fix(command): resolve relative PATH entries against cwd - #2350
Conversation
✅ Deploy Preview for viteplus-preview canceled.
|
b6e28ea to
990e718
Compare
|
@RSS1102 Can you add a snapshot test to cover this bug fix change? |
7abfc83 to
4e44ec1
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. Another round soon, please! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: af66713e7c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
fengmk2
added
test: e2e
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fb1cd60261
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| path_entry: &Path, | ||
| cwd: &AbsolutePath, | ||
| ) -> Option<std::path::PathBuf> { | ||
| if path_entry.starts_with("~") || !is_plain_relative_path(path_entry) { |
There was a problem hiding this comment.
On Windows, an empty PATH entry has no components, so is_plain_relative_path returns false and this branch delegates it to which; that lookup still interprets the empty entry relative to the vp process cwd rather than the selected command cwd. Consequently, when PATH contains a leading, trailing, or doubled ; and the executable exists only in the package cwd, vp exec --filter ... fails to resolve it. Treat the empty entry like the Unix branch does and search cwd/bin_name directly.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
I don’t think an empty Windows PATH entry should be resolved against cwd. On Windows, which intentionally ignores empty entries produced by leading, trailing, or consecutive semicolons—for example, the empty entry between ;; in C:\tools;;C:\Windows\System32. This matches Rust’s Windows command lookup behavior.
This PR only changes how non-empty relative entries such as tools, ./tools, and ../tools are resolved, so the empty-entry test is intentionally Unix-only.
2 participants
Summary
PATHentries against the command cwd before binary lookup~expansion semanticsProblem
which::which_insearches relative PATH entries against the process cwd rather than the cwd supplied for the command. The npm-distributed CLI can therefore fail to resolvenodewhen Vite+ git hooks prepend./node_modules/.bin, even though the binary exists.Validation
cargo test -p vp_command(17 passed)cargo test -p vp_global_cli relative_path_entry(1 passed)cargo clippy -p vp_command --all-targets --all-features -- -D warningscargo fmt --all -- --checkgit diff --checkvite-plus@0.2.7and a relative PATH entry, then verified the same command reachesvp checkwith the patched NAPI bindingFixes #2326