Skip to content
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

Suggestion: volta install node should error if node is being shadowed by another binary in PATH #1268

Open
untitaker opened this issue Jul 27, 2022 · 2 comments · May be fixed by #1292
Open

Comments

@untitaker
Copy link

untitaker commented Jul 27, 2022

volta setup takes care of volta being at the front of the path. However:

  1. I hand-maintain my .profile
  2. What if two programs did this?

Today I found out the answer to question 2. PATH contained additional binaries by other "shim" programs, and as a result volta's shim node was shadowed by another node

I believe volta, on every run of volta install, should check $PATH and ensure that the just-installed command actually resolves to the shim binary. The which crate can be used for this. If that isn't the case, volta should at least emit a warning, or even error. From a user POV, nothing was installed.

@untitaker
Copy link
Author

if you agree that this would be a worthwhile UX improvement i'm interested in implementing

@charlespierce
Copy link
Contributor

Hi @untitaker, thanks for the suggestion! I really like that idea since it neatly avoids showing unnecessary information to a user who explicitly shadowed Node: If they're running volta install node, then it's a reasonable assumption that they want Volta to be managing Node, which won't work if it's shadowed. It also provides the information right when it's most useful and gives us an opportunity to provide feedback for how to fix it.

We'd absolutely welcome a contribution that adds a warning when installing Node if the PATH has a different version of Node ahead of Volta. Let me know if you need any pointers around the code or anything to help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants