From 273f453e75f70a906b29618ab98bafab4abc4b67 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Wed, 6 Nov 2024 12:07:10 -0500 Subject: [PATCH] Use no reporter by default in `cache clean` (#8868) --- crates/uv-cache/src/lib.rs | 46 ++++++++++++------------ crates/uv-cache/src/removal.rs | 30 +++++++++++++--- crates/uv-distribution/src/source/mod.rs | 8 ++--- crates/uv/src/commands/cache_clean.rs | 7 ++-- crates/uv/src/commands/reporters.rs | 9 ++--- 5 files changed, 59 insertions(+), 41 deletions(-) diff --git a/crates/uv-cache/src/lib.rs b/crates/uv-cache/src/lib.rs index 1e1edd951..d580c4948 100644 --- a/crates/uv-cache/src/lib.rs +++ b/crates/uv-cache/src/lib.rs @@ -19,6 +19,7 @@ use uv_pypi_types::ResolutionMetadata; pub use crate::by_timestamp::CachedByTimestamp; #[cfg(feature = "clap")] pub use crate::cli::CacheArgs; +use crate::removal::Remover; pub use crate::removal::{rm_rf, Removal}; pub use crate::wheel::WheelCache; use crate::wheel::WheelCacheKind; @@ -320,8 +321,8 @@ impl Cache { } /// Clear the cache, removing all entries. - pub fn clear(&self, reporter: Option<&dyn CleanReporter>) -> Result { - rm_rf(&self.root, reporter) + pub fn clear(&self, reporter: Box) -> Result { + Remover::new(reporter).rm_rf(&self.root) } /// Remove a package from the cache. @@ -379,7 +380,7 @@ impl Cache { let path = fs_err::canonicalize(entry.path())?; if !after.contains(&path) && before.contains(&path) { debug!("Removing dangling cache entry: {}", path.display()); - summary += rm_rf(path, None)?; + summary += rm_rf(path)?; } } } @@ -409,13 +410,13 @@ impl Cache { if CacheBucket::iter().all(|bucket| entry.file_name() != bucket.to_str()) { let path = entry.path(); debug!("Removing dangling cache bucket: {}", path.display()); - summary += rm_rf(path, None)?; + summary += rm_rf(path)?; } } else { // If the file is not a marker file, remove it. let path = entry.path(); debug!("Removing dangling cache bucket: {}", path.display()); - summary += rm_rf(path, None)?; + summary += rm_rf(path)?; } } @@ -427,7 +428,7 @@ impl Cache { let entry = entry?; let path = fs_err::canonicalize(entry.path())?; debug!("Removing dangling cache environment: {}", path.display()); - summary += rm_rf(path, None)?; + summary += rm_rf(path)?; } } Err(err) if err.kind() == io::ErrorKind::NotFound => (), @@ -444,7 +445,7 @@ impl Cache { let path = fs_err::canonicalize(entry.path())?; if path.is_dir() { debug!("Removing unzipped wheel entry: {}", path.display()); - summary += rm_rf(path, None)?; + summary += rm_rf(path)?; } } } @@ -472,10 +473,10 @@ impl Cache { if path.is_dir() { debug!("Removing unzipped built wheel entry: {}", path.display()); - summary += rm_rf(path, None)?; + summary += rm_rf(path)?; } else if path.is_symlink() { debug!("Removing unzipped built wheel entry: {}", path.display()); - summary += rm_rf(path, None)?; + summary += rm_rf(path)?; } } } @@ -505,7 +506,7 @@ impl Cache { let path = fs_err::canonicalize(entry.path())?; if !references.contains(&path) { debug!("Removing dangling cache archive: {}", path.display()); - summary += rm_rf(path, None)?; + summary += rm_rf(path)?; } } } @@ -520,8 +521,7 @@ impl Cache { pub trait CleanReporter: Send + Sync { /// Called after one file or directory is removed. fn on_clean(&self); - /// Called after a package is cleaned. - fn on_clean_package(&self, _package: &str, _removal: &Removal) {} + /// Called after all files and directories are removed. fn on_complete(&self); } @@ -809,32 +809,32 @@ impl CacheBucket { Self::Wheels => { // For `pypi` wheels, we expect a directory per package (indexed by name). let root = cache.bucket(self).join(WheelCacheKind::Pypi); - summary += rm_rf(root.join(name.to_string()), None)?; + summary += rm_rf(root.join(name.to_string()))?; // For alternate indices, we expect a directory for every index (under an `index` // subdirectory), followed by a directory per package (indexed by name). let root = cache.bucket(self).join(WheelCacheKind::Index); for directory in directories(root) { - summary += rm_rf(directory.join(name.to_string()), None)?; + summary += rm_rf(directory.join(name.to_string()))?; } // For direct URLs, we expect a directory for every URL, followed by a // directory per package (indexed by name). let root = cache.bucket(self).join(WheelCacheKind::Url); for directory in directories(root) { - summary += rm_rf(directory.join(name.to_string()), None)?; + summary += rm_rf(directory.join(name.to_string()))?; } } Self::SourceDistributions => { // For `pypi` wheels, we expect a directory per package (indexed by name). let root = cache.bucket(self).join(WheelCacheKind::Pypi); - summary += rm_rf(root.join(name.to_string()), None)?; + summary += rm_rf(root.join(name.to_string()))?; // For alternate indices, we expect a directory for every index (under an `index` // subdirectory), followed by a directory per package (indexed by name). let root = cache.bucket(self).join(WheelCacheKind::Index); for directory in directories(root) { - summary += rm_rf(directory.join(name.to_string()), None)?; + summary += rm_rf(directory.join(name.to_string()))?; } // For direct URLs, we expect a directory for every URL, followed by a @@ -843,7 +843,7 @@ impl CacheBucket { let root = cache.bucket(self).join(WheelCacheKind::Url); for url in directories(root) { if directories(&url).any(|version| is_match(&version, name)) { - summary += rm_rf(url, None)?; + summary += rm_rf(url)?; } } @@ -853,7 +853,7 @@ impl CacheBucket { let root = cache.bucket(self).join(WheelCacheKind::Path); for path in directories(root) { if directories(&path).any(|version| is_match(&version, name)) { - summary += rm_rf(path, None)?; + summary += rm_rf(path)?; } } @@ -864,7 +864,7 @@ impl CacheBucket { for repository in directories(root) { for sha in directories(repository) { if is_match(&sha, name) { - summary += rm_rf(sha, None)?; + summary += rm_rf(sha)?; } } } @@ -872,20 +872,20 @@ impl CacheBucket { Self::Simple => { // For `pypi` wheels, we expect a rkyv file per package, indexed by name. let root = cache.bucket(self).join(WheelCacheKind::Pypi); - summary += rm_rf(root.join(format!("{name}.rkyv")), None)?; + summary += rm_rf(root.join(format!("{name}.rkyv")))?; // For alternate indices, we expect a directory for every index (under an `index` // subdirectory), followed by a directory per package (indexed by name). let root = cache.bucket(self).join(WheelCacheKind::Index); for directory in directories(root) { - summary += rm_rf(directory.join(format!("{name}.rkyv")), None)?; + summary += rm_rf(directory.join(format!("{name}.rkyv")))?; } } Self::FlatIndex => { // We can't know if the flat index includes a package, so we just remove the entire // cache entry. let root = cache.bucket(self); - summary += rm_rf(root, None)?; + summary += rm_rf(root)?; } Self::Git => { // Nothing to do. diff --git a/crates/uv-cache/src/removal.rs b/crates/uv-cache/src/removal.rs index 26aecaf36..c827b5e80 100644 --- a/crates/uv-cache/src/removal.rs +++ b/crates/uv-cache/src/removal.rs @@ -9,12 +9,34 @@ use crate::CleanReporter; /// Remove a file or directory and all its contents, returning a [`Removal`] with /// the number of files and directories removed, along with a total byte count. -pub fn rm_rf(path: impl AsRef, reporter: Option<&dyn CleanReporter>) -> io::Result { - let mut removal = Removal::default(); - removal.rm_rf(path.as_ref(), reporter)?; - Ok(removal) +pub fn rm_rf(path: impl AsRef) -> io::Result { + Remover::default().rm_rf(path) } +/// A builder for a [`Remover`] that can remove files and directories. +#[derive(Default)] +pub(crate) struct Remover { + reporter: Option>, +} + +impl Remover { + /// Create a new [`Remover`] with the given reporter. + pub(crate) fn new(reporter: Box) -> Self { + Self { + reporter: Some(reporter), + } + } + + /// Remove a file or directory and all its contents, returning a [`Removal`] with + /// the number of files and directories removed, along with a total byte count. + pub(crate) fn rm_rf(&self, path: impl AsRef) -> io::Result { + let mut removal = Removal::default(); + removal.rm_rf(path.as_ref(), self.reporter.as_deref())?; + Ok(removal) + } +} + +/// A removal operation with statistics on the number of files and directories removed. #[derive(Debug, Default)] pub struct Removal { /// The number of files removed. diff --git a/crates/uv-distribution/src/source/mod.rs b/crates/uv-distribution/src/source/mod.rs index a721c2618..399e3eff4 100644 --- a/crates/uv-distribution/src/source/mod.rs +++ b/crates/uv-distribution/src/source/mod.rs @@ -1995,8 +1995,8 @@ pub fn prune(cache: &Cache) -> Result { "Removing dangling source revision: {}", sibling.path().display() ); - removal += uv_cache::rm_rf(sibling.path(), None) - .map_err(Error::CacheWrite)?; + removal += + uv_cache::rm_rf(sibling.path()).map_err(Error::CacheWrite)?; } } } @@ -2020,8 +2020,8 @@ pub fn prune(cache: &Cache) -> Result { "Removing dangling source revision: {}", sibling.path().display() ); - removal += uv_cache::rm_rf(sibling.path(), None) - .map_err(Error::CacheWrite)?; + removal += + uv_cache::rm_rf(sibling.path()).map_err(Error::CacheWrite)?; } } } diff --git a/crates/uv/src/commands/cache_clean.rs b/crates/uv/src/commands/cache_clean.rs index 3d3d39326..ec6c7e7ca 100644 --- a/crates/uv/src/commands/cache_clean.rs +++ b/crates/uv/src/commands/cache_clean.rs @@ -3,7 +3,7 @@ use std::fmt::Write; use anyhow::{Context, Result}; use owo_colors::OwoColorize; -use uv_cache::{Cache, CleanReporter, Removal}; +use uv_cache::{Cache, Removal}; use uv_fs::Simplified; use uv_normalize::PackageName; @@ -37,7 +37,7 @@ pub(crate) fn cache_clean( let reporter = CleaningDirectoryReporter::new(printer, num_paths); cache - .clear(Some(&reporter)) + .clear(Box::new(reporter)) .with_context(|| format!("Failed to clear cache at: {}", cache.root().user_display()))? } else { let reporter = CleaningPackageReporter::new(printer, packages.len()); @@ -46,8 +46,7 @@ pub(crate) fn cache_clean( for package in packages { let removed = cache.remove(package)?; summary += removed; - - reporter.on_clean_package(package.as_str(), &summary); + reporter.on_clean(package.as_str(), &summary); } reporter.on_complete(); diff --git a/crates/uv/src/commands/reporters.rs b/crates/uv/src/commands/reporters.rs index a2b86ee3e..69982115d 100644 --- a/crates/uv/src/commands/reporters.rs +++ b/crates/uv/src/commands/reporters.rs @@ -557,6 +557,7 @@ impl uv_cache::CleanReporter for CleaningDirectoryReporter { } } +#[derive(Debug)] pub(crate) struct CleaningPackageReporter { bar: ProgressBar, } @@ -573,12 +574,8 @@ impl CleaningPackageReporter { bar.set_prefix(format!("{}", "Cleaning".bold().cyan())); Self { bar } } -} -impl uv_cache::CleanReporter for CleaningPackageReporter { - fn on_clean(&self) {} - - fn on_clean_package(&self, package: &str, removal: &Removal) { + pub(crate) fn on_clean(&self, package: &str, removal: &Removal) { self.bar.inc(1); self.bar.set_message(format!( ": {}, {} files {} folders removed", @@ -586,7 +583,7 @@ impl uv_cache::CleanReporter for CleaningPackageReporter { )); } - fn on_complete(&self) { + pub(crate) fn on_complete(&self) { self.bar.finish_and_clear(); } }