From 151d3b7970a976321008886697d925e7800d2738 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 26 Nov 2025 10:12:18 -0600 Subject: [PATCH 1/5] refactor(clean): Add extra nesting --- src/cargo/ops/cargo_clean.rs | 229 ++++++++++++++++++----------------- 1 file changed, 117 insertions(+), 112 deletions(-) diff --git a/src/cargo/ops/cargo_clean.rs b/src/cargo/ops/cargo_clean.rs index ac4a270ea1e..2dcd79d8c75 100644 --- a/src/cargo/ops/cargo_clean.rs +++ b/src/cargo/ops/cargo_clean.rs @@ -192,23 +192,86 @@ fn clean_specs( clean_ctx.progress = Box::new(CleaningPackagesBar::new(clean_ctx.gctx, packages.len())); - // Try to reduce the amount of times we iterate over the same target directory by storing away - // the directories we've iterated over (and cleaned for a given package). - let mut cleaned_packages: HashMap<_, HashSet<_>> = HashMap::default(); - for pkg in packages { - let pkg_dir = format!("{}-*", pkg.name()); - clean_ctx.progress.on_cleaning_package(&pkg.name())?; - - if clean_ctx.gctx.cli_unstable().build_dir_new_layout { - for (_compile_kind, layout) in &layouts_with_host { - let dir = layout.build_dir().build_unit(&pkg.name()); - clean_ctx.rm_rf(&dir)?; + { + // Try to reduce the amount of times we iterate over the same target directory by storing away + // the directories we've iterated over (and cleaned for a given package). + let mut cleaned_packages: HashMap<_, HashSet<_>> = HashMap::default(); + for pkg in packages { + let pkg_dir = format!("{}-*", pkg.name()); + clean_ctx.progress.on_cleaning_package(&pkg.name())?; + + if clean_ctx.gctx.cli_unstable().build_dir_new_layout { + for (_compile_kind, layout) in &layouts_with_host { + let dir = layout.build_dir().build_unit(&pkg.name()); + clean_ctx.rm_rf(&dir)?; + } + + for target in pkg.targets() { + if target.is_custom_build() { + continue; + } + for &mode in &[ + CompileMode::Build, + CompileMode::Test, + CompileMode::Check { test: false }, + ] { + for (compile_kind, layout) in &layouts { + let triple = target_data.short_name(compile_kind); + let (file_types, _unsupported) = target_data + .info(*compile_kind) + .rustc_outputs(mode, target.kind(), triple, clean_ctx.gctx)?; + let artifact_dir = layout + .artifact_dir() + .expect("artifact-dir was not locked during clean"); + let uplift_dir = match target.kind() { + TargetKind::ExampleBin | TargetKind::ExampleLib(..) => { + Some(artifact_dir.examples()) + } + // Tests/benchmarks are never uplifted. + TargetKind::Test | TargetKind::Bench => None, + _ => Some(artifact_dir.dest()), + }; + // Remove the uplifted copy. + if let Some(uplift_dir) = uplift_dir { + for file_type in file_types { + let uplifted_path = + uplift_dir.join(file_type.uplift_filename(target)); + clean_ctx.rm_rf(&uplifted_path)?; + // Dep-info generated by Cargo itself. + let dep_info = uplifted_path.with_extension("d"); + clean_ctx.rm_rf(&dep_info)?; + } + } + } + } + } + continue; + } + + // Clean fingerprints. + for (_, layout) in &layouts_with_host { + let dir = escape_glob_path(layout.build_dir().legacy_fingerprint())?; + clean_ctx.rm_rf_package_glob_containing_hash( + &pkg.name(), + &Path::new(&dir).join(&pkg_dir), + )?; } 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_dir().build())?; + clean_ctx.rm_rf_package_glob_containing_hash( + &pkg.name(), + &Path::new(&dir).join(&pkg_dir), + )?; + } continue; } + let crate_name: Rc = target.crate_name().into(); + let path_dot: &str = &format!("{crate_name}."); + let path_dash: &str = &format!("{crate_name}-"); for &mode in &[ CompileMode::Build, CompileMode::Test, @@ -222,17 +285,28 @@ fn clean_specs( let artifact_dir = layout .artifact_dir() .expect("artifact-dir was not locked during clean"); - let uplift_dir = match target.kind() { + let (dir, uplift_dir) = match target.kind() { TargetKind::ExampleBin | TargetKind::ExampleLib(..) => { - Some(artifact_dir.examples()) + (layout.build_dir().examples(), Some(artifact_dir.examples())) } // Tests/benchmarks are never uplifted. - TargetKind::Test | TargetKind::Bench => None, - _ => Some(artifact_dir.dest()), + TargetKind::Test | TargetKind::Bench => { + (layout.build_dir().legacy_deps(), None) + } + _ => (layout.build_dir().legacy_deps(), Some(artifact_dir.dest())), }; - // Remove the uplifted copy. - if let Some(uplift_dir) = uplift_dir { - for file_type in file_types { + let mut dir_glob_str = escape_glob_path(dir)?; + let dir_glob = Path::new(&dir_glob_str); + for file_type in file_types { + // Some files include a hash in the filename, some don't. + let hashed_name = file_type.output_filename(target, Some("*")); + let unhashed_name = file_type.output_filename(target, None); + + clean_ctx.rm_rf_glob(&dir_glob.join(&hashed_name))?; + clean_ctx.rm_rf(&dir.join(&unhashed_name))?; + + // Remove the uplifted copy. + if let Some(uplift_dir) = uplift_dir { let uplifted_path = uplift_dir.join(file_type.uplift_filename(target)); clean_ctx.rm_rf(&uplifted_path)?; @@ -241,105 +315,36 @@ fn clean_specs( clean_ctx.rm_rf(&dep_info)?; } } - } - } - } - continue; - } + let unhashed_dep_info = dir.join(format!("{}.d", crate_name)); + clean_ctx.rm_rf(&unhashed_dep_info)?; - // Clean fingerprints. - for (_, layout) in &layouts_with_host { - let dir = escape_glob_path(layout.build_dir().legacy_fingerprint())?; - clean_ctx - .rm_rf_package_glob_containing_hash(&pkg.name(), &Path::new(&dir).join(&pkg_dir))?; - } - - 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_dir().build())?; - clean_ctx.rm_rf_package_glob_containing_hash( - &pkg.name(), - &Path::new(&dir).join(&pkg_dir), - )?; - } - continue; - } - let crate_name: Rc = target.crate_name().into(); - let path_dot: &str = &format!("{crate_name}."); - let path_dash: &str = &format!("{crate_name}-"); - for &mode in &[ - CompileMode::Build, - CompileMode::Test, - CompileMode::Check { test: false }, - ] { - for (compile_kind, layout) in &layouts { - let triple = target_data.short_name(compile_kind); - let (file_types, _unsupported) = target_data - .info(*compile_kind) - .rustc_outputs(mode, target.kind(), triple, clean_ctx.gctx)?; - let artifact_dir = layout - .artifact_dir() - .expect("artifact-dir was not locked during clean"); - let (dir, uplift_dir) = match target.kind() { - TargetKind::ExampleBin | TargetKind::ExampleLib(..) => { - (layout.build_dir().examples(), Some(artifact_dir.examples())) - } - // Tests/benchmarks are never uplifted. - TargetKind::Test | TargetKind::Bench => { - (layout.build_dir().legacy_deps(), None) + if !dir_glob_str.ends_with(std::path::MAIN_SEPARATOR) { + dir_glob_str.push(std::path::MAIN_SEPARATOR); } - _ => (layout.build_dir().legacy_deps(), Some(artifact_dir.dest())), - }; - let mut dir_glob_str = escape_glob_path(dir)?; - let dir_glob = Path::new(&dir_glob_str); - for file_type in file_types { - // Some files include a hash in the filename, some don't. - let hashed_name = file_type.output_filename(target, Some("*")); - let unhashed_name = file_type.output_filename(target, None); - - clean_ctx.rm_rf_glob(&dir_glob.join(&hashed_name))?; - clean_ctx.rm_rf(&dir.join(&unhashed_name))?; - - // Remove the uplifted copy. - if let Some(uplift_dir) = uplift_dir { - let uplifted_path = uplift_dir.join(file_type.uplift_filename(target)); - clean_ctx.rm_rf(&uplifted_path)?; - // Dep-info generated by Cargo itself. - let dep_info = uplifted_path.with_extension("d"); - clean_ctx.rm_rf(&dep_info)?; + dir_glob_str.push('*'); + let dir_glob_str: Rc = dir_glob_str.into(); + if cleaned_packages + .entry(dir_glob_str.clone()) + .or_default() + .insert(crate_name.clone()) + { + let paths = [ + // Remove dep-info file generated by rustc. It is not tracked in + // file_types. It does not have a prefix. + (path_dash, ".d"), + // Remove split-debuginfo files generated by rustc. + (path_dot, ".o"), + (path_dot, ".dwo"), + (path_dot, ".dwp"), + ]; + clean_ctx.rm_rf_prefix_list(&dir_glob_str, &paths)?; } - } - let unhashed_dep_info = dir.join(format!("{}.d", crate_name)); - clean_ctx.rm_rf(&unhashed_dep_info)?; - if !dir_glob_str.ends_with(std::path::MAIN_SEPARATOR) { - dir_glob_str.push(std::path::MAIN_SEPARATOR); - } - dir_glob_str.push('*'); - let dir_glob_str: Rc = dir_glob_str.into(); - if cleaned_packages - .entry(dir_glob_str.clone()) - .or_default() - .insert(crate_name.clone()) - { - let paths = [ - // Remove dep-info file generated by rustc. It is not tracked in - // file_types. It does not have a prefix. - (path_dash, ".d"), - // Remove split-debuginfo files generated by rustc. - (path_dot, ".o"), - (path_dot, ".dwo"), - (path_dot, ".dwp"), - ]; - clean_ctx.rm_rf_prefix_list(&dir_glob_str, &paths)?; + // TODO: what to do about build_script_build? + let dir = escape_glob_path(layout.build_dir().incremental())?; + let incremental = Path::new(&dir).join(format!("{}-*", crate_name)); + clean_ctx.rm_rf_glob(&incremental)?; } - - // TODO: what to do about build_script_build? - let dir = escape_glob_path(layout.build_dir().incremental())?; - let incremental = Path::new(&dir).join(format!("{}-*", crate_name)); - clean_ctx.rm_rf_glob(&incremental)?; } } } From c5161ac302d3961c6892a3c3c8beab1d4dd08a3a Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 26 Nov 2025 10:14:35 -0600 Subject: [PATCH 2/5] refactor(clean): Pull out the layout conditional --- src/cargo/ops/cargo_clean.rs | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/cargo/ops/cargo_clean.rs b/src/cargo/ops/cargo_clean.rs index 2dcd79d8c75..ffc109ed134 100644 --- a/src/cargo/ops/cargo_clean.rs +++ b/src/cargo/ops/cargo_clean.rs @@ -192,12 +192,8 @@ fn clean_specs( clean_ctx.progress = Box::new(CleaningPackagesBar::new(clean_ctx.gctx, packages.len())); - { - // Try to reduce the amount of times we iterate over the same target directory by storing away - // the directories we've iterated over (and cleaned for a given package). - let mut cleaned_packages: HashMap<_, HashSet<_>> = HashMap::default(); + if clean_ctx.gctx.cli_unstable().build_dir_new_layout { for pkg in packages { - let pkg_dir = format!("{}-*", pkg.name()); clean_ctx.progress.on_cleaning_package(&pkg.name())?; if clean_ctx.gctx.cli_unstable().build_dir_new_layout { @@ -247,6 +243,14 @@ fn clean_specs( } continue; } + } + } else { + // Try to reduce the amount of times we iterate over the same target directory by storing away + // the directories we've iterated over (and cleaned for a given package). + let mut cleaned_packages: HashMap<_, HashSet<_>> = HashMap::default(); + for pkg in packages { + let pkg_dir = format!("{}-*", pkg.name()); + clean_ctx.progress.on_cleaning_package(&pkg.name())?; // Clean fingerprints. for (_, layout) in &layouts_with_host { From 7dbdd7df98ec4f8407c3db11c3b84461e7448af8 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 26 Nov 2025 10:14:54 -0600 Subject: [PATCH 3/5] refactor(clean): Remove redundant conditional --- src/cargo/ops/cargo_clean.rs | 79 +++++++++++++++++------------------- 1 file changed, 38 insertions(+), 41 deletions(-) diff --git a/src/cargo/ops/cargo_clean.rs b/src/cargo/ops/cargo_clean.rs index ffc109ed134..47aaad33205 100644 --- a/src/cargo/ops/cargo_clean.rs +++ b/src/cargo/ops/cargo_clean.rs @@ -196,52 +196,49 @@ fn clean_specs( for pkg in packages { clean_ctx.progress.on_cleaning_package(&pkg.name())?; - if clean_ctx.gctx.cli_unstable().build_dir_new_layout { - for (_compile_kind, layout) in &layouts_with_host { - let dir = layout.build_dir().build_unit(&pkg.name()); - clean_ctx.rm_rf(&dir)?; - } + for (_compile_kind, layout) in &layouts_with_host { + let dir = layout.build_dir().build_unit(&pkg.name()); + clean_ctx.rm_rf(&dir)?; + } - for target in pkg.targets() { - if target.is_custom_build() { - continue; - } - for &mode in &[ - CompileMode::Build, - CompileMode::Test, - CompileMode::Check { test: false }, - ] { - for (compile_kind, layout) in &layouts { - let triple = target_data.short_name(compile_kind); - let (file_types, _unsupported) = target_data - .info(*compile_kind) - .rustc_outputs(mode, target.kind(), triple, clean_ctx.gctx)?; - let artifact_dir = layout - .artifact_dir() - .expect("artifact-dir was not locked during clean"); - let uplift_dir = match target.kind() { - TargetKind::ExampleBin | TargetKind::ExampleLib(..) => { - Some(artifact_dir.examples()) - } - // Tests/benchmarks are never uplifted. - TargetKind::Test | TargetKind::Bench => None, - _ => Some(artifact_dir.dest()), - }; - // Remove the uplifted copy. - if let Some(uplift_dir) = uplift_dir { - for file_type in file_types { - let uplifted_path = - uplift_dir.join(file_type.uplift_filename(target)); - clean_ctx.rm_rf(&uplifted_path)?; - // Dep-info generated by Cargo itself. - let dep_info = uplifted_path.with_extension("d"); - clean_ctx.rm_rf(&dep_info)?; - } + for target in pkg.targets() { + if target.is_custom_build() { + continue; + } + for &mode in &[ + CompileMode::Build, + CompileMode::Test, + CompileMode::Check { test: false }, + ] { + for (compile_kind, layout) in &layouts { + let triple = target_data.short_name(compile_kind); + let (file_types, _unsupported) = target_data + .info(*compile_kind) + .rustc_outputs(mode, target.kind(), triple, clean_ctx.gctx)?; + let artifact_dir = layout + .artifact_dir() + .expect("artifact-dir was not locked during clean"); + let uplift_dir = match target.kind() { + TargetKind::ExampleBin | TargetKind::ExampleLib(..) => { + Some(artifact_dir.examples()) + } + // Tests/benchmarks are never uplifted. + TargetKind::Test | TargetKind::Bench => None, + _ => Some(artifact_dir.dest()), + }; + // Remove the uplifted copy. + if let Some(uplift_dir) = uplift_dir { + for file_type in file_types { + let uplifted_path = + uplift_dir.join(file_type.uplift_filename(target)); + clean_ctx.rm_rf(&uplifted_path)?; + // Dep-info generated by Cargo itself. + let dep_info = uplifted_path.with_extension("d"); + clean_ctx.rm_rf(&dep_info)?; } } } } - continue; } } } else { From 9bd1a703ec6c235391d8432cb6bbcb3a4fbbc0d1 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 26 Nov 2025 10:16:30 -0600 Subject: [PATCH 4/5] refactor(clean): Pull up comment --- src/cargo/ops/cargo_clean.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cargo/ops/cargo_clean.rs b/src/cargo/ops/cargo_clean.rs index 47aaad33205..4e5e4334aad 100644 --- a/src/cargo/ops/cargo_clean.rs +++ b/src/cargo/ops/cargo_clean.rs @@ -201,6 +201,7 @@ fn clean_specs( clean_ctx.rm_rf(&dir)?; } + // Remove the uplifted copy. for target in pkg.targets() { if target.is_custom_build() { continue; @@ -226,7 +227,6 @@ fn clean_specs( TargetKind::Test | TargetKind::Bench => None, _ => Some(artifact_dir.dest()), }; - // Remove the uplifted copy. if let Some(uplift_dir) = uplift_dir { for file_type in file_types { let uplifted_path = From 4414a60c4b417a61379216e98c6e3739637050e3 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 26 Nov 2025 10:19:15 -0600 Subject: [PATCH 5/5] refactor(clean): Describe other clean operation --- src/cargo/ops/cargo_clean.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/cargo/ops/cargo_clean.rs b/src/cargo/ops/cargo_clean.rs index 4e5e4334aad..0be8e9f9051 100644 --- a/src/cargo/ops/cargo_clean.rs +++ b/src/cargo/ops/cargo_clean.rs @@ -196,6 +196,7 @@ fn clean_specs( for pkg in packages { clean_ctx.progress.on_cleaning_package(&pkg.name())?; + // Remove intermediate artifacts for (_compile_kind, layout) in &layouts_with_host { let dir = layout.build_dir().build_unit(&pkg.name()); clean_ctx.rm_rf(&dir)?;