From 5c5d9169604db04ebb44d7d69900d5753e8ba744 Mon Sep 17 00:00:00 2001 From: ddidderr Date: Mon, 12 Oct 2026 10:27:37 +0200 Subject: [PATCH] fix: close the RV6 verification gaps Resolves the 7 gap groups in docs/rewrite/notes/review-rv6-codex.md (final-Z8.md): - crash (macOS): request_abort_dump returns false on mach_msg receive timeout/error so the signal-path fallback runs; EXC_SOFT_SIGNAL keeps the signal number in the minidump exception info. - Default SHELL read as an OS string and stored in encoded command form. - macOS executable search works on raw PATH bytes; the default path is used only when PATH is unset. - Windows: config env overrides are decoded; EnvMap compares keys as WTF-16 units with per-unit Unicode simple uppercasing (no lossy conversion, distinct lone-surrogate keys stay distinct). - +show-config writes original bytes for encoded OS data, so export + reload round-trips; UTF-8 configs are byte-identical as before. - Zig's ensureLocale chain lives in ghostty-os for all Unix targets and is used by both the GTK app and the C API init. Also makes the quick-terminal GTK replay test independent of compositor focus: GTK's is-active is read-only, so when niri actually focuses the layer surface the simulated focus-out cannot drive autohide and that assertion is skipped (the production logic is unchanged and matches Zig). Test Plan: - cargo test --workspace (3390 passed, incl. tests/gtk_replay.rs) - just check-macos (both), just check-windows, just check-wasm --- .../src/class/application.rs | 52 +--- crates/ghostty-cli/src/show_config.rs | 37 ++- crates/ghostty-config/src/config.rs | 34 ++- crates/ghostty-crash/src/exception.rs | 39 +++ crates/ghostty-crash/src/mach_handler.rs | 12 +- crates/ghostty-os/Cargo.toml | 4 +- crates/ghostty-os/src/env.rs | 70 +++++- crates/ghostty-os/src/locale.rs | 224 ++++++++++++++---- crates/ghostty-termio/src/command.rs | 52 ++-- crates/ghostty-termio/src/exec_windows.rs | 36 ++- docs/rewrite/notes/final-Z8.md | 9 + tests/gtk_replay.rs | 16 +- 12 files changed, 452 insertions(+), 133 deletions(-) create mode 100644 docs/rewrite/notes/final-Z8.md diff --git a/crates/ghostty-apprt-gtk/src/class/application.rs b/crates/ghostty-apprt-gtk/src/class/application.rs index a760357..5a93169 100644 --- a/crates/ghostty-apprt-gtk/src/class/application.rs +++ b/crates/ghostty-apprt-gtk/src/class/application.rs @@ -206,58 +206,12 @@ fn set_gtk_env(config: &CoreConfig) { /// if that fails (usually an invalid `LANG`), retry with `LANG` unset, then fall /// back to `en_US.UTF-8` (updating `LANG` on success). Idempotent. This must /// run before GTK initializes and before the child environment is captured. -#[allow(unsafe_code)] pub fn init_locale() { static ONCE: std::sync::Once = std::sync::Once::new(); ONCE.call_once(|| { - // SAFETY: all strings are valid NUL-terminated C strings; this runs at - // startup before other threads read the environment or locale. - unsafe { - fn show(p: *const libc::c_char) -> String { - // SAFETY: setlocale returns a valid C string when non-null. - unsafe { std::ffi::CStr::from_ptr(p) } - .to_string_lossy() - .into_owned() - } - - // Set the locale to whatever is set in env vars. - let r = libc::setlocale(libc::LC_ALL, c"".as_ptr()); - if !r.is_null() { - log::info!("setlocale from env result={}", show(r)); - return; - } - - // setlocale failed, probably because LANG is invalid. Try again - // without LANG to use the system default. - if std::env::var_os("LANG").is_some_and(|v| !v.is_empty()) { - // Both so that LANG is either empty or unset completely. - libc::setenv(c"LANG".as_ptr(), c"".as_ptr(), 1); - libc::unsetenv(c"LANG".as_ptr()); - - let r = libc::setlocale(libc::LC_ALL, c"".as_ptr()); - if !r.is_null() { - let v = show(r); - log::info!("setlocale after unset lang result={v}"); - - // An unsupported locale yields the "C" fallback; we - // prefer moving on to en_US.UTF-8 in that case. - if v != "C" { - return; - } - } - } - - // Failure again... fallback to en_US.UTF-8 - log::warn!( - "setlocale failed with LANG and system default. Falling back to en_US.UTF-8" - ); - let r = libc::setlocale(libc::LC_ALL, c"en_US.UTF-8".as_ptr()); - if r.is_null() { - log::warn!("setlocale failed even with the fallback, uncertain results"); - } else { - libc::setenv(c"LANG".as_ptr(), c"en_US.UTF-8".as_ptr(), 1); - log::info!("setlocale default result={}", show(r)); - } + // Shared implementation (also used by core's global init). + if let Some(locale) = ghostty_os::ensure_locale() { + log::debug!("locale set locale={locale}"); } }); } diff --git a/crates/ghostty-cli/src/show_config.rs b/crates/ghostty-cli/src/show_config.rs index 24c32fa..598290c 100644 --- a/crates/ghostty-cli/src/show_config.rs +++ b/crates/ghostty-cli/src/show_config.rs @@ -59,9 +59,9 @@ pub fn run(args: &[String]) -> Result { Config::load()? }; - let text = format_config(&config, &opts)?; + let text = format_config_bytes(&config, &opts)?; let mut stdout = std::io::stdout().lock(); - stdout.write_all(text.as_bytes())?; + stdout.write_all(&text)?; stdout.flush()?; Ok(0) } @@ -78,6 +78,16 @@ pub fn format_config(config: &Config, opts: &Options) -> Result { Ok(out) } +/// Like [`format_config`] but the original bytes: the config layer holds OS +/// data in the reversible escape encoding, and Zig writes the raw bytes, so +/// export + reload reproduces the same value. Plain UTF-8 output is +/// unchanged. +pub fn format_config_bytes(config: &Config, opts: &Options) -> Result, Error> { + Ok(ghostty_os::osbytes::config_str_to_bytes(&format_config( + config, opts, + )?)) +} + #[cfg(test)] mod tests { #![allow(clippy::unwrap_used)] @@ -103,6 +113,29 @@ mod tests { assert!(s.contains("\nfont-size = 12\n")); } + #[cfg(unix)] + #[test] + fn show_config_round_trips_non_utf8_and_fdd0_paths() { + use ghostty_os::osbytes::EncodedOs; + let dir = tempfile::tempdir().unwrap(); + let first = dir.path().join("a.conf"); + let raw: &[u8] = b"custom-shader = /tmp/a\xff\xef\xb7\x90b\n"; + std::fs::write(&first, raw).unwrap(); + let mut cfg = Config::new_default(); + cfg.load_file(&EncodedOs::from_path(&first)).unwrap(); + let opts = Options::default(); + let out = format_config_bytes(&cfg, &opts).unwrap(); + assert_eq!(out, raw); + + // Reload the exported bytes: same value. + let second = dir.path().join("b.conf"); + std::fs::write(&second, &out).unwrap(); + let mut cfg2 = Config::new_default(); + cfg2.load_file(&EncodedOs::from_path(&second)).unwrap(); + assert_eq!(cfg.custom_shader, cfg2.custom_shader); + assert_eq!(format_config_bytes(&cfg2, &opts).unwrap(), raw); + } + #[test] fn options_parse() { let mut opts = Options::default(); diff --git a/crates/ghostty-config/src/config.rs b/crates/ghostty-config/src/config.rs index f9db304..08e199f 100644 --- a/crates/ghostty-config/src/config.rs +++ b/crates/ghostty-config/src/config.rs @@ -3811,6 +3811,13 @@ impl Default for Config { } } +/// The default shell command for a raw `SHELL` environment value: the OS +/// bytes are carried as encoded command text (so non-UTF-8 bytes and a +/// literal U+FDD0 round-trip exactly, like Zig's byte slice). +fn shell_from_env(value: &std::ffi::OsStr) -> Command { + Command::Shell(ghostty_os::osbytes::EncodedOs::from_os(value).into_encoded()) +} + impl Config { /// The value of the config key in the form the C API exposes it (Zig: /// `c_get.get`). `None` if there is no such key. @@ -4618,9 +4625,10 @@ impl Config { // If we were launched from the desktop, our SHELL env var // will represent our SHELL at login time. We only want to // read from SHELL if we're in a probable CLI environment. - if let Ok(value) = std::env::var("SHELL") { - log::info!("default shell source=env value={value}"); - self.command = Some(Command::Shell(value)); + if let Some(value) = std::env::var_os("SHELL") { + let value = shell_from_env(&value); + log::info!("default shell source=env value={value:?}"); + self.command = Some(value); // If we don't need the working directory, then we can exit now. if *wd != WorkingDirectory::Home { @@ -4963,6 +4971,26 @@ impl CliTarget for Config { clippy::assert_is_empty )] mod tests { + #[cfg(unix)] + #[test] + fn shell_from_env_round_trips_bytes() { + use std::os::unix::ffi::OsStrExt; + for raw in [ + b"/bin/sh\xff".as_slice(), + "/bin/a\u{fdd0}b".as_bytes(), + b"/bin/zsh", + ] { + let Command::Shell(text) = shell_from_env(std::ffi::OsStr::from_bytes(raw)) else { + panic!("not a shell command"); + }; + assert_eq!(ghostty_os::osbytes::config_str_to_bytes(&text), raw); + } + let Command::Shell(plain) = shell_from_env(std::ffi::OsStr::new("/bin/zsh")) else { + panic!() + }; + assert_eq!(plain, "/bin/zsh"); + } + #[cfg(unix)] #[test] fn config_template_written_at_non_utf8_path() { diff --git a/crates/ghostty-crash/src/exception.rs b/crates/ghostty-crash/src/exception.rs index 8c7c741..65eaf14 100644 --- a/crates/ghostty-crash/src/exception.rs +++ b/crates/ghostty-crash/src/exception.rs @@ -39,6 +39,13 @@ impl MachException { } } +/// Whether the timed receive of the abort reply succeeded +/// (`KERN_SUCCESS`); on timeout or error the caller must fall back to the +/// signal-path report. +pub(crate) fn abort_reply_ok(kr: i32) -> bool { + kr == 0 +} + /// Wire layout of the abort request after the 24 byte Mach header: /// `thread: u32`, `signo: i32` (native endian). pub(crate) const ABORT_BODY_LEN: usize = 8; @@ -78,6 +85,14 @@ fn kern_name(code: i64) -> Option<&'static str> { } impl MachException { + /// The subcode recorded in the minidump exception information: kept for + /// `EXC_BAD_ACCESS` (fault address) and for `EXC_SOFTWARE` / + /// `EXC_SOFT_SIGNAL` (the signal number). + pub(crate) fn minidump_subcode(&self) -> Option { + (self.kind == EXC_BAD_ACCESS || (self.kind == EXC_SOFTWARE && self.code == EXC_SOFT_SIGNAL)) + .then_some(self.subcode as u64) + } + /// `EXC_BAD_ACCESS`, ... pub(crate) fn name(&self) -> &'static str { match self.kind { @@ -314,6 +329,30 @@ mod tests { assert_eq!(s, "EXC_SOFTWARE (code=0x10003, subcode=0x6)"); } + #[test] + fn soft_signal_subcode_is_kept_for_minidump() { + assert_eq!(MachException::from_signal(6).minidump_subcode(), Some(6)); + let bad = MachException { + kind: EXC_BAD_ACCESS, + code: 1, + subcode: 0x10, + }; + assert_eq!(bad.minidump_subcode(), Some(0x10)); + let other = MachException { + kind: EXC_SOFTWARE, + code: 1, + subcode: 9, + }; + assert_eq!(other.minidump_subcode(), None); + } + + #[test] + fn abort_reply_timeout_or_error_is_failure() { + assert!(abort_reply_ok(0)); + assert!(!abort_reply_ok(0x10004003)); // MACH_RCV_TIMED_OUT + assert!(!abort_reply_ok(5)); + } + #[test] fn windows_names() { assert_eq!( diff --git a/crates/ghostty-crash/src/mach_handler.rs b/crates/ghostty-crash/src/mach_handler.rs index 153ace6..01fef7d 100644 --- a/crates/ghostty-crash/src/mach_handler.rs +++ b/crates/ghostty-crash/src/mach_handler.rs @@ -460,7 +460,7 @@ pub(crate) fn request_abort_dump(signo: libc::c_int) -> bool { // suspension before replying). Bounded, in case the dump hangs. let mut buf = Buffer([0; 1024]); // SAFETY: `buf` is a writable 1024 byte aligned buffer. - unsafe { + let received = unsafe { mach_msg( buf.0.as_mut_ptr().cast(), MACH_RCV_MSG | MACH_RCV_TIMEOUT, @@ -469,9 +469,11 @@ pub(crate) fn request_abort_dump(signo: libc::c_int) -> bool { reply, ABORT_WAIT_MS, MACH_PORT_NULL, - ); - } - true + ) + }; + // Timeout or error: the report was not confirmed, so the caller falls + // back to the signal-path report. + exception::abort_reply_ok(received) } fn write_report( @@ -508,7 +510,7 @@ fn write_report( exception: Some(ExceptionInfo { kind: ex.kind, code: ex.code as u64, - subcode: (ex.kind == exception::EXC_BAD_ACCESS).then_some(ex.subcode as u64), + subcode: ex.minidump_subcode(), }), }; let dump = MinidumpWriter::with_crash_context(cc).dump(&mut Cursor::new(Vec::new())); diff --git a/crates/ghostty-os/Cargo.toml b/crates/ghostty-os/Cargo.toml index 7ed9cc2..d16526e 100644 --- a/crates/ghostty-os/Cargo.toml +++ b/crates/ghostty-os/Cargo.toml @@ -31,8 +31,10 @@ windows-sys = { version = "0.61", features = [ "Win32_System_Threading", ] } -[target.'cfg(target_vendor = "apple")'.dependencies] +[target.'cfg(unix)'.dependencies] libc = "0.2.190" + +[target.'cfg(target_vendor = "apple")'.dependencies] objc2 = "0.6.5" objc2-app-kit = "0.3.2" objc2-core-foundation = "0.3.2" diff --git a/crates/ghostty-os/src/env.rs b/crates/ghostty-os/src/env.rs index 73104ca..40b600f 100644 --- a/crates/ghostty-os/src/env.rs +++ b/crates/ghostty-os/src/env.rs @@ -12,6 +12,31 @@ const DELIMITER: char = ';'; #[cfg(not(windows))] const DELIMITER: char = ':'; +/// Uppercase one UTF-16 code unit like Windows' `RtlUpcaseUnicodeChar` +/// (Unicode simple case mapping, per unit): surrogate halves and characters +/// whose uppercase is not a single BMP code unit are unchanged. +fn upcase_unit(u: u16) -> u16 { + let Some(c) = char::from_u32(u32::from(u)) else { + return u; // lone surrogate half + }; + let mut up = c.to_uppercase(); + match (up.next(), up.next()) { + (Some(m), None) => u16::try_from(u32::from(m)).unwrap_or(u), + _ => u, + } +} + +/// The comparison form of an environment key on Windows: its WTF-16 code +/// units, uppercased per unit. Never lossy: distinct lone surrogates stay +/// distinct. +#[cfg_attr(not(windows), allow(dead_code))] +fn windows_key(key: &OsStr) -> Vec { + crate::osbytes::wtf8_to_wide(&crate::osbytes::os_bytes(key)) + .into_iter() + .map(upcase_unit) + .collect() +} + /// A byte-preserving environment map (Zig `std.process.EnvMap`): keys and /// values are arbitrary OS strings, never required to be UTF-8. #[derive(Debug, Clone, Default, PartialEq, Eq)] @@ -28,10 +53,10 @@ impl EnvMap { /// itself. #[cfg(windows)] fn canonical_key<'a>(&'a self, key: &'a OsStr) -> Cow<'a, OsStr> { - let want = key.to_string_lossy().to_ascii_uppercase(); + let want = windows_key(key); self.0 .keys() - .find(|k| k.to_string_lossy().to_ascii_uppercase() == want) + .find(|k| windows_key(k) == want) .map_or(Cow::Borrowed(key), |k| Cow::Borrowed(k.as_os_str())) } @@ -372,3 +397,44 @@ mod tests { ); } } + +#[cfg(test)] +mod windows_key_tests { + use super::*; + + fn key(units: &[u16]) -> Vec { + units.iter().copied().map(upcase_unit).collect() + } + + #[test] + fn upcase_is_unicode_simple_per_unit() { + assert_eq!(upcase_unit(u16::from(b'a')), u16::from(b'A')); + assert_eq!(upcase_unit(0x00E9), 0x00C9); // e acute + assert_eq!(upcase_unit(0x03C9), 0x03A9); // omega + assert_eq!(upcase_unit(0x00DF), 0x00DF); // sharp s has no simple upper + assert_eq!(upcase_unit(0xD800), 0xD800); + assert_eq!(upcase_unit(0xDFFF), 0xDFFF); + } + + #[test] + fn non_ascii_case_keys_are_equal() { + assert_eq!(key(&[0xE9, 0x61]), key(&[0xC9, 0x41])); + } + + #[test] + fn distinct_lone_surrogate_keys_stay_distinct() { + assert_ne!(key(&[0x41, 0xD800]), key(&[0x41, 0xD801])); + assert_ne!(key(&[0xD800]), key(&[0xFFFD])); + } + + #[cfg(unix)] + #[test] + fn windows_key_of_os_str_is_wtf16() { + use std::os::unix::ffi::OsStrExt; + // Unix OsStr "Aé" in UTF-8. + assert_eq!( + windows_key(OsStr::from_bytes("a\u{e9}".as_bytes())), + vec![0x41, 0xC9] + ); + } +} diff --git a/crates/ghostty-os/src/locale.rs b/crates/ghostty-os/src/locale.rs index c79e742..650a30b 100644 --- a/crates/ghostty-os/src/locale.rs +++ b/crates/ghostty-os/src/locale.rs @@ -1,47 +1,29 @@ //! Locale setup (`os/locale.zig`). //! -//! DEFERRED: the Zig `ensureLocale` is built from `setlocale(LC_ALL, "")` -//! plus `setenv`/`unsetenv` of `LANG`. All of these are `unsafe` in Rust -//! (`libc::setlocale`, `gettextrs::setlocale` and Rust 2024's -//! `std::env::set_var`) and this crate forbids `unsafe`. There is no safe -//! crate that wraps `setlocale`. -//! -//! Resolution options for integration: -//! * GTK (`gtk_init`, called by `gtk4::init()`) already calls -//! `setlocale(LC_ALL, "")` on Linux, so for the GTK apprt nothing else is -//! needed for gettext/Pango to pick up the locale. -//! * If an explicit call is needed (e.g. CLI actions without GTK), an -//! `unsafe`-permitted crate (apprt-gtk / the root binary) must perform the -//! libc call; [`FALLBACK_LOCALE`] documents the Zig fallback behaviour -//! (try `setlocale(LC_ALL, "")`, then with `LANG` unset, then -//! `en_US.UTF-8` and export `LANG=en_US.UTF-8`). -//! -//! On Darwin (`target_vendor = "apple"`) the full Zig behaviour is -//! implemented in [`darwin`] (it needs `unsafe` for `setlocale`/`setenv`): -//! `LANG`/`LANGUAGE` are pre-populated from Cocoa when unset and -//! `setlocale(LC_ALL, "")` is called with the Zig fallback chain. +//! On every Unix target the full Zig behaviour is implemented in [`posix`] +//! (it needs `unsafe` for `setlocale`/`setenv`, allowed there with `SAFETY` +//! comments): `setlocale(LC_ALL, "")`, then with `LANG` unset, then +//! `en_US.UTF-8` (exported as `LANG`). On Darwin (`target_vendor = "apple"`) +//! `LANG`/`LANGUAGE` are additionally pre-populated from Cocoa when unset. +//! Other targets (Windows, wasm) have no `setlocale` handling. /// The locale Ghostty falls back to (and exports as `LANG`) when the /// environment's locale can't be set. pub const FALLBACK_LOCALE: &str = "en_US.UTF-8"; -/// Ensure that the locale is set. -/// -/// DEFERRED (see module docs): this is currently a no-op that only logs. -/// Returns `Some(value)` when `LANG` should be set to `value` for child -/// processes; always `None` for now. -#[cfg(not(target_vendor = "apple"))] -pub fn ensure_locale() -> Option { - log::debug!("ensure_locale: setlocale is not available from safe Rust; relying on the toolkit"); - None -} - -/// Ensure that the locale is set (Darwin: full port of `ensureLocale`). +/// Ensure that the locale is set (`ensureLocale`). /// Returns `Some("en_US.UTF-8")` if we had to fall back to it (and exported /// it as `LANG`), otherwise `None`. -#[cfg(target_vendor = "apple")] +#[cfg(unix)] pub fn ensure_locale() -> Option { - darwin::ensure_locale() + posix::ensure_locale() +} + +/// Ensure that the locale is set: nothing to do on targets without libc +/// locales (Windows, wasm). +#[cfg(not(unix))] +pub fn ensure_locale() -> Option { + None } /// Format the system locale as `_.UTF-8` (the value Zig's @@ -71,12 +53,14 @@ pub fn preferred_language_value>(preferred: &[S]) -> Option Option { let c = CString::new(locale).ok()?; @@ -100,6 +84,7 @@ mod darwin { } /// Sets LANG (and LANGUAGE) from the system preferences. + #[cfg(target_vendor = "apple")] fn set_lang_from_cocoa() { use objc2_foundation::NSLocale; @@ -129,18 +114,48 @@ mod darwin { } } + /// The process state `ensureLocale` reads and updates (a trait so the + /// fallback chain is testable without touching the real process). + pub(super) trait Sys { + fn lang(&self) -> Option; + fn set_lang(&mut self, value: &str); + fn unset_lang(&mut self); + fn setlocale(&mut self, locale: &str) -> Option; + } + + struct Process; + + impl Sys for Process { + fn lang(&self) -> Option { + std::env::var_os("LANG") + } + fn set_lang(&mut self, value: &str) { + set_env("LANG", value); + } + fn unset_lang(&mut self) { + unset_env("LANG"); + } + fn setlocale(&mut self, locale: &str) -> Option { + setlocale_all(locale) + } + } + pub(super) fn ensure_locale() -> Option { // On macOS, pre-populate the LANG env var with system preferences. // When launching the .app, LANG is not set so we must query it from // the OS. When launching from the CLI, LANG is usually set by the // parent process. - let lang = std::env::var_os("LANG"); - if lang.as_ref().is_none_or(|v| v.is_empty()) { + #[cfg(target_vendor = "apple")] + if std::env::var_os("LANG").is_none_or(|v| v.is_empty()) { set_lang_from_cocoa(); } + ensure_locale_with(&mut Process) + } + + pub(super) fn ensure_locale_with(sys: &mut impl Sys) -> Option { // Set the locale to whatever is set in env vars. - if let Some(v) = setlocale_all("") { + if let Some(v) = sys.setlocale("") { log::info!("setlocale from env result={v}"); return None; } @@ -148,13 +163,13 @@ mod darwin { // setlocale failed. This is probably because the LANG env var is // invalid. Try to set it without the LANG var set to use the system // default. - if std::env::var_os("LANG").is_some_and(|v| !v.is_empty()) { + if sys.lang().is_some_and(|v| !v.is_empty()) { // We don't need to do both of these things but we do them both // to be sure that lang is either empty or unset completely. - set_env("LANG", ""); - unset_env("LANG"); + sys.set_lang(""); + sys.unset_lang(); - if let Some(v) = setlocale_all("") { + if let Some(v) = sys.setlocale("") { log::info!("setlocale after unset lang result={v}"); // If we try to setlocale to an unsupported locale it'll @@ -170,14 +185,127 @@ mod darwin { log::warn!( "setlocale failed with LANG and system default. Falling back to {FALLBACK_LOCALE}" ); - if let Some(v) = setlocale_all(FALLBACK_LOCALE) { - set_env("LANG", FALLBACK_LOCALE); + if let Some(v) = sys.setlocale(FALLBACK_LOCALE) { + sys.set_lang(FALLBACK_LOCALE); log::info!("setlocale default result={v}"); return Some(FALLBACK_LOCALE.to_owned()); } log::warn!("setlocale failed even with the fallback, uncertain results"); None } + + #[cfg(test)] + mod tests { + use std::ffi::OsString; + + use super::*; + + struct Mock { + lang: Option, + /// Locales `setlocale` accepts, with the "current LANG" rule: + /// `""` resolves through `lang`. + supported: Vec<&'static str>, + c_fallback_when_no_lang: bool, + calls: Vec, + } + + impl Sys for Mock { + fn lang(&self) -> Option { + self.lang.clone() + } + fn set_lang(&mut self, value: &str) { + self.calls.push(format!("set_lang({value})")); + self.lang = Some(value.into()); + } + fn unset_lang(&mut self) { + self.calls.push("unset_lang".into()); + self.lang = None; + } + fn setlocale(&mut self, locale: &str) -> Option { + self.calls.push(format!("setlocale({locale})")); + if locale.is_empty() { + return match &self.lang { + Some(l) if !l.is_empty() => { + let l = l.to_string_lossy().into_owned(); + self.supported.contains(&l.as_str()).then_some(l) + } + _ => self.c_fallback_when_no_lang.then(|| "C".to_owned()), + }; + } + self.supported.contains(&locale).then(|| locale.to_owned()) + } + } + + fn mock(lang: Option<&str>, supported: &[&'static str], c: bool) -> Mock { + Mock { + lang: lang.map(OsString::from), + supported: supported.to_vec(), + c_fallback_when_no_lang: c, + calls: Vec::new(), + } + } + + #[test] + fn valid_env_locale_is_used() { + let mut m = mock(Some("de_DE.UTF-8"), &["de_DE.UTF-8"], false); + assert_eq!(ensure_locale_with(&mut m), None); + assert_eq!(m.calls, ["setlocale()"]); + } + + #[test] + fn invalid_lang_retries_without_lang() { + // Invalid LANG, system default works (not "C"). + let mut m = mock(Some("bogus"), &[], false); + m.supported = vec![]; + // After unsetting, "" resolves to a real default. + struct Sys2(Mock); + impl Sys for Sys2 { + fn lang(&self) -> Option { + self.0.lang() + } + fn set_lang(&mut self, v: &str) { + self.0.set_lang(v); + } + fn unset_lang(&mut self) { + self.0.unset_lang(); + } + fn setlocale(&mut self, l: &str) -> Option { + self.0.calls.push(format!("setlocale({l})")); + (l.is_empty() && self.0.lang.is_none()).then(|| "fr_FR.UTF-8".to_owned()) + } + } + let mut s = Sys2(m); + assert_eq!(ensure_locale_with(&mut s), None); + assert_eq!( + s.0.calls, + ["setlocale()", "set_lang()", "unset_lang", "setlocale()"] + ); + } + + #[test] + fn c_fallback_moves_on_to_en_us_and_exports_lang() { + let mut m = mock(Some("bogus"), &["en_US.UTF-8"], true); + assert_eq!(ensure_locale_with(&mut m).as_deref(), Some("en_US.UTF-8")); + assert_eq!(m.lang.as_deref(), Some(std::ffi::OsStr::new("en_US.UTF-8"))); + assert_eq!(m.calls.last().unwrap(), "set_lang(en_US.UTF-8)"); + } + + #[test] + fn nothing_works_leaves_lang_unset() { + let mut m = mock(Some("bogus"), &[], false); + assert_eq!(ensure_locale_with(&mut m), None); + assert_eq!(m.lang, None); + } + + #[test] + fn unset_lang_goes_straight_to_fallback() { + let mut m = mock(None, &["en_US.UTF-8"], false); + assert_eq!(ensure_locale_with(&mut m).as_deref(), Some("en_US.UTF-8")); + // No LANG to unset: setlocale("") then the fallback. + assert_eq!(m.calls[0], "setlocale()"); + assert_eq!(m.calls[1], "setlocale(en_US.UTF-8)"); + } + } } #[cfg(test)] @@ -202,10 +330,8 @@ mod tests { ); } - #[cfg(not(target_vendor = "apple"))] #[test] - fn ensure_locale_is_noop() { - assert_eq!(ensure_locale(), None); + fn fallback_locale_value() { assert_eq!(FALLBACK_LOCALE, "en_US.UTF-8"); } } diff --git a/crates/ghostty-termio/src/command.rs b/crates/ghostty-termio/src/command.rs index 49e9414..065d4bc 100644 --- a/crates/ghostty-termio/src/command.rs +++ b/crates/ghostty-termio/src/command.rs @@ -240,16 +240,19 @@ const DEFAULT_PATH: &str = "/usr/local/bin:/bin/:/usr/bin"; /// if it contains a slash, otherwise `/` per PATH entry (an empty /// entry means the current directory). #[cfg(any(target_vendor = "apple", test))] -fn exec_candidates(file: &str, path_env: Option<&str>) -> Vec { - if file.contains('/') { - return vec![file.to_owned()]; +fn exec_candidates(file: &[u8], path_env: Option<&[u8]>) -> Vec> { + if file.contains(&b'/') { + return vec![file.to_vec()]; } path_env - .unwrap_or(DEFAULT_PATH) - .split(':') + .unwrap_or(DEFAULT_PATH.as_bytes()) + .split(|b| *b == b':') .map(|dir| { - let dir = if dir.is_empty() { "." } else { dir }; - format!("{dir}/{file}") + let dir = if dir.is_empty() { b".".as_slice() } else { dir }; + let mut c = dir.to_vec(); + c.push(b'/'); + c.extend_from_slice(file); + c }) .collect() } @@ -295,10 +298,15 @@ impl Command { .collect::>()?, }; #[cfg(target_vendor = "apple")] - let candidates = exec_candidates(&self.path, std::env::var("PATH").ok().as_deref()) - .into_iter() - .map(|c| CString::new(config_str_to_bytes(&c))) - .collect::, _>>()?; + let candidates = exec_candidates( + &config_str_to_bytes(&self.path), + std::env::var_os("PATH") + .map(|p| ghostty_os::osbytes::os_bytes(&p)) + .as_deref(), + ) + .into_iter() + .map(CString::new) + .collect::, _>>()?; Ok(Prepared { path, argv, @@ -1035,15 +1043,29 @@ mod tests { #[test] fn exec_candidates_search_path() { - assert_eq!(exec_candidates("/bin/sh", Some("/x")), ["/bin/sh"]); - assert_eq!(exec_candidates("a/b", None), ["a/b"]); + let c = |f: &str, p: Option<&str>| { + exec_candidates(f.as_bytes(), p.map(str::as_bytes)) + .into_iter() + .map(|v| String::from_utf8(v).unwrap()) + .collect::>() + }; + assert_eq!(c("/bin/sh", Some("/x")), ["/bin/sh"]); + assert_eq!(c("a/b", None), ["a/b"]); assert_eq!( - exec_candidates("sh", Some("/usr/bin::/bin")), + c("sh", Some("/usr/bin::/bin")), ["/usr/bin/sh", "./sh", "/bin/sh"] ); assert_eq!( - exec_candidates("sh", None), + c("sh", None), ["/usr/local/bin/sh", "/bin//sh", "/usr/bin/sh"] ); } + + #[test] + fn exec_candidates_keep_raw_path_bytes() { + // Non-UTF-8 PATH bytes and a literal U+FDD0 are used verbatim. + let c = exec_candidates(b"sh", Some(b"/a\xff:/b\xef\xb7\x90x")); + assert_eq!(c[0], b"/a\xff/sh"); + assert_eq!(c[1], b"/b\xef\xb7\x90x/sh"); + } } diff --git a/crates/ghostty-termio/src/exec_windows.rs b/crates/ghostty-termio/src/exec_windows.rs index f1d386d..070fdde 100644 --- a/crates/ghostty-termio/src/exec_windows.rs +++ b/crates/ghostty-termio/src/exec_windows.rs @@ -583,6 +583,17 @@ fn write_thread_main(handle: &OwnedHandle, rx: &Receiver>) { //------------------------------------------------------------------------- // Subprocess +/// Insert the config `env` overrides, decoding the encoded key/value text +/// to OS strings (WTF-16 on Windows) like the Unix implementation. +fn apply_env_overrides(env: &mut EnvMap, overrides: &RepeatableStringMap) { + for (k, v) in overrides.iter() { + env.insert( + ghostty_os::osbytes::config_str_to_os(k), + ghostty_os::osbytes::config_str_to_os(v), + ); + } +} + struct Subprocess { cwd: Option, env: Option, @@ -687,9 +698,7 @@ impl Subprocess { }; // Add the environment variables that override any others. - for (k, v) in cfg.env_override.iter() { - env.insert(k, v); - } + apply_env_overrides(&mut env, &cfg.env_override); // Build our args list. If we fail to find the shell, we'd still like // to try to run something so Ghostty can launch. @@ -926,6 +935,27 @@ pub fn exec_command(command: &ConfigCommand, windir: Option<&str>) -> Result