From 43e83ab3e58fb2a61301459afc167d1a739c42d5 Mon Sep 17 00:00:00 2001 From: mondaychen Date: Thu, 29 Oct 2026 19:34:22 -0400 Subject: [PATCH 1/3] fix(shim): move npm global links to the reinstalled Node.js version When the active Node.js version changes, `npm install -g ` installs a new copy into the new Node.js npm prefix. The `~/.vite-plus/bin` link kept the old target because Vite+ replaced a link only when a different package owned it. The command then kept running the old version. Replace the link also when the same package owns it but the link does not point to the binary in the current npm bin dir, record the new Node.js version, and tell the user that the link moved. Fixes #3279 Co-Authored-By: Claude Opus 5.5 --- .../.node-version | 1 + .../pkg/cli.js | 2 + .../pkg/package.json | 7 ++ .../snapshots.toml | 19 ++++ .../npm_global_install_node_version_relink.md | 62 +++++++++++++ crates/vp_global_cli/src/shim/dispatch.rs | 88 +++++++++++++++++-- 6 files changed, 174 insertions(+), 5 deletions(-) create mode 100644 crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_install_node_version_relink/.node-version create mode 100644 crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_install_node_version_relink/pkg/cli.js create mode 100644 crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_install_node_version_relink/pkg/package.json create mode 100644 crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_install_node_version_relink/snapshots.toml create mode 100644 crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_install_node_version_relink/snapshots/npm_global_install_node_version_relink.md diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_install_node_version_relink/.node-version b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_install_node_version_relink/.node-version new file mode 100644 index 0000000000..ce62013c39 --- /dev/null +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_install_node_version_relink/.node-version @@ -0,0 +1 @@ +24.22.1 diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_install_node_version_relink/pkg/cli.js b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_install_node_version_relink/pkg/cli.js new file mode 100644 index 0000000000..f7c4cc908e --- /dev/null +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_install_node_version_relink/pkg/cli.js @@ -0,0 +1,2 @@ +#!/usr/bin/env node +console.log('relink-cli works'); diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_install_node_version_relink/pkg/package.json b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_install_node_version_relink/pkg/package.json new file mode 100644 index 0000000000..f5dc747e8d --- /dev/null +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_install_node_version_relink/pkg/package.json @@ -0,0 +1,7 @@ +{ + "name": "npm-global-relink-pkg", + "version": "1.0.0", + "bin": { + "npm-global-relink-cli": "./cli.js" + } +} diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_install_node_version_relink/snapshots.toml b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_install_node_version_relink/snapshots.toml new file mode 100644 index 0000000000..d4c5d322d2 --- /dev/null +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_install_node_version_relink/snapshots.toml @@ -0,0 +1,19 @@ +# Each Vite+-managed Node.js version has its own npm global prefix. The runner +# seeds the runtime directory from a shared cache, so each step sets a +# case-local prefix per Node.js version instead. +[[case]] +name = "npm_global_install_node_version_relink" +vp = "global" +skip-platforms = ["windows"] +steps = [ + { tty = false, argv = ["npm", "install", "-g", "--ignore-scripts", "./pkg"], envs = [["NPM_CONFIG_PREFIX", "${workspace}/node-24"]], comment = "Install with Node.js 24 (creates the link)", continue-on-failure = true }, + { argv = ["vpt", "grep-file", "$VP_HOME/bins/npm-global-relink-cli.json", "\"nodeVersion\": \"24.22.1\""], comment = "BinConfig records Node.js 24", continue-on-failure = true }, + { tty = false, argv = ["npm", "install", "-g", "--ignore-scripts", "./pkg"], envs = [["VP_NODE_VERSION", "26.12.1"], ["NPM_CONFIG_PREFIX", "${workspace}/node-26"]], comment = "Install again with Node.js 26 (moves the link to the new prefix)", continue-on-failure = true }, + { argv = ["vpt", "grep-file", "$VP_HOME/bins/npm-global-relink-cli.json", "\"nodeVersion\": \"26.12.1\""], comment = "BinConfig records Node.js 26", continue-on-failure = true }, + { argv = ["vpt", "rm", "-rf", "node-24"], comment = "Remove the Node.js 24 copy: the command must not depend on it", continue-on-failure = true }, + { argv = ["npm-global-relink-cli"], comment = "Should be callable via the moved link", continue-on-failure = true }, + { tty = false, argv = ["npm", "install", "-g", "--ignore-scripts", "./pkg"], envs = [["VP_NODE_VERSION", "26.12.1"], ["NPM_CONFIG_PREFIX", "${workspace}/node-26"]], comment = "Install again with Node.js 26 (the link is current, so no message)" }, +] +after = [ + { tty = false, argv = ["npm", "uninstall", "-g", "--ignore-scripts", "npm-global-relink-pkg"], envs = [["VP_NODE_VERSION", "26.12.1"], ["NPM_CONFIG_PREFIX", "${workspace}/node-26"]], continue-on-failure = true }, +] diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_install_node_version_relink/snapshots/npm_global_install_node_version_relink.md b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_install_node_version_relink/snapshots/npm_global_install_node_version_relink.md new file mode 100644 index 0000000000..3049a9035a --- /dev/null +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_install_node_version_relink/snapshots/npm_global_install_node_version_relink.md @@ -0,0 +1,62 @@ +# npm_global_install_node_version_relink + +## `NPM_CONFIG_PREFIX=${workspace}/node-24 npm install -g --ignore-scripts ./pkg` + +Install with Node.js 24 (creates the link) + +``` + +added 1 package in +Linked 'npm-global-relink-cli' to /.vite-plus/bin/npm-global-relink-cli +``` + +## `vpt grep-file $VP_HOME/bins/npm-global-relink-cli.json '"nodeVersion": "24.22.1"'` + +BinConfig records Node.js 24 + +``` +/.vite-plus/bins/npm-global-relink-cli.json: found "\"nodeVersion\": \"24.22.1\"" +``` + +## `VP_NODE_VERSION=26.12.1 NPM_CONFIG_PREFIX=${workspace}/node-26 npm install -g --ignore-scripts ./pkg` + +Install again with Node.js 26 (moves the link to the new prefix) + +``` + +added 1 package in +note: Moving 'npm-global-relink-cli' from Node.js to 26.12.1 +Linked 'npm-global-relink-cli' to /.vite-plus/bin/npm-global-relink-cli +``` + +## `vpt grep-file $VP_HOME/bins/npm-global-relink-cli.json '"nodeVersion": "26.12.1"'` + +BinConfig records Node.js 26 + +``` +/.vite-plus/bins/npm-global-relink-cli.json: found "\"nodeVersion\": \"26.12.1\"" +``` + +## `vpt rm -rf node-24` + +Remove the Node.js 24 copy: the command must not depend on it + +``` +``` + +## `npm-global-relink-cli` + +Should be callable via the moved link + +``` +relink-cli works +``` + +## `VP_NODE_VERSION=26.12.1 NPM_CONFIG_PREFIX=${workspace}/node-26 npm install -g --ignore-scripts ./pkg` + +Install again with Node.js 26 (the link is current, so no message) + +``` + +up to date in +``` diff --git a/crates/vp_global_cli/src/shim/dispatch.rs b/crates/vp_global_cli/src/shim/dispatch.rs index 97555c7642..ebac7c1d66 100644 --- a/crates/vp_global_cli/src/shim/dispatch.rs +++ b/crates/vp_global_cli/src/shim/dispatch.rs @@ -291,16 +291,27 @@ fn check_npm_global_install_result( if config.source == BinSource::Vp { // Managed by vp install -g — warn about the conflict managed_conflicts.push((bin_name, config.package.clone())); - } else if config.source == BinSource::Npm && config.package != package_name { - // Link exists from a different npm package — recreate link for new owner. - // The old symlink points at the previous package's binary; we must - // replace it so it resolves to the new package's binary in npm's bin dir. + } else if config.source == BinSource::Npm { + // Recreate the link when a different npm package now owns the binary, + // or when a reinstall under another Node.js version put the binary + // in a different npm bin dir. Otherwise the link keeps launching the + // previous binary. #[cfg(unix)] let source_path = npm_bin_dir.join(&bin_name); #[cfg(windows)] let source_path = npm_bin_dir.join(vt_str::format!("{bin_name}.cmd")); - if source_path.as_path().exists() { + let same_owner = config.package == package_name; + if source_path.as_path().exists() + && !(same_owner + && npm_bin_link_targets(&bin_dir, &bin_name, &source_path)) + { + if same_owner && config.node_version != node_version { + output::note(&vt_str::format!( + "Moving '{bin_name}' from Node.js {} to {node_version}", + config.node_version + )); + } let _ = std::fs::remove_file(shim_path.as_path()); create_bin_link( &bin_dir, @@ -424,6 +435,27 @@ fn link_missing_npm_bins( } } +/// Whether the existing npm bin link in `bin_dir` launches `source_path`. +fn npm_bin_link_targets( + bin_dir: &AbsolutePath, + bin_name: &str, + source_path: &AbsolutePath, +) -> bool { + #[cfg(unix)] + { + std::fs::read_link(bin_dir.join(bin_name).as_path()) + .is_ok_and(|target| target == source_path.as_path()) + } + + #[cfg(windows)] + { + // `create_bin_link` writes the quoted source path into the `.cmd` wrapper. + let quoted_source = vt_str::format!("\"{}\"", source_path.as_path().display()); + std::fs::read_to_string(bin_dir.join(vt_str::format!("{bin_name}.cmd")).as_path()) + .is_ok_and(|content| content.contains(quoted_source.as_str())) + } +} + /// Create a bin link for a binary and record it via BinConfig. pub(crate) fn create_bin_link( bin_dir: &AbsolutePath, @@ -1925,6 +1957,52 @@ mod tests { }); } + #[test] + fn test_npm_reinstall_under_new_node_version_moves_link() { + let temp = TempDir::new().unwrap(); + // One npm prefix per Node.js version, as with Vite+-managed runtimes. + let install = |version: &str| { + let prefix = AbsolutePathBuf::new(temp.path().join("node").join(version)).unwrap(); + let package_dir = prefix.join("lib").join("node_modules").join("test-package"); + std::fs::create_dir_all(&package_dir).unwrap(); + std::fs::write( + package_dir.join("package.json"), + r#"{"name":"test-package","bin":{"test-cli":"cli.js"}}"#, + ) + .unwrap(); + #[cfg(unix)] + let source = prefix.join("bin").join("test-cli"); + #[cfg(windows)] + let source = prefix.join("test-cli.cmd"); + std::fs::create_dir_all(source.parent().unwrap()).unwrap(); + std::fs::write(&source, "").unwrap(); + (prefix, source) + }; + let (_, old_source) = install("22.18.0"); + let (new_prefix, new_source) = install("24.11.0"); + + vp_shared::EnvConfig::with_vars([(env_vars::VP_HOME, temp.path())], |_| { + let bin_dir = config::get_bin_dir().unwrap(); + std::fs::create_dir_all(&bin_dir).unwrap(); + create_bin_link(&bin_dir, "test-cli", &old_source, "test-package", "22.18.0"); + assert!(npm_bin_link_targets(&bin_dir, "test-cli", &old_source)); + + let packages = s(&["test-package"]); + check_npm_global_install_result(&packages, None, &new_prefix, &new_prefix, "24.11.0"); + assert!(npm_bin_link_targets(&bin_dir, "test-cli", &new_source)); + assert!(!npm_bin_link_targets(&bin_dir, "test-cli", &old_source)); + let config = BinConfig::load_sync("test-cli").unwrap().unwrap(); + assert_eq!(config.package, "test-package"); + assert_eq!(config.node_version, "24.11.0"); + + // A repeat install under the same Node.js version keeps the link as is. + let metadata = std::fs::read(BinConfig::path("test-cli").unwrap()).unwrap(); + check_npm_global_install_result(&packages, None, &new_prefix, &new_prefix, "24.11.0"); + assert!(npm_bin_link_targets(&bin_dir, "test-cli", &new_source)); + assert_eq!(std::fs::read(BinConfig::path("test-cli").unwrap()).unwrap(), metadata); + }); + } + #[test] fn test_npm_uninstall_preserves_links_until_package_is_removed() { let temp = TempDir::new().unwrap(); From ef2b9e76da6a92506a89d302ab297deaa66704c6 Mon Sep 17 00:00:00 2001 From: mondaychen Date: Fri, 30 Oct 2026 10:47:29 -0400 Subject: [PATCH 2/3] fix(shim): keep npm global links unchanged after a dry-run install A dry run changes no files in the npm prefix, but the post-install hook still saw an existing binary in that prefix and could move the link to it. With the same package in two prefixes, `npm install -g --dry-run` into the older prefix moved the command back to the older version. Skip the post-install link step when npm's effective `dry-run` setting is on. A CLI flag wins; otherwise `npm config get dry-run` reports the value from the environment and npmrc. Co-Authored-By: Claude Opus 5.5 --- .../snapshots.toml | 3 + .../npm_global_install_node_version_relink.md | 26 +++++++ crates/vp_global_cli/src/shim/dispatch.rs | 73 ++++++++++++++++++- 3 files changed, 101 insertions(+), 1 deletion(-) diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_install_node_version_relink/snapshots.toml b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_install_node_version_relink/snapshots.toml index d4c5d322d2..8a14cd94c9 100644 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_install_node_version_relink/snapshots.toml +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_install_node_version_relink/snapshots.toml @@ -10,6 +10,9 @@ steps = [ { argv = ["vpt", "grep-file", "$VP_HOME/bins/npm-global-relink-cli.json", "\"nodeVersion\": \"24.22.1\""], comment = "BinConfig records Node.js 24", continue-on-failure = true }, { tty = false, argv = ["npm", "install", "-g", "--ignore-scripts", "./pkg"], envs = [["VP_NODE_VERSION", "26.12.1"], ["NPM_CONFIG_PREFIX", "${workspace}/node-26"]], comment = "Install again with Node.js 26 (moves the link to the new prefix)", continue-on-failure = true }, { argv = ["vpt", "grep-file", "$VP_HOME/bins/npm-global-relink-cli.json", "\"nodeVersion\": \"26.12.1\""], comment = "BinConfig records Node.js 26", continue-on-failure = true }, + { tty = false, argv = ["npm", "install", "-g", "--ignore-scripts", "--dry-run", "./pkg"], envs = [["NPM_CONFIG_PREFIX", "${workspace}/node-24"]], comment = "Dry run with Node.js 24, whose prefix still has the package (the link stays on Node.js 26)", continue-on-failure = true }, + { tty = false, argv = ["npm", "install", "-g", "--ignore-scripts", "./pkg"], envs = [["NPM_CONFIG_PREFIX", "${workspace}/node-24"], ["npm_config_dry_run", "true"]], comment = "Dry run from the npm environment config (the link stays on Node.js 26)", continue-on-failure = true }, + { argv = ["vpt", "grep-file", "$VP_HOME/bins/npm-global-relink-cli.json", "\"nodeVersion\": \"26.12.1\""], comment = "BinConfig still records Node.js 26", continue-on-failure = true }, { argv = ["vpt", "rm", "-rf", "node-24"], comment = "Remove the Node.js 24 copy: the command must not depend on it", continue-on-failure = true }, { argv = ["npm-global-relink-cli"], comment = "Should be callable via the moved link", continue-on-failure = true }, { tty = false, argv = ["npm", "install", "-g", "--ignore-scripts", "./pkg"], envs = [["VP_NODE_VERSION", "26.12.1"], ["NPM_CONFIG_PREFIX", "${workspace}/node-26"]], comment = "Install again with Node.js 26 (the link is current, so no message)" }, diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_install_node_version_relink/snapshots/npm_global_install_node_version_relink.md b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_install_node_version_relink/snapshots/npm_global_install_node_version_relink.md index 3049a9035a..ae889fac13 100644 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_install_node_version_relink/snapshots/npm_global_install_node_version_relink.md +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_install_node_version_relink/snapshots/npm_global_install_node_version_relink.md @@ -37,6 +37,32 @@ BinConfig records Node.js 26 /.vite-plus/bins/npm-global-relink-cli.json: found "\"nodeVersion\": \"26.12.1\"" ``` +## `NPM_CONFIG_PREFIX=${workspace}/node-24 npm install -g --ignore-scripts --dry-run ./pkg` + +Dry run with Node.js 24, whose prefix still has the package (the link stays on Node.js 26) + +``` + +up to date in +``` + +## `NPM_CONFIG_PREFIX=${workspace}/node-24 npm_config_dry_run=true npm install -g --ignore-scripts ./pkg` + +Dry run from the npm environment config (the link stays on Node.js 26) + +``` + +up to date in +``` + +## `vpt grep-file $VP_HOME/bins/npm-global-relink-cli.json '"nodeVersion": "26.12.1"'` + +BinConfig still records Node.js 26 + +``` +/.vite-plus/bins/npm-global-relink-cli.json: found "\"nodeVersion\": \"26.12.1\"" +``` + ## `vpt rm -rf node-24` Remove the Node.js 24 copy: the command must not depend on it diff --git a/crates/vp_global_cli/src/shim/dispatch.rs b/crates/vp_global_cli/src/shim/dispatch.rs index ebac7c1d66..6eb72c1cfe 100644 --- a/crates/vp_global_cli/src/shim/dispatch.rs +++ b/crates/vp_global_cli/src/shim/dispatch.rs @@ -205,6 +205,51 @@ fn get_npm_global_prefix( node_dir.clone() } +/// Whether npm's effective `dry-run` setting is enabled for this command. +/// +/// A CLI flag wins. Otherwise `npm config get dry-run` reports the value from +/// the environment and `.npmrc`. If npm cannot report it, assume a real install. +fn npm_dry_run_enabled(args: &[String], npm_path: &AbsolutePath, env: &ToolPathEnv) -> bool { + if let Some(dry_run) = npm_dry_run_from_args(args) { + return dry_run; + } + std::process::Command::new(npm_path.as_path()) + .args(["config", "get", "dry-run"]) + .envs(env.clone().into_envs()) + .output() + .ok() + .filter(|output| output.status.success()) + .and_then(|output| std::str::from_utf8(&output.stdout).ok().map(npm_config_bool)) + .unwrap_or(false) +} + +/// The last `--dry-run`, `--dry-run=`, or `--no-dry-run` flag before `--`. +fn npm_dry_run_from_args(args: &[String]) -> Option { + let mut dry_run = None; + for arg in args.iter().take_while(|arg| *arg != "--") { + if arg == "--dry-run" { + dry_run = Some(true); + } else if arg == "--no-dry-run" { + dry_run = Some(false); + } else if let Some(value) = arg.strip_prefix("--dry-run=") { + dry_run = Some(npm_config_bool(value)); + } + } + dry_run +} + +/// Parse a boolean npm config value the way npm's option parser (nopt) does: +/// numbers are true when nonzero, `false` and `null` are false, and any other +/// string is true. `npm config get` prints environment values unparsed, such +/// as `1` for `NPM_CONFIG_DRY_RUN=1`. +fn npm_config_bool(value: &str) -> bool { + let value = value.trim(); + if let Ok(number) = value.parse::() { + return number != 0.0; + } + !value.is_empty() && value != "false" && value != "null" +} + /// After npm install -g completes, check if installed binaries are on PATH. /// /// First determines the actual npm global bin directory (which may differ from the @@ -948,7 +993,8 @@ pub async fn dispatch(tool: &str, args: &[String], env: ToolPathEnv) -> i32 { if tool == "npm" { if let Some(parsed) = parse_npm_global_install(args) { let exit_code = exec::spawn_tool(&tool_path, args, child_env.clone()); - if exit_code == 0 { + // A dry run changes no files in the npm prefix, so the links stay as they are. + if exit_code == 0 && !npm_dry_run_enabled(args, &tool_path, &child_env) { let node_dir = node_prefix_from_binary(&node_path); let node_version = resolution.as_ref().map_or_else( || read_node_version(&node_path).unwrap_or_else(|| "unknown".into()), @@ -1957,6 +2003,31 @@ mod tests { }); } + #[test] + fn test_npm_dry_run_from_args() { + assert_eq!(npm_dry_run_from_args(&s(&["install", "-g", "pkg"])), None); + assert_eq!(npm_dry_run_from_args(&s(&["install", "-g", "--dry-run", "pkg"])), Some(true)); + assert_eq!(npm_dry_run_from_args(&s(&["i", "-g", "pkg", "--dry-run=false"])), Some(false)); + assert_eq!(npm_dry_run_from_args(&s(&["i", "-g", "--dry-run=1", "pkg"])), Some(true)); + // The last flag wins, as in npm. + assert_eq!( + npm_dry_run_from_args(&s(&["i", "-g", "--dry-run", "--no-dry-run", "pkg"])), + Some(false) + ); + // Arguments after `--` are not npm flags. + assert_eq!(npm_dry_run_from_args(&s(&["i", "-g", "pkg", "--", "--dry-run"])), None); + } + + #[test] + fn test_npm_config_bool() { + for value in ["true", "1", "yes", "true\n"] { + assert!(npm_config_bool(value), "{value:?} should be true"); + } + for value in ["false", "0", "null", "", "false\n"] { + assert!(!npm_config_bool(value), "{value:?} should be false"); + } + } + #[test] fn test_npm_reinstall_under_new_node_version_moves_link() { let temp = TempDir::new().unwrap(); From ca167240b02400c4bc5ef0375249aedb19f74c3e Mon Sep 17 00:00:00 2001 From: mondaychen Date: Mon, 2 Nov 2026 11:46:12 -0500 Subject: [PATCH 3/3] fix(shim): read npm's dry-run setting with the install's own arguments The dry-run probe ran `npm config get dry-run` without the install's arguments, and Vite+ parsed the dry-run flags itself. Both could disagree with npm: - Without `-g`, the probe failed with ENOWORKSPACES in a workspace child, and read a project .npmrc that `install -g` ignores. - An explicit `--userconfig` file was not read. - `--dry-run false`, `--no-dry-run false`, and the `--dry` abbreviation were parsed differently from npm. Pass the install's arguments to the probe so npm applies the same global mode, config files, and flag parsing. The extra positionals become extra keys, so read the `dry-run=` line. When npm cannot report the setting, leave the links unchanged and tell the user, instead of assuming a real install. Co-Authored-By: Claude Opus 5.5 --- .../dry-run.npmrc | 1 + .../project/.npmrc | 1 + .../project/package.json | 4 + .../snapshots.toml | 7 ++ .../npm_global_install_node_version_relink.md | 65 +++++++++++++++ .../ws/package.json | 5 ++ .../ws/packages/child/package.json | 4 + crates/vp_global_cli/src/shim/dispatch.rs | 82 ++++++++++--------- 8 files changed, 131 insertions(+), 38 deletions(-) create mode 100644 crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_install_node_version_relink/dry-run.npmrc create mode 100644 crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_install_node_version_relink/project/.npmrc create mode 100644 crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_install_node_version_relink/project/package.json create mode 100644 crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_install_node_version_relink/ws/package.json create mode 100644 crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_install_node_version_relink/ws/packages/child/package.json diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_install_node_version_relink/dry-run.npmrc b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_install_node_version_relink/dry-run.npmrc new file mode 100644 index 0000000000..13e3e4991e --- /dev/null +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_install_node_version_relink/dry-run.npmrc @@ -0,0 +1 @@ +dry-run=true diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_install_node_version_relink/project/.npmrc b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_install_node_version_relink/project/.npmrc new file mode 100644 index 0000000000..13e3e4991e --- /dev/null +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_install_node_version_relink/project/.npmrc @@ -0,0 +1 @@ +dry-run=true diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_install_node_version_relink/project/package.json b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_install_node_version_relink/project/package.json new file mode 100644 index 0000000000..9759d6f3bc --- /dev/null +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_install_node_version_relink/project/package.json @@ -0,0 +1,4 @@ +{ + "name": "npm-global-relink-project", + "private": true +} diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_install_node_version_relink/snapshots.toml b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_install_node_version_relink/snapshots.toml index 8a14cd94c9..a5c60ae2a5 100644 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_install_node_version_relink/snapshots.toml +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_install_node_version_relink/snapshots.toml @@ -12,7 +12,14 @@ steps = [ { argv = ["vpt", "grep-file", "$VP_HOME/bins/npm-global-relink-cli.json", "\"nodeVersion\": \"26.12.1\""], comment = "BinConfig records Node.js 26", continue-on-failure = true }, { tty = false, argv = ["npm", "install", "-g", "--ignore-scripts", "--dry-run", "./pkg"], envs = [["NPM_CONFIG_PREFIX", "${workspace}/node-24"]], comment = "Dry run with Node.js 24, whose prefix still has the package (the link stays on Node.js 26)", continue-on-failure = true }, { tty = false, argv = ["npm", "install", "-g", "--ignore-scripts", "./pkg"], envs = [["NPM_CONFIG_PREFIX", "${workspace}/node-24"], ["npm_config_dry_run", "true"]], comment = "Dry run from the npm environment config (the link stays on Node.js 26)", continue-on-failure = true }, + { tty = false, argv = ["npm", "install", "-g", "--ignore-scripts", "./pkg", "--no-dry-run", "false"], envs = [["NPM_CONFIG_PREFIX", "${workspace}/node-24"]], comment = "npm reads `--no-dry-run false` as a dry run (the link stays on Node.js 26)", continue-on-failure = true }, + { tty = false, argv = ["npm", "install", "-g", "--ignore-scripts", "--userconfig", "dry-run.npmrc", "./pkg"], envs = [["NPM_CONFIG_PREFIX", "${workspace}/node-24"]], comment = "Dry run from an explicit --userconfig file (the link stays on Node.js 26)", continue-on-failure = true }, + { tty = false, cwd = "ws/packages/child", argv = ["npm", "install", "-g", "--ignore-scripts", "../../../pkg"], envs = [["NPM_CONFIG_PREFIX", "${workspace}/../../../node-24"], ["npm_config_dry_run", "true"]], comment = "Dry run from an npm workspace child (the link stays on Node.js 26)", continue-on-failure = true }, { argv = ["vpt", "grep-file", "$VP_HOME/bins/npm-global-relink-cli.json", "\"nodeVersion\": \"26.12.1\""], comment = "BinConfig still records Node.js 26", continue-on-failure = true }, + { tty = false, cwd = "project", argv = ["npm", "install", "-g", "--ignore-scripts", "../pkg"], envs = [["NPM_CONFIG_PREFIX", "${workspace}/../node-24"]], comment = "Real install with Node.js 24: `install -g` ignores the project .npmrc that sets dry-run (moves the link)", continue-on-failure = true }, + { argv = ["vpt", "grep-file", "$VP_HOME/bins/npm-global-relink-cli.json", "\"nodeVersion\": \"24.22.1\""], comment = "BinConfig records Node.js 24", continue-on-failure = true }, + { tty = false, argv = ["npm", "install", "-g", "--ignore-scripts", "./pkg", "--dry-run", "false"], envs = [["VP_NODE_VERSION", "26.12.1"], ["NPM_CONFIG_PREFIX", "${workspace}/node-26"]], comment = "npm reads `--dry-run false` as a real install with Node.js 26 (moves the link)", continue-on-failure = true }, + { argv = ["vpt", "grep-file", "$VP_HOME/bins/npm-global-relink-cli.json", "\"nodeVersion\": \"26.12.1\""], comment = "BinConfig records Node.js 26", continue-on-failure = true }, { argv = ["vpt", "rm", "-rf", "node-24"], comment = "Remove the Node.js 24 copy: the command must not depend on it", continue-on-failure = true }, { argv = ["npm-global-relink-cli"], comment = "Should be callable via the moved link", continue-on-failure = true }, { tty = false, argv = ["npm", "install", "-g", "--ignore-scripts", "./pkg"], envs = [["VP_NODE_VERSION", "26.12.1"], ["NPM_CONFIG_PREFIX", "${workspace}/node-26"]], comment = "Install again with Node.js 26 (the link is current, so no message)" }, diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_install_node_version_relink/snapshots/npm_global_install_node_version_relink.md b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_install_node_version_relink/snapshots/npm_global_install_node_version_relink.md index ae889fac13..3d370a293e 100644 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_install_node_version_relink/snapshots/npm_global_install_node_version_relink.md +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_install_node_version_relink/snapshots/npm_global_install_node_version_relink.md @@ -55,6 +55,33 @@ Dry run from the npm environment config (the link stays on Node.js 26) up to date in ``` +## `NPM_CONFIG_PREFIX=${workspace}/node-24 npm install -g --ignore-scripts ./pkg --no-dry-run false` + +npm reads `--no-dry-run false` as a dry run (the link stays on Node.js 26) + +``` + +up to date in +``` + +## `NPM_CONFIG_PREFIX=${workspace}/node-24 npm install -g --ignore-scripts --userconfig dry-run.npmrc ./pkg` + +Dry run from an explicit --userconfig file (the link stays on Node.js 26) + +``` + +up to date in +``` + +## `cd ws/packages/child && NPM_CONFIG_PREFIX=${workspace}/../../../node-24 npm_config_dry_run=true npm install -g --ignore-scripts ../../../pkg` + +Dry run from an npm workspace child (the link stays on Node.js 26) + +``` + +up to date in +``` + ## `vpt grep-file $VP_HOME/bins/npm-global-relink-cli.json '"nodeVersion": "26.12.1"'` BinConfig still records Node.js 26 @@ -63,6 +90,44 @@ BinConfig still records Node.js 26 /.vite-plus/bins/npm-global-relink-cli.json: found "\"nodeVersion\": \"26.12.1\"" ``` +## `cd project && NPM_CONFIG_PREFIX=${workspace}/../node-24 npm install -g --ignore-scripts ../pkg` + +Real install with Node.js 24: `install -g` ignores the project .npmrc that sets dry-run (moves the link) + +``` + +up to date in +note: Moving 'npm-global-relink-cli' from Node.js to 24.22.1 +Linked 'npm-global-relink-cli' to /.vite-plus/bin/npm-global-relink-cli +``` + +## `vpt grep-file $VP_HOME/bins/npm-global-relink-cli.json '"nodeVersion": "24.22.1"'` + +BinConfig records Node.js 24 + +``` +/.vite-plus/bins/npm-global-relink-cli.json: found "\"nodeVersion\": \"24.22.1\"" +``` + +## `VP_NODE_VERSION=26.12.1 NPM_CONFIG_PREFIX=${workspace}/node-26 npm install -g --ignore-scripts ./pkg --dry-run false` + +npm reads `--dry-run false` as a real install with Node.js 26 (moves the link) + +``` + +up to date in +note: Moving 'npm-global-relink-cli' from Node.js to 26.12.1 +Linked 'npm-global-relink-cli' to /.vite-plus/bin/npm-global-relink-cli +``` + +## `vpt grep-file $VP_HOME/bins/npm-global-relink-cli.json '"nodeVersion": "26.12.1"'` + +BinConfig records Node.js 26 + +``` +/.vite-plus/bins/npm-global-relink-cli.json: found "\"nodeVersion\": \"26.12.1\"" +``` + ## `vpt rm -rf node-24` Remove the Node.js 24 copy: the command must not depend on it diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_install_node_version_relink/ws/package.json b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_install_node_version_relink/ws/package.json new file mode 100644 index 0000000000..d6755c91d9 --- /dev/null +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_install_node_version_relink/ws/package.json @@ -0,0 +1,5 @@ +{ + "name": "npm-global-relink-workspace", + "private": true, + "workspaces": ["packages/*"] +} diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_install_node_version_relink/ws/packages/child/package.json b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_install_node_version_relink/ws/packages/child/package.json new file mode 100644 index 0000000000..4362bf04c6 --- /dev/null +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_install_node_version_relink/ws/packages/child/package.json @@ -0,0 +1,4 @@ +{ + "name": "npm-global-relink-child", + "version": "1.0.0" +} diff --git a/crates/vp_global_cli/src/shim/dispatch.rs b/crates/vp_global_cli/src/shim/dispatch.rs index 6eb72c1cfe..67fea3150e 100644 --- a/crates/vp_global_cli/src/shim/dispatch.rs +++ b/crates/vp_global_cli/src/shim/dispatch.rs @@ -205,37 +205,34 @@ fn get_npm_global_prefix( node_dir.clone() } -/// Whether npm's effective `dry-run` setting is enabled for this command. +/// Whether npm's effective `dry-run` setting was enabled for an install, or +/// `None` when npm cannot report it. /// -/// A CLI flag wins. Otherwise `npm config get dry-run` reports the value from -/// the environment and `.npmrc`. If npm cannot report it, assume a real install. -fn npm_dry_run_enabled(args: &[String], npm_path: &AbsolutePath, env: &ToolPathEnv) -> bool { - if let Some(dry_run) = npm_dry_run_from_args(args) { - return dry_run; - } - std::process::Command::new(npm_path.as_path()) +/// The probe receives the install's own arguments, so npm applies the same +/// global mode, config files (`--userconfig`, global-only npmrc lookup), and +/// flag parsing (`--dry-run false`, `--no-dry-run`, abbreviations) as it did +/// for the install. The subcommand and package specs become extra keys, so +/// npm prints one `key=value` line per key, starting with `dry-run`. +fn npm_dry_run_enabled( + args: &[String], + npm_path: &AbsolutePath, + env: &ToolPathEnv, +) -> Option { + let output = std::process::Command::new(npm_path.as_path()) .args(["config", "get", "dry-run"]) + .args(args) .envs(env.clone().into_envs()) .output() - .ok() - .filter(|output| output.status.success()) - .and_then(|output| std::str::from_utf8(&output.stdout).ok().map(npm_config_bool)) - .unwrap_or(false) + .ok()?; + if !output.status.success() { + return None; + } + npm_dry_run_from_config_output(std::str::from_utf8(&output.stdout).ok()?) } -/// The last `--dry-run`, `--dry-run=`, or `--no-dry-run` flag before `--`. -fn npm_dry_run_from_args(args: &[String]) -> Option { - let mut dry_run = None; - for arg in args.iter().take_while(|arg| *arg != "--") { - if arg == "--dry-run" { - dry_run = Some(true); - } else if arg == "--no-dry-run" { - dry_run = Some(false); - } else if let Some(value) = arg.strip_prefix("--dry-run=") { - dry_run = Some(npm_config_bool(value)); - } - } - dry_run +/// Read the `dry-run=` line that `npm config get dry-run ` prints. +fn npm_dry_run_from_config_output(stdout: &str) -> Option { + stdout.lines().find_map(|line| line.strip_prefix("dry-run=")).map(npm_config_bool) } /// Parse a boolean npm config value the way npm's option parser (nopt) does: @@ -993,8 +990,18 @@ pub async fn dispatch(tool: &str, args: &[String], env: ToolPathEnv) -> i32 { if tool == "npm" { if let Some(parsed) = parse_npm_global_install(args) { let exit_code = exec::spawn_tool(&tool_path, args, child_env.clone()); + if exit_code != 0 { + return exit_code; + } // A dry run changes no files in the npm prefix, so the links stay as they are. - if exit_code == 0 && !npm_dry_run_enabled(args, &tool_path, &child_env) { + // When npm cannot report the setting, do not guess that the install was real. + let Some(dry_run) = npm_dry_run_enabled(args, &tool_path, &child_env) else { + output::note( + "Could not read npm's dry-run setting, so links in ~/.vite-plus/bin/ were not changed.", + ); + return exit_code; + }; + if !dry_run { let node_dir = node_prefix_from_binary(&node_path); let node_version = resolution.as_ref().map_or_else( || read_node_version(&node_path).unwrap_or_else(|| "unknown".into()), @@ -2004,18 +2011,17 @@ mod tests { } #[test] - fn test_npm_dry_run_from_args() { - assert_eq!(npm_dry_run_from_args(&s(&["install", "-g", "pkg"])), None); - assert_eq!(npm_dry_run_from_args(&s(&["install", "-g", "--dry-run", "pkg"])), Some(true)); - assert_eq!(npm_dry_run_from_args(&s(&["i", "-g", "pkg", "--dry-run=false"])), Some(false)); - assert_eq!(npm_dry_run_from_args(&s(&["i", "-g", "--dry-run=1", "pkg"])), Some(true)); - // The last flag wins, as in npm. - assert_eq!( - npm_dry_run_from_args(&s(&["i", "-g", "--dry-run", "--no-dry-run", "pkg"])), - Some(false) - ); - // Arguments after `--` are not npm flags. - assert_eq!(npm_dry_run_from_args(&s(&["i", "-g", "pkg", "--", "--dry-run"])), None); + fn test_npm_dry_run_from_config_output() { + let output = "dry-run=true\ninstall=undefined\npkg=undefined\n"; + assert_eq!(npm_dry_run_from_config_output(output), Some(true)); + // `npm config get` prints environment values unparsed. + assert_eq!(npm_dry_run_from_config_output("dry-run=1\ninstall=undefined\n"), Some(true)); + // A package spec that looks like the key does not hide the real value. + let output = "dry-run=false\ninstall=undefined\ndry-run=true=undefined\n"; + assert_eq!(npm_dry_run_from_config_output(output), Some(false)); + // `--help` and `--version` replace the key lines with other output. + assert_eq!(npm_dry_run_from_config_output("10.9.9\n"), None); + assert_eq!(npm_dry_run_from_config_output(""), None); } #[test]