fix-deviant(termio): resolve bare program names on PATH on Windows
Intentional deviation from Ghostty 1.3.1 (DEVIANT_FIXES.md row 39). Zig's Command.startWindows (Command.zig:261,347-358) passes the command path as CreateProcessW's lpApplicationName, which is taken literally: a partial name is completed with the current directory only, never the search path. A bare `cmd.exe` (e.g. `command = direct:cmd.exe` or `-e cmd.exe`) therefore fails to start in Ghostty on Windows. A bare program name (no drive or separator) is now looked up on PATH, appending .exe when there is no extension as CreateProcess does; anything else or a failed lookup is passed through unchanged. Verified under Wine by the exec_windows ConPTY tests. Test Plan: - cargo test -p ghostty-termio resolve_application_searches_path_for_bare_names - just test-windows-wine
This commit is contained in:
1 parent
97a262dbe6
commit
4d7ad7cdba
2 files changed
+55
-1
No files matched your search
@@ -135,7 +135,11 @@ impl Command {
|
|||||||
// OS strings); validate UTF-8 and convert to UTF-16 for the Windows API.
|
// OS strings); validate UTF-8 and convert to UTF-16 for the Windows API.
|
||||||
// Like Zig's fallible `utf8ToUtf16Le*`, malformed input fails the
|
// Like Zig's fallible `utf8ToUtf16Le*`, malformed input fails the
|
||||||
// launch instead of selecting a U+FFFD-substituted path or argument.
|
// launch instead of selecting a U+FFFD-substituted path or argument.
|
||||||
let application_w = EncodedOs::from_encoded(&self.path).try_to_utf16_nul()?;
|
// DEVIANT (see DEVIANT_FIXES.md): `lpApplicationName` is never
|
||||||
|
// searched on PATH, so a bare `cmd.exe` (a `command = direct:` /
|
||||||
|
// `-e` argv[0]) would not start in Zig. Resolve it ourselves.
|
||||||
|
let resolved = resolve_application(&self.path);
|
||||||
|
let application_w = EncodedOs::from_encoded(&resolved).try_to_utf16_nul()?;
|
||||||
let cwd_w = self
|
let cwd_w = self
|
||||||
.cwd
|
.cwd
|
||||||
.as_ref()
|
.as_ref()
|
||||||
@@ -344,10 +348,59 @@ impl Command {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// `CreateProcessW` takes `lpApplicationName` literally (a partial name is
|
||||||
|
/// completed with the current directory only, the search path is *not* used).
|
||||||
|
/// For a bare program name (no drive or separator) look it up on `PATH`,
|
||||||
|
/// appending `.exe` when it has no extension as `CreateProcess` would.
|
||||||
|
/// Anything else, or a failed lookup, is returned unchanged.
|
||||||
|
fn resolve_application(path: &str) -> String {
|
||||||
|
if path.is_empty() || path.contains(['\\', '/', ':']) {
|
||||||
|
return path.to_owned();
|
||||||
|
}
|
||||||
|
let name = if std::path::Path::new(path).extension().is_some() {
|
||||||
|
path.to_owned()
|
||||||
|
} else {
|
||||||
|
format!("{path}.exe")
|
||||||
|
};
|
||||||
|
match ghostty_os::path::expand_os(std::ffi::OsStr::new(&name)) {
|
||||||
|
Ok(Some(found)) => found
|
||||||
|
.to_str()
|
||||||
|
.map_or_else(|| path.to_owned(), str::to_owned),
|
||||||
|
_ => path.to_owned(),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
#[cfg(test)]
|
#[cfg(test)]
|
||||||
mod tests {
|
mod tests {
|
||||||
use super::*;
|
use super::*;
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn resolve_application_searches_path_for_bare_names() {
|
||||||
|
let found = resolve_application("cmd.exe");
|
||||||
|
assert!(found.to_ascii_lowercase().ends_with("\\cmd.exe"), "{found}");
|
||||||
|
assert_ne!(found, "cmd.exe");
|
||||||
|
// Extension is implied like CreateProcess.
|
||||||
|
assert!(
|
||||||
|
resolve_application("cmd")
|
||||||
|
.to_ascii_lowercase()
|
||||||
|
.ends_with("\\cmd.exe")
|
||||||
|
);
|
||||||
|
// Paths and unknown names are left alone.
|
||||||
|
assert_eq!(resolve_application(r"C:\x\y.exe"), r"C:\x\y.exe");
|
||||||
|
assert_eq!(resolve_application(r".\y.exe"), r".\y.exe");
|
||||||
|
assert_eq!(
|
||||||
|
resolve_application("no-such-program-zzz"),
|
||||||
|
"no-such-program-zzz"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn bare_program_name_starts_via_path_search() {
|
||||||
|
let mut cmd = Command::new("cmd.exe", &["cmd.exe", "/c", "exit 3"]);
|
||||||
|
cmd.start().unwrap();
|
||||||
|
assert_eq!(cmd.wait(true).unwrap(), Exit::Exited(3));
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn run_cmd_and_wait() {
|
fn run_cmd_and_wait() {
|
||||||
let mut cmd = Command::new(
|
let mut cmd = Command::new(
|
||||||
|
|||||||
@@ -64,6 +64,7 @@ when this file was written.
|
|||||||
| 36 | Font (wasm/browser Canvas): atlas pixel format | `font/face/web_canvas.zig` (atlas write branch on `.rgba`) | The branch tests a stale `.rgba` format against an enum that contains `.bgra`, so BGRA atlases are never converted. | The BGRA atlas conversion is implemented. No claim of equivalence with a real browser (not testable here). | d89691b | `ghostty-font` web_canvas tests (compile/Node level) |
|
| 36 | Font (wasm/browser Canvas): atlas pixel format | `font/face/web_canvas.zig` (atlas write branch on `.rgba`) | The branch tests a stale `.rgba` format against an enum that contains `.bgra`, so BGRA atlases are never converted. | The BGRA atlas conversion is implemented. No claim of equivalence with a real browser (not testable here). | d89691b | `ghostty-font` web_canvas tests (compile/Node level) |
|
||||||
| 37 | Termio: subprocess cleanup | `termio/Exec.zig:1188-1224` (`getpgid`) | Loops forever while `getpgid(child) == our pgid`, i.e. also when the child died before `setsid` and is an unreaped zombie (its pgid stays ours), hanging the surface close. | Still waits without a cap (no 5 s limit) while the child is alive, but stops (no SIGHUP, nothing to kill) once `waitid(WNOWAIT)` shows the child exited. | uncommitted (P3) | `getpgid_gives_up_on_exited_child_without_setsid`, `getpgid_waits_for_late_setsid` (`ghostty-termio/src/exec.rs`) |
|
| 37 | Termio: subprocess cleanup | `termio/Exec.zig:1188-1224` (`getpgid`) | Loops forever while `getpgid(child) == our pgid`, i.e. also when the child died before `setsid` and is an unreaped zombie (its pgid stays ours), hanging the surface close. | Still waits without a cap (no 5 s limit) while the child is alive, but stops (no SIGHUP, nothing to kill) once `waitid(WNOWAIT)` shows the child exited. | uncommitted (P3) | `getpgid_gives_up_on_exited_child_without_setsid`, `getpgid_waits_for_late_setsid` (`ghostty-termio/src/exec.rs`) |
|
||||||
| 38 | Config (Windows): absolute theme paths | `config/Config.zig:9788-9799` (`Theme.parseCLI`) | Any value containing `,`, `=` or `:` is parsed as a `light:..,dark:..` pair, and a Windows absolute path (`C:\\themes\\x`, `\\\\?\\C:\\...`) always contains a colon, so `theme = <absolute path>` is rejected on Windows (found by running the config tests under Wine). | On Windows a value that starts like an absolute Windows path (drive letter + `:` + separator, or `\\\\`) is a single theme; the pair syntax is unchanged (quote backslashes). Other platforms are unchanged. | uncommitted (agent WINE) | `windows_absolute_path_detection`, `theme_windows_absolute_path_and_pair_with_paths` (`ghostty-config/src/types_b.rs`), `theme_loading` under Wine |
|
| 38 | Config (Windows): absolute theme paths | `config/Config.zig:9788-9799` (`Theme.parseCLI`) | Any value containing `,`, `=` or `:` is parsed as a `light:..,dark:..` pair, and a Windows absolute path (`C:\\themes\\x`, `\\\\?\\C:\\...`) always contains a colon, so `theme = <absolute path>` is rejected on Windows (found by running the config tests under Wine). | On Windows a value that starts like an absolute Windows path (drive letter + `:` + separator, or `\\\\`) is a single theme; the pair syntax is unchanged (quote backslashes). Other platforms are unchanged. | uncommitted (agent WINE) | `windows_absolute_path_detection`, `theme_windows_absolute_path_and_pair_with_paths` (`ghostty-config/src/types_b.rs`), `theme_loading` under Wine |
|
||||||
|
| 39 | Termio (Windows): bare program names | `Command.zig:261,347-358` (`startWindows`, `CreateProcessW(application_w, ...)`) with `termio/Exec.zig:1004` (`.path = self.args[0]`) | `lpApplicationName` is never searched on `PATH` (a partial name only completes against the current directory), so a `direct:` command / `-e` argv[0] such as `cmd.exe` fails with `FileNotFound`; only the default shell works (it is wrapped as `%WINDIR%\\System32\\cmd.exe /C ...`). | A name without drive/separator is looked up on `PATH` (`.exe` implied) before `CreateProcessW`; failures keep the original name (same error as Zig). | uncommitted (agent WINE) | `resolve_application_searches_path_for_bare_names`, `bare_program_name_starts_via_path_search` (`ghostty-termio/src/command_windows.rs`), `echo_output_reaches_terminal_and_exit_is_reported` (`tests/exec_windows.rs`, Wine) |
|
||||||
|
|
||||||
## Upstream quirks deliberately kept (not fixed)
|
## Upstream quirks deliberately kept (not fixed)
|
||||||
|
|
||||||
|
|||||||
Reference in new issue
Block a user