Only allow absolute paths for EM_CONFIG/EM_CACHE, etc. #23402
+9
−1
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Allowing relative paths here breaks when emcc is run as a subprocess in a different directory. This happens in when building system libraries in batch build mode. One alternative to doing this would be convert all these paths to absolute paths when we read them and then re-export them with those different values for our sub-processes, but that seems more complex and error prone than just requiring them to be absolute in the first place. I'm also not sure it makes much sense to use relative paths here since if the environment contains a relative path then the compiler cannot be used from different directories. e.g. ``` $ export EM_CACHE=foo $ emcc hello.c $ cd foo && emcc foo.c # confusingly would use a different cache ``` Fixes: emscripten-core#23374
|
I think this is a good idea. Unlike, say, a config file, an environment variable is going to be specific to a particular installation or environment and won't move between systems. |
|
You also don't want the environment variable to effectively move ever time you do |
Right, I would expect the cache location not to be relative to the working directory. If it's relative to anything, it seems like it should be the emscripten root. Even if for some reason you do want a separate cache for, say, every build project, you can still accomplish that with the build system. |
|
OK to land this? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Allowing relative paths here breaks when emcc is run as a subprocess in a different directory. This happens in when building system libraries in batch build mode.
One alternative to doing this would be convert all these paths to absolute paths when we read them and then re-export them with those different values for our sub-processes, but that seems more complex and error prone than just requiring them to be absolute in the first place.
I'm also not sure it makes much sense to use relative paths here since if the environment contains a relative path then the compiler cannot be used from different directories. e.g.
Fixes: #23374