Skip to content

Commit dbbf6fa

Browse files
committed
xtask desktop: resolve pnpm explicitly on Windows (.exe preferred, .cmd via cmd /c)
Rust Command (CreateProcess) only resolves .exe on PATH, not the .cmd shims a pnpm install leaves behind, so build_frontend failed with 'failed to run pnpm: program not found' on Windows. Resolve PATH/PATHEXT explicitly; fail closed naming pnpm and the fix when absent. Linux/macOS behavior unchanged.
1 parent 3cf4575 commit dbbf6fa

1 file changed

Lines changed: 143 additions & 1 deletion

File tree

crates/xtask/src/desktop.rs

Lines changed: 143 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -414,7 +414,7 @@ fn check_bundle_prereqs() -> Result<(), String> {
414414
}
415415

416416
fn build_frontend() -> Result<(), String> {
417-
let status = Command::new("pnpm")
417+
let status = pnpm_command()?
418418
.args(["--filter", "./apps/desktop", "build"])
419419
.current_dir(super::repo_root())
420420
.status()
@@ -436,6 +436,75 @@ fn build_frontend() -> Result<(), String> {
436436
Ok(())
437437
}
438438

439+
/// Build the pnpm invocation for the desktop frontend.
440+
///
441+
/// On Linux/macOS this is `pnpm` exactly as before. On Windows, Rust's
442+
/// `Command` (CreateProcess) only resolves `.exe` on PATH, not the
443+
/// `.cmd` shims a pnpm install leaves behind, so resolve explicitly: a
444+
/// real `pnpm.exe` runs directly, otherwise the resolved PATHEXT shim runs
445+
/// via `cmd /c` (batch files need the command interpreter). A truly absent
446+
/// pnpm fails closed naming the program and the fix.
447+
fn pnpm_command() -> Result<Command, String> {
448+
if cfg!(windows) {
449+
pnpm_command_windows()
450+
} else {
451+
Ok(Command::new("pnpm"))
452+
}
453+
}
454+
455+
fn pnpm_command_windows() -> Result<Command, String> {
456+
let dirs: Vec<std::path::PathBuf> = std::env::var_os("PATH")
457+
.map(|path| std::env::split_paths(&path).collect())
458+
.unwrap_or_default();
459+
let pathext = std::env::var("PATHEXT").unwrap_or_else(|_| ".COM;.EXE;.BAT;.CMD".to_string());
460+
match resolve_windows_program("pnpm", &dirs, &pathext) {
461+
Some((exe, false)) => Ok(Command::new(exe)),
462+
Some((shim, true)) => {
463+
let mut cmd = Command::new("cmd");
464+
cmd.arg("/c").arg(shim);
465+
Ok(cmd)
466+
}
467+
None => Err("pnpm not found on PATH (install the pnpm version pinned in the packageManager field of package.json and ensure it is on PATH)".to_string()),
468+
}
469+
}
470+
471+
/// Resolve `stem` against `dirs` honoring `pathext`, preferring a real
472+
/// `.exe` (CreateProcess runs it directly) over shell shims. Returns the
473+
/// resolved path plus whether it needs a shell (`cmd /c`): batch shims
474+
/// such as `pnpm.cmd` cannot run directly. `.exe` wins even from a later
475+
/// directory so a directly-runnable binary is never routed through a
476+
/// shell. Extension case follows the `pathext` entry as written; the
477+
/// Windows filesystem matches it case-insensitively.
478+
fn resolve_windows_program(
479+
stem: &str,
480+
dirs: &[std::path::PathBuf],
481+
pathext: &str,
482+
) -> Option<(std::path::PathBuf, bool)> {
483+
for dir in dirs {
484+
let exe = dir.join(format!("{stem}.exe"));
485+
if exe.is_file() {
486+
return Some((exe, false));
487+
}
488+
}
489+
for ext in pathext
490+
.split(';')
491+
.map(str::trim)
492+
.filter(|ext| !ext.is_empty())
493+
{
494+
if ext.eq_ignore_ascii_case(".exe") {
495+
continue;
496+
}
497+
let ext = ext.strip_prefix('.').unwrap_or(ext);
498+
for dir in dirs {
499+
let candidate = dir.join(format!("{stem}.{ext}"));
500+
if candidate.is_file() {
501+
return Some((candidate, true));
502+
}
503+
}
504+
}
505+
None
506+
}
507+
439508
fn bundle() -> Result<(), String> {
440509
check_bundle_prereqs()?;
441510
let targets = bundle_targets();
@@ -544,4 +613,77 @@ mod tests {
544613
assert!(root.join(name).is_file(), "missing generated icon {name}");
545614
}
546615
}
616+
617+
/// Scratch PATH tree for the Windows pnpm resolver tests: `files` are
618+
/// `(subdir, filename)` pairs. Filenames use the exact case the test's
619+
/// PATHEXT entry produces (Windows matches case-insensitively; the
620+
/// Linux/macOS test runner does not).
621+
fn pnpm_resolve_fixture(tag: &str, files: &[(&str, &str)]) -> std::path::PathBuf {
622+
let root = std::env::temp_dir().join(format!(
623+
"dezoomify-pnpm-resolve-{tag}-{}",
624+
std::process::id()
625+
));
626+
let _ = std::fs::remove_dir_all(&root);
627+
for (dir, file) in files {
628+
let dir = root.join(dir);
629+
std::fs::create_dir_all(&dir).expect("create fixture dir");
630+
std::fs::write(dir.join(file), "fixture").expect("write fixture file");
631+
}
632+
root
633+
}
634+
635+
#[test]
636+
fn pnpm_resolve_prefers_exe() {
637+
// A real pnpm.exe wins over a .CMD shim even from a later PATH
638+
// directory, and runs directly (no shell).
639+
let root = pnpm_resolve_fixture("exe", &[("bin", "pnpm.CMD"), ("tools", "pnpm.exe")]);
640+
let dirs = vec![root.join("bin"), root.join("tools")];
641+
let found = super::resolve_windows_program("pnpm", &dirs, ".COM;.EXE;.BAT;.CMD")
642+
.expect("pnpm must resolve");
643+
assert_eq!(found.0, root.join("tools").join("pnpm.exe"));
644+
assert!(!found.1, "pnpm.exe must run directly, not via a shell");
645+
let _ = std::fs::remove_dir_all(&root);
646+
}
647+
648+
#[test]
649+
fn pnpm_resolve_falls_back_to_cmd_shim() {
650+
// Shim-only install (the failing Windows CI layout): resolves the
651+
// .CMD shim and marks it as needing `cmd /c`.
652+
let root = pnpm_resolve_fixture("cmd", &[("bin", "pnpm.CMD")]);
653+
let dirs = vec![root.join("bin")];
654+
let found = super::resolve_windows_program("pnpm", &dirs, ".COM;.EXE;.BAT;.CMD")
655+
.expect("pnpm.CMD must resolve");
656+
assert_eq!(found.0, root.join("bin").join("pnpm.CMD"));
657+
assert!(found.1, "pnpm.CMD needs cmd /c");
658+
let _ = std::fs::remove_dir_all(&root);
659+
}
660+
661+
#[test]
662+
fn pnpm_resolve_honors_pathext_and_reports_missing() {
663+
// PATHEXT without .CMD skips pnpm.CMD and takes pnpm.BAT instead.
664+
let root = pnpm_resolve_fixture("pathext", &[("bin", "pnpm.CMD"), ("bin", "pnpm.BAT")]);
665+
let dirs = vec![root.join("bin")];
666+
let found = super::resolve_windows_program("pnpm", &dirs, ".COM;.EXE;.BAT")
667+
.expect("pnpm.BAT must resolve");
668+
assert_eq!(found.0, root.join("bin").join("pnpm.BAT"));
669+
assert!(found.1, "pnpm.BAT needs cmd /c");
670+
// Nothing installed: no resolution, so the caller fails closed.
671+
let empty = pnpm_resolve_fixture("empty", &[]);
672+
assert!(super::resolve_windows_program(
673+
"pnpm",
674+
std::slice::from_ref(&empty),
675+
".COM;.EXE;.BAT;.CMD"
676+
)
677+
.is_none());
678+
let _ = std::fs::remove_dir_all(&root);
679+
let _ = std::fs::remove_dir_all(&empty);
680+
}
681+
682+
#[test]
683+
#[cfg(not(windows))]
684+
fn pnpm_command_unix_is_bare_pnpm() {
685+
// Linux/macOS behavior stays byte-identical: a bare `pnpm` lookup.
686+
let cmd = super::pnpm_command().expect("pnpm command builds");
687+
assert_eq!(cmd.get_program(), "pnpm");
688+
}
547689
}

0 commit comments

Comments
 (0)