-
Notifications
You must be signed in to change notification settings - Fork 21
fix: prefer .ps1 over .cmd shim on Windows #345
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
fengmk2
wants to merge
13
commits into
main
Choose a base branch
from
fix/prefer-ps1-over-cmd-shim
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+299
−5
Open
Changes from all commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
592f313
fix(plan): prefer .ps1 over .cmd shim on Windows
fengmk2 62c9ce4
refactor(plan): extract powershell prefix flags to const
fengmk2 c03c631
fix(spawn): move .cmd->.ps1 rewrite from plan layer to spawn layer
fengmk2 c2738fa
fix(task): add missing `which` dep on Windows
fengmk2 89ace99
chore: update Cargo.lock for vite_task which dep
fengmk2 9bc5d8e
refactor(spawn): simplify powershell rewrite
fengmk2 fb1b8d2
fix(spawn): scope .ps1 rewrite to node_modules/.bin shims only
fengmk2 3e1fc38
refactor(spawn): tighten powershell rewrite review points
fengmk2 bbc7fc7
refactor: move .cmd->.ps1 selection to plan, keep only ps1 wrapping a…
fengmk2 26021ca
refactor(ps1_shim): align style with powershell module
fengmk2 bed8407
refactor(ps1_shim): drop .bat handling
fengmk2 99493b5
fix(ps1_shim): fall back to .cmd when no PowerShell host is on PATH
fengmk2 be08530
refactor(spawn): use cfg-gated import for powershell_host
fengmk2 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,130 @@ | ||
| //! On Windows, `CreateProcess` cannot execute a `.ps1` script directly. | ||
| //! When the planner has already picked a `.ps1` (see `vite_task_plan::ps1_shim`), | ||
| //! this module wraps the spawn as `powershell.exe -File <ps1> <args…>`, | ||
| //! preferring `pwsh.exe` when available. The PowerShell host is the same one | ||
| //! the planner consulted, so plan-time and spawn-time decisions stay in sync. | ||
|
|
||
| use std::sync::Arc; | ||
|
|
||
| use vite_path::AbsolutePath; | ||
| use vite_str::Str; | ||
| #[cfg(windows)] | ||
| use vite_task_plan::ps1_shim::powershell_host; | ||
|
|
||
| /// Fixed arguments prepended before the `.ps1` path. `-NoProfile`/`-NoLogo` | ||
| /// skip user profile loading; `-ExecutionPolicy Bypass` allows running the | ||
| /// unsigned shims that npm/pnpm install into `node_modules/.bin`. | ||
| #[cfg(any(windows, test))] | ||
| const POWERSHELL_PREFIX: &[&str] = | ||
| &["-NoProfile", "-NoLogo", "-ExecutionPolicy", "Bypass", "-File"]; | ||
|
|
||
| /// If `program_path` is a `.ps1`, return a rewritten | ||
| /// `(powershell.exe, [-File <ps1>, ...args])` invocation. Returns `None` | ||
| /// when the program isn't a `.ps1` or no PowerShell host is available, so | ||
| /// callers can reuse the original references without cloning. | ||
| #[cfg(windows)] | ||
| pub(super) fn wrap_ps1_with_powershell( | ||
| program_path: &Arc<AbsolutePath>, | ||
| args: &Arc<[Str]>, | ||
| ) -> Option<(Arc<AbsolutePath>, Arc<[Str]>)> { | ||
| wrap_with_host(program_path, args, powershell_host()?) | ||
| } | ||
|
|
||
| #[cfg(not(windows))] | ||
| pub(super) const fn wrap_ps1_with_powershell( | ||
| _program_path: &Arc<AbsolutePath>, | ||
| _args: &Arc<[Str]>, | ||
| ) -> Option<(Arc<AbsolutePath>, Arc<[Str]>)> { | ||
| None | ||
| } | ||
|
|
||
| /// Pure rewrite logic, factored out so tests can exercise it on any platform | ||
| /// without depending on a real `powershell.exe` being on PATH. | ||
| #[cfg(any(windows, test))] | ||
| fn wrap_with_host( | ||
| program_path: &Arc<AbsolutePath>, | ||
| args: &Arc<[Str]>, | ||
| host: &Arc<AbsolutePath>, | ||
| ) -> Option<(Arc<AbsolutePath>, Arc<[Str]>)> { | ||
| let ext = program_path.as_path().extension().and_then(|e| e.to_str())?; | ||
| if !ext.eq_ignore_ascii_case("ps1") { | ||
| return None; | ||
| } | ||
|
|
||
| let ps1_str = program_path.as_path().to_str()?; | ||
|
|
||
| tracing::debug!( | ||
| "wrapping .ps1 with powershell: {} -File {}", | ||
| host.as_path().display(), | ||
| ps1_str, | ||
| ); | ||
|
|
||
| let new_args: Arc<[Str]> = POWERSHELL_PREFIX | ||
| .iter() | ||
| .copied() | ||
| .map(Str::from) | ||
| .chain(std::iter::once(Str::from(ps1_str))) | ||
| .chain(args.iter().cloned()) | ||
| .collect(); | ||
|
|
||
| Some((Arc::clone(host), new_args)) | ||
| } | ||
|
|
||
| #[cfg(test)] | ||
| mod tests { | ||
| use std::{fs, sync::Arc}; | ||
|
|
||
| use tempfile::tempdir; | ||
| use vite_path::{AbsolutePath, AbsolutePathBuf}; | ||
| use vite_str::Str; | ||
|
|
||
| use super::wrap_with_host; | ||
|
|
||
| #[expect(clippy::disallowed_types, reason = "tempdir bridges std PathBuf into AbsolutePath")] | ||
| fn abs_path(buf: std::path::PathBuf) -> Arc<AbsolutePath> { | ||
| Arc::<AbsolutePath>::from(AbsolutePathBuf::new(buf).unwrap()) | ||
| } | ||
|
|
||
| #[test] | ||
| fn wraps_ps1_with_powershell_host() { | ||
| let dir = tempdir().unwrap(); | ||
| let ps1_path = dir.path().join("vite.ps1"); | ||
| fs::write(&ps1_path, "").unwrap(); | ||
|
|
||
| let host = abs_path(dir.path().join("powershell.exe")); | ||
| let program = abs_path(ps1_path.clone()); | ||
| let args: Arc<[Str]> = Arc::from(vec![Str::from("--port"), Str::from("3000")]); | ||
|
|
||
| let (rewritten_program, rewritten_args) = | ||
| wrap_with_host(&program, &args, &host).expect("should wrap"); | ||
|
|
||
| assert_eq!(rewritten_program.as_path(), host.as_path()); | ||
| let as_strs: Vec<&str> = rewritten_args.iter().map(Str::as_str).collect(); | ||
| assert_eq!( | ||
| as_strs, | ||
| vec![ | ||
| "-NoProfile", | ||
| "-NoLogo", | ||
| "-ExecutionPolicy", | ||
| "Bypass", | ||
| "-File", | ||
| ps1_path.to_str().unwrap(), | ||
| "--port", | ||
| "3000", | ||
| ] | ||
| ); | ||
| } | ||
|
|
||
| #[test] | ||
| fn returns_none_for_non_ps1_programs() { | ||
| let dir = tempdir().unwrap(); | ||
| let cmd_path = dir.path().join("vite.cmd"); | ||
| fs::write(&cmd_path, "").unwrap(); | ||
|
|
||
| let host = abs_path(dir.path().join("powershell.exe")); | ||
| let program = abs_path(cmd_path); | ||
| let args: Arc<[Str]> = Arc::from(vec![Str::from("build")]); | ||
|
|
||
| assert!(wrap_with_host(&program, &args, &host).is_none()); | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,152 @@ | ||
| //! Windows-specific: substitute `.cmd` shims in `node_modules/.bin` with | ||
| //! their sibling `.ps1`. | ||
| //! | ||
| //! Running a `.cmd` shim from any shell causes `cmd.exe` to prompt "Terminate | ||
| //! batch job (Y/N)?" on Ctrl+C, which leaves the terminal corrupt. Picking the | ||
| //! `.ps1` sibling at plan time makes the task graph record the script that | ||
| //! actually runs; spawn-time logic wraps it with `powershell.exe -File` so | ||
| //! Ctrl+C propagates without the `cmd.exe` hop. | ||
| //! | ||
| //! The substitution is limited to `node_modules/.bin/` triplets produced by | ||
| //! npm/pnpm/yarn (via cmd-shim, which only emits `.cmd` — not `.bat`) so | ||
| //! unrelated `.cmd` files elsewhere on PATH are left alone. | ||
| //! | ||
| //! See <https://github.com/voidzero-dev/vite-plus/issues/1176>. | ||
|
|
||
| use std::sync::Arc; | ||
|
|
||
| use vite_path::{AbsolutePath, AbsolutePathBuf}; | ||
|
|
||
| /// Cached location of the PowerShell host used to run `.ps1` shims. Prefers | ||
| /// cross-platform `pwsh.exe` when present, falling back to the Windows | ||
| /// built-in `powershell.exe`. `None` means no host was found in PATH (or we | ||
| /// aren't on Windows at all). | ||
| /// | ||
| /// Exposed so the spawn layer wraps the `.ps1` with the same host this module | ||
| /// validated at plan time — there's one cache, one lookup. | ||
| #[cfg(windows)] | ||
| pub fn powershell_host() -> Option<&'static Arc<AbsolutePath>> { | ||
| use std::sync::LazyLock; | ||
|
|
||
| static POWERSHELL_HOST: LazyLock<Option<Arc<AbsolutePath>>> = LazyLock::new(|| { | ||
| let resolved = which::which("pwsh.exe").or_else(|_| which::which("powershell.exe")).ok()?; | ||
| AbsolutePathBuf::new(resolved).map(Arc::<AbsolutePath>::from) | ||
| }); | ||
| POWERSHELL_HOST.as_ref() | ||
| } | ||
|
|
||
| #[cfg(not(windows))] | ||
| #[must_use] | ||
| pub const fn powershell_host() -> Option<&'static Arc<AbsolutePath>> { | ||
| None | ||
| } | ||
|
|
||
| /// If `resolved` is a `.cmd` shim under `node_modules/.bin` with a sibling | ||
| /// `.ps1` **and** a PowerShell host is available to run that `.ps1`, return | ||
| /// the `.ps1` path. Otherwise return the input unchanged — so callers that | ||
| /// couldn't use PowerShell anyway fall back to the original `.cmd` instead of | ||
| /// producing a path `CreateProcess` can't execute. | ||
| #[cfg(windows)] | ||
| pub fn prefer_ps1_sibling(resolved: AbsolutePathBuf) -> AbsolutePathBuf { | ||
| if powershell_host().is_none() { | ||
| return resolved; | ||
| } | ||
| find_ps1_sibling(&resolved).unwrap_or(resolved) | ||
| } | ||
|
|
||
| #[cfg(not(windows))] | ||
| #[must_use] | ||
| pub const fn prefer_ps1_sibling(resolved: AbsolutePathBuf) -> AbsolutePathBuf { | ||
| resolved | ||
| } | ||
|
|
||
| #[cfg(any(windows, test))] | ||
| fn find_ps1_sibling(resolved: &AbsolutePath) -> Option<AbsolutePathBuf> { | ||
| let path = resolved.as_path(); | ||
| let ext = path.extension().and_then(|e| e.to_str())?; | ||
| if !ext.eq_ignore_ascii_case("cmd") { | ||
| return None; | ||
| } | ||
|
|
||
| let mut parents = path.components().rev(); | ||
| parents.next()?; // shim filename | ||
| if !parents.next()?.as_os_str().eq_ignore_ascii_case(".bin") { | ||
| return None; | ||
| } | ||
| if !parents.next()?.as_os_str().eq_ignore_ascii_case("node_modules") { | ||
| return None; | ||
| } | ||
|
|
||
| let ps1 = path.with_extension("ps1"); | ||
| if !ps1.is_file() { | ||
| return None; | ||
| } | ||
|
|
||
| let ps1 = AbsolutePathBuf::new(ps1)?; | ||
| tracing::debug!("preferring .ps1 sibling: {} -> {}", path.display(), ps1.as_path().display()); | ||
| Some(ps1) | ||
| } | ||
|
|
||
| #[cfg(test)] | ||
| mod tests { | ||
| use std::fs; | ||
|
|
||
| use tempfile::tempdir; | ||
|
|
||
| use super::{AbsolutePath, AbsolutePathBuf, Arc, find_ps1_sibling}; | ||
|
|
||
| #[expect(clippy::disallowed_types, reason = "tempdir bridges std PathBuf into AbsolutePath")] | ||
| fn abs(buf: std::path::PathBuf) -> Arc<AbsolutePath> { | ||
| Arc::<AbsolutePath>::from(AbsolutePathBuf::new(buf).unwrap()) | ||
| } | ||
|
|
||
| #[expect(clippy::disallowed_types, reason = "tempdir hands out std Path for the test root")] | ||
| fn bin_dir(root: &std::path::Path) -> std::path::PathBuf { | ||
| let bin = root.join("node_modules").join(".bin"); | ||
| fs::create_dir_all(&bin).unwrap(); | ||
| bin | ||
| } | ||
|
|
||
| #[test] | ||
| fn substitutes_cmd_when_ps1_sibling_exists_in_node_modules_bin() { | ||
| let dir = tempdir().unwrap(); | ||
| let bin = bin_dir(dir.path()); | ||
| fs::write(bin.join("vite.CMD"), "").unwrap(); | ||
| fs::write(bin.join("vite.ps1"), "").unwrap(); | ||
|
|
||
| let resolved = abs(bin.join("vite.CMD")); | ||
| let result = find_ps1_sibling(&resolved).expect("should substitute"); | ||
| assert_eq!(result.as_path(), bin.join("vite.ps1")); | ||
| } | ||
|
|
||
| #[test] | ||
| fn leaves_cmd_alone_without_ps1_sibling() { | ||
| let dir = tempdir().unwrap(); | ||
| let bin = bin_dir(dir.path()); | ||
| fs::write(bin.join("vite.cmd"), "").unwrap(); | ||
|
|
||
| let resolved = abs(bin.join("vite.cmd")); | ||
| assert!(find_ps1_sibling(&resolved).is_none()); | ||
| } | ||
|
|
||
| #[test] | ||
| fn leaves_cmd_alone_outside_node_modules_bin() { | ||
| let dir = tempdir().unwrap(); | ||
| fs::write(dir.path().join("where.cmd"), "").unwrap(); | ||
| fs::write(dir.path().join("where.ps1"), "").unwrap(); | ||
|
|
||
| let resolved = abs(dir.path().join("where.cmd")); | ||
| assert!(find_ps1_sibling(&resolved).is_none()); | ||
| } | ||
|
|
||
| #[test] | ||
| fn leaves_non_shim_extensions_alone() { | ||
| let dir = tempdir().unwrap(); | ||
| let bin = bin_dir(dir.path()); | ||
| fs::write(bin.join("node.exe"), "").unwrap(); | ||
| fs::write(bin.join("node.ps1"), "").unwrap(); | ||
|
|
||
| let resolved = abs(bin.join("node.exe")); | ||
| assert!(find_ps1_sibling(&resolved).is_none()); | ||
| } | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.