Skip to content

Don't pollute the START variable#18

Closed
bronson wants to merge 1 commit intonvm-sh:masterfrom
bronson:master
Closed

Don't pollute the START variable#18
bronson wants to merge 1 commit intonvm-sh:masterfrom
bronson:master

Conversation

@bronson
Copy link
Contributor

@bronson bronson commented Feb 14, 2011

It's bad to leak the START variable into the user's environment so do the install steps in a subshell.
Also, don't try to install npm if the node install fails.

This change is mostly indentation, here's the git diff -b:

@@ -36,21 +36,25 @@ nvm()
         nvm help
         return;
       fi
-      START=`pwd`
-      mkdir -p "$NVM_DIR/src" && \
+      if (
+        mkdir -p "$NVM_DIR/src" &&
       cd "$NVM_DIR/src" && \
       wget "http://nodejs.org/dist/node-$2.tar.gz" -N && \
       tar -xzf "node-$2.tar.gz" && \
       cd "node-$2" && \
       ./configure --prefix="$NVM_DIR/$2" && \
       make && \
-      make install && \
+        make install
+        )
+      then
       nvm use $2
       if ! which npm ; then

         echo "Installing npm..."
         curl http://npmjs.org/install.sh | sh
       fi
-      cd $START
+      else
+        echo "nvm: install $2 failed!"
+      fi
     ;;
     "deactivate" )
       if [[ $PATH == *$NVM_DIR/*/bin* ]]; then

also don't try to install npm if node installation fails.
@creationix
Copy link
Collaborator

Not sure why this wasn't closed, but I merged this in a while back.

This pull request was closed.
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

Successfully merging this pull request may close these issues.

2 participants