diff --git a/crates/vp_global_cli/src/cli.rs b/crates/vp_global_cli/src/cli.rs index c4ba970c5d..737dbf2a80 100644 --- a/crates/vp_global_cli/src/cli.rs +++ b/crates/vp_global_cli/src/cli.rs @@ -756,13 +756,24 @@ async fn system_package_manager( kind: vp_pm_cli::PackageManagerType, executable: &vt_path::AbsolutePath, ) -> Option { - let output = - tokio::process::Command::new(executable.as_path()).arg("--version").output().await.ok()?; - if !output.status.success() { - return None; - } - let version = std::str::from_utf8(&output.stdout).ok()?.trim(); - js_semver::Version::parse(version).ok()?; + // Only npm skips `--version`; `npm_version_from_package_json` explains why. + let version = if kind == vp_pm_cli::PackageManagerType::Npm + && let Some(version) = vp_pm_cli::npm_version_from_package_json(executable) + { + version.to_string() + } else { + let output = tokio::process::Command::new(executable.as_path()) + .arg("--version") + .output() + .await + .ok()?; + if !output.status.success() { + return None; + } + let version = std::str::from_utf8(&output.stdout).ok()?.trim(); + js_semver::Version::parse(version).ok()?; + version.to_owned() + }; let bin_prefix = executable.parent()?.to_absolute_path_buf(); Some(vp_pm_cli::PackageManager::from_bin_prefix(kind, version, bin_prefix)) } @@ -1461,4 +1472,47 @@ mod tests { fn unknown_subcommand_does_not_suppress_header() { assert!(!should_suppress_header_for_subcommand("test", &["--lsp".to_string()])); } + + #[cfg(unix)] + #[tokio::test] + async fn system_npm_version_is_read_from_its_package_without_running_it() { + use std::os::unix::fs::PermissionsExt; + + use super::system_package_manager; + + // A Node.js install. Running npm-cli.js fails, so a spawned `npm --version` would + // return `None`. + // + // / + // ├── bin/npm -> ../lib/node_modules/npm/bin/npm-cli.js + // └── lib/node_modules/npm/ + // ├── package.json {"name":"npm","version":"10.9.2"} + // └── bin/npm-cli.js exit 1 + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + let package = root.join("lib/node_modules/npm"); + std::fs::create_dir_all(package.join("bin")).unwrap(); + std::fs::write(package.join("package.json"), r#"{"name":"npm","version":"10.9.2"}"#) + .unwrap(); + let cli = package.join("bin/npm-cli.js"); + std::fs::write(&cli, "#!/bin/sh\nexit 1\n").unwrap(); + std::fs::set_permissions(&cli, std::fs::Permissions::from_mode(0o755)).unwrap(); + std::fs::create_dir_all(root.join("bin")).unwrap(); + std::os::unix::fs::symlink("../lib/node_modules/npm/bin/npm-cli.js", root.join("bin/npm")) + .unwrap(); + let npm = vt_path::AbsolutePathBuf::new(root.join("bin/npm")).unwrap(); + + let manager = system_package_manager(vp_pm_cli::PackageManagerType::Npm, &npm) + .await + .expect("npm's version is read without running npm"); + // Scripts see the version in the user agent. + let context = + vp_pm_cli::LifecycleEnvContext { vp_version: "0.0.0".into(), node_version: None }; + let user_agent = manager + .lifecycle_env_vars(&context) + .into_iter() + .find_map(|(name, value)| (name == "npm_config_user_agent").then_some(value)) + .unwrap(); + assert!(user_agent.to_string_lossy().starts_with("vp/0.0.0 npm/10.9.2 "), "{user_agent:?}"); + } } diff --git a/crates/vp_pm_cli/src/lib.rs b/crates/vp_pm_cli/src/lib.rs index 12ad5fa286..412d5b4a88 100644 --- a/crates/vp_pm_cli/src/lib.rs +++ b/crates/vp_pm_cli/src/lib.rs @@ -30,10 +30,10 @@ pub use package_manager::{ PackageManagerResolution, PackageManagerSource, PackageManagerType, download_package_manager, download_package_manager_for_cwd, ensure_package_manager_bin, fetch_package_manager_versions, fetch_package_manager_versions_for_cwd, get_package_manager_type_and_version, - is_package_manager_dist_tag, package_manager_bin_path, package_manager_install_dir, - resolve_environment_package_manager, resolve_environment_package_manager_spec, - resolve_package_manager_from_package_json, resolve_package_manager_version, - resolve_package_manager_version_for_cwd, + is_package_manager_dist_tag, npm_version_from_package_json, package_manager_bin_path, + package_manager_install_dir, resolve_environment_package_manager, + resolve_environment_package_manager_spec, resolve_package_manager_from_package_json, + resolve_package_manager_version, resolve_package_manager_version_for_cwd, }; pub use request::HttpClient; pub use resolution::{ diff --git a/crates/vp_pm_cli/src/package_manager.rs b/crates/vp_pm_cli/src/package_manager.rs index 0192726e2e..25af983f05 100644 --- a/crates/vp_pm_cli/src/package_manager.rs +++ b/crates/vp_pm_cli/src/package_manager.rs @@ -265,20 +265,62 @@ impl PackageManagerBuilder { } } +/// Reads npm's version from the `package.json` of the npm package that `npm` resolves into. +/// +/// `npm --version` prints this field, but it starts Node.js and loads npm first. That cost is a +/// large share of short commands such as `vp run`. Only npm reads its manifest: +/// +/// - In an npm project without a pinned version, `vp` uses the npm on PATH and must ask for +/// its version. For pnpm, yarn, and bun, `vp` resolves the version itself; they run +/// `--version` only in system-first mode. +/// - pnpm and yarn can switch to the version that the project selects (`packageManager`, +/// `yarnPath`) before they answer, so their manifest on disk can disagree with `--version`. +/// npm does not switch. +/// +/// The PATH entry is normally a symlink into the package (`bin/npm` -> +/// `lib/node_modules/npm/bin/npm-cli.js`). Returns `None` when the nearest `package.json` +/// within four parents is not npm's, for example for a version-manager shim, Corepack, or +/// Windows `npm.cmd`; the caller then runs `npm --version`. +pub fn npm_version_from_package_json(npm: &AbsolutePath) -> Option { + #[derive(Deserialize)] + struct Manifest { + #[serde(default)] + name: Str, + #[serde(default)] + version: Str, + } + + let executable = fs::canonicalize(npm.as_path()).ok()?; + let manifest = executable + .ancestors() + .skip(1) + .take(4) + .find_map(|dir| fs::read(dir.join("package.json")).ok())?; + let manifest: Manifest = serde_json::from_slice(&manifest).ok()?; + if manifest.name != "npm" { + return None; + } + Version::parse(&manifest.version).ok() +} + // Version gates and migration must use the npm on PATH, not the latest registry release. async fn resolve_npm_from_path(cwd: &AbsolutePath) -> Result { let npm = vp_command::resolve_bin("npm", None, cwd)?; - let output = tokio::process::Command::new(npm.as_path()) - .arg("--version") - .current_dir(cwd) - // User preloads can print to stdout; only the actual command should run them. - .env_remove("NODE_OPTIONS") - .output() - .await?; - if !output.status.success() { - return Err(io::Error::other("failed to read npm version").into()); - } - let version = Version::parse(String::from_utf8_lossy(&output.stdout).trim())?; + let version = if let Some(version) = npm_version_from_package_json(&npm) { + version + } else { + let output = tokio::process::Command::new(npm.as_path()) + .arg("--version") + .current_dir(cwd) + // User preloads can print to stdout; only the actual command should run them. + .env_remove("NODE_OPTIONS") + .output() + .await?; + if !output.status.success() { + return Err(io::Error::other("failed to read npm version").into()); + } + Version::parse(String::from_utf8_lossy(&output.stdout).trim())? + }; let bin_prefix = npm .parent() .ok_or_else(|| Error::CannotFindBinaryPath("npm".into()))? @@ -2190,6 +2232,91 @@ mod tests { .await; } + /// Lays out npm like a Node.js install and returns `/bin/npm`: + /// + /// ```text + /// / + /// ├── bin/npm -> ../lib/node_modules/npm/bin/npm-cli.js + /// └── lib/node_modules/npm/ + /// ├── package.json `package_json` + /// └── bin/npm-cli.js `cli` + /// ``` + #[cfg(unix)] + fn install_npm_package(root: &AbsolutePath, package_json: &str, cli: &str) -> AbsolutePathBuf { + use std::os::unix::fs::PermissionsExt; + + let package = root.join("lib/node_modules/npm"); + fs::create_dir_all(package.join("bin")).unwrap(); + fs::write(package.join("package.json"), package_json).unwrap(); + let cli_path = package.join("bin/npm-cli.js"); + fs::write(&cli_path, cli).unwrap(); + fs::set_permissions(&cli_path, fs::Permissions::from_mode(0o755)).unwrap(); + fs::create_dir_all(root.join("bin")).unwrap(); + let npm = root.join("bin/npm"); + std::os::unix::fs::symlink("../lib/node_modules/npm/bin/npm-cli.js", &npm).unwrap(); + npm + } + + #[cfg(unix)] + #[test] + fn npm_version_is_read_from_the_package_that_npm_links_into() { + let dir = create_temp_dir(); + let root = AbsolutePathBuf::new(dir.path().to_path_buf()).unwrap(); + let npm = install_npm_package(&root, r#"{"name":"npm","version":"11.19.0"}"#, ""); + assert_eq!(npm_version_from_package_json(&npm), Some(Version::new(11, 19, 0))); + } + + #[cfg(unix)] + #[test] + fn npm_version_is_not_read_unless_the_nearest_manifest_is_npms() { + for package_json in [ + // Corepack's `npm` shim resolves into the corepack package. + r#"{"name":"corepack","version":"0.34.0"}"#, + r#"{"name":"npm"}"#, + r#"{"name":"npm","version":"not-a-version"}"#, + "{ broken", + ] { + let dir = create_temp_dir(); + let root = AbsolutePathBuf::new(dir.path().to_path_buf()).unwrap(); + let npm = install_npm_package(&root, package_json, ""); + assert_eq!(npm_version_from_package_json(&npm), None, "{package_json}"); + } + + // A shim outside any package, five levels below an npm manifest. + let dir = create_temp_dir(); + let root = AbsolutePathBuf::new(dir.path().to_path_buf()).unwrap(); + fs::write(root.join("package.json"), r#"{"name":"npm","version":"11.19.0"}"#).unwrap(); + let shim_dir = root.join("a/b/c/d"); + fs::create_dir_all(&shim_dir).unwrap(); + let shim = shim_dir.join("npm"); + fs::write(&shim, "#!/bin/sh\n").unwrap(); + assert_eq!(npm_version_from_package_json(&shim), None); + } + + #[cfg(unix)] + #[tokio::test(flavor = "current_thread")] + async fn npm_default_reads_the_version_of_the_npm_on_path_without_running_it() { + let dir = create_temp_dir(); + let root = AbsolutePathBuf::new(dir.path().to_path_buf()).unwrap(); + // A separate global prefix whose npm differs from the one bundled with Node.js. Running + // it fails, so a spawned `npm --version` would surface as an error. + let npm = install_npm_package( + &root, + r#"{"name":"npm","version":"10.9.2"}"#, + "#!/bin/sh\nexit 1\n", + ); + let bin_dir = npm.parent().unwrap(); + + EnvConfig::with_vars_async([("PATH", bin_dir.as_path())], |_| async { + let version = + resolve_package_manager_version_for_cwd(&root, PackageManagerType::Npm, "default") + .await + .unwrap(); + assert_eq!(version, "10.9.2"); + }) + .await; + } + #[test] fn bin_names_only_include_executables_in_the_installation() { for kind in [