Mirror entire environment under node #18820
Conversation
One can still use `-sDETERMINISITIC` to avoid this behaviour. Fixes: #18816
| #if ENVIRONMENT_MAY_BE_NODE && !DETERMINISTIC | ||
| if (ENVIRONMENT_IS_NODE) { | ||
| // Under node we mirror then entire outer environment | ||
| env = process.env; |
There was a problem hiding this comment.
this is breaking change, I think.
For example, there could be OS shell variable which would point to files which do not exist on VFS.
There was a problem hiding this comment.
It is possible yes. Should we prune the environment? Or perhaps only do this when NODERAWFS is used (i.e. when all paths would be valid)?
There was a problem hiding this comment.
Also, given that folks can still override with using the ENV global in JS, do you think it would be a problem in practice?
There was a problem hiding this comment.
I am also a little worried about breaking changes here.
Comparing this to the filesystem, by default we are sandboxed, and only use minimal things from the environment as makes sense (or no things, in DETERMINISTIC mode). The user can opt into NODEFS or NODERAWFS to get direct/unsandboxed access to the outside. Maybe something similar makes sense for the environment?
If that direction sounds relevant, I'm not sure what's best but options could include a new option, or adding a new "node direct" option (for both this and files), or perhaps bundling with NODE[RAW]FS somehow.
There was a problem hiding this comment.
How about in the default case.. should certain env vars be allowed to pass though by default? e.g. LANG and LC_* were specifically requested in #18816. Is there any reason to not pass those ones through?
There was a problem hiding this comment.
I think specific ones might be ok on a case by case basis.
|
Instead of this PR, perhaps it would be useful to document at https://emscripten.org/docs/porting/connecting_cpp_and_javascript/Interacting-with-code.html#interacting-with-code-environment-variables that it's quite easy to mirror the entire environment on Node.js with: Module.preRun = () => {ENV = Object.create(process.env)};(see #3948 (comment) where this was originally mentioned) |
|
FWIW, the reason I mentioned this is that Rust no longer test the |
One can still use
-sDETERMINISITICto avoid this behaviour.Fixes: #18816