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

source nvm.sh; nvm install --lts fails #1664

Closed
3 tasks done
norpol opened this issue Nov 15, 2017 · 9 comments
Closed
3 tasks done

source nvm.sh; nvm install --lts fails #1664

norpol opened this issue Nov 15, 2017 · 9 comments
Labels
pull request wanted This is a great way to contribute! Help us out :-D

Comments

@norpol
Copy link

norpol commented Nov 15, 2017

Hi, short overview is that set -u in a script is letting my nvm install --lts fail, due obvious reasons.
A quick response would is super welcomed.

  • What steps did you perform?
#!/bin/sh
setup_nvm() {
  export NVM_DIR="$(mktemp -d /tmp/nvm-XXXXXXXX)"
  [ -d "${NVM_DIR}" ]
  git clone --depth=10 https://github.com/creationix/nvm.git "${NVM_DIR}"
  cd "${NVM_DIR}"
  git checkout $(git describe --abbrev=0 --tags --match "v[0-9]*" origin)
  . "${NVM_DIR}/nvm.sh"
  nvm install --lts
  echo "${NVM_DIR}"
}

setup_nvm # works
set -u
setup_nvm # fails
  • What happened?

I get the error message: NVM_LS_REMOTE_IOJS_OUTPUT: parameter not set.
This happens, because I'm using set -u in my script. I'd like to have a very quick conversation regards how we should resolve the issue:

  • How can I properly workaround this? (I'd like to avoid set +u; setup_nvm; set -u in my script)
  • How should this be handled in our code? Should we get set -eu enabled by default inside nvm?
  • Send pull request to fix at least NVM_LS_REMOTE_IOJS_OUTPUT is assigned outside the condition above. https://github.com/creationix/nvm/blob/master/nvm.sh#L525
@ljharb
Copy link
Member

ljharb commented Nov 15, 2017

The first option is the proper workaround; setting shell options is hostile to other things that run in the same shell. The second option isn’t an option.

The third option, however, is a fine longer-term fix. Thanks!

@ljharb ljharb added the pull request wanted This is a great way to contribute! Help us out :-D label Nov 15, 2017
@norpol
Copy link
Author

norpol commented Nov 15, 2017

There your pull-request goes. If it's alright, I've also added set -u to your test case for lts.
I could also do local foo=, but I'm not sure if that is part of your practices.

@ljharb
Copy link
Member

ljharb commented Nov 17, 2017

In tests, we don't worry about local; in actual code, we always do:

local foo
foo=1

to account for ksh.

@norpol
Copy link
Author

norpol commented Nov 17, 2017

Great :-), then feel free to merge #1665 at some point. Wish you a great weekend!

@ljharb
Copy link
Member

ljharb commented Nov 18, 2017

Closed in #1665.

@ljharb ljharb closed this as completed Nov 18, 2017
ljharb added a commit that referenced this issue Nov 18, 2017
@liqiang372
Copy link

liqiang372 commented Nov 22, 2017

Hi @norpol , I wondered how your script works, you are sourcing and installing node inside a script. But the parent shell does not have node installed.
The node and npm still not found in command line.
I'm trying to automate the nvm installation, but got stuck here.

This is the script I tried to run

#!/bin/bash
setup_nvm() {
  export NVM_DIR="${HOME}/.nvm"
  export NODE_VERSION=6.11.5
  curl -o- https://raw.githubusercontent.com/creationix/nvm/v0.33.6/install.sh | bash
  . "${NVM_DIR}/nvm.sh"
  nvm list
  nvm install ${NODE_VERSION}
  nvm use ${NODE_VERSION}
  nvm alias default ${NODE_VERSION}
  npm install -g grunt
}

setup_nvm

I can see them installed correctly. But when I type node, it says command not found

@ljharb
Copy link
Member

ljharb commented Nov 22, 2017

@liqiang372 You have to re-source nvm.sh in your parent shell after doing that, if you're doing that in a script.

Also, you don't need the nvm use command, or the nvm alias default command if it's the first node version you're installing.

@liqiang372
Copy link

@ljharb got it, thanks! So there is no ultimate automation of this installation. I have to at last run

$ . ~/.nvm/nvm.sh

by myself to make current shell aware of the change

@ljharb
Copy link
Member

ljharb commented Nov 23, 2017

Yes, that's correct.

ljharb added a commit to ljharb/nvm that referenced this issue Dec 9, 2017
Fixes
 - fix unassigned variable (nvm-sh#1665, nvm-sh#1664)
 - Fix for $path used by zsh (nvm-sh#1669)
 - `set -u`: ensure `NVM_USE_OUTPUT` is always set (nvm-sh#1671)
 - `install.sh`: Fix a bug that block that installation of node in install.sh (nvm-sh#1676)
 - `nvm install-latest-npm`: fix node 4-4.6

Documentation
 - Make `nvm cache clear` message less ambiguous (nvm-sh#1644)
 - Added missing piece (nvm-sh#1658)
edwmurph pushed a commit to edwmurph/nvm that referenced this issue Apr 9, 2018
edwmurph pushed a commit to edwmurph/nvm that referenced this issue Apr 9, 2018
Fixes
 - fix unassigned variable (nvm-sh#1665, nvm-sh#1664)
 - Fix for $path used by zsh (nvm-sh#1669)
 - `set -u`: ensure `NVM_USE_OUTPUT` is always set (nvm-sh#1671)
 - `install.sh`: Fix a bug that block that installation of node in install.sh (nvm-sh#1676)
 - `nvm install-latest-npm`: fix node 4-4.6

Documentation
 - Make `nvm cache clear` message less ambiguous (nvm-sh#1644)
 - Added missing piece (nvm-sh#1658)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pull request wanted This is a great way to contribute! Help us out :-D
Projects
None yet
Development

No branches or pull requests

3 participants