From ebee726d8f4105dca185ddb6253508ca09b7a8f0 Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Sun, 10 Sep 2023 12:02:17 -0700 Subject: [PATCH] Separately track files and directories removed. The previous status line was a little awkward in the way it combined both counts. I don't think showing the directories is particularly interesting, so they are only displayed when no files are deleted. --- src/cargo/ops/cargo_clean.rs | 32 ++++++++++++++++++++----------- tests/testsuite/clean.rs | 14 +++++++------- tests/testsuite/profile_custom.rs | 2 +- tests/testsuite/script.rs | 2 +- 4 files changed, 30 insertions(+), 20 deletions(-) diff --git a/src/cargo/ops/cargo_clean.rs b/src/cargo/ops/cargo_clean.rs index 6cba612c237..84ecc5dbfa9 100644 --- a/src/cargo/ops/cargo_clean.rs +++ b/src/cargo/ops/cargo_clean.rs @@ -31,7 +31,8 @@ pub struct CleanContext<'cfg> { pub config: &'cfg Config, progress: Box, pub dry_run: bool, - num_files_folders_cleaned: u64, + num_files_removed: u64, + num_dirs_removed: u64, total_bytes_removed: u64, } @@ -270,7 +271,8 @@ impl<'cfg> CleanContext<'cfg> { config, progress: Box::new(progress), dry_run: false, - num_files_folders_cleaned: 0, + num_files_removed: 0, + num_dirs_removed: 0, total_bytes_removed: 0, } } @@ -352,6 +354,7 @@ impl<'cfg> CleanContext<'cfg> { // byte sizes and not the block sizes. self.total_bytes_removed += meta.len(); } + self.num_files_removed += 1; if !self.dry_run { paths::remove_file(path)?; } @@ -359,20 +362,19 @@ impl<'cfg> CleanContext<'cfg> { }; if !meta.is_dir() { - self.num_files_folders_cleaned += 1; return rm_file(path, Ok(meta)); } for entry in walkdir::WalkDir::new(path).contents_first(true) { let entry = entry?; self.progress.on_clean()?; - self.num_files_folders_cleaned += 1; if self.dry_run { self.config .shell() .verbose(|shell| Ok(writeln!(shell.out(), "{}", entry.path().display())?))?; } if entry.file_type().is_dir() { + self.num_dirs_removed += 1; // The contents should have been removed by now, but sometimes a race condition is hit // where other files have been added by the OS. `paths::remove_dir_all` also falls back // to `std::fs::remove_dir_all`, which may be more reliable than a simple walk in @@ -401,13 +403,21 @@ impl<'cfg> CleanContext<'cfg> { format!(", {bytes:.1}{unit} total") } }; - self.config.shell().status( - status, - format!( - "{} files/directories{byte_count}", - self.num_files_folders_cleaned - ), - ) + // I think displaying the number of directories removed isn't + // particularly interesting to the user. However, if there are 0 + // files, and a nonzero number of directories, cargo should indicate + // that it did *something*, so directory counts are only shown in that + // case. + let file_count = match (self.num_files_removed, self.num_dirs_removed) { + (0, 0) => format!("0 files"), + (0, 1) => format!("1 directory"), + (0, 2..) => format!("{} directories", self.num_dirs_removed), + (1, _) => format!("1 file"), + (2.., _) => format!("{} files", self.num_files_removed), + }; + self.config + .shell() + .status(status, format!("{file_count}{byte_count}")) } /// Deletes all of the given paths, showing a progress bar as it proceeds. diff --git a/tests/testsuite/clean.rs b/tests/testsuite/clean.rs index fd66fbb139e..9474e39965e 100644 --- a/tests/testsuite/clean.rs +++ b/tests/testsuite/clean.rs @@ -417,7 +417,7 @@ fn clean_verbose() { expected.push_str(&format!("[REMOVING] [..]{}\n", obj.unwrap().display())); } } - expected.push_str("[REMOVED] [..] files/directories, [..] total\n"); + expected.push_str("[REMOVED] [..] files, [..] total\n"); p.cargo("clean -p bar --verbose") .with_stderr_unordered(&expected) .run(); @@ -609,7 +609,7 @@ error: package ID specification `baz` did not match any packages .with_stderr( "warning: version qualifier in `-p bar:0.1.0` is ignored, \ cleaning all versions of `bar` found\n\ - [REMOVED] [..] files/directories, [..] total", + [REMOVED] [..] files, [..] total", ) .run(); let mut walker = walkdir::WalkDir::new(p.build_dir()) @@ -664,7 +664,7 @@ error: package ID specification `baz` did not match any packages .with_stderr( "warning: version qualifier in `-p bar:0.1` is ignored, \ cleaning all versions of `bar` found\n\ - [REMOVED] [..] files/directories, [..] total", + [REMOVED] [..] files, [..] total", ) .run(); let mut walker = walkdir::WalkDir::new(p.build_dir()) @@ -719,7 +719,7 @@ error: package ID specification `baz` did not match any packages .with_stderr( "warning: version qualifier in `-p bar:0` is ignored, \ cleaning all versions of `bar` found\n\ - [REMOVED] [..] files/directories, [..] total", + [REMOVED] [..] files, [..] total", ) .run(); let mut walker = walkdir::WalkDir::new(p.build_dir()) @@ -817,13 +817,13 @@ fn clean_dry_run() { // Start with no files. p.cargo("clean --dry-run") .with_stdout("") - .with_stderr("[SUMMARY] 0 files/directories") + .with_stderr("[SUMMARY] 0 files") .run(); p.cargo("check").run(); let before = ls_r(); p.cargo("clean --dry-run") .with_stdout("[CWD]/target") - .with_stderr("[SUMMARY] [..] files/directories, [..] total") + .with_stderr("[SUMMARY] [..] files, [..] total") .run(); // Verify it didn't delete anything. let after = ls_r(); @@ -833,7 +833,7 @@ fn clean_dry_run() { // Verify the verbose output. p.cargo("clean --dry-run -v") .with_stdout_unordered(expected) - .with_stderr("[SUMMARY] [..] files/directories, [..] total") + .with_stderr("[SUMMARY] [..] files, [..] total") .run(); } diff --git a/tests/testsuite/profile_custom.rs b/tests/testsuite/profile_custom.rs index 32e471796a4..f7139e55267 100644 --- a/tests/testsuite/profile_custom.rs +++ b/tests/testsuite/profile_custom.rs @@ -544,7 +544,7 @@ fn clean_custom_dirname() { // This should clean 'other' p.cargo("clean --profile=other") - .with_stderr("[REMOVED] [..] files/directories, [..] total") + .with_stderr("[REMOVED] [..] files, [..] total") .run(); assert!(p.build_dir().join("debug").is_dir()); assert!(!p.build_dir().join("other").is_dir()); diff --git a/tests/testsuite/script.rs b/tests/testsuite/script.rs index fe1e33c00cc..c869b43f7d6 100644 --- a/tests/testsuite/script.rs +++ b/tests/testsuite/script.rs @@ -980,7 +980,7 @@ fn cmd_clean_with_embedded() { .with_stderr( "\ [WARNING] `package.edition` is unspecified, defaulting to `2021` -[REMOVED] [..] files/directories, [..] total +[REMOVED] [..] files, [..] total ", ) .run();