Skip to content

Commit 4f14ebe

Browse files
fengmk2fengmk2
committed
refactor(spawn): simplify powershell rewrite
- Return Option<(Arc, Arc)>; caller reuses original references on the passthrough path, dropping two unconditional Arc clones per spawn - Inline the `imp` submodule: two cfg-gated definitions replace the `cfg_attr(expect(missing_const_for_fn))` workaround - Use eq_ignore_ascii_case to drop the lowercase String allocation - Gate pure rewrite on `any(windows, test)` so tests run on every OS
1 parent 36b23b1 commit 4f14ebe

2 files changed

Lines changed: 93 additions & 114 deletions

File tree

‎crates/vite_task/src/session/execute/powershell.rs‎

Lines changed: 83 additions & 104 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,11 @@
11
//! Windows-specific: prefer `.ps1` shims over `.cmd` shims at spawn time.
22
//!
3-
//! npm/pnpm/yarn install binary shims as `.cmd`, `.ps1`, and POSIX `.sh` triplets.
4-
//! Spawning the `.cmd` wrapper from any shell causes `cmd.exe` to prompt
5-
//! "Terminate batch job (Y/N)?" when the child exits via Ctrl+C, which leaves
6-
//! the terminal in a corrupt state. Routing through the `.ps1` variant via
7-
//! `powershell.exe -File` sidesteps that prompt.
3+
//! Spawning a `.cmd` shim from any shell causes `cmd.exe` to prompt
4+
//! "Terminate batch job (Y/N)?" on Ctrl+C, leaving the terminal corrupt.
5+
//! Routing through the `.ps1` sibling via `powershell.exe -File` sidesteps it.
86
//!
9-
//! This lives in the spawn layer — not the plan layer — so the rewrite does
10-
//! not leak absolute `.ps1` paths or `powershell.exe` into `SpawnFingerprint`,
7+
//! Lives in the spawn layer — not the plan layer — so the rewrite does not
8+
//! leak absolute `.ps1` paths or `powershell.exe` into `SpawnFingerprint`,
119
//! keeping cache keys portable across machines and OSes.
1210
//!
1311
//! See <https://github.com/voidzero-dev/vite-plus/issues/1176>.
@@ -17,109 +15,95 @@ use std::sync::Arc;
1715
use vite_path::AbsolutePath;
1816
use vite_str::Str;
1917

20-
/// If `program_path` is a `.cmd`/`.bat` shim with a sibling `.ps1`, rewrite the
21-
/// spawn to invoke the `.ps1` via PowerShell. Returns the inputs unchanged on
22-
/// non-Windows platforms, when no sibling `.ps1` exists, or when no PowerShell
23-
/// host (`pwsh.exe` / `powershell.exe`) can be located.
24-
#[cfg_attr(
25-
not(windows),
26-
expect(clippy::missing_const_for_fn, reason = "Windows branch has runtime-only logic")
27-
)]
18+
/// Fixed arguments prepended before the `.ps1` path. `-NoProfile`/`-NoLogo`
19+
/// skip user profile loading; `-ExecutionPolicy Bypass` allows running the
20+
/// unsigned shims that npm/pnpm install into `node_modules/.bin`.
21+
#[cfg(any(windows, test))]
22+
const POWERSHELL_PREFIX: &[&str] =
23+
&["-NoProfile", "-NoLogo", "-ExecutionPolicy", "Bypass", "-File"];
24+
25+
/// If `program_path` is a `.cmd`/`.bat` shim with a sibling `.ps1`, return a
26+
/// rewritten `(powershell.exe, [-File <ps1>, ...args])` invocation. Returns
27+
/// `None` when no rewrite applies, so callers can reuse the original
28+
/// references without cloning.
29+
#[cfg(windows)]
2830
pub(super) fn rewrite_cmd_shim_to_powershell(
29-
program_path: Arc<AbsolutePath>,
30-
args: Arc<[Str]>,
31-
) -> (Arc<AbsolutePath>, Arc<[Str]>) {
32-
#[cfg(windows)]
33-
{
34-
let Some(host) = imp::cached_host() else {
35-
return (program_path, args);
36-
};
37-
imp::rewrite_with_host(program_path, args, host)
38-
}
31+
program_path: &Arc<AbsolutePath>,
32+
args: &Arc<[Str]>,
33+
) -> Option<(Arc<AbsolutePath>, Arc<[Str]>)> {
34+
rewrite_with_host(program_path, args, POWERSHELL_HOST.as_ref()?)
35+
}
3936

40-
#[cfg(not(windows))]
41-
{
42-
(program_path, args)
43-
}
37+
#[cfg(not(windows))]
38+
pub(super) const fn rewrite_cmd_shim_to_powershell(
39+
_program_path: &Arc<AbsolutePath>,
40+
_args: &Arc<[Str]>,
41+
) -> Option<(Arc<AbsolutePath>, Arc<[Str]>)> {
42+
None
4443
}
4544

45+
/// Cached location of the PowerShell host used to run `.ps1` shims. Prefers
46+
/// cross-platform `pwsh.exe` when present, falling back to the Windows
47+
/// built-in `powershell.exe`. `None` means no host was found in PATH.
4648
#[cfg(windows)]
47-
mod imp {
48-
use std::sync::{Arc, LazyLock};
49-
50-
use vite_path::{AbsolutePath, AbsolutePathBuf};
51-
use vite_str::Str;
52-
53-
/// Fixed arguments prepended before the `.ps1` path. `-NoProfile`/`-NoLogo`
54-
/// skip user profile loading; `-ExecutionPolicy Bypass` allows running the
55-
/// unsigned shims that npm/pnpm install into `node_modules/.bin`.
56-
const POWERSHELL_PREFIX: &[&str] =
57-
&["-NoProfile", "-NoLogo", "-ExecutionPolicy", "Bypass", "-File"];
58-
59-
/// Cached location of the PowerShell host used to run `.ps1` shims. Prefers
60-
/// cross-platform `pwsh.exe` when present, falling back to the Windows
61-
/// built-in `powershell.exe`. `None` means no host was found in PATH.
62-
static POWERSHELL_HOST: LazyLock<Option<Arc<AbsolutePath>>> = LazyLock::new(|| {
49+
static POWERSHELL_HOST: std::sync::LazyLock<Option<Arc<AbsolutePath>>> =
50+
std::sync::LazyLock::new(|| {
6351
let resolved = which::which("pwsh.exe").or_else(|_| which::which("powershell.exe")).ok()?;
64-
AbsolutePathBuf::new(resolved).map(Arc::<AbsolutePath>::from)
52+
vite_path::AbsolutePathBuf::new(resolved).map(Arc::<AbsolutePath>::from)
6553
});
6654

67-
pub(super) fn cached_host() -> Option<&'static Arc<AbsolutePath>> {
68-
POWERSHELL_HOST.as_ref()
55+
/// Pure rewrite logic, factored out so tests can exercise it on any platform
56+
/// without depending on a real `powershell.exe` being on PATH.
57+
#[cfg(any(windows, test))]
58+
fn rewrite_with_host(
59+
program_path: &Arc<AbsolutePath>,
60+
args: &Arc<[Str]>,
61+
host: &Arc<AbsolutePath>,
62+
) -> Option<(Arc<AbsolutePath>, Arc<[Str]>)> {
63+
let ext = program_path.as_path().extension().and_then(|e| e.to_str())?;
64+
if !(ext.eq_ignore_ascii_case("cmd") || ext.eq_ignore_ascii_case("bat")) {
65+
return None;
6966
}
7067

71-
pub(super) fn rewrite_with_host(
72-
program_path: Arc<AbsolutePath>,
73-
args: Arc<[Str]>,
74-
host: &Arc<AbsolutePath>,
75-
) -> (Arc<AbsolutePath>, Arc<[Str]>) {
76-
let ext = program_path
77-
.as_path()
78-
.extension()
79-
.and_then(|e| e.to_str())
80-
.map(str::to_ascii_lowercase);
81-
if !matches!(ext.as_deref(), Some("cmd" | "bat")) {
82-
return (program_path, args);
83-
}
84-
85-
let ps1_path = program_path.as_path().with_extension("ps1");
86-
if !ps1_path.is_file() {
87-
return (program_path, args);
88-
}
89-
90-
let Some(ps1_str) = ps1_path.to_str() else {
91-
return (program_path, args);
92-
};
93-
94-
tracing::debug!(
95-
"rewriting cmd shim to powershell: {} -> {} -File {}",
96-
program_path.as_path().display(),
97-
host.as_path().display(),
98-
ps1_str,
99-
);
68+
let ps1_path = program_path.as_path().with_extension("ps1");
69+
if !ps1_path.is_file() {
70+
return None;
71+
}
10072

101-
let new_args: Arc<[Str]> = POWERSHELL_PREFIX
102-
.iter()
103-
.copied()
104-
.map(Str::from)
105-
.chain(std::iter::once(Str::from(ps1_str)))
106-
.chain(args.iter().cloned())
107-
.collect();
73+
let ps1_str = ps1_path.to_str()?;
10874

109-
(Arc::clone(host), new_args)
110-
}
75+
tracing::debug!(
76+
"rewriting cmd shim to powershell: {} -> {} -File {}",
77+
program_path.as_path().display(),
78+
host.as_path().display(),
79+
ps1_str,
80+
);
81+
82+
let new_args: Arc<[Str]> = POWERSHELL_PREFIX
83+
.iter()
84+
.copied()
85+
.map(Str::from)
86+
.chain(std::iter::once(Str::from(ps1_str)))
87+
.chain(args.iter().cloned())
88+
.collect();
89+
90+
Some((Arc::clone(host), new_args))
11191
}
11292

113-
#[cfg(all(test, windows))]
93+
#[cfg(test)]
11494
mod tests {
11595
use std::{fs, sync::Arc};
11696

11797
use tempfile::tempdir;
11898
use vite_path::{AbsolutePath, AbsolutePathBuf};
11999
use vite_str::Str;
120100

121-
use super::imp::rewrite_with_host;
101+
use super::rewrite_with_host;
122102

103+
#[expect(
104+
clippy::disallowed_types,
105+
reason = "tempdir yields std PathBuf; test helper is the narrowest conversion point"
106+
)]
123107
fn abs_path(buf: std::path::PathBuf) -> Arc<AbsolutePath> {
124108
Arc::<AbsolutePath>::from(AbsolutePathBuf::new(buf).unwrap())
125109
}
@@ -133,12 +117,14 @@ mod tests {
133117
fs::write(&ps1_path, "").unwrap();
134118

135119
let host = abs_path(dir.path().join("powershell.exe"));
120+
let program = abs_path(cmd_path);
136121
let args: Arc<[Str]> = Arc::from(vec![Str::from("--port"), Str::from("3000")]);
137122

138-
let (program, new_args) = rewrite_with_host(abs_path(cmd_path), args, &host);
123+
let (rewritten_program, rewritten_args) =
124+
rewrite_with_host(&program, &args, &host).expect("rewrite should apply");
139125

140-
assert_eq!(program.as_path(), host.as_path());
141-
let as_strs: Vec<&str> = new_args.iter().map(Str::as_str).collect();
126+
assert_eq!(rewritten_program.as_path(), host.as_path());
127+
let as_strs: Vec<&str> = rewritten_args.iter().map(Str::as_str).collect();
142128
assert_eq!(
143129
as_strs,
144130
vec![
@@ -155,37 +141,30 @@ mod tests {
155141
}
156142

157143
#[test]
158-
fn leaves_cmd_unchanged_when_no_ps1_sibling() {
144+
fn returns_none_when_no_ps1_sibling() {
159145
let dir = tempdir().unwrap();
160146
let cmd_path = dir.path().join("vite.cmd");
161147
fs::write(&cmd_path, "").unwrap();
162148

163149
let host = abs_path(dir.path().join("powershell.exe"));
150+
let program = abs_path(cmd_path);
164151
let args: Arc<[Str]> = Arc::from(vec![Str::from("build")]);
165-
let original_program = abs_path(cmd_path);
166152

167-
let (program, new_args) =
168-
rewrite_with_host(Arc::clone(&original_program), Arc::clone(&args), &host);
169-
170-
assert_eq!(program.as_path(), original_program.as_path());
171-
let as_strs: Vec<&str> = new_args.iter().map(Str::as_str).collect();
172-
assert_eq!(as_strs, vec!["build"]);
153+
assert!(rewrite_with_host(&program, &args, &host).is_none());
173154
}
174155

175156
#[test]
176-
fn leaves_non_cmd_extensions_unchanged() {
157+
fn returns_none_for_non_shim_extensions() {
177158
let dir = tempdir().unwrap();
178159
let exe_path = dir.path().join("node.exe");
179160
fs::write(&exe_path, "").unwrap();
180161
// Even with a sibling .ps1, non-cmd/bat programs must not be rewritten.
181162
fs::write(dir.path().join("node.ps1"), "").unwrap();
182163

183164
let host = abs_path(dir.path().join("powershell.exe"));
165+
let program = abs_path(exe_path);
184166
let args: Arc<[Str]> = Arc::from(vec![Str::from("--version")]);
185-
let original_program = abs_path(exe_path);
186-
187-
let (program, _new_args) = rewrite_with_host(Arc::clone(&original_program), args, &host);
188167

189-
assert_eq!(program.as_path(), original_program.as_path());
168+
assert!(rewrite_with_host(&program, &args, &host).is_none());
190169
}
191170
}

‎crates/vite_task/src/session/execute/spawn.rs‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
//! cancellation-aware `wait` future. Draining the pipes is [`super::pipe`]'s
55
//! job; normalizing fspy path accesses is [`super::tracked_accesses`]'s.
66
7-
use std::{io, process::Stdio, sync::Arc};
7+
use std::{io, process::Stdio};
88

99
use fspy::PathAccessIterable;
1010
use futures_util::{FutureExt, future::LocalBoxFuture};
@@ -54,15 +54,15 @@ pub async fn spawn(
5454
stdio: SpawnStdio,
5555
cancellation_token: CancellationToken,
5656
) -> anyhow::Result<ChildHandle> {
57-
// Re-route `.cmd`/`.bat` shims through PowerShell on Windows when a sibling
58-
// `.ps1` exists. Done here (not at plan time) so cache fingerprints still
59-
// key on the original tool's relative path and args.
60-
let (program_path, args) = super::powershell::rewrite_cmd_shim_to_powershell(
61-
Arc::clone(&cmd.program_path),
62-
Arc::clone(&cmd.args),
63-
);
64-
65-
let mut fspy_cmd = fspy::Command::new(program_path.as_path());
57+
// Rewrite happens here, not at plan time, so cache fingerprints keep the
58+
// original tool's portable path instead of leaking `powershell.exe` keys.
59+
let rewrite = super::powershell::rewrite_cmd_shim_to_powershell(&cmd.program_path, &cmd.args);
60+
let (program_path, args) = match rewrite.as_ref() {
61+
Some((p, a)) => (p.as_path(), a.as_ref()),
62+
None => (cmd.program_path.as_path(), cmd.args.as_ref()),
63+
};
64+
65+
let mut fspy_cmd = fspy::Command::new(program_path);
6666
fspy_cmd.args(args.iter().map(vite_str::Str::as_str));
6767
fspy_cmd.envs(cmd.all_envs.iter());
6868
fspy_cmd.current_dir(&*cmd.cwd);

0 commit comments

Comments
 (0)