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

nvm use fails when NVM_DIR ends with a slash #1708

Closed
chrisse27 opened this issue Jan 15, 2018 · 17 comments
Closed

nvm use fails when NVM_DIR ends with a slash #1708

chrisse27 opened this issue Jan 15, 2018 · 17 comments

Comments

@chrisse27
Copy link

  • Operating system and version: Linux

  • nvm debug output:

nvm --version: v0.33.8
$SHELL: /bin/bash
$HOME: /home/bamboo
$NVM_DIR: '$HOME/.nvm/'
$PREFIX: ''
$NPM_CONFIG_PREFIX: ''
$NVM_NODEJS_ORG_MIRROR: 'https://nodejs.org/dist'
$NVM_IOJS_ORG_MIRROR: 'https://iojs.org/dist'
shell version: 'GNU bash, version 4.3.48(1)-release (x86_64-pc-linux-gnu)'
uname -a: 'Linux 4.4.0-109-generic #132-Ubuntu SMP Tue Jan 9 19:52:39 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux'
OS version: Ubuntu 16.04.3 LTS
curl: /usr/bin/curl, curl 7.47.0 (x86_64-pc-linux-gnu) libcurl/7.47.0 GnuTLS/3.4.10 zlib/1.2.8 libidn/1.32 librtmp/2.3
wget: /usr/bin/wget, GNU Wget 1.17.1 built on linux-gnu.
git: /usr/bin/git, git version 2.7.4
grep: /bin/grep, grep (GNU grep) 2.25
awk: /usr/bin/awk, GNU Awk 4.1.3, API: 1.1 (GNU MPFR 3.1.4, GNU MP 6.1.0)
sed: /bin/sed, sed (GNU sed) 4.2.2
cut: /usr/bin/cut, cut (GNU coreutils) 8.25
basename: /usr/bin/basename, basename (GNU coreutils) 8.25
rm: /bin/rm, rm (GNU coreutils) 8.25
mkdir: /bin/mkdir, mkdir (GNU coreutils) 8.25
xargs: /usr/bin/xargs, xargs (GNU findutils) 4.7.0-git
nvm current: v6.9.1
which node: $NVM_DIR/versions/node/v6.9.1/bin/node
which iojs:
which npm: $NVM_DIR/versions/node/v6.9.1/bin/npm
npm config get prefix: $NVM_DIR/versions/node/v6.9.1
npm root -g: $NVM_DIR/versions/node/v6.9.1/lib/node_modules
  • nvm ls output:
  • How did you install nvm? (e.g. install script in readme, homebrew):

Install script in readme

  • What steps did you perform?
nvm use 9
  • What happened?
nvm is not compatible with the npm config "prefix" option: currently set to "/home/bamboo/.nvm/versions/node/v8.9.1"
Run `nvm use --delete-prefix v8.9.1` to unset it.
  • What did you expect to happen?

That nvm switches to the new Node version.

This is a bug in the function nvm_tree_contains_path which cannot deal with trailing slashes in the NVM_DIR variable. removing the slash solves the problem. I anyway opened this issue to spare future users hours of painful debugging.

  • Is there anything in any of your profile files (.bashrc, .bash_profile, .zshrc, etc) that modifies the PATH?
  • If you are having installation issues, or getting "N/A", what does curl -I --compressed -v https://nodejs.org/dist/ print out?
@ljharb
Copy link
Member

ljharb commented Jan 15, 2018

Hmm, how did the trailing slash end up there in the first place?

@chrisse27
Copy link
Author

@ljharb That was a configuration that our Admins were setting up in our Bamboo CI. Of course we had them change that now, but I found it a bit disturbing that a slash in the "wrong" place can do this harm. It's not only the function I mentioned above that has a problem with this. Also calling nvm current starts reporting the wrong version when the NVM_DIR has the trailing slash.

@ljharb
Copy link
Member

ljharb commented Jan 15, 2018

@chrisse27 would you be interested in submitting a PR that explicitly fails with a helpful error message when a directory path has a trailing slash, in all the places you find, with associated tests?

@ljharb
Copy link
Member

ljharb commented Jan 15, 2018

(please do note; a slash in the wrong place with mv can do harm too; it's simply the nature of posix that a directory path does not end in a slash; the slash indicates "inside the directory" - your admins should know that)

@chrisse27
Copy link
Author

@ljharb I can do that, although we are still discussing among the team whether it's better to educate the users or help them by normalizing the path.

@ljharb
Copy link
Member

ljharb commented Jan 16, 2018

I think that since the primary use case for nvm is the install script populating NVM_DIR, and users manually providing it is an edge case, that it's more important to educate them.

@chrisse27
Copy link
Author

Just had a look at the code. Adding an error message everywhere where NVM_DIR is used is too much of an effort. I will submit an PR for a change in the documentation.

@PeterDaveHello
Copy link
Collaborator

PeterDaveHello commented Feb 18, 2018

Path ends with trailing / isn't that invalid in fact, #1734 can fix it :)

@chrisse27
Copy link
Author

Thanks for the fix @PeterDaveHello. I am looking forward to the discussions with @ljharb regarding how valid the trailing slash is.

@PeterDaveHello
Copy link
Collaborator

@chrisse27 let me know if it fixes your problem if you have time to test it, thanks :)

@ljharb
Copy link
Member

ljharb commented Feb 18, 2018

It can be fixed, but no, it's not valid. Try using mv or rm on a directory with a trailing slash and you'll see that it's not :-)

@PeterDaveHello
Copy link
Collaborator

Yes, I tested mv with Ubuntu and Alpine, it works perfectly. rm is not for directory but rmdir is, but rm -r also works fine here.

PeterDaveHello added a commit to PeterDaveHello/nvm that referenced this issue Feb 19, 2018
@ljharb
Copy link
Member

ljharb commented Feb 19, 2018

You may have been trying with the source arg, or with a nonexistent dest arg. Try mv src dest vs mv src dest/ when dest exists.

@PeterDaveHello
Copy link
Collaborator

I got what do you mean, still no problem here though.

@ljharb
Copy link
Member

ljharb commented Feb 19, 2018

In the former example it will replace dest; in the latter it will move src inside dest. That’s because the trailing slash means “inside the dir”, and without the slash it’s “the dir”.

@PeterDaveHello
Copy link
Collaborator

Yeah, I know the difference, but in the case of $NVM_DIR, I think we won't have this issue?

@ljharb
Copy link
Member

ljharb commented Feb 20, 2018

Not at the moment; but that doesn’t change that the proper path to a directory has no trailing slash :-)

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

No branches or pull requests

3 participants