From 82d2ed3382a6abdc7a3f39a934cc1350f56eff4e Mon Sep 17 00:00:00 2001 From: Valentin Haudiquet Date: Fri, 2 Oct 2026 23:51:42 +0200 Subject: [PATCH] refactor(extract): report extraction progress through the sink The extraction spinner drew straight to stderr from inside the library, so every consumer got terminal UI it never asked for. extract_tarball and its helpers now count extracted files themselves and emit Progress events; rendering belongs to the caller. --- crates/ecr-cli/src/main.rs | 2 +- crates/ecr/src/extract.rs | 185 +++++++++++++++++-------------------- crates/ecr/src/rootfs.rs | 30 +++--- 3 files changed, 106 insertions(+), 111 deletions(-) diff --git a/crates/ecr-cli/src/main.rs b/crates/ecr-cli/src/main.rs index 0cb49c7..c5e3013 100644 --- a/crates/ecr-cli/src/main.rs +++ b/crates/ecr-cli/src/main.rs @@ -38,7 +38,7 @@ fn main() -> Result<()> { arch: args.arch.clone().unwrap_or_default(), no_cache: args.no_cache, }; - let rootfs = cache.prepare(&request)?; + let rootfs = cache.prepare(&request, &ecr::NoProgress)?; // Branch based on --kernel flag // Option> (require_equals: the value must use =PATH syntax diff --git a/crates/ecr/src/extract.rs b/crates/ecr/src/extract.rs index 644e27b..d99b7df 100644 --- a/crates/ecr/src/extract.rs +++ b/crates/ecr/src/extract.rs @@ -1,13 +1,13 @@ +use crate::progress::{Progress, Unit}; use crate::veprintln; use anyhow::{Context, Result}; -use indicatif::{ProgressBar, ProgressStyle}; use std::fs::File; use std::io::{BufReader, Read}; use std::os::unix::fs::PermissionsExt; use std::path::{Component, Path, PathBuf}; /// Extract a tarball to the specified directory -pub fn extract_tarball(tarball: &Path, dest: &Path) -> Result<()> { +pub fn extract_tarball(tarball: &Path, dest: &Path, progress: &dyn Progress) -> Result<()> { let filename = tarball.file_name().and_then(|n| n.to_str()).unwrap_or(""); // OCI bundle files are always named "oci-…" by generate_cache_filename. @@ -16,15 +16,15 @@ pub fn extract_tarball(tarball: &Path, dest: &Path) -> Result<()> { // reading the entire compressed archive twice for large OCI images). if filename.starts_with("oci-") { veprintln!("Detected multi-layer OCI image, extracting layers..."); - return extract_multi_layer_oci(tarball, dest); + return extract_multi_layer_oci(tarball, dest, progress); } - extract_single_archive(tarball, dest) + extract_single_archive(tarball, dest, progress) } /// Extract a plain rootfs tarball, detecting the compression format from /// the filename with a magic-byte fallback. -fn extract_single_archive(tarball: &Path, dest: &Path) -> Result<()> { +fn extract_single_archive(tarball: &Path, dest: &Path, progress: &dyn Progress) -> Result<()> { let filename = tarball.file_name().and_then(|n| n.to_str()).unwrap_or(""); let file = File::open(tarball) @@ -35,13 +35,13 @@ fn extract_single_archive(tarball: &Path, dest: &Path) -> Result<()> { // Detect compression format from filename if filename.ends_with(".tar.gz") || filename.ends_with(".tgz") { - extract_gz(reader, dest, &mut fixups)?; + extract_gz(reader, dest, &mut fixups, progress)?; } else if filename.ends_with(".tar.xz") || filename.ends_with(".txz") { - extract_xz(reader, dest, &mut fixups)?; + extract_xz(reader, dest, &mut fixups, progress)?; } else if filename.ends_with(".tar.zst") || filename.ends_with(".tar.zstd") { - extract_zst(reader, dest, &mut fixups)?; + extract_zst(reader, dest, &mut fixups, progress)?; } else if filename.ends_with(".tar") { - extract_tar(reader, dest, &mut fixups)?; + extract_tar(reader, dest, &mut fixups, progress)?; } else { // Try to detect from magic bytes let mut magic = [0u8; 6]; @@ -53,25 +53,25 @@ fn extract_single_archive(tarball: &Path, dest: &Path) -> Result<()> { // gzip magic drop(peek_reader); let file = File::open(tarball)?; - extract_gz(BufReader::new(file), dest, &mut fixups)?; + extract_gz(BufReader::new(file), dest, &mut fixups, progress)?; } [0xfd, b'7', b'z', b'X', b'Z', 0x00] => { // xz magic drop(peek_reader); let file = File::open(tarball)?; - extract_xz(BufReader::new(file), dest, &mut fixups)?; + extract_xz(BufReader::new(file), dest, &mut fixups, progress)?; } [0x28, 0xb5, 0x2f, 0xfd, ..] => { // zstd magic drop(peek_reader); let file = File::open(tarball)?; - extract_zst(BufReader::new(file), dest, &mut fixups)?; + extract_zst(BufReader::new(file), dest, &mut fixups, progress)?; } _ => { // Assume uncompressed tar drop(peek_reader); let file = File::open(tarball)?; - extract_tar(BufReader::new(file), dest, &mut fixups)?; + extract_tar(BufReader::new(file), dest, &mut fixups, progress)?; } } } @@ -80,7 +80,7 @@ fn extract_single_archive(tarball: &Path, dest: &Path) -> Result<()> { } /// Extract a multi-layer OCI image -fn extract_multi_layer_oci(tarball: &Path, dest: &Path) -> Result<()> { +fn extract_multi_layer_oci(tarball: &Path, dest: &Path, progress: &dyn Progress) -> Result<()> { let filename = tarball.file_name().and_then(|n| n.to_str()).unwrap_or(""); // Extract the outer OCI bundle (layers.manifest + layer_N.tar.gz files) @@ -116,7 +116,7 @@ fn extract_multi_layer_oci(tarball: &Path, dest: &Path) -> Result<()> { // an OCI cache entry (the dispatch above keys on the "oci-" filename // prefix). The outer archive is a plain rootfs tarball. veprintln!("No layers.manifest in bundle; extracting as plain rootfs tarball"); - return extract_single_archive(tarball, dest); + return extract_single_archive(tarball, dest, progress); } // Read the layers manifest @@ -135,7 +135,7 @@ fn extract_multi_layer_oci(tarball: &Path, dest: &Path) -> Result<()> { continue; } veprintln!("Extracting layer: {}", layer_name); - extract_oci_layer(&layer_path, dest, &mut fixups)?; + extract_oci_layer(&layer_path, dest, &mut fixups, progress)?; } apply_fixups(dest, &mut fixups) @@ -144,7 +144,12 @@ fn extract_multi_layer_oci(tarball: &Path, dest: &Path) -> Result<()> { /// Decompress and extract one OCI layer tarball into `dest`, honouring /// whiteout markers. Compression is inferred from the filename then from /// magic bytes. -fn extract_oci_layer(layer_path: &Path, dest: &Path, fixups: &mut DeferredFixups) -> Result<()> { +fn extract_oci_layer( + layer_path: &Path, + dest: &Path, + fixups: &mut DeferredFixups, + progress: &dyn Progress, +) -> Result<()> { use std::io::Read; let layer_name = layer_path .file_name() @@ -154,82 +159,63 @@ fn extract_oci_layer(layer_path: &Path, dest: &Path, fixups: &mut DeferredFixups .with_context(|| format!("Failed to open layer: {}", layer_path.display()))?; let reader = BufReader::new(file); - if layer_name.ends_with(".tar.gz") || layer_name.ends_with(".tgz") { - extract_with_progress( - tar::Archive::new(flate2::read::GzDecoder::new(reader)), - dest, - "Extracting OCI layer", - fixups, - ) - } else if layer_name.ends_with(".tar.xz") || layer_name.ends_with(".txz") { - extract_with_progress( - tar::Archive::new(xz2::read::XzDecoder::new(reader)), - dest, - "Extracting OCI layer", - fixups, - ) - } else if layer_name.ends_with(".tar.zst") || layer_name.ends_with(".tar.zstd") { - extract_with_progress( - tar::Archive::new(zstd::stream::read::Decoder::new(reader)?), - dest, - "Extracting OCI layer", - fixups, - ) - } else { - // Fall back to magic-byte detection - let mut magic = [0u8; 6]; - let mut peek = BufReader::new(File::open(layer_path)?); - let _ = peek.read_exact(&mut magic); // short reads are fine for detection - drop(peek); - match magic { - [0x1f, 0x8b, ..] => extract_with_progress( - tar::Archive::new(flate2::read::GzDecoder::new(BufReader::new(File::open( - layer_path, - )?))), - dest, - "Extracting OCI layer", - fixups, - ), - [0xfd, b'7', b'z', b'X', b'Z', 0x00] => extract_with_progress( - tar::Archive::new(xz2::read::XzDecoder::new(BufReader::new(File::open( - layer_path, - )?))), - dest, - "Extracting OCI layer", - fixups, - ), - [0x28, 0xb5, 0x2f, 0xfd, ..] => extract_with_progress( - tar::Archive::new(zstd::stream::read::Decoder::new(BufReader::new( - File::open(layer_path)?, - ))?), - dest, - "Extracting OCI layer", - fixups, - ), - _ => extract_with_progress( - tar::Archive::new(BufReader::new(File::open(layer_path)?)), - dest, - "Extracting OCI layer", - fixups, - ), - } - } + // The decompressor type differs per format; erase it behind a trait + // object so a single archive-extraction call site serves every branch. + let archive: tar::Archive> = + if layer_name.ends_with(".tar.gz") || layer_name.ends_with(".tgz") { + tar::Archive::new(Box::new(flate2::read::GzDecoder::new(reader))) + } else if layer_name.ends_with(".tar.xz") || layer_name.ends_with(".txz") { + tar::Archive::new(Box::new(xz2::read::XzDecoder::new(reader))) + } else if layer_name.ends_with(".tar.zst") || layer_name.ends_with(".tar.zstd") { + tar::Archive::new(Box::new(zstd::stream::read::Decoder::new(reader)?)) + } else { + // Fall back to magic-byte detection + let mut magic = [0u8; 6]; + let mut peek = BufReader::new(File::open(layer_path)?); + let _ = peek.read_exact(&mut magic); // short reads are fine for detection + drop(peek); + let file = File::open(layer_path)?; + match magic { + [0x1f, 0x8b, ..] => { + tar::Archive::new(Box::new(flate2::read::GzDecoder::new(BufReader::new(file)))) + } + [0xfd, b'7', b'z', b'X', b'Z', 0x00] => { + tar::Archive::new(Box::new(xz2::read::XzDecoder::new(BufReader::new(file)))) + } + [0x28, 0xb5, 0x2f, 0xfd, ..] => tar::Archive::new(Box::new( + zstd::stream::read::Decoder::new(BufReader::new(file))?, + )), + _ => tar::Archive::new(Box::new(BufReader::new(file))), + } + }; + + extract_archive(archive, dest, "Extracting OCI layer", fixups, progress) } -fn extract_gz(reader: R, dest: &Path, fixups: &mut DeferredFixups) -> Result<()> { +fn extract_gz( + reader: R, + dest: &Path, + fixups: &mut DeferredFixups, + progress: &dyn Progress, +) -> Result<()> { let gz_decoder = flate2::read::GzDecoder::new(reader); let archive = tar::Archive::new(gz_decoder); - extract_with_progress(archive, dest, "Extracting gzip archive", fixups)?; + extract_archive(archive, dest, "Extracting gzip archive", fixups, progress)?; Ok(()) } -fn extract_xz(reader: R, dest: &Path, fixups: &mut DeferredFixups) -> Result<()> { +fn extract_xz( + reader: R, + dest: &Path, + fixups: &mut DeferredFixups, + progress: &dyn Progress, +) -> Result<()> { let xz_decoder = xz2::read::XzDecoder::new(reader); let archive = tar::Archive::new(xz_decoder); - extract_with_progress(archive, dest, "Extracting xz archive", fixups)?; + extract_archive(archive, dest, "Extracting xz archive", fixups, progress)?; Ok(()) } @@ -238,11 +224,12 @@ fn extract_zst( reader: R, dest: &Path, fixups: &mut DeferredFixups, + progress: &dyn Progress, ) -> Result<()> { let zst_decoder = zstd::Decoder::new(reader)?; let archive = tar::Archive::new(zst_decoder); - extract_with_progress(archive, dest, "Extracting zstd archive", fixups)?; + extract_archive(archive, dest, "Extracting zstd archive", fixups, progress)?; Ok(()) } @@ -251,10 +238,11 @@ fn extract_tar( reader: R, dest: &Path, fixups: &mut DeferredFixups, + progress: &dyn Progress, ) -> Result<()> { let archive = tar::Archive::new(reader); - extract_with_progress(archive, dest, "Extracting tar archive", fixups)?; + extract_archive(archive, dest, "Extracting tar archive", fixups, progress)?; Ok(()) } @@ -344,20 +332,16 @@ fn apply_hard_link(dest: &Path, dst: &Path, target: &Path) -> Result<()> { Ok(()) } -/// Extract tar archive with progress bar, handling whiteout files for OCI layers -fn extract_with_progress( +/// Extract tar archive entries, reporting a per-file count through +/// `progress` and handling whiteout files for OCI layers +fn extract_archive( mut archive: tar::Archive, dest: &Path, - msg: &str, + label: &str, fixups: &mut DeferredFixups, + progress: &dyn Progress, ) -> Result<()> { - let pb = ProgressBar::new_spinner(); - pb.set_style( - ProgressStyle::default_spinner() - .template("{spinner:.green} {msg} ({pos} files)") - .unwrap(), - ); - pb.set_message(msg.to_string()); + progress.begin(label, Unit::Files, None); archive.set_preserve_permissions(true); archive.set_preserve_ownerships(false); @@ -367,6 +351,7 @@ fn extract_with_progress( .entries() .context("Failed to read archive entries")?; + let mut count: u64 = 0; for entry in entries { let mut entry = entry.context("Failed to read archive entry")?; @@ -431,11 +416,12 @@ fn extract_with_progress( .with_context(|| format!("Failed to extract {}", path.display()))?; } - pb.inc(1); + count += 1; + progress.advance(1); } - pb.finish_and_clear(); - veprintln!("Extracted {} files", pb.position()); + progress.finish(); + veprintln!("Extracted {} files", count); Ok(()) } @@ -455,6 +441,7 @@ fn remove_path(path: &Path) -> std::io::Result<()> { #[cfg(test)] mod tests { use super::*; + use crate::progress::NoProgress; use std::os::unix::fs::{MetadataExt, PermissionsExt}; use tar::{Builder, EntryType, Header}; @@ -533,7 +520,7 @@ mod tests { ], ); - extract_tarball(&tarball, out.path()).unwrap(); + extract_tarball(&tarball, out.path(), &NoProgress).unwrap(); let base = out .path() @@ -575,7 +562,7 @@ mod tests { ], ); - extract_tarball(&tarball, out.path()).unwrap(); + extract_tarball(&tarball, out.path(), &NoProgress).unwrap(); assert_eq!( std::fs::read(out.path().join("etc/dir/link.0")).unwrap(), b"payload" @@ -618,7 +605,7 @@ mod tests { ], ); - extract_tarball(&tarball, out.path()).unwrap(); + extract_tarball(&tarball, out.path(), &NoProgress).unwrap(); assert_eq!( std::fs::read(out.path().join("etc/dir/keep.txt")).unwrap(), b"kept", diff --git a/crates/ecr/src/rootfs.rs b/crates/ecr/src/rootfs.rs index bc2fdb4..fece38f 100644 --- a/crates/ecr/src/rootfs.rs +++ b/crates/ecr/src/rootfs.rs @@ -8,13 +8,13 @@ //! entry and skips both the download and the provisioning step: //! //! ```no_run -//! use ecr::{chroot, ExecOptions, PrepareRequest, RootfsCache}; +//! use ecr::{chroot, ExecOptions, NoProgress, PrepareRequest, RootfsCache}; //! //! # fn main() -> anyhow::Result<()> { //! let cache = RootfsCache::new(RootfsCache::default_dir()?); //! let req = PrepareRequest::new("alpine:3.23"); //! -//! let rootfs = cache.prepare_provisioned(&req, |rootfs| { +//! let rootfs = cache.prepare_provisioned(&req, &NoProgress, |rootfs| { //! let code = rootfs.exec(&ExecOptions { //! command: vec!["apk".into(), "add".into(), "curl".into()], //! env: chroot::default_env(rootfs.dir()), @@ -47,6 +47,7 @@ use crate::distro::{ use crate::download::{digest_sidecar, download_image, fetch_oci_digest}; use crate::exec::ExecOptions; use crate::extract::extract_tarball; +use crate::progress::Progress; use crate::veprintln; /// Sidecar suffix marking a cache entry that holds a provisioned rootfs. @@ -89,8 +90,9 @@ impl RootfsCache { /// Resolve `req`, download through the cache when needed, and extract /// into a fresh scratch directory (removed when the returned - /// [`PreparedRootfs`] is dropped). - pub fn prepare(&self, req: &PrepareRequest) -> Result { + /// [`PreparedRootfs`] is dropped). Download and extraction progress is + /// reported through `progress`. + pub fn prepare(&self, req: &PrepareRequest, progress: &dyn Progress) -> Result { let arch = req.resolved_arch(); let source = resolve_source(&req.image, &arch)?; let cache_path = self.tarball_path(&source, &arch); @@ -143,7 +145,7 @@ impl RootfsCache { veprintln!("Using cached tarball: {}", cache_path.display()); } - extract_prepared(cache_path, arch) + extract_prepared(cache_path, arch, progress) } /// Prepare a provisioned rootfs with cache amortization: when the cache @@ -161,6 +163,7 @@ impl RootfsCache { pub fn prepare_provisioned( &self, req: &PrepareRequest, + progress: &dyn Progress, provision: F, ) -> Result where @@ -173,10 +176,10 @@ impl RootfsCache { if !req.no_cache && cache_path.exists() && marker.exists() { veprintln!("Using provisioned cache entry: {}", cache_path.display()); - return extract_prepared(cache_path, arch); + return extract_prepared(cache_path, arch, progress); } - let rootfs = self.prepare(req)?; + let rootfs = self.prepare(req, progress)?; provision(&rootfs)?; rootfs.persist().with_context(|| { format!( @@ -298,12 +301,16 @@ fn resolve_source(image: &str, arch: &str) -> Result { } } -fn extract_prepared(cache_path: PathBuf, arch: String) -> Result { +fn extract_prepared( + cache_path: PathBuf, + arch: String, + progress: &dyn Progress, +) -> Result { let temp = tempfile::tempdir().context("Failed to create temporary directory")?; let rootfs = temp.path().to_path_buf(); veprintln!("Extracting to: {}", rootfs.display()); - extract_tarball(&cache_path, &rootfs)?; + extract_tarball(&cache_path, &rootfs, progress)?; Ok(PreparedRootfs { rootfs: temp, @@ -461,6 +468,7 @@ fn get_tarball_extension(url: &str) -> &str { #[cfg(test)] mod tests { use super::*; + use crate::progress::NoProgress; use std::os::unix::fs::PermissionsExt; fn direct_tarball(distro: Distro, version: &str) -> ImageSource { @@ -539,7 +547,7 @@ mod tests { pack_rootfs(src.path(), &dest).unwrap(); let out = tempfile::tempdir().unwrap(); - crate::extract::extract_tarball(&dest, out.path()).unwrap(); + crate::extract::extract_tarball(&dest, out.path(), &NoProgress).unwrap(); assert_eq!( std::fs::read(out.path().join("hello.txt")).unwrap(), @@ -596,7 +604,7 @@ mod tests { // The persisted entry extracts back to the original contents let out = tempfile::tempdir().unwrap(); - crate::extract::extract_tarball(&cache_path, out.path()).unwrap(); + crate::extract::extract_tarball(&cache_path, out.path(), &NoProgress).unwrap(); assert_eq!(std::fs::read(out.path().join("file.txt")).unwrap(), b"data"); }