diff --git a/crates/ghostty-apprt-gtk/src/class/surface.rs b/crates/ghostty-apprt-gtk/src/class/surface.rs index 2492f0d..7bb1614 100644 --- a/crates/ghostty-apprt-gtk/src/class/surface.rs +++ b/crates/ghostty-apprt-gtk/src/class/surface.rs @@ -1818,7 +1818,9 @@ impl Surface { log::warn!("invalid bell audio path: BadPathName"); return; }; - debug_assert!(path.is_absolute()); + debug_assert!(ghostty_os::osbytes::zig_is_absolute( + &ghostty_os::osbytes::os_bytes(path.as_os_str()) + )); let media_file = gtk::MediaFile::for_filename(&path); // If the audio file is marked as required, we'll emit an error if diff --git a/crates/ghostty-cli/src/crash_report.rs b/crates/ghostty-cli/src/crash_report.rs index 0ce4df8..160236b 100644 --- a/crates/ghostty-cli/src/crash_report.rs +++ b/crates/ghostty-cli/src/crash_report.rs @@ -42,7 +42,7 @@ pub fn run(args: &[impl AsRef<[u8]>]) -> Result { let stdout = std::io::stdout(); let is_tty = stdout.is_terminal(); - let mut out = std::io::BufWriter::new(stdout.lock()); + let mut out = crate::zig_writer::ZigWriter::new(crate::zig_writer::stdout_sink(), 1024); let result = run_inner(&default_dir()?, is_tty, now_ns(), &mut out); let _ = out.flush(); result diff --git a/crates/ghostty-cli/src/edit_config.rs b/crates/ghostty-cli/src/edit_config.rs index c245276..9370807 100644 --- a/crates/ghostty-cli/src/edit_config.rs +++ b/crates/ghostty-cli/src/edit_config.rs @@ -31,7 +31,7 @@ pub use crate::options::NoOptions as Options; /// if both are set. If neither are set, it will print an error /// and exit. pub fn run(args: &[impl AsRef<[u8]>]) -> Result { - let mut stderr = std::io::stderr().lock(); + let mut stderr = crate::zig_writer::ZigWriter::new(crate::zig_writer::stderr_sink(), 1024); let mut opts = Options; crate::parse_options(&mut opts, args)?; diff --git a/crates/ghostty-cli/src/ghostty.rs b/crates/ghostty-cli/src/ghostty.rs index 895df76..339abaf 100644 --- a/crates/ghostty-cli/src/ghostty.rs +++ b/crates/ghostty-cli/src/ghostty.rs @@ -165,7 +165,8 @@ impl Action { // to stdout. In the future we can style this much prettier // for all commands by just changing this one place. let text = format!("{}\n", self.help_text()); - let mut stdout = std::io::stdout().lock(); + let mut stdout = + crate::zig_writer::ZigWriter::new(crate::zig_writer::stdout_sink(), 1024); if let Err(e) = stdout.write_all(text.as_bytes()) { log::warn!("failed to write help text: {e}\n"); return Ok(1); diff --git a/crates/ghostty-cli/src/help.rs b/crates/ghostty-cli/src/help.rs index 01299e8..16f7788 100644 --- a/crates/ghostty-cli/src/help.rs +++ b/crates/ghostty-cli/src/help.rs @@ -26,7 +26,7 @@ pub fn run(args: &[impl AsRef<[u8]>]) -> Result { let mut opts = Options::default(); crate::parse_options(&mut opts, args)?; - let mut out = std::io::BufWriter::new(std::io::stdout().lock()); + let mut out = crate::zig_writer::ZigWriter::new(crate::zig_writer::stdout_sink(), 2048); write_help(&mut out)?; out.flush()?; Ok(0) diff --git a/crates/ghostty-cli/src/lib.rs b/crates/ghostty-cli/src/lib.rs index da9d7cd..01d268a 100644 --- a/crates/ghostty-cli/src/lib.rs +++ b/crates/ghostty-cli/src/lib.rs @@ -40,6 +40,7 @@ pub mod ssh_cache; pub mod tui; pub mod validate_config; pub mod version; +pub(crate) mod zig_writer; pub use error::Error; pub use ghostty::{Action, run_from_args}; diff --git a/crates/ghostty-cli/src/list_actions.rs b/crates/ghostty-cli/src/list_actions.rs index 593e1bd..fb57496 100644 --- a/crates/ghostty-cli/src/list_actions.rs +++ b/crates/ghostty-cli/src/list_actions.rs @@ -27,7 +27,7 @@ pub fn run(args: &[impl AsRef<[u8]>]) -> Result { crate::parse_options(&mut opts, args)?; let out = generate_plaintext(opts.docs); - let mut stdout = std::io::stdout().lock(); + let mut stdout = crate::zig_writer::ZigWriter::new(crate::zig_writer::stdout_sink(), 4096); stdout.write_all(out.as_bytes())?; stdout.flush()?; Ok(0) diff --git a/crates/ghostty-cli/src/list_fonts.rs b/crates/ghostty-cli/src/list_fonts.rs index 310336b..c06db32 100644 --- a/crates/ghostty-cli/src/list_fonts.rs +++ b/crates/ghostty-cli/src/list_fonts.rs @@ -97,7 +97,7 @@ pub fn run(args: &[impl AsRef<[u8]>]) -> Result { } // Output each - let mut stdout = std::io::BufWriter::new(std::io::stdout().lock()); + let mut stdout = crate::zig_writer::ZigWriter::new(crate::zig_writer::stdout_sink(), 2048); for family in &families { let Some(list) = map.get_mut(family) else { continue; diff --git a/crates/ghostty-cli/src/list_keybinds.rs b/crates/ghostty-cli/src/list_keybinds.rs index eabe5e1..f3c0853 100644 --- a/crates/ghostty-cli/src/list_keybinds.rs +++ b/crates/ghostty-cli/src/list_keybinds.rs @@ -73,7 +73,7 @@ pub fn run(args: &[impl AsRef<[u8]>]) -> Result { } let text = format_plain(&config.keybind, opts.docs)?; - let mut out = std::io::BufWriter::new(stdout.lock()); + let mut out = crate::zig_writer::ZigWriter::new(crate::zig_writer::stdout_sink(), 1024); out.write_all(text.as_bytes())?; // Don't forget to flush! out.flush()?; diff --git a/crates/ghostty-cli/src/list_themes.rs b/crates/ghostty-cli/src/list_themes.rs index 6fee1a0..1a9e06c 100644 --- a/crates/ghostty-cli/src/list_themes.rs +++ b/crates/ghostty-cli/src/list_themes.rs @@ -233,7 +233,7 @@ pub fn run(args: &[impl AsRef<[u8]>]) -> Result { crate::parse_options(&mut opts, args)?; let stdout = std::io::stdout(); - let mut stderr = std::io::stderr(); + let mut stderr = crate::zig_writer::ZigWriter::new(crate::zig_writer::stderr_sink(), 4096); let resources_dir = resources_dir(); if resources_dir.is_none() { @@ -262,7 +262,7 @@ pub fn run(args: &[impl AsRef<[u8]>]) -> Result { return Ok(0); } - let mut out = std::io::BufWriter::new(stdout.lock()); + let mut out = crate::zig_writer::ZigWriter::new(crate::zig_writer::stdout_sink(), 4096); write_plain(&mut out, &themes, &opts)?; // Don't forget to flush! out.flush()?; diff --git a/crates/ghostty-cli/src/new_window.rs b/crates/ghostty-cli/src/new_window.rs index 5bb435f..ba26b29 100644 --- a/crates/ghostty-cli/src/new_window.rs +++ b/crates/ghostty-cli/src/new_window.rs @@ -187,6 +187,13 @@ impl CliTarget for Options { /// /// Available since: 1.2.0 pub fn run(args: &[impl AsRef<[u8]>]) -> Result { + let mut stderr = crate::zig_writer::ZigWriter::new(crate::zig_writer::stderr_sink(), 1024); + let result = run_args(args, &mut stderr); + let _ = std::io::Write::flush(&mut stderr); + result +} + +fn run_args(args: &[impl AsRef<[u8]>], stderr: &mut dyn std::io::Write) -> Result { let mut opts = Options::default(); let mut iter = ghostty_config::cli_args::ArgsIterator::new(args.iter().skip(1).map(AsRef::as_ref)); @@ -196,7 +203,7 @@ pub fn run(args: &[impl AsRef<[u8]>]) -> Result { return Err(Error::ActionHelpRequested); } Err(err) => { - eprintln!("Error parsing args: {err}"); + writeln!(stderr, "Error parsing args: {err}")?; return Ok(1); } } @@ -237,7 +244,7 @@ pub fn run(args: &[impl AsRef<[u8]>]) -> Result { return Ok(1); } Err(e) => { - eprint!("Sending the IPC failed: {e}"); + write!(stderr, "Sending the IPC failed: {e}")?; return Ok(1); } }, @@ -248,7 +255,7 @@ pub fn run(args: &[impl AsRef<[u8]>]) -> Result { } // If we get here, the platform is not supported. - eprintln!("+new-window is not supported on this platform."); + writeln!(stderr, "+new-window is not supported on this platform.")?; Ok(1) } diff --git a/crates/ghostty-cli/src/show_config.rs b/crates/ghostty-cli/src/show_config.rs index feda24d..eca88b2 100644 --- a/crates/ghostty-cli/src/show_config.rs +++ b/crates/ghostty-cli/src/show_config.rs @@ -60,9 +60,9 @@ pub fn run(args: &[impl AsRef<[u8]>]) -> Result { }; let text = format_config_bytes(&config, &opts)?; - let mut stdout = std::io::stdout().lock(); + let mut stdout = crate::zig_writer::ZigWriter::new(crate::zig_writer::stdout_sink(), 4096); stdout.write_all(&text)?; - stdout.flush()?; + stdout.end()?; Ok(0) } diff --git a/crates/ghostty-cli/src/show_face.rs b/crates/ghostty-cli/src/show_face.rs index c94e12a..5fa4c03 100644 --- a/crates/ghostty-cli/src/show_face.rs +++ b/crates/ghostty-cli/src/show_face.rs @@ -1,6 +1,9 @@ //! `+show-face` (ZIG/src/cli/show_face.zig). +use std::io::Write; + use ghostty_config::{ + cli_args::U21, config::Config, config_enum, config_struct, @@ -14,8 +17,12 @@ use ghostty_font::{ face::DesiredSize, shared_grid_set::DerivedConfig, }; +use ghostty_os::osbytes::EncodedOs; -use crate::error::Error; +use crate::{ + error::Error, + zig_writer::{Sink, ZigWriter, stdout_sink}, +}; config_enum! { /// Font style (Zig: `font.Style`). @@ -38,10 +45,10 @@ config_enum! { config_struct! { pub struct Options { /// The codepoint to search for. - "cp" => cp: Option = None, + "cp" => cp: Option = None, /// Search for all of the codepoints in the string. - "string" => string: Option = None, + "string" => string: Option = None, /// Font style to search for. "style" => style: Style = Style::Regular, @@ -82,7 +89,15 @@ pub fn run(args: &[impl AsRef<[u8]>]) -> Result { let mut opts = Options::default(); crate::parse_options(&mut opts, args)?; - run_args(&opts, &mut std::io::stdout(), &mut std::io::stdout()) + // Zig creates both writers on stdout (`File.stdout()`), each with its own + // 1024 byte buffer, and flushes stdout then stderr at the end. + let sink: Sink = stdout_sink(); + let mut stdout = ZigWriter::new(sink.clone(), 1024); + let mut stderr = ZigWriter::new(sink, 1024); + let result = run_args(&opts, &mut stdout, &mut stderr); + let _ = stdout.flush(); + let _ = stderr.flush(); + result } fn run_args( @@ -175,16 +190,25 @@ fn run_args( let mut rc = None; if let Some(cp) = opts.cp { - rc = lookup(stdout, stderr, &grid, style, presentation, cp)?; + rc = lookup(stdout, stderr, &grid, style, presentation, cp.0)?; } if rc.is_none() && let Some(string) = &opts.string { - for ch in string.chars() { - rc = lookup(stdout, stderr, &grid, style, presentation, ch as u32)?; - if rc.is_some() { - break; + // `Utf8View.init` validates the whole string before any lookup. + if let Ok(text) = std::str::from_utf8(&string.to_bytes()) { + for ch in text.chars() { + rc = lookup(stdout, stderr, &grid, style, presentation, ch as u32)?; + if rc.is_some() { + break; + } } + } else { + write!( + stderr, + "Unable to parse string as unicode: error.InvalidUtf8" + )?; + rc = Some(1); } } @@ -230,3 +254,67 @@ fn lookup( )?; Ok(None) } + +#[cfg(test)] +mod tests { + #![allow(clippy::unwrap_used)] + use super::*; + + fn parse(args: &[&[u8]]) -> Options { + let mut full: Vec<&[u8]> = vec![b"ghostty"]; + full.extend_from_slice(args); + let mut opts = Options::default(); + crate::parse_options(&mut opts, &full).unwrap(); + opts + } + + #[test] + fn cp_is_u21() { + let o = parse(&[b"--cp=0x1FFFFF"]); + assert_eq!(o.cp, Some(U21(0x1f_ffff))); + assert!(o.diagnostics.is_empty()); + for bad in ["--cp=0x200000", "--cp=4294967295", "--cp=-1"] { + let o = parse(&[bad.as_bytes()]); + assert_eq!(o.cp, None, "{bad}"); + let d = &o.diagnostics.items()[0]; + assert_eq!(d.key, "cp"); + assert!(d.to_string().contains("invalid value"), "{d}"); + } + } + + #[test] + fn invalid_cp_exits_1_without_panic() { + let o = parse(&[b"--cp=4294967295"]); + let mut out = Vec::new(); + let mut err = Vec::new(); + assert_eq!(run_args(&o, &mut out, &mut err).unwrap(), 1); + let text = String::from_utf8(err).unwrap(); + assert!( + text.starts_with("config error: cli:") && text.contains("cp: invalid value"), + "{text}" + ); + } + + #[test] + fn string_accepts_raw_bytes_at_parse() { + let o = parse(&[b"--cp=65", b"--string=\xff"]); + assert!(o.diagnostics.is_empty()); + assert_eq!(o.string.unwrap().to_bytes(), b"\xff"); + } + + #[test] + fn invalid_utf8_string_fails_after_cp_lookup() { + let o = parse(&[b"--config-default-files=false", b"--string=\xff"]); + let mut out = Vec::new(); + let mut err = Vec::new(); + // Without a usable font setup this can fail earlier; only assert the + // Zig message when the grid was available. + let rc = run_args(&o, &mut out, &mut err).unwrap(); + let err = String::from_utf8(err).unwrap(); + if err.contains("Unable to") && !err.contains("as unicode") { + return; + } + assert_eq!(rc, 1); + assert_eq!(err, "Unable to parse string as unicode: error.InvalidUtf8"); + } +} diff --git a/crates/ghostty-cli/src/ssh_cache.rs b/crates/ghostty-cli/src/ssh_cache.rs index d5a1f0c..09654a6 100644 --- a/crates/ghostty-cli/src/ssh_cache.rs +++ b/crates/ghostty-cli/src/ssh_cache.rs @@ -50,8 +50,8 @@ pub fn run(args: &[impl AsRef<[u8]>]) -> Result { let mut opts = Options::default(); crate::parse_options(&mut opts, args)?; - let mut stdout = std::io::BufWriter::new(std::io::stdout().lock()); - let mut stderr = std::io::BufWriter::new(std::io::stderr().lock()); + let mut stdout = crate::zig_writer::ZigWriter::new(crate::zig_writer::stdout_sink(), 1024); + let mut stderr = crate::zig_writer::ZigWriter::new(crate::zig_writer::stderr_sink(), 1024); let result = run_inner(&opts, &mut stdout, &mut stderr); diff --git a/crates/ghostty-cli/src/validate_config.rs b/crates/ghostty-cli/src/validate_config.rs index a624fa7..7e13de4 100644 --- a/crates/ghostty-cli/src/validate_config.rs +++ b/crates/ghostty-cli/src/validate_config.rs @@ -29,9 +29,9 @@ pub fn run(args: &[impl AsRef<[u8]>]) -> Result { let mut opts = Options::default(); crate::parse_options(&mut opts, args)?; - let mut stdout = std::io::BufWriter::new(std::io::stdout().lock()); + let mut stdout = crate::zig_writer::ZigWriter::new(crate::zig_writer::stdout_sink(), 1024); let result = run_inner(&opts, &mut stdout); - stdout.flush()?; + stdout.end()?; result } diff --git a/crates/ghostty-cli/src/version.rs b/crates/ghostty-cli/src/version.rs index 742e4df..eb8a4b7 100644 --- a/crates/ghostty-cli/src/version.rs +++ b/crates/ghostty-cli/src/version.rs @@ -12,7 +12,7 @@ use crate::{ pub fn run(_args: &[impl AsRef<[u8]>]) -> Result { let stdout = std::io::stdout(); let tty = stdout.is_terminal(); - let mut out = std::io::BufWriter::new(stdout.lock()); + let mut out = crate::zig_writer::ZigWriter::new(crate::zig_writer::stdout_sink(), 1024); write_version(&mut out, &build_info::get(), tty)?; out.flush()?; Ok(0) diff --git a/crates/ghostty-cli/src/zig_writer.rs b/crates/ghostty-cli/src/zig_writer.rs new file mode 100644 index 0000000..f8bb532 --- /dev/null +++ b/crates/ghostty-cli/src/zig_writer.rs @@ -0,0 +1,157 @@ +//! A model of Zig 0.15 `std.fs.File.writer(&buffer).interface`. +//! +//! Zig actions write through a fixed-size buffer that is only written to the +//! file when it overflows or when the action calls `flush`, so two writers on +//! the same file (as in `+show-face`) interleave in flush order, not call +//! order. This type keeps that ordering. + +#[cfg(unix)] +use std::os::fd::AsFd; +use std::{ + cell::RefCell, + io::{self, Write}, + rc::Rc, +}; + +/// Where a [`ZigWriter`] drains to. +pub enum Target { + /// A duplicate of a standard descriptor. + #[cfg(unix)] + Fd(std::fs::File), + /// A plain stream. + Stream(Box), +} + +/// A shared output target so several [`ZigWriter`]s can drain to one file. +pub type Sink = Rc>; + +/// Standard output as a [`Sink`]. +#[must_use] +pub fn stdout_sink() -> Sink { + #[cfg(unix)] + if let Ok(fd) = io::stdout().as_fd().try_clone_to_owned() { + return Rc::new(RefCell::new(Target::Fd(fd.into()))); + } + Rc::new(RefCell::new(Target::Stream(Box::new(io::stdout())))) +} + +/// Standard error as a [`Sink`]. +#[must_use] +pub fn stderr_sink() -> Sink { + #[cfg(unix)] + if let Ok(fd) = io::stderr().as_fd().try_clone_to_owned() { + return Rc::new(RefCell::new(Target::Fd(fd.into()))); + } + Rc::new(RefCell::new(Target::Stream(Box::new(io::stderr())))) +} + +/// A sink over any stream (tests). +#[cfg(test)] +#[must_use] +pub fn stream_sink(w: impl Write + 'static) -> Sink { + Rc::new(RefCell::new(Target::Stream(Box::new(w)))) +} + +/// `File.Writer` with a `cap` byte buffer. +pub struct ZigWriter { + buf: Vec, + cap: usize, + sink: Sink, +} + +impl ZigWriter { + #[must_use] + pub fn new(sink: Sink, cap: usize) -> Self { + Self { + buf: Vec::with_capacity(cap), + cap, + sink, + } + } + + /// `File.Writer.end`. + pub fn end(&mut self) -> io::Result<()> { + self.flush() + } + + fn drain(&mut self, extra: &[u8]) -> io::Result<()> { + let mut pending = std::mem::take(&mut self.buf); + pending.extend_from_slice(extra); + if pending.is_empty() { + return Ok(()); + } + match &mut *self.sink.borrow_mut() { + #[cfg(unix)] + Target::Fd(file) => file.write_all(&pending), + Target::Stream(w) => { + w.write_all(&pending)?; + w.flush() + } + } + } +} + +impl Write for ZigWriter { + fn write(&mut self, data: &[u8]) -> io::Result { + self.write_all(data)?; + Ok(data.len()) + } + + /// Zig `Writer.write`: data that fits is buffered; otherwise the drain + /// writes the buffered bytes followed by the data straight to the file. + fn write_all(&mut self, data: &[u8]) -> io::Result<()> { + if self.buf.len() + data.len() <= self.cap { + self.buf.extend_from_slice(data); + return Ok(()); + } + self.drain(data) + } + + fn flush(&mut self) -> io::Result<()> { + self.drain(&[]) + } +} + +#[cfg(test)] +mod tests { + #![allow(clippy::unwrap_used)] + use super::*; + + #[derive(Clone, Default)] + struct Mem(Rc>>); + impl Write for Mem { + fn write(&mut self, d: &[u8]) -> io::Result { + self.0.borrow_mut().extend_from_slice(d); + Ok(d.len()) + } + fn flush(&mut self) -> io::Result<()> { + Ok(()) + } + } + + #[test] + fn buffers_until_flush_in_flush_order() { + let mem = Mem::default(); + let sink: Sink = stream_sink(mem.clone()); + let mut a = ZigWriter::new(sink.clone(), 1024); + let mut b = ZigWriter::new(sink, 1024); + b.write_all(b"err\n").unwrap(); + a.write_all(b"out\n").unwrap(); + assert!(mem.0.borrow().is_empty()); + a.flush().unwrap(); + b.flush().unwrap(); + assert_eq!(&*mem.0.borrow(), b"out\nerr\n"); + } + + #[test] + fn overflow_drains_buffer_then_data() { + let mem = Mem::default(); + let sink: Sink = stream_sink(mem.clone()); + let mut w = ZigWriter::new(sink, 4); + w.write_all(b"ab").unwrap(); + w.write_all(b"cdefg").unwrap(); + assert_eq!(&*mem.0.borrow(), b"abcdefg"); + w.write_all(b"h").unwrap(); + assert_eq!(&*mem.0.borrow(), b"abcdefg"); + } +} diff --git a/crates/ghostty-config/src/cli_args.rs b/crates/ghostty-config/src/cli_args.rs index ecb4412..8e64676 100644 --- a/crates/ghostty-config/src/cli_args.rs +++ b/crates/ghostty-config/src/cli_args.rs @@ -605,6 +605,30 @@ impl_int_field!( isize => |_| CValue::Unsupported ); +/// Zig `u21` (a Unicode scalar range integer, `0..=0x1F_FFFF`). +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] +pub struct U21(pub u32); + +impl U21 { + pub const MAX: u32 = 0x1f_ffff; +} + +impl ConfigField for U21 { + fn parse_bytes(&mut self, value: Option<&[u8]>) -> Result<(), ArgError> { + let v = value.ok_or(ArgError::ValueRequired)?; + let n = ghostty_os::zig_fmt::parse_int::(v, 0).map_err(|_| ArgError::InvalidValue)?; + if n > Self::MAX { + return Err(ArgError::InvalidValue); + } + *self = U21(n); + Ok(()) + } + + fn format_entry(&self, f: &mut EntryFormatter<'_>) -> fmt::Result { + writeln!(f.writer, "{} = {}", f.name, self.0) + } +} + macro_rules! impl_float_field { ($($t:ty => $cv:expr),*) => {$( impl ConfigField for $t { @@ -1021,6 +1045,29 @@ mod tests { } } + config_struct! { + pub struct Cp { + "cp" => cp: Option = None, + } + } + + #[test] + fn u21_field_matches_zig_range() { + let mut c = Cp::raw_default(); + c.parse_field("cp", Some("0x1FFFFF")).unwrap(); + assert_eq!(c.cp, Some(U21(0x1f_ffff))); + c.parse_field("cp", Some("0b1000001")).unwrap(); + assert_eq!(c.cp, Some(U21(65))); + for bad in ["0x200000", "4294967295", "99999999999999999999", "-1", "x"] { + assert_eq!( + c.parse_field("cp", Some(bad)), + Err(ArgError::InvalidValue), + "{bad}" + ); + } + assert_eq!(c.parse_field("cp", None), Err(ArgError::ValueRequired)); + } + config_enum! { pub enum OneTwo { One => "one", Two => "two" } } diff --git a/crates/ghostty-config/src/config.rs b/crates/ghostty-config/src/config.rs index d5acf31..b717767 100644 --- a/crates/ghostty-config/src/config.rs +++ b/crates/ghostty-config/src/config.rs @@ -3935,7 +3935,8 @@ impl Config { /// /// `path` must be resolved and absolute. pub fn load_file(&mut self, path: &EncodedOs) -> Result<(), Error> { - assert!(path.to_path()?.is_absolute()); + path.to_path()?; + assert!(path.is_zig_absolute()); let file = match crate::file_load::open(path) { Ok(f) => f, Err(crate::file_load::OpenFileError::NotAFile) => { @@ -4260,7 +4261,8 @@ impl Config { // All paths should already be absolute at this point because // they're fixed up after each load. - assert!(path.to_path()?.is_absolute()); + path.to_path()?; + assert!(path.is_zig_absolute()); // We must only load a unique file once if !loaded.insert(path.clone()) { diff --git a/crates/ghostty-config/src/file_load.rs b/crates/ghostty-config/src/file_load.rs index 570b240..33dd3c4 100644 --- a/crates/ghostty-config/src/file_load.rs +++ b/crates/ghostty-config/src/file_load.rs @@ -163,7 +163,7 @@ pub fn open(path: &EncodedOs) -> Result { log::warn!("invalid config path: {err}"); OpenFileError::FileOpenFailed })?; - assert!(real.is_absolute()); + assert!(path.is_zig_absolute()); let file = File::open(&real).map_err(|err| { if err.kind() == std::io::ErrorKind::NotFound { diff --git a/crates/ghostty-config/src/path.rs b/crates/ghostty-config/src/path.rs index 340e7f6..d84a978 100644 --- a/crates/ghostty-config/src/path.rs +++ b/crates/ghostty-config/src/path.rs @@ -112,12 +112,15 @@ impl Path { pub fn expand(&mut self, base: &EncodedOs, diags: &mut DiagnosticList) -> std::io::Result<()> { // Filesystem calls use the decoded OS paths. let base_path = base.to_path()?; - assert!(base_path.is_absolute()); + assert!(base.is_zig_absolute()); let path = self.path().clone(); // If it is already absolute we can ignore it. - if path.is_empty() || path.to_path()?.is_absolute() { + if path.is_empty() || { + path.to_path()?; + path.is_zig_absolute() + } { return Ok(()); } diff --git a/crates/ghostty-config/src/theme.rs b/crates/ghostty-config/src/theme.rs index e209a09..318878b 100644 --- a/crates/ghostty-config/src/theme.rs +++ b/crates/ghostty-config/src/theme.rs @@ -229,7 +229,7 @@ pub fn open( let theme_text = theme.as_encoded(); // Absolute themes are loaded a different path. - if theme_path.is_absolute() { + if theme.is_zig_absolute() { let file = open_absolute(theme, diags)?; if !check_regular(&file, theme_text, diags) { return None; @@ -446,6 +446,21 @@ mod tests { assert_eq!(zig_basename("./"), "."); } + #[test] + fn windows_basename_matches_zig_0_15_2() { + // The drive colon is recognized only at byte index 1. + assert_eq!(basename_windows("\u{3bb}:theme"), "\u{3bb}:theme"); + assert_eq!( + basename_windows("light:\u{3bb}:theme"), + "light:\u{3bb}:theme" + ); + assert_eq!(basename_windows("1:theme"), "theme"); + assert_eq!(basename_windows("c:"), ""); + assert_eq!(basename_windows("a\\b/c\\"), "c"); + assert_eq!(basename_windows("\\"), ""); + assert_eq!(basename_windows("theme"), "theme"); + } + #[test] fn dot_theme_names_are_directories() { let dir = std::env::temp_dir().join(format!("ghostty-theme-dot-{}", std::process::id())); diff --git a/crates/ghostty-core/src/surface/mouse.rs b/crates/ghostty-core/src/surface/mouse.rs index 1fb23c5..f7d73bf 100644 --- a/crates/ghostty-core/src/surface/mouse.rs +++ b/crates/ghostty-core/src/surface/mouse.rs @@ -402,7 +402,7 @@ fn resolve_path_for_opening( ) -> Option { use std::path::{Component, Path, PathBuf}; - if Path::new(path).is_absolute() { + if ghostty_os::osbytes::zig_is_absolute(path.as_bytes()) { return None; } diff --git a/crates/ghostty-os/src/osbytes.rs b/crates/ghostty-os/src/osbytes.rs index 36bda32..9a83734 100644 --- a/crates/ghostty-os/src/osbytes.rs +++ b/crates/ghostty-os/src/osbytes.rs @@ -81,6 +81,36 @@ pub fn os_bytes(s: &OsStr) -> Vec { s.to_str().map_or_else(Vec::new, |s| s.as_bytes().to_vec()) } +/// `std.fs.path.isAbsolutePosix` (Zig 0.15.2). +#[must_use] +pub fn zig_is_absolute_posix(path: &[u8]) -> bool { + path.first() == Some(&b'/') +} + +/// `std.fs.path.isAbsoluteWindows` (Zig 0.15.2): any leading separator (a +/// rooted or UNC path) or `:` followed by a separator is absolute. +#[must_use] +pub fn zig_is_absolute_windows(path: &[u8]) -> bool { + match path { + [] => false, + [b'/' | b'\\', ..] => true, + [_, b':', b'/' | b'\\', ..] => true, + _ => false, + } +} + +/// `std.fs.path.isAbsolute` for the host platform. Rust's +/// `Path::is_absolute` differs on Windows (it requires a prefix, so `\theme` +/// and `/theme` are not absolute there). +#[must_use] +pub fn zig_is_absolute(path: &[u8]) -> bool { + if cfg!(windows) { + zig_is_absolute_windows(path) + } else { + zig_is_absolute_posix(path) + } +} + /// Build an OS string from raw bytes (WTF-8 on Windows; bytes that are not /// well-formed WTF-8 become U+FFFD). Filesystem and process operations must /// use their checked conversions instead. @@ -290,6 +320,12 @@ impl EncodedOs { Self::from_os(p.as_os_str()) } + /// `std.fs.path.isAbsolute` on the decoded bytes. + #[must_use] + pub fn is_zig_absolute(&self) -> bool { + zig_is_absolute(&self.to_bytes()) + } + /// Encode raw bytes. #[must_use] pub fn from_bytes(b: &[u8]) -> Self { @@ -798,3 +834,37 @@ mod tests { assert_eq!(config_str_to_os(&s), o); } } + +#[cfg(test)] +mod zig_absolute_tests { + use super::*; + + #[test] + fn windows_is_absolute_matches_zig_0_15_2() { + for (p, want) in [ + ("", false), + ("/", true), + ("//server/file", true), + ("\\\\server", true), + ("\\theme", true), + ("/theme", true), + ("c", false), + ("c:", false), + ("c:\\", true), + ("c:/", true), + ("1:/x", true), + ("C:cwd/another", false), + ("directory\\directory", false), + ] { + assert_eq!(zig_is_absolute_windows(p.as_bytes()), want, "{p}"); + } + } + + #[test] + fn posix_is_absolute_matches_zig_0_15_2() { + assert!(!zig_is_absolute_posix(b"")); + assert!(zig_is_absolute_posix(b"/home/foo")); + assert!(!zig_is_absolute_posix(b"bar/")); + assert!(!zig_is_absolute_posix(b"\\theme")); + } +} diff --git a/crates/ghostty-os/src/xev.rs b/crates/ghostty-os/src/xev.rs index 0335688..1b7e3e4 100644 --- a/crates/ghostty-os/src/xev.rs +++ b/crates/ghostty-os/src/xev.rs @@ -108,24 +108,13 @@ pub fn available(be: Backend) -> bool { } } -/// libxev performs a harmless `io_uring_setup(256)` to see if we have -/// io_uring support. We additionally require what our loop needs -/// (`IORING_OP_POLL_ADD`, `IORING_OP_POLL_REMOVE` and the `EXT_ARG` timeout), -/// so that "available" never selects a ring that cannot run the IO loop. +/// libxev's `io_uring.available()`: a harmless `IoUring.init(256, 0)` (an +/// `io_uring_setup` with 256 entries and no flags) that either succeeds or +/// not. Nothing more is required; a reactor that wants a feature the ring +/// lacks (such as the `EXT_ARG` wait timeout) has to cope with its absence. #[cfg(target_os = "linux")] fn io_uring_available() -> bool { - let Ok(ring) = io_uring::IoUring::new(256) else { - return false; - }; - if !ring.params().is_feature_ext_arg() { - return false; - } - let mut probe = io_uring::Probe::new(); - if ring.submitter().register_probe(&mut probe).is_err() { - return false; - } - probe.is_supported(io_uring::opcode::PollAdd::CODE) - && probe.is_supported(io_uring::opcode::PollRemove::CODE) + io_uring::IoUring::new(256).is_ok() } #[cfg(not(target_os = "linux"))] @@ -195,6 +184,16 @@ mod tests { assert_eq!(backend(), before); } + #[cfg(target_os = "linux")] + #[test] + fn io_uring_available_is_plain_setup() { + // libxev's check is exactly a successful `IoUring.init(256, 0)`. + assert_eq!( + available(Backend::IoUring), + io_uring::IoUring::new(256).is_ok() + ); + } + #[cfg(target_os = "linux")] #[test] fn prefer_epoll_then_detect() { diff --git a/crates/ghostty-termio/src/reactor.rs b/crates/ghostty-termio/src/reactor.rs index 3bfc96a..5df1ec3 100644 --- a/crates/ghostty-termio/src/reactor.rs +++ b/crates/ghostty-termio/src/reactor.rs @@ -7,7 +7,8 @@ //! //! * [`Backend::IoUring`]: one `IORING_OP_POLL_ADD` per interest (re-armed //! after it fires, removed with `IORING_OP_POLL_REMOVE` when the interest -//! goes away) and an `EXT_ARG` timeout on the wait. +//! goes away) and, for the wait timeout, `EXT_ARG` when the ring has it or +//! else an `IORING_OP_TIMEOUT` entry (libxev's own timers use the latter). //! * [`Backend::Epoll`]: a persistent epoll instance whose registrations are //! diffed against the interests every wait (level-triggered). //! * `Poll`: plain `poll(2)`, used where libxev has a single, different @@ -53,6 +54,14 @@ impl Reactor { } } + /// An io_uring reactor behaving as on a ring without `EXT_ARG`. + #[cfg(all(test, target_os = "linux"))] + fn uring_without_ext_arg() -> io::Result { + Ok(Reactor::IoUring(Box::new( + UringReactor::new_without_ext_arg()?, + ))) + } + /// A plain `poll(2)` reactor regardless of platform. pub fn poll() -> Reactor { Reactor::Poll @@ -229,6 +238,10 @@ impl EpollReactor { /// The user_data of `IORING_OP_POLL_REMOVE` submissions (ids start at 1). #[cfg(target_os = "linux")] const REMOVE_ID: u64 = 0; +/// The first user_data of the `IORING_OP_TIMEOUT` entries used when `EXT_ARG` +/// is missing (poll ids count up from 1 and never reach this range). +#[cfg(target_os = "linux")] +const TIMEOUT_BASE: u64 = 1 << 63; /// io_uring based poller (`IORING_OP_POLL_ADD` one-shot polls). #[cfg(target_os = "linux")] @@ -237,19 +250,38 @@ pub struct UringReactor { /// token -> (raw fd, write, id of the armed poll) armed: HashMap, next_id: u64, + /// Whether waits may carry their timeout as `EXT_ARG` (kernel 5.11+). + ext_arg: bool, + /// user_data of the `IORING_OP_TIMEOUT` of the current wait and whether + /// it has completed (the `EXT_ARG`-less path). + timeout_id: u64, + timeout_fired: bool, } #[cfg(target_os = "linux")] impl UringReactor { fn new() -> io::Result { - // Same ring size as libxev's default. + // Same ring size and (absent) flags as libxev's `IoUring.init(256, 0)`. + let ring = io_uring::IoUring::new(256)?; + let ext_arg = ring.params().is_feature_ext_arg(); Ok(UringReactor { - ring: io_uring::IoUring::new(256)?, + ring, armed: HashMap::new(), next_id: 1, + ext_arg, + timeout_id: TIMEOUT_BASE, + timeout_fired: false, }) } + /// A reactor that behaves as on a ring without `EXT_ARG`. + #[cfg(test)] + fn new_without_ext_arg() -> io::Result { + let mut r = UringReactor::new()?; + r.ext_arg = false; + Ok(r) + } + /// Queue an entry, flushing the submission queue first if it is full. #[allow(unsafe_code)] fn push(&mut self, entry: &io_uring::squeue::Entry) -> io::Result<()> { @@ -318,6 +350,13 @@ impl UringReactor { if id == REMOVE_ID { continue; } + if id >= TIMEOUT_BASE { + // A stale (cancelled) timeout of an earlier wait is ignored. + if id == self.timeout_id { + self.timeout_fired = true; + } + continue; + } let Some(token) = self .armed .iter() @@ -348,6 +387,7 @@ impl UringReactor { // Results already waiting (e.g. from a previous partial reap). let res = match timeout { None => self.ring.submit_and_wait(1), + Some(d) if !self.ext_arg => return self.wait_with_timeout_op(d, interests, ready), Some(d) => { let ts = types::Timespec::new() .sec(d.as_secs()) @@ -366,6 +406,53 @@ impl UringReactor { self.reap(interests, ready); Ok(()) } + + /// Wait with a timeout on a ring without `EXT_ARG`: queue an + /// `IORING_OP_TIMEOUT` (5.4+, what libxev's timers use) whose completion + /// ends the wait. Completions that are not for the interests (a cancelled + /// timeout of an earlier wait) do not end it. + fn wait_with_timeout_op( + &mut self, + d: Duration, + interests: &[Interest<'_>], + ready: &mut Vec, + ) -> io::Result<()> { + use io_uring::{opcode, types}; + + let ts = types::Timespec::new() + .sec(d.as_secs()) + .nsec(d.subsec_nanos()); + self.timeout_id = self.timeout_id.wrapping_add(1).max(TIMEOUT_BASE); + self.timeout_fired = false; + // The kernel copies the timespec while consuming the entry in + // `submit_and_wait` below; `ts` outlives that call. + let entry = opcode::Timeout::new(std::ptr::addr_of!(ts)) + .build() + .user_data(self.timeout_id); + self.push(&entry)?; + loop { + let res = self.ring.submit_and_wait(1); + self.reap(interests, ready); + match res { + Ok(_) => {} + Err(e) => match e.raw_os_error() { + Some(libc::ETIME | libc::EINTR | libc::EBUSY) => break, + _ => return Err(e), + }, + } + if self.timeout_fired || !ready.is_empty() { + break; + } + } + if !self.timeout_fired { + // Cancel it so it cannot wake a later wait. + let entry = opcode::TimeoutRemove::new(self.timeout_id) + .build() + .user_data(REMOVE_ID); + self.push(&entry)?; + } + Ok(()) + } } #[cfg(test)] @@ -393,6 +480,46 @@ mod tests { rustix::pipe::pipe().unwrap() } + #[cfg(target_os = "linux")] + #[test] + fn io_uring_without_ext_arg_uses_timeout_op() { + let Ok(mut r) = Reactor::uring_without_ext_arg() else { + eprintln!("SKIP io_uring reactor: unavailable"); + return; + }; + let (rd, wr) = pipe(); + let interest = [Interest { + token: 7, + fd: rd.as_fd(), + write: false, + }]; + let mut ready = vec![]; + // Times out when nothing is ready, repeatedly (a cancelled timeout of + // the previous wait must not end the next one early). + for _ in 0..3 { + let start = std::time::Instant::now(); + r.wait(&interest, Some(Duration::from_millis(30)), &mut ready) + .unwrap(); + assert!(ready.is_empty()); + assert!(start.elapsed() >= Duration::from_millis(25)); + } + // Readiness ends the wait long before the timeout. + rustix::io::write(&wr, b"x").unwrap(); + let start = std::time::Instant::now(); + r.wait(&interest, Some(Duration::from_secs(10)), &mut ready) + .unwrap(); + assert_eq!(ready, vec![7]); + assert!(start.elapsed() < Duration::from_secs(5)); + // The abandoned 10 s timeout does not wake the following wait. + let mut buf = [0u8; 1]; + rustix::io::read(&rd, &mut buf).unwrap(); + let start = std::time::Instant::now(); + r.wait(&interest, Some(Duration::from_millis(30)), &mut ready) + .unwrap(); + assert!(ready.is_empty()); + assert!(start.elapsed() >= Duration::from_millis(25)); + } + #[test] fn times_out_with_nothing_ready() { for be in backends() {