As of 0.24.1, bash -e option causes failures

[nicholascloud]

NOTE I'm still investigating this and it sounds like it might be an edge case, but I'm opening the issue now with the information I have in case others are experiencing this.

We use RightScale to deploy virtual servers. Today our instances, which all install nvm when launched, all started failing. We traced the audit output (shown below) to the nvm.sh script:

19:53:12: + echo 'installing node version manager...'
19:53:12: + git clone https://github.com/creationix/nvm.git /opt/nvm
19:53:12: installing node version manager...
19:53:12: Cloning into '/opt/nvm'...
19:53:12: + cd /opt/nvm
19:53:12: ++ git describe --abbrev=0 --tags
19:53:12: + git checkout v0.24.1
19:53:12: Note: checking out 'v0.24.1'.

You are in 'detached HEAD' state. You can look around, make experimental
changes and commit them, and you can discard any commits you make in this
state without impacting any branches by performing another checkout.

If you want to create a new branch to retain commits you create, you may
do so (now or later) by using -b with the checkout command again. Example:

  git checkout -b new_branch_name
19:53:12: HEAD is now at c966204... v0.24.1
19:53:12: + . /opt/nvm/nvm.sh
++ NVM_SCRIPT_SOURCE=v0.24.1
++ nvm_has unsetopt
++ type unsetopt
++ '[' -z '' ']'
++ '[' -n /opt/nvm/nvm.sh ']'
++ NVM_SCRIPT_SOURCE=/opt/nvm/nvm.sh
19:53:12: ++++ dirname /opt/nvm/nvm.sh
19:53:12: +++ cd /opt/nvm
+++ pwd
19:53:12: ++ export NVM_DIR=/opt/nvm
++ NVM_DIR=/opt/nvm
++ unset NVM_SCRIPT_SOURCE
++ '[' -z '' ']'
++ export NVM_NODEJS_ORG_MIRROR=https://nodejs.org/dist
++ NVM_NODEJS_ORG_MIRROR=https://nodejs.org/dist
++ '[' -z '' ']'
++ export NVM_IOJS_ORG_MIRROR=https://iojs.org/dist
++ NVM_IOJS_ORG_MIRROR=https://iojs.org/dist
++ '[' -z '' ']'
++ export NVM_IOJS_ORG_VERSION_LISTING=https://iojs.org/dist/index.tab
++ NVM_IOJS_ORG_VERSION_LISTING=https://iojs.org/dist/index.tab
19:53:12: +++ nvm_alias default
19:53:12: ++ VERSION=
19:53:12: Script exit status: 2

The install script that generates this audit output looks like this (E_NVM_DIR=/opt/nvm):

#!/bin/bash -ex

# install node version manager
echo "installing node version manager..."
git clone https://github.com/creationix/nvm.git $E_NVM_DIR
cd $E_NVM_DIR
git checkout `git describe --abbrev=0 --tags`
. $E_NVM_DIR/nvm.sh

We changed the git checkout line to read:

git checkout v0.24.0

and the images are all launching successfully now. So something in the v0.24.1 release, seemingly around automatic default version assignment, seems to be affecting our images. We are running Ubuntu 12.04.1 LTS with bash.

[ljharb]

Do you perhaps have your deploy script set up to exit whenever any function exits nonzero? If so, that's likely to be the problem, as a nonzero exit code is being used here intentionally.

[ropez]

+1

This causes our CI to fail, since we're running scripts with bash -e. It was introduced in commit 96e7382

[ljharb]

Yes, that's going to continue to be a problem. Running with -e is always problematic for this reason, and that makes your CI tests more brittle.

You could always set -e after nvm is loaded, if you need to.

[nicholascloud]

@ljharb yes, RightScale aborts on non-zero exit codes. I will try the set -e approach for now, then test for $NVM_DIR and exit the script with an error code if it cannot be found.

[nicholascloud]

@ljharb Using set -e works. Here is my modified script:

#!/bin/bash -x
# install node version manager
echo "installing node version manager..."
git clone https://github.com/creationix/nvm.git $E_NVM_DIR
cd $E_NVM_DIR
git checkout `git describe --abbrev=0 --tags`
. $E_NVM_DIR/nvm.sh
set -e
if [ -z ${NVM_DIR+x} ] ; then
  echo "Unable to source nvm."
  exit 1
fi

[jameshartig]

Just spent a few hours figuring out why packer was returning exit code 2 when trying to install nvm and finally traced it down to this issue. It would be nice if anything that cared about exit codes didn't break just because there isn't an alias for default installed yet. Especially if the next thing you're going to do is install a default next... Is there a way to source nvm.sh without having it immediately try to use a version of nvm?

Could there just be another elif at the bottom that returns 0? From what I can tell returning exit code 2 was a regression introduced with a completely unrelated goal from #709 (goal being to increase performance by only calling nvm_alias once).

[ljharb]

@fastest963 no, I'm afraid not - I'd be open to a PR adding that as an option (like --install). However, blindly exiting on any failure code doesn't seem to me to be particularly wise.

However, on re-reviewing this issue, it seems like perhaps it's simply that https://github.com/creationix/nvm/blob/master/nvm.sh#L1787 doesn't always return 0, and since there's nothing to return 0 at the end of the script, $? is forwarded along. If adding a else; return 0 to the end of the if blocks would fix this, I'd definitely be willing to do that. Can someone verify?

[ropez]

FYI, We resolved this in our scripts, by creating https://github.com/ropez/nvx and using that instead of nvm.

[jameshartig]

@ljharb I tried else return 0 and seems to fix the issue:

--- a/nvm.sh
+++ b/nvm.sh
@@ -1795,6 +1795,8 @@ elif [ -n "$VERSION" ]; then
   nvm use "$VERSION" >/dev/null
 elif nvm_rc_version >/dev/null 2>&1; then
   nvm use >/dev/null
+else
+  return 0
 fi

 } # this ensures the entire script is downloaded #

[vagrant@localhost ~]$ source ~/.nvm/nvm.sh; echo $?
0

[ljharb]

Great, I'll add that tonight then once I can add a test for it.

[ljharb]

@fastest963 hm, I can't reproduce the failing exit code. If you just comment out the entire if/else block, does it fail with the same code it failed with before?

[jameshartig]

Subshells spawned to execute command substitutions inherit the value of the -e option from the parent shell. When not in posix mode, Bash clears the -e option in such subshells.

@ljharb Ah... so it appears that when source ~/.nvm/nvm.sh from within a script with set -e that applies to the nvm.sh as well (including any commands run in $()). But when just source nvm.sh from the command-line, it doesn't actually return a non-zero exit code.

So actually something like:

--- a/nvm.sh
+++ b/nvm.sh
@@ -1784,7 +1784,7 @@ nvm_supports_source_options() {
   [ "_$(echo 'echo $1' | . /dev/stdin yes 2> /dev/null)" = "_yes" ]
 }

-VERSION="$(nvm_alias default 2>/dev/null)"
+VERSION="$(nvm_alias default 2>/dev/null || echo)"
 if nvm_supports_source_options && [ "_$1" = "_--install" ]; then
   if [ -n "$VERSION" ]; then
     nvm install "$VERSION" >/dev/null

The echo doesn't print anything and therefore still makes VERSION="" but also returns 0.

Tested with:

#!/bin/bash
set -e
source ~/.nvm/nvm.sh
echo "worked"

Prints worked even if you don't have an alias for default. Without the || echo it won't print worked.

[jameshartig]

Awesome! Thanks @ljharb 👍

[ljharb]

Released in v0.24.2 and v0.25.3.