Skip to content

Commit

Permalink
[Fix] use: Prepend instead of changing if shadowed by system dirs (f…
Browse files Browse the repository at this point in the history
…ixes #1652)
  • Loading branch information
zeorin authored and ljharb committed Jun 8, 2018
1 parent 0cdc184 commit 90cfb5d
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 2 deletions.
11 changes: 9 additions & 2 deletions nvm.sh
Original file line number Diff line number Diff line change
Expand Up @@ -638,12 +638,19 @@ nvm_change_path() {
elif ! nvm_echo "${1-}" | nvm_grep -q "${NVM_DIR}/[^/]*${2-}" \
&& ! nvm_echo "${1-}" | nvm_grep -q "${NVM_DIR}/versions/[^/]*/[^/]*${2-}"; then
nvm_echo "${3-}${2-}:${1-}"
# if the initial path contains BOTH an nvm path (checked for above) and
# that nvm path is preceded by a system binary path, just prepend the
# supplementary path instead of replacing it.
# https://github.com/creationix/nvm/issues/1652#issuecomment-342571223
elif nvm_echo "${1-}" | nvm_grep -Eq "(^|:)(/usr(/local)?)?${2-}:.*${NVM_DIR}/[^/]*${2-}" \

This comment has been minimized.

Copy link
@dblachut-adb

dblachut-adb Dec 8, 2023

What about ~/.npm-packages/bin? Is there any reason to replace existing path in place instead of always removing it and prepending? The latter would be consistent, would not require additional ifs like this and would work in more cases...

This comment has been minimized.

Copy link
@ljharb

ljharb Dec 8, 2023

Member

what about that? Global packages must NEVER be shared across node installs, so a ~/.npm-packages approach is a bad and harmful practice.

|| nvm_echo "${1-}" | nvm_grep -Eq "(^|:)(/usr(/local)?)?${2-}:.*${NVM_DIR}/versions/[^/]*/[^/]*${2-}"; then
nvm_echo "${3-}${2-}:${1-}"
# use sed to replace the existing nvm path with the supplementary path. This
# preserves the order of the path.
else
nvm_echo "${1-}" | command sed \
-e "s#${NVM_DIR}/[^/]*${2-}[^:]*#${3-}${2-}#g" \
-e "s#${NVM_DIR}/versions/[^/]*/[^/]*${2-}[^:]*#${3-}${2-}#g"
-e "s#${NVM_DIR}/[^/]*${2-}[^:]*#${3-}${2-}#" \
-e "s#${NVM_DIR}/versions/[^/]*/[^/]*${2-}[^:]*#${3-}${2-}#"
fi
}

Expand Down
14 changes: 14 additions & 0 deletions test/fast/Unit tests/nvm_change_path
Original file line number Diff line number Diff line change
Expand Up @@ -41,3 +41,17 @@ NEW_PATH=`nvm_change_path "$EMPTY_PATH" "/bin" "$NVM_DIR/versions/node/v7.1.0"`
NEW_PATH=`nvm_change_path "$EMPTY_PATH" "/bin" "$NVM_DIR/v0.1.2"`

[ "$NEW_PATH" = "$NVM_DIR/v0.1.2/bin" ] || die "Not correctly prepended: $NEW_PATH "


# https://github.com/creationix/nvm/issues/1652#issuecomment-342571223
MAC_OS_NESTED_SESSION_PATH=/usr/bin:/usr/local/bin:$NVM_DIR/versions/node/v4.5.0/bin

# New version dir
NEW_PATH=`nvm_change_path "$MAC_OS_NESTED_SESSION_PATH" "/bin" "$NVM_DIR/versions/node/v7.1.0"`

[ "$NEW_PATH" = "$NVM_DIR/versions/node/v7.1.0/bin:/usr/bin:/usr/local/bin:$NVM_DIR/versions/node/v4.5.0/bin" ] || die "Not correctly changed: $NEW_PATH "

# Old version dir
NEW_PATH=`nvm_change_path "$MAC_OS_NESTED_SESSION_PATH" "/bin" "$NVM_DIR/v0.1.2"`

[ "$NEW_PATH" = "$NVM_DIR/v0.1.2/bin:/usr/bin:/usr/local/bin:$NVM_DIR/versions/node/v4.5.0/bin" ] || die "Not correctly changed: $NEW_PATH "

0 comments on commit 90cfb5d

Please sign in to comment.