Skip to content

Commit

Permalink
Separately track files and directories removed.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
ehuss committed Sep 20, 2023
1 parent e9110aa commit ebee726
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 20 deletions.
32 changes: 21 additions & 11 deletions src/cargo/ops/cargo_clean.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ pub struct CleanContext<'cfg> {
pub config: &'cfg Config,
progress: Box<dyn CleaningProgressBar + 'cfg>,
pub dry_run: bool,
num_files_folders_cleaned: u64,
num_files_removed: u64,
num_dirs_removed: u64,
total_bytes_removed: u64,
}

Expand Down Expand Up @@ -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,
}
}
Expand Down Expand Up @@ -352,27 +354,27 @@ 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)?;
}
Ok(())
};

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
Expand Down Expand Up @@ -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.
Expand Down
14 changes: 7 additions & 7 deletions tests/testsuite/clean.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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();
Expand All @@ -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();
}

Expand Down
2 changes: 1 addition & 1 deletion tests/testsuite/profile_custom.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
2 changes: 1 addition & 1 deletion tests/testsuite/script.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down

0 comments on commit ebee726

Please sign in to comment.