fix(os,termio): FreeBSD support, dev-profile resource precedence, docs
Final fix pass per the stopping criterion (docs/rewrite/PLAN.md), resolving the remaining RV18 build/behaviour items (final-Z21.md): - termio errno access uses the per-target libc accessor (__error on FreeBSD/DragonFly/Apple, __errno on OpenBSD/NetBSD/Android) instead of __errno_location everywhere, unblocking non-Linux BSD builds. - FreeBSD resource discovery follows Zig (site-terminfo, local/share). - Zig's Debug-only resource precedence (and the dev workspace-resources fallback) key off a ghostty_debug_build cfg set from PROFILE=="debug" instead of debug_assertions, so release builds with assertions honor GHOSTTY_RESOURCES_DIR first like Zig ReleaseSafe. - KNOWN_DIFFERENCES.md lists the remaining P3 differences deliberately left unfixed (theme-preview editing details, a cleanup timeout edge case) and what cannot be verified on this machine. - DEVIANT_FIXES.md registers the PTY-hangup output drain (35), the canvas BGRA conversion (36) and the dev resources fallback. Test Plan: - cargo test --workspace --no-fail-fast (exit 0) - cargo check --target x86_64-unknown-freebsd -p ghostty-os
This commit is contained in:
1 parent
729b544820
commit
7b59457cb8
6 files changed
+83
-7
No files matched your search
@@ -0,0 +1,12 @@
|
||||
//! Emits `cfg(ghostty_debug_build)` for the Cargo `dev` profile (Zig
|
||||
//! `builtin.mode == .Debug`). `debug_assertions` is not equivalent: the
|
||||
//! repository's `release` profile enables it, like Zig's `ReleaseSafe`.
|
||||
|
||||
fn main() {
|
||||
println!("cargo:rerun-if-changed=build.rs");
|
||||
println!("cargo:rerun-if-env-changed=PROFILE");
|
||||
println!("cargo:rustc-check-cfg=cfg(ghostty_debug_build)");
|
||||
if std::env::var("PROFILE").is_ok_and(|p| p == "debug") {
|
||||
println!("cargo:rustc-cfg=ghostty_debug_build");
|
||||
}
|
||||
}
|
||||
@@ -48,9 +48,13 @@ impl ResourcesDir {
|
||||
|
||||
/// Sentinels we look for in the path to know we've found the resources
|
||||
/// directory (Linux).
|
||||
#[cfg(not(any(windows, target_os = "macos")))]
|
||||
#[cfg(not(any(windows, target_os = "macos", target_os = "freebsd")))]
|
||||
const SENTINELS: [&str; 2] = ["terminfo/g/ghostty", "terminfo/x/xterm-ghostty"];
|
||||
|
||||
/// Sentinels on FreeBSD (`site-terminfo`, like Zig).
|
||||
#[cfg(target_os = "freebsd")]
|
||||
const SENTINELS: [&str; 2] = ["site-terminfo/g/ghostty", "site-terminfo/x/xterm-ghostty"];
|
||||
|
||||
/// Sentinel on macOS (terminfo directories are named by hex there).
|
||||
#[cfg(target_os = "macos")]
|
||||
const SENTINELS: [&str; 1] = ["terminfo/78/xterm-ghostty"];
|
||||
@@ -66,12 +70,14 @@ const DEV_SENTINELS: [&str; 1] = ["terminfo/x/xterm-ghostty"];
|
||||
/// Gets the directory to the bundled resources directory, if it
|
||||
/// exists (not all platforms or packages have it).
|
||||
///
|
||||
/// Zig's `builtin.mode == .Debug` is mapped to `cfg!(debug_assertions)`.
|
||||
/// Zig's `builtin.mode == .Debug` is mapped to the Cargo `dev` profile
|
||||
/// (`cfg(ghostty_debug_build)`, set by `build.rs`), not to `debug_assertions`:
|
||||
/// the repository's `release` profile enables assertions like ReleaseSafe.
|
||||
pub fn resources_dir() -> io::Result<ResourcesDir> {
|
||||
let exe = std::env::current_exe().ok();
|
||||
let env = std::env::var_os("GHOSTTY_RESOURCES_DIR");
|
||||
Ok(resources_dir_impl(
|
||||
cfg!(debug_assertions),
|
||||
cfg!(ghostty_debug_build),
|
||||
exe.as_deref(),
|
||||
env,
|
||||
))
|
||||
@@ -112,9 +118,14 @@ fn resources_dir_impl(debug: bool, exe: Option<&Path>, env: Option<OsString>) ->
|
||||
}
|
||||
}
|
||||
|
||||
// We look for a /usr/share style path.
|
||||
// We look for a /usr/share style path (/usr/local/share on FreeBSD).
|
||||
let share = if cfg!(target_os = "freebsd") {
|
||||
"local/share"
|
||||
} else {
|
||||
"share"
|
||||
};
|
||||
for sentinel in SENTINELS {
|
||||
if let Some(v) = maybe_dir_os(dir, "share", sentinel) {
|
||||
if let Some(v) = maybe_dir_os(dir, share, sentinel) {
|
||||
return ResourcesDir {
|
||||
app_path: Some(enc(&v.join("ghostty"))),
|
||||
host_path: None,
|
||||
|
||||
@@ -546,20 +546,38 @@ fn execvpe(prep: &Prepared, argv: &[*const c_char], envp: &[*const c_char]) -> E
|
||||
})
|
||||
}
|
||||
|
||||
#[cfg(not(target_vendor = "apple"))]
|
||||
#[cfg(not(any(
|
||||
target_vendor = "apple",
|
||||
target_os = "freebsd",
|
||||
target_os = "dragonfly",
|
||||
target_os = "openbsd",
|
||||
target_os = "netbsd",
|
||||
target_os = "android"
|
||||
)))]
|
||||
#[allow(unsafe_code)]
|
||||
fn errno_now() -> i32 {
|
||||
// SAFETY: __errno_location returns a valid thread-local pointer.
|
||||
unsafe { *libc::__errno_location() }
|
||||
}
|
||||
|
||||
#[cfg(target_vendor = "apple")]
|
||||
#[cfg(any(
|
||||
target_vendor = "apple",
|
||||
target_os = "freebsd",
|
||||
target_os = "dragonfly"
|
||||
))]
|
||||
#[allow(unsafe_code)]
|
||||
fn errno_now() -> i32 {
|
||||
// SAFETY: __error returns a valid thread-local pointer.
|
||||
unsafe { *libc::__error() }
|
||||
}
|
||||
|
||||
#[cfg(any(target_os = "openbsd", target_os = "netbsd", target_os = "android"))]
|
||||
#[allow(unsafe_code)]
|
||||
fn errno_now() -> i32 {
|
||||
// SAFETY: __errno returns a valid thread-local pointer.
|
||||
unsafe { *libc::__errno() }
|
||||
}
|
||||
|
||||
fn write_stderr(msg: &[u8]) {
|
||||
// Best effort, no allocation.
|
||||
let _ = rustix::io::write(borrow_stderr(), msg);
|
||||
|
||||
@@ -60,6 +60,8 @@ when this file was written.
|
||||
| 32 | OS: URL opener child reaping | `os/open.zig` (`try collectOutput(...)` before `wait()`) | When collecting the opener's output fails (e.g. more than 50 KiB on a stream), the error returns before `wait()`, leaving the child as a zombie. | The child is always reaped; the collection error is still logged. | `4a55abf` fix-deviant(os) | `ghostty-os` open tests |
|
||||
| 33 | OS: maximum-length `NOTIFY_SOCKET` | `os/systemd.zig:100-116` (`socket_address.path.len < socket_path.len`, then `path[socket_path.len] = 0`) | A 108-byte path (the size of `sockaddr_un.path`) passes the length check, and the terminating NUL is written at `path[108]`, one byte past the array (out-of-bounds write). Separate from row 13 (the abstract-socket `@` handling). | The address is built with rustix's `SocketAddrUnix::new`, which takes the 108 bytes without a terminator (valid for Linux `AF_UNIX`) and rejects longer paths; no out-of-bounds write. Verified in code. | f8c5ce0 (`git log -S` on `ghostty-os/src/systemd.rs`: the only commit touching it, besides formatting/macOS changes) | `notify_maximum_length_socket_path_is_rejected_safely` (`ghostty-os/src/systemd.rs`) |
|
||||
| 34 | Renderer (Metal): buffer reallocation size | `renderer/metal/buffer.zig:89` (`Buffer.sync`) and `:146` (`syncFromArrayLists`) | `size = req_bytes * 2` is already in bytes, then `size * @sizeOf(T)` is passed to `newBufferWithLength:options:`, so the new buffer is `sizeof(T)` times larger than intended (GPU-memory over-allocation). | The new buffer is exactly `2 * req_bytes` (`metal_layout::grown_buffer_bytes`); the element count is `2 * total_len`. | bab49ee; registered + test in `2040677` fix-deviant(renderer) | `metal_layout::tests::grown_buffer_bytes_is_double_the_required_bytes_not_scaled_by_size_of_t` (runs on any host; the Metal buffer itself is compile-checked only) |
|
||||
| 35 | Termio: PTY hangup drops unread output | `termio/Exec.zig` `ReadThread.threadMainPosix` (exits as soon as `POLL.HUP` is seen) | `POLLIN \| POLLHUP` can arrive together with unread final output (a Linux PTY check confirmed this); exiting immediately loses the tail of the child's output. | After HUP the read thread makes one more pass over the fd, so the remaining output is delivered before exit. | 6629fb0 | none (needs a live PTY; verified by RV18 on Linux) |
|
||||
| 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) |
|
||||
|
||||
## Upstream quirks deliberately kept (not fixed)
|
||||
|
||||
@@ -100,6 +102,7 @@ Only entries with an observable behavioural difference are listed.
|
||||
| OSC 52 base64 acceptance | Surface.zig:2149,2163 rules | Was different in both directions (Codex core audit); fixed to match Zig | `notes/review-core-codex.md` | 2f58f02 |
|
||||
| Benchmark / synthetic tooling extensions | `ghostty-bench` has no `--time` option and only the simplified terminal handler; `ghostty-synthetic` has the actions `ascii`, `osc`, `utf8` | `ghostty-bench --time` prints the measured step time to stderr; `ghostty-bench --handler=full` runs the full `stream_readonly` handler instead of the print-only one; `ghostty-synthetic +sgr` generates SGR-heavy output. Dev/benchmark tooling only; otherwise upstream behaviour is unchanged | `crates/ghostty-bench/src/main.rs`, `src/benches.rs`, `src/bin/ghostty-synthetic.rs`; `notes/review-rv16-codex.md` | 0c4f0ee |
|
||||
| Inspector font rasterizer (Windows, Apple, wasm) | Dear ImGui is built with `IMGUI_ENABLE_FREETYPE` (`pkg/dcimgui`), so the 32 px inspector font is rasterized by FreeType | Linux and other Unix targets: FreeType (`dear-imgui-rs` feature `freetype`, system FreeType through pkg-config), 32 px. Windows, Apple and wasm32 keep Dear ImGui's stb_truetype loader at 32 px, because `dear-imgui-sys` can only locate FreeType through pkg-config/vcpkg and those targets cannot be built or run here. Glyph anti-aliasing differs slightly there. | `crates/ghostty-inspector/Cargo.toml`, `src/inspector.rs` (`embedded_font_source`) | Z20 (uncommitted) |
|
||||
| Development resources fallback | `resourcesDir` returns nothing when no terminfo bundle is found next to the binary | In the Cargo `dev` profile, if neither installed resources nor `GHOSTTY_RESOURCES_DIR` apply, the in-repo `<workspace>/resources` directory is used when the binary lives inside this workspace | `crates/ghostty-os/src/resourcesdir.rs` (`dev_resources_dir`) | 967e3a1 |
|
||||
|
||||
## Maintenance
|
||||
|
||||
|
||||
@@ -0,0 +1,25 @@
|
||||
# Known differences (left by the stopping criterion)
|
||||
|
||||
These are known minor differences between the Rust port and Ghostty 1.3.1
|
||||
found in the final audit (RV18, `notes/review-rv18-codex.md`). They were
|
||||
recorded, not fixed, because the stopping criterion in `PLAN.md` does not
|
||||
require chasing P3 items. All are severity P3. Each was verified from source
|
||||
and, where noted, by PTY reproduction.
|
||||
|
||||
| # | Area | Rust | Zig | Observable effect | Sev |
|
||||
|---|------|------|-----|-------------------|-----|
|
||||
| 5 | Theme search: word navigation | `ghostty-cli/src/list_themes/preview.rs:113` has no word-navigation editing | upstream list-themes search has word navigation | Enter `blue red`, Alt+B, `X`: Zig gives `blue Xred`, Rust `blue redX` | P3 |
|
||||
| 6 | Theme search: grapheme backspace | `preview.rs:100` deletes one Unicode scalar | deletes one grapheme | `e` + U+0301 then Backspace: Zig removes both, Rust leaves `e` | P3 |
|
||||
| 7 | Theme search: non-ASCII case | `preview.rs:150` Unicode-lowercases theme names but ASCII-lowercases the query | ASCII-only on both sides | Query `Ä` does not match theme `Ä` in Rust, matches in Zig | P3 |
|
||||
| 8 | Theme preview: initial colors | `preview.rs:385` consults `COLORFGBG` | starts light until terminal color reports arrive | Without terminal replies, changing `COLORFGBG` changes Rust's colors only | P3 |
|
||||
| 9 | Theme preview: load errors | `preview.rs:440` ignores `load_file` errors and keeps filtering with the retained config | the error propagates and the preview exits | A config load error is silently swallowed in Rust | P3 |
|
||||
| 10 | Subprocess cleanup cap | `ghostty-termio/src/exec.rs:1348` waits at most about 5 s for the child's new process group | keeps waiting | If the child is delayed before `setsid` for longer, Rust returns without sending SIGHUP | P3 |
|
||||
|
||||
## Not verifiable on this machine
|
||||
|
||||
- Native Apple execution: Metal, IOSurface, CoreText, Apple input APIs, Mach handling and Apple packaging (compile-checked only, `just check-macos`).
|
||||
- Windows ConPTY and Win32 behavior (compile-checked only, `just check-windows`).
|
||||
- Native FreeBSD execution (cross `cargo check` only, if the target could be added).
|
||||
- Live portal / D-Bus integration.
|
||||
- Real-browser rendering (wasm is checked with Node only).
|
||||
- Native io_uring execution; Swift code was excluded.
|
||||
@@ -0,0 +1,7 @@
|
||||
# Z21 final fix pass
|
||||
|
||||
- RV18 #1: removed the reachable `debug_assert!(!b)` in `Terminal::print_grapheme` (Zig ReleaseFast ignores it). The panic hunt (`difftest_fuzz panics`, new Rust-only mode, debug build, ~60k cases) then found a second input-reachable defect: Devanagari conjunct at the right margin on the bottom row (margin wrap scrolls) returned `InvalidCoordinate` and left a grapheme flag without data, so the formatter panicked later. Fixed by moving the grapheme from the cell that holds it. 0 panics afterwards; Zig differential 3000 cases, 0 divergences.
|
||||
- RV18 #2-#4: errno accessor per target, FreeBSD resources conventions, `ghostty_debug_build` cfg (new `crates/ghostty-os/build.rs`).
|
||||
- FreeBSD check: `cargo check --target x86_64-unknown-freebsd -p ghostty-os` passes (with FREETYPE_NO_PKG_CONFIG=1); `-p ghostty-termio` is blocked by dear-imgui-sys (no FreeBSD bindings) via ghostty-inspector; the `errno_now` cfg block was compiled alone against libc for FreeBSD and Linux.
|
||||
- Docs: DEVIANT_FIXES.md entry 10 rewritten, entries 35 (PTY hangup drain, 6629fb0) and 36 (canvas BGRA, d89691b), dev resources fallback row (967e3a1); new KNOWN_DIFFERENCES.md.
|
||||
- Side effect: the in-workspace resources fallback is now dev-profile only.
|
||||
Reference in new issue
Block a user