diff --git a/local/docs/BUILD-SYSTEM-IMPROVEMENTS.md b/local/docs/BUILD-SYSTEM-IMPROVEMENTS.md index 0bd725a20e..2a341cac5b 100644 --- a/local/docs/BUILD-SYSTEM-IMPROVEMENTS.md +++ b/local/docs/BUILD-SYSTEM-IMPROVEMENTS.md @@ -256,7 +256,7 @@ Eliminates the "delete and pray" pattern. | 1 | Parallel-safe cook pool | M | 2-3x | M | open | | 2 | `cook --repair` mode | S | 5-10x per-failure | L | **DONE** (`local/scripts/repair-cook.sh`) | | 3 | Per-recipe patch idempotency auditor | S | Catch at lint | None | **DONE** (commit 03c8a38a1) | -| 4 | Cook TUI status | M | UX | None | open | +| 4 | Cook TUI status | M | UX | None | **DONE** (`src/cook/status.rs`) | | 5 | Build-time recipe lint | M | Catch at lint | None | **DONE** (`local/scripts/lint-recipe.py`) | | 6 | `recipes/kf6-*` recipe dep audit | S | Prevent bugs | None | **DONE** | | 7 | QML gate | L | Unblock KDE | A: L | open | @@ -264,7 +264,7 @@ Eliminates the "delete and pray" pattern. | 9 | Failure classifier | M | 5-10x diagnosis | None | **DONE** (commit bd18eefc6) | | 10 | Cookbook scratch-rebuild system | L | Predictable | M | open | -**Implemented (commits 03c8a38a1, bd18eefc6):** +**Implemented (commits 03c8a38a1, bd18eefc6, ae749ffb2, current):** - **#3 (patch idempotency auditor):** `local/scripts/audit-patch-idempotency.py` validates every external patch in `local/patches/` against a fresh @@ -308,6 +308,27 @@ Eliminates the "delete and pray" pattern. read-only analysis, no build side effects. Supports `--last`, `--explain-rule `, and `--json` for CI integration. +- **#4 (cook TUI status):** `src/cook/status.rs` adds a one-line + per-recipe progress reporter for the non-TUI path. Auto-enables + when `config.cook.tui == false` AND `config.cook.logs == false` + AND stderr is a TTY (i.e., `CI=1 repo cook ...` from a real + terminal, e.g. SSH or a backgrounded shell). Output format: + ``` + [05/15] kf6-kio: starting + [05/15] kf6-kio: fetched (3.2s) + [05/15] kf6-kio: built (4m 18s) + [05/15] kf6-kio: done (total 4m 23s) + ``` + Cached recipes emit `[NN/MM] recipe: cached` (no phase breakdown). + Writes to stderr (eprintln!) so it never gets mixed with the + captured build-script log. Threading a `&mut StatusReporter` + through `repo_inner` and the per-phase closures in `src/bin/repo.rs` + was the minimum-impact change — no rewrite of the cook pipeline. + 6 unit tests cover format_elapsed boundaries, the disabled + no-op path, and the phase-tracking. The ratatui TUI + (`run_tui_cook` in `src/bin/repo.rs`) is unchanged; this is + the parallel status path for non-interactive cooks. + - **#2 (`cook --repair` mode):** `local/scripts/repair-cook.sh` wraps `repo cook ` with a fast-path that skips configure + build when the existing `CMakeCache.txt` is newer than the source tree @@ -388,5 +409,5 @@ Eliminates the "delete and pray" pattern. the Mesa row correctly references the 5 active mesa patches and the 2026-06-11 build success. -Recommended order for the remaining 4: #4, #10, #1, #7A. +Recommended order for the remaining 3: #10, #1, #7A. diff --git a/src/bin/repo.rs b/src/bin/repo.rs index 8f26043ce2..9d1e60b763 100644 --- a/src/bin/repo.rs +++ b/src/bin/repo.rs @@ -260,9 +260,26 @@ fn main_inner() -> anyhow::Result<()> { } let verbose = config.cook.verbose; - for recipe in &recipes { - match repo_inner(&config, &command, recipe) { + let status_enabled = cookbook::cook::status::status_reporter_enabled( + config.cook.tui, + config.cook.logs, + ); + let total = recipes.len(); + for (idx, recipe) in recipes.iter().enumerate() { + let mut status = cookbook::cook::status::StatusReporter::new( + status_enabled, + idx + 1, + total, + recipe.name.as_str(), + ); + status.start(); + match repo_inner(&config, &command, recipe, &mut status) { Ok(cached) => { + if cached { + status.cached(); + } else { + status.done(); + } if !command.is_informational() { if cached { print_cached(&command, &recipe.name); @@ -272,6 +289,7 @@ fn main_inner() -> anyhow::Result<()> { } } Err(e) => { + status.phase("failed"); if config.cook.nonstop { if verbose { eprintln!("{:?}", e); @@ -342,14 +360,20 @@ fn repo_inner( config: &CliConfig, command: &CliCommand, recipe: &CookRecipe, + status: &mut cookbook::cook::status::StatusReporter, ) -> Result { Ok(match *command { CliCommand::Fetch | CliCommand::Cook => { - let repo_inner_fn = move |logger: &PtyOut| -> Result { + let mut repo_inner_fn = move |logger: &PtyOut| -> Result { let is_cook = *command == CliCommand::Cook; let fetch_result = handle_fetch(recipe, config, is_cook, logger)?; let cached = if is_cook { - handle_cook(recipe, config, fetch_result.source_dir, logger)? + status.phase("fetched"); + let cached = handle_cook(recipe, config, fetch_result.source_dir, logger)?; + if !cached { + status.phase("built"); + } + cached } else { fetch_result.cached }; diff --git a/src/cook.rs b/src/cook.rs index 9c989215c3..867e997b92 100644 --- a/src/cook.rs +++ b/src/cook.rs @@ -7,4 +7,5 @@ pub mod ident; pub mod package; pub mod pty; pub mod script; +pub mod status; pub mod tree; diff --git a/src/cook/status.rs b/src/cook/status.rs new file mode 100644 index 0000000000..8b5a325acc --- /dev/null +++ b/src/cook/status.rs @@ -0,0 +1,197 @@ +//! StatusReporter — one-line cook progress for the non-TUI path. +//! +//! When the cookbook runs without its ratatui TUI (e.g. `CI=1 repo cook` +//! in a background shell, or via SSH), the only progress output is the +//! per-recipe tail of the build script. There's no aggregate +//! "5/15 done" view, no per-phase signal (fetch vs build vs package), +//! and no elapsed time. StatusReporter fills that gap with one-line +//! status updates to stderr. +//! +//! Activated automatically by the cookbook CLI when the TUI is off +//! AND log capture is off (i.e., `CI=1` mode). When the TUI is on, +//! the user already sees aggregate progress via ratatui; when log +//! capture is on, the per-recipe log file in `build/logs/` provides +//! the per-recipe context. In both of those cases StatusReporter is +//! a no-op so it never duplicates the existing UX. +//! +//! Output format: +//! `[05/15] kf6-kio: starting` +//! `[05/15] kf6-kio: fetched (3.2s)` +//! `[05/15] kf6-kio: built (4m 18s)` +//! `[05/15] kf6-kio: done (total 4m 23s)` +//! +//! All output is `eprintln!` so it never gets mixed with the +//! captured log output of the build script. +//! +//! Per build-system improvement #4 in +//! `local/docs/BUILD-SYSTEM-IMPROVEMENTS.md`. + +use std::io::IsTerminal; +use std::time::Instant; + +/// Returns true if the cook status reporter should emit progress +/// lines. Auto-enables when stdin AND stderr are both TTYs AND +/// neither the TUI nor log capture is wanted. (The TUI is the +/// ratatui dashboard; log capture writes per-recipe build logs to +/// `build/logs/.log` for postmortem review.) +pub fn status_reporter_enabled(tui: bool, logs: bool) -> bool { + !tui && !logs && std::io::stderr().is_terminal() +} + +pub struct StatusReporter { + enabled: bool, + index: usize, + total: usize, + name: String, + start: Instant, + phase_start: Instant, + last_phase: String, +} + +impl StatusReporter { + /// Create a per-recipe status reporter. `index` is 1-based. + /// If `enabled` is false, all methods are no-ops and the + /// reporter can be dropped without effect. + pub fn new(enabled: bool, index: usize, total: usize, name: &str) -> Self { + Self { + enabled, + index, + total, + name: name.to_string(), + start: Instant::now(), + phase_start: Instant::now(), + last_phase: String::new(), + } + } + + /// Emit a "starting" line. Call once per recipe. + pub fn start(&mut self) { + if !self.enabled { + return; + } + self.phase_start = Instant::now(); + eprintln!( + "[{:02}/{:02}] {}: starting", + self.index, self.total, self.name, + ); + } + + /// Emit a phase transition. The `phase` arg is a short label + /// like "fetched", "building", "built", "packaging", "done". + /// Elapsed time printed is from the previous phase boundary + /// (or the recipe start for the first phase). + pub fn phase(&mut self, phase: &str) { + if !self.enabled { + return; + } + let phase_elapsed = self.phase_start.elapsed(); + eprintln!( + "[{:02}/{:02}] {}: {} ({})", + self.index, + self.total, + self.name, + phase, + format_elapsed(phase_elapsed), + ); + self.last_phase = phase.to_string(); + self.phase_start = Instant::now(); + } + + /// Emit the final "done" line with total elapsed time. + pub fn done(&mut self) { + if !self.enabled { + return; + } + eprintln!( + "[{:02}/{:02}] {}: done (total {})", + self.index, + self.total, + self.name, + format_elapsed(self.start.elapsed()), + ); + self.last_phase = "done".to_string(); + } + + pub fn cached(&mut self) { + if !self.enabled { + return; + } + eprintln!( + "[{:02}/{:02}] {}: cached", + self.index, self.total, self.name, + ); + } + + pub fn last_phase(&self) -> &str { + &self.last_phase + } +} + +fn format_elapsed(d: std::time::Duration) -> String { + let total = d.as_secs(); + if total < 60 { + format!("{}s", total) + } else if total < 3600 { + format!("{}m {:02}s", total / 60, total % 60) + } else { + format!("{}h {:02}m {:02}s", total / 3600, (total / 60) % 60, total % 60) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn format_elapsed_under_minute() { + assert_eq!(format_elapsed(std::time::Duration::from_secs(5)), "5s"); + assert_eq!(format_elapsed(std::time::Duration::from_secs(59)), "59s"); + } + + #[test] + fn format_elapsed_under_hour() { + assert_eq!(format_elapsed(std::time::Duration::from_secs(60)), "1m 00s"); + assert_eq!(format_elapsed(std::time::Duration::from_secs(125)), "2m 05s"); + } + + #[test] + fn format_elapsed_over_hour() { + assert_eq!( + format_elapsed(std::time::Duration::from_secs(3725)), + "1h 02m 05s" + ); + } + + #[test] + fn disabled_reporter_is_noop() { + let mut r = StatusReporter::new(false, 1, 3, "kf6-kio"); + r.start(); + r.phase("fetched"); + r.phase("built"); + r.done(); + // No panic, no output, no state mutation when disabled. + assert_eq!(r.last_phase(), ""); + } + + #[test] + fn enabled_reporter_tracks_phases() { + let mut r = StatusReporter::new(true, 5, 15, "kf6-kio"); + assert!(r.enabled); + r.start(); + r.phase("fetched"); + assert_eq!(r.last_phase(), "fetched"); + r.phase("built"); + assert_eq!(r.last_phase(), "built"); + r.done(); + assert_eq!(r.last_phase(), "done"); + } + + #[test] + fn status_reporter_enabled_logic() { + // All false: enabled (no TUI, no logs, stderr is a tty in tests... well, maybe not) + // In unit tests stderr is typically a tty capture. We just verify the + // boolean logic is correct for the in-process check. + assert!(!status_reporter_enabled(true, false)); // TUI on + assert!(!status_reporter_enabled(false, true)); // Logs on + } +}