Fix an issue if user passes a relative path to Python

[juj]

Fix an issue if user passes a relative path to Python, and some system cache libraries need rebuilding. Emcc tool would spawn sub-emcc tools to rebuild the cache, with different CWD, resulting in the relative path lookups pointing to the wrong relative directories. To fix the issue, normalize the relative tool paths first.

[sbc100]

Surely you don't want to be setting EMSDK_PYTHON to a relative path, do you? Otherwise you cannot run it from any directory other than one you are in.

i.e. if you did:

$ EMSDK_PYTHON=../path/to/python
$ emcc
$ cd out
$ emcc  # this one will now fail, right?

i.e. your compile will then only be usable from a single location, even after this change, right?

Another alternative here would be to set EMCC_BATCH_BUILD=0 since the issue here is that the batch builder needs to change the CWD.. which is kind of sad..

[sbc100]

Another example that simply won't work, even with this change, if you have a relative path like this:

$ make CC=emcc
.. as soon as make tries to run the compiler from a different directory this will fail

Maybe the launcher scripts (currently the only place we reference EMSDK_PYTHON) could warn/assert if its a relative path like this.

[juj]

My gut impression here was to disable this support, as "this does not make sense, why would you do that?"

But then when I thought about it,

a) Unity has been doing this since forever, where Unity's build system programmatically launches Emscripten. So it crafts the paths on the fly. Other build system might be doing the same, so why actively break them. Sure, it is easy to migrate to absolute paths (which we did), but was equally easy to fix up Emscripten, which this PR shows.

b) It can be useful for ad hoc cmdline runs in testing, e.g. EMSDK_PYTHON ../pythonA/python emcc ... vs EMSDK_PYTHON ../pythonB/python emcc ...

c) fixing this issue is simple - why actively break something that is easy to fix.

Surely you don't want to be setting EMSDK_PYTHON to a relative path, do you? Otherwise you cannot run it from any directory other than one you are in.

Yes, but the above use cases a) (programmatic launching) and b) (ad hoc cmdline use) do not navigate to other directories.

Another example that simply won't work, even with this change, if you have a relative path like this:

Yes, but here the failure will occur when make is launching emcc, not when emcc is internally launching other emcc. It is easier to debug top-level emcc launches from the calling tools, than emcc internally launching other tools.

Maybe the launcher scripts (currently the only place we reference EMSDK_PYTHON) could warn/assert if its a relative path like this.

We could add an assert and say "this is not allowed to work", but since the fix is so simple, I thought it's more straightforward to just fix the root cause?

[sbc100]

Another option is that when we re-execute emcc internally we skip the launcher scripts and use sys.executable emcc.py instead? I think that would fix the problem without introducing the EMSDK_PYTHON into the emscripten source code.

It might also speed up the library builds.

[juj]

That could work, although it seems conceptually simpler to have a single loop that absolute-pathizes, rather than use sys.executable for python, and then something else for Node. (I presume there could exist a similar issue if a sub-emcc launched by emcc attempted to spawn Node)

[sbc100]

That could work, although it seems conceptually simpler to have a single loop that absolute-pathizes, rather than use sys.executable for python, and then something else for Node. (I presume there could exist a similar issue if a sub-emcc launched by emcc attempted to spawn Node)

I don't think the latter can happen since since the only sub-compiler commands emcc ever runs are emcc -c (compile only, no node) and emar.

[sbc100]

Note to self: This issue really only exists because of EMCC_BATCH_BUILD=1.. because its only that is the only place where we run in a different CWD.

I don't love EMCC_BATCH_BUILD=1 at all and kind of which we could remove it.. but I understand it makes a big difference on windows?

[juj]

I don't think the latter can happen since since the only sub-compiler commands emcc ever runs are emcc -c (compile only, no node) and emar.

Ok, updated the implementation to only address Python. I think that's correct, that this won't happen to other sub-tools.

[juj]

I think the batch build is probably quite a significant speed improvement. Now looking at this, the code area has been refactored by quite a bit since I last looked into this. Though the important bit is that Python multiprocessing is not being used, but we run the subprocess.Popen() items manually.

[sbc100]

Nice! That seems much better.

I also think we could probably localize this to run_build_commands in system_libs.py (since that is the only place its really needed.

But if you want to land as-is and can do some followups.

[sbc100]

This kind of has the wrong name now.

[juj]

Hmm, yeah, not sure what's a better name here. Though I'll leave it if you want to address further.

[sbc100]

Yeah no worries, feel free to land

[sbc100]

you can ignore the fp6 failures, those are due to a v8 update and will be fixed when llvm changes next roll in.