From 21099065c49c4de13b470e7b7412c34092f3ec77 Mon Sep 17 00:00:00 2001 From: Breadway Date: Wed, 29 Jul 2026 11:33:06 +0800 Subject: [PATCH] bread-capture: generalize target registry, fix Drop-skipped-on-exit leak MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaces the breadbar-only hardcoded view list with a registry keyed by app name (--app-name, defaulting to --app-path's file stem) so wiring up each new app just means adding one entry, not touching the CLI shape. Also fixes a real leak found while testing breadbox against this: std::process::exit() on the failure path skipped every destructor, including Isolation's Drop — so a failed capture run (or, more subtly, *any* run against an app whose view list didn't match yet, which is exactly what happened testing this) permanently orphaned the headless Sway process and its wayland-N/.lock socket pair. main() now returns ExitCode instead of calling process::exit directly, so Drop always runs. --- bread-capture/src/main.rs | 66 ++++++++++++++++++++++++++++----------- 1 file changed, 47 insertions(+), 19 deletions(-) diff --git a/bread-capture/src/main.rs b/bread-capture/src/main.rs index 3309bcc..e72d785 100644 --- a/bread-capture/src/main.rs +++ b/bread-capture/src/main.rs @@ -2,10 +2,13 @@ //! //! Drives each target app's `--screenshot --output ` mode (see //! `bread-screenshots` for what that mode does inside the app) and reports -//! pass/fail per view. Foundation-phase scope: one target (breadbar), a -//! hardcoded view list, and a flat output directory — no versioned -//! `screenshots/vX.Y.Z/latest` structure or manifest file yet, since those -//! only earn their complexity once more apps are wired up. +//! pass/fail per view. One app per invocation, selected by `--app-name` +//! (defaults to `--app-path`'s file stem, so `--app-path +//! ./target/release/breadbox` needs no separate `--app-name`) — the view +//! list for each app is looked up from [`TARGETS`] below. Flat output +//! directory for now — no versioned `screenshots/vX.Y.Z/latest` structure +//! or manifest file yet, since that's still not earning its complexity over +//! a handful of apps. //! //! By default every capture runs inside a throwaway headless Sway instance //! (see [`isolation`]) rather than the operator's live desktop, so another @@ -16,25 +19,35 @@ mod isolation; -use anyhow::Result; +use anyhow::{bail, Result}; use clap::Parser; use std::path::PathBuf; +use std::process::ExitCode; use std::time::Duration; const CAPTURE_TIMEOUT: Duration = Duration::from_secs(10); -/// (view name, output filename) -const BREADBAR_TARGETS: &[(&str, &str)] = &[ - ("bar", "breadbar-bar.png"), - ("control-panel", "breadbar-control-panel.png"), +/// Per-app (view name, output filename) lists. Keyed by the app's binary +/// name — see `--app-name`. +const TARGETS: &[(&str, &[(&str, &str)])] = &[ + ( + "breadbar", + &[("bar", "breadbar-bar.png"), ("control-panel", "breadbar-control-panel.png")], + ), + ("breadbox", &[("launcher", "breadbox-launcher.png")]), ]; #[derive(Parser)] struct Cli { - /// Path to the breadbar binary (resolved via $PATH if not a path). - #[arg(long, default_value = "breadbar")] + /// Path to the target app's binary (resolved via $PATH if not a path). + #[arg(long)] app_path: String, + /// Which app's view list to use (see `TARGETS`). Defaults to + /// `--app-path`'s file stem, e.g. `./target/release/breadbox` -> `breadbox`. + #[arg(long)] + app_name: Option, + /// Directory to write captured PNGs into. #[arg(long, default_value = "./screenshots")] out_dir: PathBuf, @@ -54,9 +67,27 @@ struct Cli { isolate_height: u32, } -fn main() -> Result<()> { +fn main() -> Result { let cli = Cli::parse(); + let app_name = cli.app_name.clone().unwrap_or_else(|| { + PathBuf::from(&cli.app_path) + .file_stem() + .map(|s| s.to_string_lossy().into_owned()) + .unwrap_or_else(|| cli.app_path.clone()) + }); + let Some((_, views)) = TARGETS.iter().find(|(name, _)| *name == app_name) else { + bail!( + "no known view list for app '{app_name}' (known: {})", + TARGETS.iter().map(|(n, _)| *n).collect::>().join(", ") + ); + }; + + // Bound, not dropped-and-discarded: `_isolation`'s teardown (kill the + // compositor, remove its socket/config) must run via Drop regardless of + // how this function returns below — returning an ExitCode rather than + // calling `std::process::exit` (which skips destructors entirely) is + // what makes that true on the failure path too. let _isolation = if cli.no_isolate { None } else { @@ -67,7 +98,7 @@ fn main() -> Result<()> { let height_str = cli.isolate_height.to_string(); let mut failed = false; - for (view, filename) in BREADBAR_TARGETS { + for (view, filename) in *views { let out_path = cli.out_dir.join(filename); let out_str = out_path.to_string_lossy(); let result = bread_utils::proc::run( @@ -81,15 +112,12 @@ fn main() -> Result<()> { CAPTURE_TIMEOUT, ); if result.success { - println!("ok breadbar/{view} -> {}", out_path.display()); + println!("ok {app_name}/{view} -> {}", out_path.display()); } else { failed = true; - println!("FAIL breadbar/{view}: {}", result.stderr.trim()); + println!("FAIL {app_name}/{view}: {}", result.stderr.trim()); } } - if failed { - std::process::exit(1); - } - Ok(()) + Ok(if failed { ExitCode::FAILURE } else { ExitCode::SUCCESS }) }