-
Notifications
You must be signed in to change notification settings - Fork 697
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Installing emsdk overwrites node on PATH #1142
Comments
This issue has been automatically marked as stale because there has been no activity in the past 2 years. It will be closed automatically if no further activity occurs in the next 7 days. Feel free to re-open at any time if this issue is still relevant. |
Any updates? |
Reopening, as this does sound like it would be nice to fix. I think just not adding those things to the path would be fine, as emscripten does have direct paths to them in the config file for its own use. Perhaps someone has time to test that and see if it works? If so let me know if you have any questions about how to modify the emsdk for this! |
Any updates again? It's been 2 years and I can't use emscripten without this being fixed as I need node.js 16 and emscripten overwrites my nvm version with 14. Update: I found https://github.com/Sxxov/emscripten-sdk-npm and it seems to be working, node -v still reports v16.10.0. |
We no longer inject clang or llvm into the PATH, but we do still inject our node in to the PATH. I tried to make a change where node would only be added to the PATH if it wasn't already there: But we decided against landed it. Do you really need to have emsdk and your own version of node active at the same time in the same shell? Can you explain a little about your use case? If its really probelem maybe we can re-open #714 In #714 it was mentioned that as a workaround you should be able to install and activate just the sub-components of the SDK without also installing node. e.g.:
|
Note that you could also do: # Prefer the default system-installed version of Node.js
echo "NODE_JS = '$(which node)'" >> .emscripten Just before: # Activate PATH and other environment variables in the current terminal
source ./emsdk_env.sh To ensure that the Node.js from emsdk doesn't clobber the # Prefer the default system-installed version of Node.js
./emsdk uninstall $(./emsdk list | grep INSTALLED | tr -d \(\*\) | grep node | awk '{print $1}') (from: #96 (comment)) Just before: ./emsdk activate latest But that doesn't seems to work anymore ( |
Does the emsdk_env.sh script do anything other than modify the PATH? And will the emscripten tools be okay if its node executable isn't found on the path? If so, I'm just going to settle for manually adding its stuff to the end of my path and excluding its node executable. I found it surprising (in an annoying way) that my snap installation of node wasn't the one found on my PATH. |
@david-fong that solution should work fine, yes. All you should really need is
|
@sbc100 I am running into a similar problem. With the release of firefox 100 wasm exceptions seems to now be supported by all major browsers by default. So i was switching my build process to use wasm exceptions, what i found was when using node 14 i was hitting the following error It seems that emsdk_env.sh is still overwriting node even if it exists in the path contrary to what #714
|
We go push back on #714 so it didn't land. I'll see if we can revist. |
Originally Emsdk was designed to allow people to control what tools they want to have in their PATH. If a user wanted to use all of Emscripten's provided tools (the most common scenario), they would do e.g.
(or some other sdk target, depending on the version they choose) The SDKs are defined as convenience shorthands to collections of SDK Tools to use at the same time. For example, sdk-upstream-main-64bit is a collection of the following Tools:
This means that typing
is identical to typing
If I wanted to omit one of the Tools, I can issue an Emsdk install/activate step which skips that tool altogether. E.g.
does an installation that does not bring Emsdk node into the mix. So if you wanted to drive Emscripten with your own node and not an emsdk provided node, you could do an installation/activation like that. One should also recognize the behavior of the Emsdk
it is the same as typing
i.e. the user is explicitly requesting a global registry PATH installation of python and node. In this case, if it is undesirable, the recommended solution would be not to run that command, but instead do a
The command prompt emsdk_env intends to enter a prompt environment that contains all the tools that are activated at the time, so it is intentional that this prompt has the Node and Python etc. tools explicitly and up front in PATH (if user installed and activated those tools via emsdk). The intent is to provide developers with a method of entering the exact environment where the same tools are available to the prompt that Emscripten uses. If the issue is that emsdk_env adds those tools to PATH that clobbers the use of some other commands, then maybe the root realization is that those other commands would not be appropriate to be run under the emsdk_env provided environment? Nothing should force developers to run their own tool command lines under emsdk_env if they so choose. Emscripten command invocations do not need or care about any of the emsdk provided node and python tools being in the PATH. They only care about the EM_CONFIG file. This ensures that it is possible to invoke Emscripten tools outside emsdk_env environment (using emsdk_env is optional), as long as you provide the appropriate config file, either by using the For example, to run emcc outside the emsdk prompt while pointing it to the configuration that emsdk has provided, one can run
or
This kind of use can ensure that people don't get any adjustments to their PATH if not desired. Overall, if Emsdk did not add the needed tools to PATH, my impression is that there would be more users who would be confused of not getting the tools readily available. E.g. the getting started instructions
would not work out of the box if emsdk did not provide all the tools for the users as necessary. Also I think the idea of pushing the tools to the PATH last is not great, as it would weaken the reproducibility of |
@juj Fair enough i will work around it. I would rather not have to try and keep up to date with all the dependencies of emscripten because Trying to keep this aligned seems painful as well
Could there be an omit/exclude flag added instead?
Let me ask a another question, and more specific to why i need to exclude node. What is the cadence that emscripten will update nodejs? For example emscripten is producing wasm that the node version it includes does not support. That's really the only reason i need to use a different node. I would happily use the bundled nodejs, unfortunately i can't. For dependencies in the toolchain like clang emscripten point to trunk, are there any plan to keep nodejs more inline with active versions? |
I can see @juj pointer that changing the defaults might cause more confusion that it solves. I do want to make a change to allow the behaviour that @arsnyder16 want though. I kind of want the same thing since I have version of node that I like to use in my PATH already. I kind of like the idea of something like (BTW in your example above |
Dont overwrite shell's node. use an environment variable or something else. |
The principle of least surprise.
…On Mon, Jun 13, 2022, 11:47 Sam Clegg ***@***.***> wrote:
Dont overwrite shell's node. use an environment variable or something else.
@juj <https://github.com/juj> makes the opposite argument in emscripten-core/emsdk#1142
(comment)
<#1142>.
Its not clear which approach is better/easier.
—
Reply to this email directly, view it on GitHub
<#1142>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AEFHWPKD3PHM5DZLP37BZZTVO5QZLANCNFSM4C35GRIQ>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
@juj:
I think the main issue here is that the documentation suggests that you should always be in
and it is not at all clear that doing the "Configure emsdk in your shell startup scripts by running [...]" step is also going to clobber the system version of node for any future shell you open. That was very surprising to me. It sounds like If you want to clobber the system version of To put it another way, here's the process I went through:
I would like to go through exactly the same process, including the "emcc works in a new shell part", except without the last step. |
Currently we do recommend using I think the expression "clobber the system version node" is a little strong. What we are doing is putting the emsdk version of node first in the PATH when emsdk is active, the system version is still there, its just no longer first in the PATH. |
I sent a message to the list to try to see who out there is depending on this: https://groups.google.com/g/emscripten-discuss/c/KnG7io3zu2Q |
Agree, this problem makes sense - in order to be able to get "emsdk activate everything - node", one will currently need to first learn and track what "everything" is in order to arrive to that everything minus node. That is a shortcoming.
This would be a clean feature to add. Name bikeshedding, I'd prefer
would match to skip node-*-*bit (i.e. all tools with name node, any version, any bitness) However, reading further about the confusion, I think this would probably not be a best of all worlds solution, because it would be solving the problem by layering more concepts on people (rather than simplifying). That being said, as a general feature, I think such a
If/when Emscripten is producing wasm content that Emsdk-bundled node is unable to run, that is by itself an issue with Emsdk, and I think we should address that issue as well. If Emsdk has fallen badly behind, then updating is certainly in order. Though using a relatively old version in Emsdk to ensure that our CI tests against the "minimum supported version" is also important - but I think here we can explicitly instruct the CI to download the min spec version, and default users to download a new version (in case we get into these types of testing trouble). Maybe straddle different testing targets to download different nodes.
I'd recommend against Also, if we do a minimal mode that only has Emscripten in PATH and nothing else, then we can still get other users complaining why they don't have a way to say they want Emscripten-provided Python or Clang in PATH, but not Node or Java. The permutations are large, so a general option like
I agree with this characterisation. But like people have mentioned in the above comments, they are running into a problem that the node that Emsdk provides is not enough to run their Emscripten produced wasm features. If that is the case, then that issue should specifically be addressed - sounds like Emsdk bundled node has fallen too far behind(?). Please raise specific issues about those (sorry if you did already, my attention bandwidth may be struggling)
In this scenario, do you want Emscripten to invoke your supplied
Thanks, now I understand. I agree this is to a great extent a documentation problem.
I agree with this characterization. To solve these issues, I recommend that we modify the behavior of
then emsdk_env would enter an environment where these two tools only will be brought into the user's PATH. The earlier Then, let's create an alias
then all the tools that were previously activated with If no parameters are specified, i.e.
then it will be the same as if one had written
(this will be a breaking change. For a transition period, we can issue a hint if this produces no node at all in user's PATH) This way, we can have the getting started docs recommend people to run Hopefully that would make it easy for people to target all scenarios they wish for, with minimal pain? |
That sounds good to me!
I would suggest instead testing if And then |
The above seems more complicated than it should be. While a few people may want to expose emsdk's node, most won't. IMO exposing node should be opt-in not opt-out. |
@curiousdannii with the above proposal it would be opt-in, as I read it? |
Modifying emsdk_env like this SGTM. (And yes my reading is that node would then be opt in.. either via writing |
Yes it's opt-in. But I don't think the script should be looking to see if node already exists in path. The docs shouldn't encourage exposing emsdk's node, but instead should recommend people install node for their OS as normal. I don't see the wisdom in a But then I'm not part of the project team, so you decide what you want to do :) |
Sure, I think whether and how we recommend using The main point is that there exists some way to preserve the old behaviour for those what do want emsdk to be a completely solution (some folks I think do want emsdk to install all dependencies without having to go use apt-get for some of the stuff). |
It would be interesting to know how many developers using emscripten do not install their own node on their system, it's not a small unknown dependency of emscripten, it's a common package for every dev I know. |
Is there no workaround for this issue as of today? I am working both on a project that uses emscripten and an unrelated project that needs Node v18, and I don't want to be forced to manually call Is my best shot |
The simplest solution might just be to avoid using |
See #1142 (comment). |
IIUC the ideal solution doesn't avoid installing the emsdk version of node, or have emscripten use an unexpected/system version. The goal is to avoid having emsdk's override system node in the PATH, right? Avoiding emsdk node, even within emsdk itself, seems like overkill and could cause other issues. |
As mentioned in that comment, you cannot activate Emscripten without installing the bundled Node.js provided by emsdk.
This assumes that the globally-installed Node.js version is newer than what emsdk provides. emsdk currently provides Node.js 14.x, which active support ended one year ago. Security updates for this version ends in 5 months. So, I would argue to at least update the bundled Node.js version to the latest LTS version, which would also fix issues like emscripten-core/emscripten#18084. FWIW, this workaround is currently used in at least libjxl and wasm-vips to avoid compatibility issues with Wasm SIMD, which requires Node.js 16.4.0 or later to match the final SIMD opcodes.
I agree that would be the ideal solution. I only pointed out that comment since he was looking for a workaround.
Just wondering, what do you mean with other issues? With the same assumption as above, at least for Emscripten, using a globally-installed Node.js should be safe after PR emscripten-core/emscripten#18070. Another solution would be that emsdk only installs the bundled Node.js, if a globally-installed Node.js is not available, or if the globally-installed Node.js version is lower than the minimum required Node.js version (or the one provided by emsdk). This would also fix this |
We only test emcc itself with a specific version of node, so using any other version is vearing into untested teritory. Its probably fine, but we only test our internal compiler tools against that fixed version.
I tried to land that solution in #714 but it was deemed to magical. We could try to re-litigate that one. I think its quite a nice solution myself. |
I understand. There's a
Great, I had forgotten about that PR. IIUC, that only affects the If so, perhaps a better solution would be to do this check in the $ ./emsdk install node-14.18.2-64bit
$ ./emsdk active node-14.18.2-64bit |
Yes, in fact it still uses the bundled version of node within emscripten (i.e. it sets NODE_JS in the config file). All it does it avoid adding this version of node to the PATH during
That would be an alternative, and slightly more intrusive/risky change yes. It would solve a slightly different issue which is "I want emcc to use my system version of node". |
This change means that aliases such as `latest` and `3.10.2` can be used to install jsut the emscipten-releases packages and not the full SDK with all the dependencies. This allows uses who do not want to install dependencies such as node, python or java to still use this package. These packages were already available to install on their own via their full names such as: ``` $ ./emsdk install releases-upstream-48ce0b44015d0182fc8c27aa9fbc0a4474b55982-64bi ``` Now these base packages can also be install via: ``` $ ./emsdk install latest-base ``` Fixes: #1142
This change means that aliases such as `latest` and `3.10.2` can be used to install jsut the emscipten-releases packages and not the full SDK with all the dependencies. This allows uses who do not want to install dependencies such as node, python or java to still use this package. These packages were already available to install on their own via their full names such as: ``` $ ./emsdk install releases-upstream-48ce0b44015d0182fc8c27aa9fbc0a4474b55982-64bi ``` Now these base packages can also be install via: ``` $ ./emsdk install latest-base ``` Fixes: #1142
@sbc100 Is there anyway to pin the current solution to the top of this issue? It took me a while to find this amongst the full discussion. As a JavaScript developer learning Emscripten, this was a very confusing. I am using Emscripten along with JavaScript/TypeScript build tools to bundle an application and I am using a very recent version of Node.js. |
(I don't think there is any way to do that, but I will see if I can edit the description of the add a note). |
This change means that aliases such as `latest` and `3.10.2` can be used to install jsut the emscipten-releases packages and not the full SDK with all the dependencies. This allows uses who do not want to install dependencies such as node, python or java to still use this package. These packages were already available to install on their own via their full names such as: ``` $ ./emsdk install releases-upstream-48ce0b44015d0182fc8c27aa9fbc0a4474b55982-64bi ``` Now these base packages can also be install via: ``` $ ./emsdk install latest-base ``` Fixes: #1142
This change means that aliases such as `latest` and `3.10.2` can be used to install jsut the emscipten-releases packages and not the full SDK with all the dependencies. This allows uses who do not want to install dependencies such as node, python or java to still use this package. These packages were already available to install on their own via their full names such as: ``` $ ./emsdk install releases-upstream-48ce0b44015d0182fc8c27aa9fbc0a4474b55982-64bi ``` Now these base packages can also be install via: ``` $ ./emsdk install latest-base ``` Fixes: #1142
This change means that aliases such as `latest` and `3.10.2` can be used to install jsut the emscipten-releases packages and not the full SDK with all the dependencies. This allows uses who do not want to install dependencies such as node, python or java to still use this package. These packages were already available to install on their own via their full names such as: ``` $ ./emsdk install releases-upstream-48ce0b44015d0182fc8c27aa9fbc0a4474b55982-64bi ``` Now these base packages can also be install via: ``` $ ./emsdk install latest-base ``` Fixes: #1142
This was fixed in #1189 |
This is a small but annoying issue. When installing the emscripten sdk, or whenever you install a new version of the toolchain, you're told to execute
source ./emsdk_env.sh
.This is rather annoying, as the toolchain comes packaged with its own specific version of node, and this conflicts with all other node tools we're used to using. For example, it put node 4.1.1 or my path while I previously had 7.4.0. I've noticed the same goes for
clang
, but it doesn't bother my that much. Even when relying on a tool like nvm, it's annoying because switching to another version would essentially break emscripten/emsdk?If emscripten is reliant on a specific version of node, should it not be its own responsibility to make sure its scripts run with its own internally packaged version? This just seems a tad messy.
(Note: One possible temporary workaround is to simply avoid
emsdk_env.sh
in this case and just manually addemsdk/upstream/emscripten
to your path. That is all that should be needed to get emscripten working on many systems).The text was updated successfully, but these errors were encountered: