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

Cannot call nvm exec in bash script with set -e #993

Closed
errm opened this issue Feb 1, 2016 · 22 comments
Closed

Cannot call nvm exec in bash script with set -e #993

errm opened this issue Feb 1, 2016 · 22 comments

Comments

@errm
Copy link

errm commented Feb 1, 2016

If I call nvm exec node foobar.js in a bash script with set -e everything exits early, this is not great for being able to use nvm in an automated build for example . . .

@ljharb
Copy link
Member

ljharb commented Feb 1, 2016

Using inline nonzero return codes is a common practice, and set -e is an intrusive option that isn't compatible with nvm. Rather than using the catchall of -e, I'd recommend explicit error checking and exits where errors are possible.

An alternative is to source nvm.sh prior to setting the -e option, and use nvm use rather than nvm exec.

@joscha
Copy link

joscha commented Mar 17, 2016

For anyone using nvm in a CI environment, where non-zero exit codes are usually used to indicate a problem, I have this little snippet:

export NODE_VERSION="5.9.0"
export NPM_VERSION="3.7.3"
export NVM_DIR="`pwd`/.nvm" # truly local nvm install not needing sudo
mkdir -p $NVM_DIR

curl -o- https://raw.githubusercontent.com/creationix/nvm/v0.31.0/install.sh | bash
[ -s "$NVM_DIR/nvm.sh" ] && . "$NVM_DIR/nvm.sh"  # This loads nvm

nvm install $NODE_VERSION
nvm use --silent $NODE_VERSION

set -e # make the script fail if anything goes wrong from here

export npm_install=$NPM_VERSION
curl -L https://npmjs.org/install.sh | sh

npm install your-module
node `npm bin`/your-module

@ljharb
Copy link
Member

ljharb commented Mar 17, 2016

@joscha is this still a problem if you move the set -e above the sourcing of nvm.sh?

While I do think that set -e is a silly option, and failures should explicitly be checked for, I don't want to make life hard for nvm users who use set -e. If there's still issues with this, and it's possible to solve them in a practical way, I'd like to (which is why the issue is still open)

@joscha
Copy link

joscha commented Mar 17, 2016

Yes, it doesnt't work, because most nvm xxx tasks return with 127 instead of 0. Especially for nvm exec $NODE_VERSION npm test this is dangerous.

@ljharb
Copy link
Member

ljharb commented Mar 17, 2016

Hmm, 127 should only be the exit code when an incorrect command was entered, and the help output goes to stderr. Do you have a specific line where it exits?

@joscha
Copy link

joscha commented Mar 18, 2016

Unfortunately the build logs where I had problems with that (hence why I found this ticket) are gone, so I am not sure which of the commands failed exactly, but I remember that an order like this:

set -e
export NVM_DIR="`pwd`/.nvm"
mkdir -p $NVM_DIR

curl -o- https://raw.githubusercontent.com/creationix/nvm/v0.31.0/install.sh | bash
[ -s "$NVM_DIR/nvm.sh" ] && . "$NVM_DIR/nvm.sh"

nvm install 5.9.0
nvm use --silent 5.9.0

export npm_install=3.7.3
curl -L https://npmjs.org/install.sh | sh

nvm exec --silent 5.9.0 npm install your-module
nvm exec --silent 5.9.0 node index.js

did not work.

@ljharb
Copy link
Member

ljharb commented Mar 20, 2016

Executing this script works as expected with and without a default, with and without an .nvmrc:

#!/usr/bin/env bash
set -e

. ~/.nvm/nvm.sh
echo $(nvm current)

I'm going to close this for now. Please reopen if the issue occurs again and if anyone can be more specific about which command fails.

@ljharb ljharb closed this as completed Mar 20, 2016
@joscha
Copy link

joscha commented Mar 29, 2016

I looked into this again and I think in some of our builds the $NODE_VERSION was empty and the command was this:

nvm exec --silent $NODE_VERSION npm -v

so I believe that it actually only fails when the version is omitted. The OP of this ticket also uses it without a version, so that would make sense.

@ljharb
Copy link
Member

ljharb commented Mar 29, 2016

@joscha when the version is omitted and no .nvmrc is present, it does fail (and should) - is that the situation for you?

@joscha
Copy link

joscha commented Mar 29, 2016

Yes, it fails with exit code 3

@ljharb
Copy link
Member

ljharb commented Mar 29, 2016

Great - that's intentional then :-)

@kncs
Copy link

kncs commented Apr 4, 2016

I don't know if it could help someone but I just solve a similar problem using set -e and nvm into a bash script. The problem was a missing line break into the .nvmrc file.

How to reproduce it :

  1. Create a bash file
#!/bin/bash
set -e
source ~/.nvm/nvm.sh
nvm --version 
nvm use 
  1. Remove line break into the .nvmrc

  2. Launch script and observe.

@ljharb
Copy link
Member

ljharb commented Apr 4, 2016

@kncs hm, you're saying that if the nvmrc file omits a trailing line break (something every file in unix should always have), it breaks? That would be a concrete bug I could fix.

@kncs
Copy link

kncs commented Apr 4, 2016

My bad, of course I wouldn't ask you to fix it, my comment was only to help people that could have similar problems

@ljharb
Copy link
Member

ljharb commented Apr 4, 2016

@kncs i'll try to reproduce (i want to fix it!) but your confirmation would help :-)

@kncs
Copy link

kncs commented Apr 4, 2016

Yes it seems that it breaks something. I join a zip containing a broken nvmrc and the bash script to help you reproduce it.
test.zip

@ljharb
Copy link
Member

ljharb commented Apr 5, 2016

@kncs thanks - i was able to find the line causing the error. Definitely every file ever should have a trailing newline, but, I'll figure out a way to make this not break.

@hedefalk
Copy link

I'm having this problem using drone ci https://github.com/drone/dronehttps://github.com/drone/drone

Drone automatically creates a script with set -e on top of my "user commands" so can't really do anything about it other than || trueon all nvm commands.

@ljharb
Copy link
Member

ljharb commented Aug 26, 2016

@hedefalk if you can narrow down which commands are failing, and ideally using set -x which place they are failing at, please open a new issue for each and I'll tackle them one at a time.

@andyearnshaw
Copy link

andyearnshaw commented Apr 4, 2017

It appears nvm install <version> fails when set -e is used also. Here's an example script I'm using:

#!/bin/sh
set -e   
[ -z "${DEBUG}" ] || set -x

green="$(tput setaf 2)"
reset="$(tput sgr0)"

export NVM_DIR="$HOME/.nvm"

NODE_VERSION="7.8"
BASEDIR="$(dirname "$0")/.."
cd "$(dirname "$0")/.."

SHORT_PIPELINE_VERSION=${PIPELINE_VERSION#*/}

if [ ! -d $NVM_DIR ]; then
    echo $green "\n==> Installing Node Version Manager" $reset
    (
        git clone -n https://github.com/creationix/nvm.git "$NVM_DIR"
        cd "$NVM_DIR"
        git checkout -q `git describe --abbrev=0 --tags --match "v[0-9]*" origin`
    )
fi

. $NVM_DIR/nvm.sh

echo $green "\n==> Installing node" $reset

nvm install $NODE_VERSION

The offending non-zero returning function is nvm_version_greater, called from nvm_version_path. Should I open a new ticket for this?

@ljharb
Copy link
Member

ljharb commented Apr 4, 2017

@andyearnshaw yes, please; a new issue would be great.

@matheusfaustino
Copy link

Executing this script works as expected with and without a default, with and without an .nvmrc:

#!/usr/bin/env bash
set -e

. ~/.nvm/nvm.sh
echo $(nvm current)

I'm going to close this for now. Please reopen if the issue occurs again and if anyone can be more specific about which command fails.

I just wanna point out that this solved my problem with Bitbucket Pipeline using atlassian/ssh-run. I added this part on the script because only when it was running by the Pipeline it couldn't find node installation. Thanks ^^

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

7 participants