From ce157343002638bfef112adbf811728435a29daf Mon Sep 17 00:00:00 2001 From: Jordan Harband Date: Mon, 29 Jun 2026 14:52:21 -0500 Subject: [PATCH] [Fix] `nvm install`: migrate packages/set alias when target is already installed When `nvm install ` found that `` was already installed, the post-`nvm use` steps (`--reinstall-packages-from`, default packages, and `--alias`/`--default`) were gated on `[ $EXIT_CODE -ne 0 ]`. Since that branch is only entered when `nvm use` succeeded (EXIT_CODE == 0), those conditions were always false, so the steps were silently skipped. The fresh-install branch correctly uses `-eq 0`; mirror that here so `--reinstall-packages-from` actually migrates global packages (and the alias/default get set) when the target version already exists. Includes a regression test covering the already-installed path. Fixes #3858 --- nvm.sh | 6 +- .../install while reinstalling packages | 74 ++++++++++++++++++- 2 files changed, 76 insertions(+), 4 deletions(-) diff --git a/nvm.sh b/nvm.sh index e36b7382..738c5e60 100755 --- a/nvm.sh +++ b/nvm.sh @@ -3736,10 +3736,10 @@ nvm() { nvm install-latest-npm EXIT_CODE=$? fi - if [ $EXIT_CODE -ne 0 ] && [ -z "${SKIP_DEFAULT_PACKAGES-}" ]; then + if [ $EXIT_CODE -eq 0 ] && [ -z "${SKIP_DEFAULT_PACKAGES-}" ]; then nvm_install_default_packages fi - if [ $EXIT_CODE -ne 0 ] && [ -n "${REINSTALL_PACKAGES_FROM-}" ] && [ "_${REINSTALL_PACKAGES_FROM}" != "_N/A" ]; then + if [ $EXIT_CODE -eq 0 ] && [ -n "${REINSTALL_PACKAGES_FROM-}" ] && [ "_${REINSTALL_PACKAGES_FROM}" != "_N/A" ]; then nvm reinstall-packages "${REINSTALL_PACKAGES_FROM}" EXIT_CODE=$? fi @@ -3757,7 +3757,7 @@ nvm() { EXIT_CODE=$? fi - if [ $EXIT_CODE -ne 0 ] && [ -n "${ALIAS-}" ]; then + if [ $EXIT_CODE -eq 0 ] && [ -n "${ALIAS-}" ]; then nvm alias "${ALIAS}" "${provided_version}" EXIT_CODE=$? fi diff --git a/test/installation_node/install while reinstalling packages b/test/installation_node/install while reinstalling packages index b180bcc8..0328eb51 100755 --- a/test/installation_node/install while reinstalling packages +++ b/test/installation_node/install while reinstalling packages @@ -1,6 +1,27 @@ #!/bin/sh -die () { echo "$@" ; exit 1; } +# State touched by the regression section below; cleanup restores it even on failure. +DEFAULT_PACKAGES_FILE="${NVM_DIR}/default-packages" +DEFAULT_PACKAGES_BACKUP="" +DEFAULT_PACKAGES_WROTE="" +TEST_ALIAS="" + +cleanup () { + if [ -n "${DEFAULT_PACKAGES_WROTE}" ]; then + rm -f "${DEFAULT_PACKAGES_FILE}" + DEFAULT_PACKAGES_WROTE="" + fi + if [ -n "${DEFAULT_PACKAGES_BACKUP}" ] && [ -f "${DEFAULT_PACKAGES_BACKUP}" ]; then + mv "${DEFAULT_PACKAGES_BACKUP}" "${DEFAULT_PACKAGES_FILE}" + DEFAULT_PACKAGES_BACKUP="" + fi + if [ -n "${TEST_ALIAS}" ]; then + nvm unalias "${TEST_ALIAS}" > /dev/null 2>&1 || true + TEST_ALIAS="" + fi +} + +die () { echo "$@" ; cleanup ; exit 1; } \. ../../nvm.sh @@ -44,3 +65,54 @@ nvm use --lts=argon node --version | grep v4.9.1 > /dev/null || die "nvm ls --lts=argon didn't use v4.9.1" npm list --global | grep object-is > /dev/null || die "object-is isn't installed" + + +# Regression: `--reinstall-packages-from` must still migrate packages when the +# target version is *already installed*. See https://github.com/nvm-sh/nvm/issues/3858 + +# install a fresh global package on the source (v9.7.0) only +nvm use 9.7.0 +npm install -g is-nan@1.0.0 || die "npm install -g is-nan failed" +npm list --global | grep is-nan > /dev/null || die "is-nan isn't installed on v9.7.0" + +# precondition: the already-installed target (v9.10.0) must not have it yet +nvm use 9.10.0 +npm list --global | grep is-nan > /dev/null && die "is-nan should not be installed on v9.10.0 before reinstall" + +# target is already installed, so this should report so AND still migrate packages +OUTPUT="$(nvm install --reinstall-packages-from=9.7.0 9.10.0 2>&1)" || die "nvm install --reinstall-packages-from=9.7.0 9.10.0 failed: ${OUTPUT}" +nvm_echo "${OUTPUT}" | grep "is already installed" > /dev/null || die "expected 'already installed' message, got: ${OUTPUT}" + +nvm use 9.10.0 +npm list --global | grep is-nan > /dev/null || die "is-nan was not migrated to already-installed v9.10.0" + + +# ...the same is true for default packages: they must install on the already-installed path too. + +# back up any real default-packages file, then point it at a package the target lacks +if [ -f "${DEFAULT_PACKAGES_FILE}" ]; then + DEFAULT_PACKAGES_BACKUP="${DEFAULT_PACKAGES_FILE}.3858.bak" + mv "${DEFAULT_PACKAGES_FILE}" "${DEFAULT_PACKAGES_BACKUP}" || die "could not back up ${DEFAULT_PACKAGES_FILE}" +fi +DEFAULT_PACKAGES_WROTE=1 +nvm_echo 'object-inspect@1.0.2' > "${DEFAULT_PACKAGES_FILE}" + +# precondition: the already-installed target must not have the default package yet +nvm use 9.10.0 +npm list --global | grep object-inspect > /dev/null && die "object-inspect should not be installed on v9.10.0 before default-packages test" + +nvm install 9.10.0 > /dev/null 2>&1 || die "nvm install 9.10.0 (already installed, default-packages) failed" +nvm use 9.10.0 +npm list --global | grep object-inspect > /dev/null || die "default packages were not installed on already-installed v9.10.0" + +# restore the default-packages file before the next assertion +cleanup + + +# ...and `--alias` must be set on the already-installed path too. +TEST_ALIAS='nvm-3858-already-installed' +nvm unalias "${TEST_ALIAS}" > /dev/null 2>&1 || true # ensure a clean precondition +nvm install --alias="${TEST_ALIAS}" 9.10.0 > /dev/null 2>&1 || die "nvm install --alias=${TEST_ALIAS} 9.10.0 (already installed) failed" +TERM=dumb nvm alias "${TEST_ALIAS}" | grep 'v9.10.0' > /dev/null || die "--alias was not set on already-installed v9.10.0" + +cleanup