Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a progress indicator for cargo clean #10236

Merged
merged 5 commits into from
Apr 10, 2022
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
163 changes: 135 additions & 28 deletions src/cargo/ops/cargo_clean.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use crate::ops;
use crate::util::errors::CargoResult;
use crate::util::interning::InternedString;
use crate::util::lev_distance;
use crate::util::Config;
use crate::util::{Config, Progress, ProgressStyle};

use anyhow::Context as _;
use cargo_util::paths;
Expand Down Expand Up @@ -34,7 +34,7 @@ pub fn clean(ws: &Workspace<'_>, opts: &CleanOptions<'_>) -> CargoResult<()> {
// If the doc option is set, we just want to delete the doc directory.
if opts.doc {
target_dir = target_dir.join("doc");
return rm_rf(&target_dir.into_path_unlocked(), config);
return clean_entire_folder(&target_dir.into_path_unlocked(), config);
}

let profiles = Profiles::new(ws, opts.requested_profile)?;
Expand All @@ -53,7 +53,7 @@ pub fn clean(ws: &Workspace<'_>, opts: &CleanOptions<'_>) -> CargoResult<()> {
// Note that we don't bother grabbing a lock here as we're just going to
// blow it all away anyway.
if opts.spec.is_empty() {
return rm_rf(&target_dir.into_path_unlocked(), config);
return clean_entire_folder(&target_dir.into_path_unlocked(), config);
}

// Clean specific packages.
Expand Down Expand Up @@ -133,21 +133,23 @@ pub fn clean(ws: &Workspace<'_>, opts: &CleanOptions<'_>) -> CargoResult<()> {
}
let packages = pkg_set.get_many(pkg_ids)?;

let mut progress = CleaningPackagesBar::new(config, packages.len());
for pkg in packages {
let pkg_dir = format!("{}-*", pkg.name());
progress.on_cleaning_package(&pkg.name())?;

// Clean fingerprints.
for (_, layout) in &layouts_with_host {
let dir = escape_glob_path(layout.fingerprint())?;
rm_rf_glob(&Path::new(&dir).join(&pkg_dir), config)?;
rm_rf_glob(&Path::new(&dir).join(&pkg_dir), config, &mut progress)?;
}

for target in pkg.targets() {
if target.is_custom_build() {
// Get both the build_script_build and the output directory.
for (_, layout) in &layouts_with_host {
let dir = escape_glob_path(layout.build())?;
rm_rf_glob(&Path::new(&dir).join(&pkg_dir), config)?;
rm_rf_glob(&Path::new(&dir).join(&pkg_dir), config, &mut progress)?;
}
continue;
}
Expand Down Expand Up @@ -178,33 +180,33 @@ pub fn clean(ws: &Workspace<'_>, opts: &CleanOptions<'_>) -> CargoResult<()> {
let dir_glob = escape_glob_path(dir)?;
let dir_glob = Path::new(&dir_glob);

rm_rf_glob(&dir_glob.join(&hashed_name), config)?;
rm_rf(&dir.join(&unhashed_name), config)?;
rm_rf_glob(&dir_glob.join(&hashed_name), config, &mut progress)?;
rm_rf(&dir.join(&unhashed_name), config, &mut progress)?;
// Remove dep-info file generated by rustc. It is not tracked in
// file_types. It does not have a prefix.
let hashed_dep_info = dir_glob.join(format!("{}-*.d", crate_name));
rm_rf_glob(&hashed_dep_info, config)?;
rm_rf_glob(&hashed_dep_info, config, &mut progress)?;
let unhashed_dep_info = dir.join(format!("{}.d", crate_name));
rm_rf(&unhashed_dep_info, config)?;
rm_rf(&unhashed_dep_info, config, &mut progress)?;
// Remove split-debuginfo files generated by rustc.
let split_debuginfo_obj = dir_glob.join(format!("{}.*.o", crate_name));
rm_rf_glob(&split_debuginfo_obj, config)?;
rm_rf_glob(&split_debuginfo_obj, config, &mut progress)?;
let split_debuginfo_dwo = dir_glob.join(format!("{}.*.dwo", crate_name));
rm_rf_glob(&split_debuginfo_dwo, config)?;
rm_rf_glob(&split_debuginfo_dwo, config, &mut progress)?;

// Remove the uplifted copy.
if let Some(uplift_dir) = uplift_dir {
let uplifted_path = uplift_dir.join(file_type.uplift_filename(target));
rm_rf(&uplifted_path, config)?;
rm_rf(&uplifted_path, config, &mut progress)?;
// Dep-info generated by Cargo itself.
let dep_info = uplifted_path.with_extension("d");
rm_rf(&dep_info, config)?;
rm_rf(&dep_info, config, &mut progress)?;
}
}
// TODO: what to do about build_script_build?
let dir = escape_glob_path(layout.incremental())?;
let incremental = Path::new(&dir).join(format!("{}-*", crate_name));
rm_rf_glob(&incremental, config)?;
rm_rf_glob(&incremental, config, &mut progress)?;
}
}
}
Expand All @@ -220,29 +222,134 @@ fn escape_glob_path(pattern: &Path) -> CargoResult<String> {
Ok(glob::Pattern::escape(pattern))
}

fn rm_rf_glob(pattern: &Path, config: &Config) -> CargoResult<()> {
fn rm_rf_glob(
pattern: &Path,
config: &Config,
progress: &mut dyn CleaningProgressBar,
) -> CargoResult<()> {
// TODO: Display utf8 warning to user? Or switch to globset?
let pattern = pattern
.to_str()
.ok_or_else(|| anyhow::anyhow!("expected utf-8 path"))?;
for path in glob::glob(pattern)? {
rm_rf(&path?, config)?;
rm_rf(&path?, config, progress)?;
}
Ok(())
}

fn rm_rf(path: &Path, config: &Config) -> CargoResult<()> {
let m = fs::symlink_metadata(path);
if m.as_ref().map(|s| s.is_dir()).unwrap_or(false) {
config
.shell()
.verbose(|shell| shell.status("Removing", path.display()))?;
paths::remove_dir_all(path).with_context(|| "could not remove build directory")?;
} else if m.is_ok() {
config
.shell()
.verbose(|shell| shell.status("Removing", path.display()))?;
paths::remove_file(path).with_context(|| "failed to remove build artifact")?;
fn rm_rf(path: &Path, config: &Config, progress: &mut dyn CleaningProgressBar) -> CargoResult<()> {
if fs::symlink_metadata(path).is_err() {
return Ok(());
}

config
.shell()
.verbose(|shell| shell.status("Removing", path.display()))?;
progress.display_now()?;

for entry in walkdir::WalkDir::new(path).contents_first(true) {
let entry = entry?;
progress.on_clean()?;
if entry.file_type().is_dir() {
paths::remove_dir(entry.path()).with_context(|| "could not remove build directory")?;
} else {
paths::remove_file(entry.path()).with_context(|| "failed to remove build artifact")?;
}
}

Ok(())
}

fn clean_entire_folder(path: &Path, config: &Config) -> CargoResult<()> {
let num_paths = walkdir::WalkDir::new(path).into_iter().count();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This naively seems like it would have a large impact and slow things down because we have to walk everything before actually removing everything. I don't have a great sense for the practical impact of this though.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An alternative to this I think would be to perform the walk once, collect a list of all paths to remove, then remove them all in a loop. That would mean we only have to walk once and should be similar in performance I think.

let mut progress = CleaningFolderBar::new(config, num_paths);
rm_rf(path, config, &mut progress)
}

trait CleaningProgressBar {
fn display_now(&mut self) -> CargoResult<()>;
fn on_clean(&mut self) -> CargoResult<()>;
}

struct CleaningFolderBar<'cfg> {
bar: Progress<'cfg>,
max: usize,
cur: usize,
}

impl<'cfg> CleaningFolderBar<'cfg> {
fn new(cfg: &'cfg Config, max: usize) -> Self {
Self {
bar: Progress::with_style("Cleaning", ProgressStyle::Percentage, cfg),
max,
cur: 0,
}
}

fn cur_progress(&self) -> usize {
std::cmp::min(self.cur, self.max)
}
}

impl<'cfg> CleaningProgressBar for CleaningFolderBar<'cfg> {
fn display_now(&mut self) -> CargoResult<()> {
self.bar.tick_now(self.cur_progress(), self.max, "")
}

fn on_clean(&mut self) -> CargoResult<()> {
self.cur += 1;
self.bar.tick(self.cur_progress(), self.max, "")
}
}

struct CleaningPackagesBar<'cfg> {
bar: Progress<'cfg>,
max: usize,
cur: usize,
num_files_folders_cleaned: usize,
package_being_cleaned: String,
}

impl<'cfg> CleaningPackagesBar<'cfg> {
fn new(cfg: &'cfg Config, max: usize) -> Self {
Self {
bar: Progress::with_style("Cleaning", ProgressStyle::Ratio, cfg),
max,
cur: 0,
num_files_folders_cleaned: 0,
package_being_cleaned: String::new(),
}
}

fn on_cleaning_package(&mut self, package: &str) -> CargoResult<()> {
self.cur += 1;
self.package_being_cleaned = String::from(package);
self.bar
.tick(self.cur_progress(), self.max, &self.format_message())
}

fn cur_progress(&self) -> usize {
std::cmp::min(self.cur, self.max)
}

fn format_message(&self) -> String {
format!(
": {}, {} files/folders cleaned",
self.package_being_cleaned, self.num_files_folders_cleaned
)
}
}

impl<'cfg> CleaningProgressBar for CleaningPackagesBar<'cfg> {
fn display_now(&mut self) -> CargoResult<()> {
self.bar
.tick_now(self.cur_progress(), self.max, &self.format_message())
}

fn on_clean(&mut self) -> CargoResult<()> {
self.bar
.tick(self.cur_progress(), self.max, &self.format_message())?;
self.num_files_folders_cleaned += 1;
Ok(())
}
}