Nvm: become a safer shell script

[herzi]

In our CI setup, we prefix shell scripts with

set -o errexit # exit on first error
set -o nounset # unset variables are errors

in order to make sure script errors will become more obvious. However, I can only enable these lines _after_ including nvm.shdue to this error:

/usr/local/opt/nvm/nvm.sh: line 1918: $1: unbound variable

Please consider adding the aforementioned shell settings in your code (and to fix unbound variable access).

Debug Info

bash-3.2$ nvm --version
0.26.1

[ljharb]

Using nonzero exit codes for flow control is a normal part of programming, and nvm is not compatible with set -e (the errexit option) - see https://github.com/creationix/nvm/issues/865#issuecomment-146948066.

In addition, v0.26.1 is out of date - please try installing v0.29.0. I'm happy to look at any variable errors you see on the latest version, but errexit interferes with normal operation of nvm. (Relates to #865, #866, #721).

Note that this particular nounset error is complaining about $1 which is being used as an argument to the "source nvm" command in your profile file - the whole point is that it's unset, and I'm _testing that it's unset_, so that I can conditionally install-on-source.

In other words, these shell options are a crutch that both interfere with normal operation as well as enable developers to not properly check all possible failure conditions.

[herzi]

About errexit: Note that this does not apply to any invocation that is either:

• the argument deciding which branch to take for an if, or
• code that has error handling capabilities like || <your error handler here>

WRT nonuset: You can safely check the value of $# instead of just referencing into the arguments array.

[ljharb]

That's a fair point - I'll be happy to fix that variable check by checking $# prior to $1.

If you can point at specific instances where errexit breaks, and a to a place where nvm can fix it without impeding code clarity, I'm all for it!

[ljharb]

I've taken care of the nounset error. Let me know if you see any others.

[ljharb]

Reopening this in case someone wants to attempt to identify and fix these issues.

[danistefanovic]

I've found the following unbound variables so far:

• line 77: NVM_DIR: unbound variable
• line 1655: PROVIDED_REINSTALL_PACKAGES_FROM: unbound variable
• line 1658: PROVIDED_REINSTALL_PACKAGES_FROM: unbound variable
• line 662: NVM_ADD_SYSTEM: unbound variable

NVM version: v0.30.1

[ljharb]

NVM_DIR is explicitly intended to be one, but the others aren't - thanks!

[ljharb]

@danistefanovic I see how PROVIDED_REINSTALL_PACKAGES_FROM is being checked in one case even when it's empty, but I explicitly set $NVM_ADD_SYSTEM to false on line 603 - can you elaborate on the problem with that one? Both are declared with local so I'm also not clear on what you mean by "unbound"

[danistefanovic]

"Unbound variable" is the error message in the terminal if you enable the nounset option.

set -o nounset
. ./nvm/nvm.sh

./nvm/nvm.sh: line 77: NVM_DIR: unbound variable

I understand that NVM_DIR should be provided from "outside" to set a custom NVM directory. For this case, you can use parameter expansion for the check:

if [ -z "${NVM_DIR-}" ]; then

# instead of
# if [ -z "$NVM_DIR" ]; then

This seems to work with the nounset option. I've tested it in bash (Ubuntu) and zsh (OS X).

[ljharb]

Thanks, this has actually helped me find a ton of these already. I'm going to do a full pass, and I'll close this issue with that commit.

[danistefanovic]

Glad I could help :beers:

[stouf]

Hi there !

Just ran into another unbound variable issue :(

nvm/nvm.sh: line 95: NVM_IOJS_ORG_MIRROR: unbound variable

This error occurs when I have the set -u option in a bash script.

Version of NVM: 0.31.0

[ljharb]

Thanks, will fix.

[stouf]

@ljharb Awesome. I forked the repo and was about to send a Pull Request, but I guess it's better to leave that to you :)

[ljharb]

@stouf thanks! PRs are always welcome but this is small enough that it's nbd ¯_(ツ)_/¯