Remove JS_ENGINES and COMPILER_ENGINE from config file

[sbc100]

COMPILER_ENGINE was completely removed from emscripten in emscripten-core/emscripten#9469.

JS_ENGINES is only used/needed for testing and is no longer needed as of emscripten-core/emscripten#9542.

[kripken]

lgtm otherwise, versions look right.

[kripken]

This is in a loop - is it not possible that we get these more than once? Or do we guarantee that EMSCRIPTEN_ROOT appears exactly once in a single activated config?

[sbc100]

I suppose we could have more than one version of emscripten installed? But in that case the desired behaviour is that the config file should be a super-set of all the settings required I think.

The exact same pattern is used here:

emsdk/emsdk.py

Lines 2490 to 2494 in 7815dca

	for tool in tools_to_activate:
	config = tool.activated_config()
	if 'EMSCRIPTEN_ROOT' in config:
	# For older emscripten versions that don't use an embedded cache by
	# default we need to export EM_CACHE.

[kripken]

The superset makes sense, but I think the current code can end up with duplication? That is,

JS_ENGINES = [NODE_JS]
JS_ENGINES = [NODE_JS]
JS_ENGINES = [NODE_JS]

[sbc100]

But I don't think it matters because you can't really have more than one version of emscripten installed can you?

Also, those repeated keys would be harmless if you somehow did mange to activate more than one emscripten install at once

[kripken]

But I don't think it matters because you can't really have more than one version of emscripten installed can you?

Yeah, I think only one can be active at once, but just wanted to check that my understanding was the same as yours. However, reading the code I can't easily confirm that, as in the manifest file EMSCRIPTEN_ROOT appears in things with id "releases" and also id "emscripten". I think a release contains emscripten among other things, so that suggests we use that field in more than one active thing?

Also, those repeated keys would be harmless if you somehow did mange to activate more than one emscripten install at once

I agree, it should not break anything, but would just look weird.

[sbc100]

What solution would your propose here?

Remember this only effects very old version of emscripten. So in order to see this "wierd" duplication one would need to:

1. Somehow install more than one version of emscripten (I'm pretty sure this is not support and/or would never work)
2. Both version of emscripten would need to be these very old versions.

[kripken]

Perhaps we could in the loop set a boolean "need legacy vars", and then outside of it if that is set then we add those two? That doesn't seem much more complex, and it seems safer to me.

[sbc100]

But there is no safety issue so I'm not sure adding complexity here is worth it. Have those duplicate keys would be a very minor cosmetic issue (more folks are not going to look at this config file ever).

[sbc100]

I refactored and added find_emscripten_root() which returns the currently active emscripten root.

[kripken]

lgtm % comments