From 4e44ec1ba3eda63b419b46089bb458ab1b79096a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=98=BF=E8=8F=9C=20Cai?= Date: Thu, 6 Aug 2026 04:36:25 +0800 Subject: [PATCH] fix(command): resolve relative PATH entries against cwd --- .../package.json | 6 + .../packages/app/package.json | 3 + .../command_exec_relative_path_cwd/setup.js | 9 ++ .../snapshots.toml | 9 ++ .../command_exec_relative_path_cwd.md | 14 +++ crates/vp_command/src/lib.rs | 108 +++++++++++++++++- 6 files changed, 148 insertions(+), 1 deletion(-) create mode 100644 crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_exec_relative_path_cwd/package.json create mode 100644 crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_exec_relative_path_cwd/packages/app/package.json create mode 100644 crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_exec_relative_path_cwd/setup.js create mode 100644 crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_exec_relative_path_cwd/snapshots.toml create mode 100644 crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_exec_relative_path_cwd/snapshots/command_exec_relative_path_cwd.md diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_exec_relative_path_cwd/package.json b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_exec_relative_path_cwd/package.json new file mode 100644 index 0000000000..f74441e9ab --- /dev/null +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_exec_relative_path_cwd/package.json @@ -0,0 +1,6 @@ +{ + "name": "exec-relative-path-cwd", + "workspaces": [ + "packages/*" + ] +} diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_exec_relative_path_cwd/packages/app/package.json b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_exec_relative_path_cwd/packages/app/package.json new file mode 100644 index 0000000000..fd1f8f6386 --- /dev/null +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_exec_relative_path_cwd/packages/app/package.json @@ -0,0 +1,3 @@ +{ + "name": "app" +} diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_exec_relative_path_cwd/setup.js b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_exec_relative_path_cwd/setup.js new file mode 100644 index 0000000000..ee06996a73 --- /dev/null +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_exec_relative_path_cwd/setup.js @@ -0,0 +1,9 @@ +const fs = require('fs'); + +fs.mkdirSync('packages/app/tools', { recursive: true }); +fs.writeFileSync( + 'packages/app/tools/fake-node', + '#!/usr/bin/env node\nconsole.log("resolved from package cwd");\n', + { mode: 0o755 }, +); +fs.writeFileSync('packages/app/tools/fake-node.cmd', '@node "%~dp0\\fake-node" %*\n'); diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_exec_relative_path_cwd/snapshots.toml b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_exec_relative_path_cwd/snapshots.toml new file mode 100644 index 0000000000..831192feb1 --- /dev/null +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_exec_relative_path_cwd/snapshots.toml @@ -0,0 +1,9 @@ +[[case]] +name = "command_exec_relative_path_cwd" +vp = "local" +skip-platforms = ["windows"] +comment = "A relative PATH entry must resolve against the selected package cwd, not the vp process cwd." +steps = [ + { argv = ["node", "setup.js"], snapshot = false, continue-on-failure = true }, + { argv = ["vp", "exec", "--filter", "app", "--", "fake-node"], envs = [["PATH", "./tools:${PATH}"]], comment = "relative PATH entry resolves from the selected package", continue-on-failure = true }, +] diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_exec_relative_path_cwd/snapshots/command_exec_relative_path_cwd.md b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_exec_relative_path_cwd/snapshots/command_exec_relative_path_cwd.md new file mode 100644 index 0000000000..4dc3f5b257 --- /dev/null +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_exec_relative_path_cwd/snapshots/command_exec_relative_path_cwd.md @@ -0,0 +1,14 @@ +# command_exec_relative_path_cwd + +A relative PATH entry must resolve against the selected package cwd, not the vp process cwd. + +## `node setup.js` + + +## `PATH=./tools:${PATH} vp exec --filter app -- fake-node` + +relative PATH entry resolves from the selected package + +``` +resolved from package cwd +``` diff --git a/crates/vp_command/src/lib.rs b/crates/vp_command/src/lib.rs index d388688756..045dadd41a 100644 --- a/crates/vp_command/src/lib.rs +++ b/crates/vp_command/src/lib.rs @@ -21,6 +21,15 @@ use vt_path::{AbsolutePath, AbsolutePathBuf, RelativePathBuf}; mod ps1_shim; +fn normalize_path_env( + path_env: &OsStr, + cwd: &AbsolutePath, +) -> Result { + std::env::join_paths(std::env::split_paths(path_env).map(|path| { + if path.is_absolute() || path.starts_with("~") { path } else { cwd.as_path().join(path) } + })) +} + /// Result of running a command with fspy tracking. #[derive(Debug)] pub struct FspyCommandResult { @@ -39,6 +48,7 @@ pub fn resolve_bin( path_env: Option<&OsStr>, cwd: impl AsRef, ) -> Result { + let cwd = cwd.as_ref(); let current_path; let path_env = if let Some(p) = path_env { p @@ -46,8 +56,14 @@ pub fn resolve_bin( current_path = std::env::var_os("PATH").unwrap_or_default(); ¤t_path }; - let path = which::which_in(bin_name, Some(path_env), cwd.as_ref()) + // `which` resolves relative PATH entries against the process cwd instead of the supplied + // command cwd. Commands are spawned with `cwd`, so resolve the entries the same way first; + // leave `~` entries for `which` to expand against the user's home directory. + let path_env = normalize_path_env(path_env, cwd) .map_err(|_| Error::CannotFindBinaryPath(bin_name.into()))?; + let path = which::which_in(bin_name, Some(&path_env), cwd) + .map_err(|_| Error::CannotFindBinaryPath(bin_name.into()))?; + let path = if path.is_absolute() { path } else { cwd.as_path().join(path) }; AbsolutePathBuf::new(path).ok_or_else(|| Error::CannotFindBinaryPath(bin_name.into())) } @@ -399,6 +415,96 @@ mod tests { tempdir().expect("Failed to create temp directory") } + #[cfg(unix)] + fn create_executable(path: &std::path::Path) { + use std::{fs, os::unix::fs::PermissionsExt}; + + fs::create_dir_all(path.parent().unwrap()).unwrap(); + fs::write(path, "#!/bin/sh\nexit 0\n").unwrap(); + let mut permissions = fs::metadata(path).unwrap().permissions(); + permissions.set_mode(0o755); + fs::set_permissions(path, permissions).unwrap(); + } + + #[cfg(unix)] + #[test] + fn test_resolve_bin_with_relative_path_entry() { + use std::path::PathBuf; + + let temp_dir = create_temp_dir(); + let cwd_path = temp_dir.path().canonicalize().unwrap(); + let cwd = AbsolutePathBuf::new(cwd_path.clone()).unwrap(); + let bin_dir = cwd_path.join("node_modules/.bin"); + let bin_path = bin_dir.join("fake-node"); + let fallback_bin_dir = cwd_path.join("fallback-bin"); + let fallback_bin_path = fallback_bin_dir.join("fake-node"); + + create_executable(&bin_path); + create_executable(&fallback_bin_path); + + let path_env = + std::env::join_paths([PathBuf::from("./node_modules/.bin"), fallback_bin_dir]).unwrap(); + let resolved = resolve_bin("fake-node", Some(&path_env), &cwd).unwrap(); + + assert_eq!(resolved.into_path_buf(), bin_path); + } + + #[cfg(unix)] + #[test] + fn test_resolve_bin_continues_after_missing_relative_path_entry() { + use std::path::PathBuf; + + let temp_dir = create_temp_dir(); + let cwd_path = temp_dir.path().canonicalize().unwrap(); + let cwd = AbsolutePathBuf::new(cwd_path.clone()).unwrap(); + let fallback_bin_dir = cwd_path.join("fallback-bin"); + let fallback_bin_path = fallback_bin_dir.join("fake-node"); + + create_executable(&fallback_bin_path); + + let path_env = + std::env::join_paths([PathBuf::from("./missing-bin"), fallback_bin_dir]).unwrap(); + let resolved = resolve_bin("fake-node", Some(&path_env), &cwd).unwrap(); + + assert_eq!(resolved.into_path_buf(), fallback_bin_path); + } + + #[cfg(unix)] + #[test] + fn test_resolve_bin_with_empty_path_entry() { + use std::path::PathBuf; + + let temp_dir = create_temp_dir(); + let cwd_path = temp_dir.path().canonicalize().unwrap(); + let cwd = AbsolutePathBuf::new(cwd_path.clone()).unwrap(); + let bin_path = cwd_path.join("fake-node"); + + create_executable(&bin_path); + + let path_env = std::env::join_paths([PathBuf::new()]).unwrap(); + let resolved = resolve_bin("fake-node", Some(&path_env), &cwd).unwrap(); + + assert_eq!(resolved.into_path_buf(), bin_path); + } + + #[cfg(unix)] + #[test] + fn test_normalize_path_env_preserves_tilde_entry() { + use std::path::PathBuf; + + let temp_dir = create_temp_dir(); + let cwd_path = temp_dir.path().canonicalize().unwrap(); + let cwd = AbsolutePathBuf::new(cwd_path).unwrap(); + let path_env = std::env::join_paths([PathBuf::from("~/bin")]).unwrap(); + + let normalized = normalize_path_env(&path_env, &cwd).unwrap(); + + assert_eq!( + std::env::split_paths(&normalized).collect::>(), + [PathBuf::from("~/bin")] + ); + } + mod run_command_tests { use super::*;