Skip to content
Open
Show file tree
Hide file tree
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
11 changes: 10 additions & 1 deletion src/bin/cargo/commands/clean.rs
Copy link
Member

Choose a reason for hiding this comment

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

  1. During testing we've found that Workspace::members returns path dependencies of workspace members as workspace members; this means that cargo clean --workspace will clean up artifacts of path dependencies as well. We are not sure if that's a good behaviour and would love to get some more guidance on it.

This is a documented behavior, so they should be pruned as well.

All path dependencies residing in the workspace directory automatically become members. Additional members can be listed with the members key, which should be an array of strings containing directories with Cargo.toml files.

See also

fn find_path_deps(
&mut self,
manifest_path: &Path,
root_manifest: &Path,
is_path_dep: bool,
) -> CargoResult<()> {
let manifest_path = paths::normalize_path(manifest_path);
if self.members.contains(&manifest_path) {
return Ok(());
}
if is_path_dep && self.root_maybe().is_embedded() {
// Embedded manifests cannot have workspace members
return Ok(());
}
if is_path_dep
&& !manifest_path.parent().unwrap().starts_with(self.root())
&& self.find_root(&manifest_path)? != self.root_manifest
{
// If `manifest_path` is a path dependency outside of the workspace,
// don't add it, or any of its dependencies, as a members.
return Ok(());
}
if let WorkspaceConfig::Root(ref root_config) =
*self.packages.load(root_manifest)?.workspace_config()
{
if root_config.is_excluded(&manifest_path) {
return Ok(());
}
}
debug!("find_path_deps - {}", manifest_path.display());
self.members.push(manifest_path.clone());
let candidates = {
let pkg = match *self.packages.load(&manifest_path)? {
MaybePackage::Package(ref p) => p,
MaybePackage::Virtual(_) => return Ok(()),
};
self.member_ids.insert(pkg.package_id());
pkg.dependencies()
.iter()
.map(|d| (d.source_id(), d.package_name()))
.filter(|(s, _)| s.is_path())
.filter_map(|(s, n)| s.url().to_file_path().ok().map(|p| (p, n)))
.map(|(p, n)| (p.join("Cargo.toml"), n))
.collect::<Vec<_>>()
};
for (path, name) in candidates {
self.find_path_deps(&path, root_manifest, true)
.with_context(|| format!("failed to load manifest for dependency `{}`", name))
.map_err(|err| ManifestError::new(err, manifest_path.clone()))?;
}
Ok(())
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see, thank you!

Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use cargo::ops::CleanContext;
use cargo::ops::{self, CleanOptions};
use cargo::util::print_available_packages;
use clap_complete::ArgValueCandidates;
use indexmap::IndexSet;
use std::time::Duration;

pub fn cli() -> Command {
Expand All @@ -18,6 +19,10 @@ pub fn cli() -> Command {
"Package to clean artifacts for",
ArgValueCandidates::new(get_pkg_name_candidates),
)
.arg(
flag("workspace", "Clean artifacts of the workspace members")
.help_heading(heading::PACKAGE_SELECTION),
)
.arg_release("Whether or not to clean release artifacts")
.arg_profile("Clean artifacts of the specified profile")
.arg_target_triple("Target triple to clean output for")
Expand Down Expand Up @@ -146,10 +151,14 @@ pub fn exec(gctx: &mut GlobalContext, args: &ArgMatches) -> CliResult {
if args.is_present_with_zero_values("package") {
print_available_packages(&ws)?;
}
let mut spec = IndexSet::from_iter(values(args, "package"));

if args.flag("workspace") {
spec.extend(ws.members().map(|package| package.name().to_string()))
};
let opts = CleanOptions {
gctx,
spec: values(args, "package"),
spec,
targets: args.targets()?,
requested_profile: args.get_profile_name("dev", ProfileChecking::Custom)?,
profile_specified: args.contains_id("profile") || args.flag("release"),
Expand Down
5 changes: 3 additions & 2 deletions src/cargo/ops/cargo_clean.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use crate::util::interning::InternedString;
use crate::util::{GlobalContext, Progress, ProgressStyle};
use anyhow::bail;
use cargo_util::paths;
use indexmap::IndexSet;
use std::collections::{HashMap, HashSet};
use std::fs;
use std::path::{Path, PathBuf};
Expand All @@ -17,7 +18,7 @@ use std::rc::Rc;
pub struct CleanOptions<'gctx> {
pub gctx: &'gctx GlobalContext,
/// A list of packages to clean. If empty, everything is cleaned.
pub spec: Vec<String>,
pub spec: IndexSet<String>,
/// The target arch triple to clean, or None for the host arch
pub targets: Vec<String>,
/// Whether to clean the release directory
Expand Down Expand Up @@ -108,7 +109,7 @@ fn clean_specs(
ws: &Workspace<'_>,
profiles: &Profiles,
targets: &[String],
spec: &[String],
spec: &IndexSet<String>,
dry_run: bool,
) -> CargoResult<()> {
// Clean specific packages.
Expand Down
6 changes: 6 additions & 0 deletions src/doc/man/cargo-clean.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,16 @@ When no packages are selected, all packages and all dependencies in the
workspace are cleaned.

{{#options}}

{{#option "`-p` _spec_..." "`--package` _spec_..." }}
Clean only the specified packages. This flag may be specified
multiple times. See {{man "cargo-pkgid" 1}} for the SPEC format.
{{/option}}

{{#option "`--workspace`" }}
Clean artifacts of the workspace members.
{{/option}}

{{/options}}

### Clean Options
Expand Down
3 changes: 3 additions & 0 deletions src/doc/man/generated_txt/cargo-clean.txt
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ OPTIONS
Clean only the specified packages. This flag may be specified
multiple times. See cargo-pkgid(1) for the SPEC format.

--workspace
Clean artifacts of the workspace members.

Clean Options
--dry-run
Displays a summary of what would be deleted without deleting
Expand Down
7 changes: 7 additions & 0 deletions src/doc/src/commands/cargo-clean.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,19 @@ When no packages are selected, all packages and all dependencies in the
workspace are cleaned.

<dl>

<dt class="option-term" id="option-cargo-clean--p"><a class="option-anchor" href="#option-cargo-clean--p"><code>-p</code> <em>spec</em>…</a></dt>
<dt class="option-term" id="option-cargo-clean---package"><a class="option-anchor" href="#option-cargo-clean---package"><code>--package</code> <em>spec</em>…</a></dt>
<dd class="option-desc"><p>Clean only the specified packages. This flag may be specified
multiple times. See <a href="cargo-pkgid.html">cargo-pkgid(1)</a> for the SPEC format.</p>
</dd>


<dt class="option-term" id="option-cargo-clean---workspace"><a class="option-anchor" href="#option-cargo-clean---workspace"><code>--workspace</code></a></dt>
<dd class="option-desc"><p>Clean artifacts of the workspace members.</p>
</dd>


</dl>

### Clean Options
Expand Down
5 changes: 5 additions & 0 deletions src/etc/man/cargo-clean.1
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,11 @@ workspace are cleaned.
Clean only the specified packages. This flag may be specified
multiple times. See \fBcargo\-pkgid\fR(1) for the SPEC format.
.RE
.sp
\fB\-\-workspace\fR
.RS 4
Clean artifacts of the workspace members.
.RE
.SS "Clean Options"
.sp
\fB\-\-dry\-run\fR
Expand Down
38 changes: 20 additions & 18 deletions tests/testsuite/cargo_clean/help/stdout.term.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
139 changes: 139 additions & 0 deletions tests/testsuite/clean.rs
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,145 @@ fn clean_p_only_cleans_specified_package() {
);
}

#[cargo_test]
fn clean_workspace_does_not_touch_non_workspace_packages() {
Package::new("external_dependency", "0.1.0").publish();
let foo_manifest = r#"
[package]
name = "foo"
version = "0.1.0"
edition = "2015"

[dependencies]
external_dependency = "0.1.0"
"#;
let p = project()
.file(
"Cargo.toml",
r#"
[workspace]
members = [
"foo",
"foo_core",
"foo-base",
]
"#,
)
.file("foo/Cargo.toml", foo_manifest)
.file("foo/src/lib.rs", "//! foo")
.file("foo_core/Cargo.toml", &basic_manifest("foo_core", "0.1.0"))
.file("foo_core/src/lib.rs", "//! foo_core")
.file("foo-base/Cargo.toml", &basic_manifest("foo-base", "0.1.0"))
.file("foo-base/src/lib.rs", "//! foo-base")
.build();

let fingerprint_path = &p.build_dir().join("debug").join(".fingerprint");

p.cargo("check -p foo -p foo_core -p foo-base").run();

let mut fingerprint_names = get_fingerprints_without_hashes(fingerprint_path);

// Artifacts present for all after building
assert!(fingerprint_names.iter().any(|e| e == "foo"));
assert!(fingerprint_names.iter().any(|e| e == "foo_core"));
assert!(fingerprint_names.iter().any(|e| e == "foo-base"));

let num_external_dependency_artifacts = fingerprint_names
.iter()
.filter(|&e| e == "external_dependency")
.count();
assert_ne!(num_external_dependency_artifacts, 0);

p.cargo("clean --workspace").run();

fingerprint_names = get_fingerprints_without_hashes(fingerprint_path);

// Cleaning workspace members leaves artifacts for the external dependency
assert!(
!fingerprint_names
.iter()
.any(|e| e == "foo" || e == "foo_core" || e == "foo-base")
);
assert_eq!(
fingerprint_names
.iter()
.filter(|&e| e == "external_dependency")
.count(),
num_external_dependency_artifacts,
);
}

#[cargo_test]
fn clean_workspace_with_extra_package_specifiers() {
Package::new("external_dependency_1", "0.1.0").publish();
Package::new("external_dependency_2", "0.1.0").publish();
let foo_manifest = r#"
[package]
name = "foo"
version = "0.1.0"
edition = "2015"

[dependencies]
external_dependency_1 = "0.1.0"
external_dependency_2 = "0.1.0"
"#;
let p = project()
.file(
"Cargo.toml",
r#"
[workspace]
members = [
"foo",
"foo_core",
"foo-base",
]
"#,
)
.file("foo/Cargo.toml", foo_manifest)
.file("foo/src/lib.rs", "//! foo")
.file("foo_core/Cargo.toml", &basic_manifest("foo_core", "0.1.0"))
.file("foo_core/src/lib.rs", "//! foo_core")
.file("foo-base/Cargo.toml", &basic_manifest("foo-base", "0.1.0"))
.file("foo-base/src/lib.rs", "//! foo-base")
.build();

let fingerprint_path = &p.build_dir().join("debug").join(".fingerprint");

p.cargo("check -p foo -p foo_core -p foo-base").run();

let mut fingerprint_names = get_fingerprints_without_hashes(fingerprint_path);

// Artifacts present for all after building
assert!(fingerprint_names.iter().any(|e| e == "foo"));
assert!(fingerprint_names.iter().any(|e| e == "foo_core"));
assert!(fingerprint_names.iter().any(|e| e == "foo-base"));

let num_external_dependency_2_artifacts = fingerprint_names
.iter()
.filter(|&e| e == "external_dependency_2")
.count();
assert_ne!(num_external_dependency_2_artifacts, 0);

p.cargo("clean --workspace -p external_dependency_1").run();

fingerprint_names = get_fingerprints_without_hashes(fingerprint_path);

// Cleaning workspace members and external_dependency_1 leaves artifacts for the external_dependency_2
assert!(
!fingerprint_names.iter().any(|e| e == "foo"
|| e == "foo_core"
|| e == "foo-base"
|| e == "external_dependency_1")
);
assert_eq!(
fingerprint_names
.iter()
.filter(|&e| e == "external_dependency_2")
.count(),
num_external_dependency_2_artifacts,
);
}

fn get_fingerprints_without_hashes(fingerprint_path: &Path) -> Vec<String> {
std::fs::read_dir(fingerprint_path)
.expect("Build dir should be readable")
Expand Down