Skip to content

Commit 3ed9d63

Browse files
committed
progress: Move indicatif to CLI, introduce ProgressReporter API
Library crates composefs-oci and composefs-http previously created indicatif progress bars directly, coupling them to terminal output. But callers like bootc really need structured metadata so they can render their own progress. Closes: #140 Generated-by: OpenCode (Claude Sonnet 4.6) Signed-off-by: Colin Walters <walters@verbum.org>
1 parent 7e86960 commit 3ed9d63

14 files changed

Lines changed: 1326 additions & 133 deletions

File tree

Justfile

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,9 @@ fmt:
4040
check-fuzz:
4141
cargo check --manifest-path crates/composefs/fuzz/Cargo.toml
4242

43+
# Run unit + non-privileged integration tests (no VM, no root)
44+
test-all: test test-integration
45+
4346
# Run all checks (clippy + fmt + test + fuzz build)
4447
check: clippy check-feature-combos fmt-check test check-fuzz
4548

crates/composefs-ctl/src/lib.rs

Lines changed: 249 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,12 @@ pub use composefs_http;
2222
#[cfg(feature = "oci")]
2323
pub use composefs_oci;
2424

25+
#[cfg(any(feature = "oci", feature = "http"))]
26+
use std::collections::HashMap;
2527
use std::io::Read;
2628
use std::path::Path;
29+
#[cfg(any(feature = "oci", feature = "http"))]
30+
use std::sync::Mutex;
2731
use std::{ffi::OsString, path::PathBuf};
2832

2933
#[cfg(feature = "oci")]
@@ -35,9 +39,15 @@ use anyhow::{Context as _, Result};
3539
use clap::{Parser, Subcommand, ValueEnum};
3640
#[cfg(feature = "oci")]
3741
use comfy_table::{Table, presets::UTF8_FULL};
42+
#[cfg(any(feature = "oci", feature = "http"))]
43+
use indicatif::{MultiProgress, ProgressBar, ProgressStyle};
3844
use rustix::fs::{CWD, Mode, OFlags};
3945
use serde::Serialize;
4046

47+
#[cfg(any(feature = "oci", feature = "http"))]
48+
use composefs::progress::{
49+
ComponentId, ProgressEvent, ProgressReporter, ProgressUnit, SharedReporter,
50+
};
4151
use composefs_boot::BootOps;
4252
#[cfg(feature = "oci")]
4353
use composefs_boot::write_boot;
@@ -53,6 +63,94 @@ use composefs::{
5363
tree::RegularFile,
5464
};
5565

66+
/// An `indicatif`-backed [`ProgressReporter`] for use in the CLI.
67+
///
68+
/// Renders per-component progress bars via [`MultiProgress`]. When a component
69+
/// completes or is skipped the bar is removed; human-readable messages are
70+
/// printed above the bar group via [`MultiProgress::println`].
71+
#[cfg(any(feature = "oci", feature = "http"))]
72+
struct IndicatifReporter {
73+
multi: MultiProgress,
74+
bars: Mutex<HashMap<ComponentId, ProgressBar>>,
75+
}
76+
77+
#[cfg(any(feature = "oci", feature = "http"))]
78+
impl IndicatifReporter {
79+
fn new() -> Self {
80+
IndicatifReporter {
81+
multi: MultiProgress::new(),
82+
bars: Mutex::new(HashMap::new()),
83+
}
84+
}
85+
86+
/// Build a shared reporter from this instance.
87+
fn into_shared(self) -> SharedReporter {
88+
Arc::new(self)
89+
}
90+
}
91+
92+
#[cfg(any(feature = "oci", feature = "http"))]
93+
impl std::fmt::Debug for IndicatifReporter {
94+
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
95+
f.debug_struct("IndicatifReporter").finish_non_exhaustive()
96+
}
97+
}
98+
99+
#[cfg(any(feature = "oci", feature = "http"))]
100+
impl ProgressReporter for IndicatifReporter {
101+
fn report(&self, event: ProgressEvent) {
102+
match event {
103+
ProgressEvent::Started { id, total, unit } => {
104+
let bar = if let Some(total) = total {
105+
self.multi.add(ProgressBar::new(total))
106+
} else {
107+
self.multi.add(ProgressBar::new_spinner())
108+
};
109+
let style = match unit {
110+
ProgressUnit::Bytes => ProgressStyle::with_template(
111+
"[eta {eta}] {bar:40.cyan/blue} {decimal_bytes:>7}/{decimal_total_bytes:7} {msg}",
112+
),
113+
ProgressUnit::Items => ProgressStyle::with_template(
114+
"[eta {eta}] {bar:40.cyan/blue} {pos:>7}/{len:7} objects {msg}",
115+
),
116+
// Future unit variants fall back to a generic spinner.
117+
_ => ProgressStyle::with_template(
118+
"[eta {eta}] {bar:40.cyan/blue} {pos}/{len} {msg}",
119+
),
120+
};
121+
bar.set_style(
122+
style
123+
.unwrap_or_else(|_| ProgressStyle::default_bar())
124+
.progress_chars("##-"),
125+
);
126+
bar.set_message(id.to_string());
127+
self.bars.lock().unwrap().insert(id, bar);
128+
}
129+
ProgressEvent::Progress { id, fetched, .. } => {
130+
if let Some(bar) = self.bars.lock().unwrap().get(&id) {
131+
bar.set_position(fetched);
132+
}
133+
}
134+
ProgressEvent::Done { id, .. } => {
135+
if let Some(bar) = self.bars.lock().unwrap().remove(&id) {
136+
bar.finish_and_clear();
137+
}
138+
}
139+
ProgressEvent::Skipped { id } => {
140+
if let Some(bar) = self.bars.lock().unwrap().remove(&id) {
141+
bar.finish_with_message("skipped");
142+
}
143+
}
144+
ProgressEvent::Message(msg) => {
145+
let _ = self.multi.println(msg);
146+
}
147+
// `ProgressEvent` is #[non_exhaustive]: new variants added to the library
148+
// will be silently ignored here until cfsctl is updated to handle them.
149+
_ => {}
150+
}
151+
}
152+
}
153+
56154
/// JSON output wrapper for `cfsctl fsck --json`.
57155
#[derive(Serialize)]
58156
struct FsckJsonOutput {
@@ -986,8 +1084,10 @@ where
9861084
// If no explicit name provided, use the image reference as the tag
9871085
let tag_name = name.as_deref().unwrap_or(image);
9881086

1087+
let reporter: SharedReporter = IndicatifReporter::new().into_shared();
9891088
let opts = composefs_oci::PullOptions {
9901089
local_fetch: local_fetch.into(),
1090+
progress: Some(reporter),
9911091
..Default::default()
9921092
};
9931093

@@ -1244,10 +1344,158 @@ where
12441344
}
12451345
#[cfg(feature = "http")]
12461346
Command::Fetch { url, name } => {
1247-
let (digest, verity) = composefs_http::download(&url, &name, Arc::clone(&repo)).await?;
1347+
let reporter: SharedReporter = IndicatifReporter::new().into_shared();
1348+
let (digest, verity) = composefs_http::download(
1349+
&url,
1350+
&name,
1351+
Arc::clone(&repo),
1352+
composefs_http::DownloadOptions {
1353+
progress: Some(reporter),
1354+
},
1355+
)
1356+
.await?;
12481357
println!("content {digest}");
12491358
println!("verity {}", verity.to_hex());
12501359
}
12511360
}
12521361
Ok(())
12531362
}
1363+
1364+
#[cfg(test)]
1365+
#[cfg(any(feature = "oci", feature = "http"))]
1366+
mod tests {
1367+
use super::*;
1368+
use composefs::progress::{ProgressEvent, ProgressUnit};
1369+
1370+
// ── IndicatifReporter ────────────────────────────────────────────────────
1371+
1372+
/// A complete valid lifecycle (Started → Progress → Done) must not panic,
1373+
/// even without a real terminal (indicatif handles headless gracefully).
1374+
#[test]
1375+
fn test_indicatif_reporter_valid_lifecycle() {
1376+
let reporter = IndicatifReporter::new();
1377+
// Message before any component
1378+
reporter.report(ProgressEvent::Message("starting pull".into()));
1379+
// Byte-tracked component
1380+
reporter.report(ProgressEvent::Started {
1381+
id: "sha256:abc".into(),
1382+
total: Some(1_000_000),
1383+
unit: ProgressUnit::Bytes,
1384+
});
1385+
reporter.report(ProgressEvent::Progress {
1386+
id: "sha256:abc".into(),
1387+
fetched: 500_000,
1388+
total: Some(1_000_000),
1389+
});
1390+
reporter.report(ProgressEvent::Done {
1391+
id: "sha256:abc".into(),
1392+
transferred: 1_000_000,
1393+
});
1394+
// Item-counted component (HTTP objects)
1395+
reporter.report(ProgressEvent::Started {
1396+
id: "objects:stream".into(),
1397+
total: Some(200),
1398+
unit: ProgressUnit::Items,
1399+
});
1400+
reporter.report(ProgressEvent::Progress {
1401+
id: "objects:stream".into(),
1402+
fetched: 100,
1403+
total: Some(200),
1404+
});
1405+
reporter.report(ProgressEvent::Done {
1406+
id: "objects:stream".into(),
1407+
transferred: 200,
1408+
});
1409+
// Skipped component
1410+
reporter.report(ProgressEvent::Started {
1411+
id: "sha256:cached".into(),
1412+
total: None,
1413+
unit: ProgressUnit::Bytes,
1414+
});
1415+
reporter.report(ProgressEvent::Skipped {
1416+
id: "sha256:cached".into(),
1417+
});
1418+
}
1419+
1420+
/// Progress/Done events for an ID that was never `Started` must not panic.
1421+
///
1422+
/// This guards against error-recovery paths where a `Started` event may
1423+
/// have been suppressed or the reporter was attached after the operation
1424+
/// began.
1425+
#[test]
1426+
fn test_indicatif_reporter_unknown_id_no_panic() {
1427+
let reporter = IndicatifReporter::new();
1428+
// Progress for unknown ID — should silently ignore
1429+
reporter.report(ProgressEvent::Progress {
1430+
id: "ghost".into(),
1431+
fetched: 42,
1432+
total: None,
1433+
});
1434+
// Done for unknown ID — should silently ignore
1435+
reporter.report(ProgressEvent::Done {
1436+
id: "ghost".into(),
1437+
transferred: 42,
1438+
});
1439+
// Skipped for unknown ID — should silently ignore
1440+
reporter.report(ProgressEvent::Skipped { id: "ghost".into() });
1441+
}
1442+
1443+
/// A spinner-style bar (unknown total) must not panic.
1444+
#[test]
1445+
fn test_indicatif_reporter_spinner_lifecycle() {
1446+
let reporter = IndicatifReporter::new();
1447+
// Started with unknown total → spinner
1448+
reporter.report(ProgressEvent::Started {
1449+
id: "layer:unknown-size".into(),
1450+
total: None,
1451+
unit: ProgressUnit::Bytes,
1452+
});
1453+
reporter.report(ProgressEvent::Progress {
1454+
id: "layer:unknown-size".into(),
1455+
fetched: 1024,
1456+
total: None,
1457+
});
1458+
reporter.report(ProgressEvent::Done {
1459+
id: "layer:unknown-size".into(),
1460+
transferred: 2048,
1461+
});
1462+
}
1463+
1464+
/// Multiple concurrent components must not interfere with each other.
1465+
#[test]
1466+
fn test_indicatif_reporter_multiple_concurrent_components() {
1467+
let reporter = IndicatifReporter::new();
1468+
// Start two layers in parallel
1469+
reporter.report(ProgressEvent::Started {
1470+
id: "layer:a".into(),
1471+
total: Some(100),
1472+
unit: ProgressUnit::Bytes,
1473+
});
1474+
reporter.report(ProgressEvent::Started {
1475+
id: "layer:b".into(),
1476+
total: Some(200),
1477+
unit: ProgressUnit::Bytes,
1478+
});
1479+
// Interleaved progress
1480+
reporter.report(ProgressEvent::Progress {
1481+
id: "layer:a".into(),
1482+
fetched: 50,
1483+
total: Some(100),
1484+
});
1485+
reporter.report(ProgressEvent::Progress {
1486+
id: "layer:b".into(),
1487+
fetched: 100,
1488+
total: Some(200),
1489+
});
1490+
// Layer B finishes first
1491+
reporter.report(ProgressEvent::Done {
1492+
id: "layer:b".into(),
1493+
transferred: 200,
1494+
});
1495+
// Layer A finishes
1496+
reporter.report(ProgressEvent::Done {
1497+
id: "layer:a".into(),
1498+
transferred: 100,
1499+
});
1500+
}
1501+
}

crates/composefs-http/Cargo.toml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ anyhow = { version = "1.0.87", default-features = false }
1515
bytes = { version = "1.7.1", default-features = false }
1616
composefs = { workspace = true }
1717
hex = { version = "0.4.0", default-features = false }
18-
indicatif = { version = "0.18.0", default-features = false }
1918
reqwest = { version = "0.13.0", features = ["zstd"] }
2019
sha2 = { version = "0.11.0", default-features = false }
2120
tokio = { version = "1.24.2", default-features = false }

0 commit comments

Comments
 (0)