Skip to content

Commit

Permalink
Auto merge of #11242 - cassaundra:remove-workspace, r=epage
Browse files Browse the repository at this point in the history
Clean up workspace dependencies after cargo remove

### What does this PR try to resolve?

After successful removal of an inherited dependency from a workspace member, clean up the root workspace manifest.

This PR is part of the continued working on cargo remove (#11099, see deferred work).

### How should we test and review this PR?

Make sure the tests cover all possible use cases. After posting this PR, I will post a short self-review regarding some design concerns.

### Additional information

#11194 is currently blocked on this feature.
  • Loading branch information
bors committed Nov 1, 2022
2 parents 16c9c4e + 38b23d5 commit 65b2149
Show file tree
Hide file tree
Showing 34 changed files with 361 additions and 1 deletion.
53 changes: 53 additions & 0 deletions src/bin/cargo/commands/remove.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
use cargo::core::dependency::DepKind;
use cargo::core::Workspace;
use cargo::ops::cargo_remove::remove;
use cargo::ops::cargo_remove::RemoveOptions;
use cargo::ops::resolve_ws;
use cargo::util::command_prelude::*;
use cargo::util::toml_mut::manifest::DepTable;
use cargo::util::toml_mut::manifest::LocalManifest;
use cargo::CargoResult;

pub fn cli() -> clap::Command {
clap::Command::new("remove")
Expand Down Expand Up @@ -85,6 +88,9 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
remove(&options)?;

if !dry_run {
// Clean up workspace dependencies
gc_workspace(&workspace, &options.dependencies)?;

// Reload the workspace since we've changed dependencies
let ws = args.workspace(config)?;
resolve_ws(&ws)?;
Expand Down Expand Up @@ -114,3 +120,50 @@ fn parse_section(args: &ArgMatches) -> DepTable {

table
}

/// Clean up workspace dependencies which no longer have a reference to them.
fn gc_workspace(workspace: &Workspace<'_>, dependencies: &[String]) -> CargoResult<()> {
let mut manifest: toml_edit::Document =
cargo_util::paths::read(workspace.root_manifest())?.parse()?;

let members = workspace
.members()
.map(|p| LocalManifest::try_new(p.manifest_path()))
.collect::<CargoResult<Vec<_>>>()?;

for dep in dependencies {
if !dep_in_workspace(dep, &members) {
remove_workspace_dep(dep, &mut manifest);
}
}

cargo_util::paths::write(workspace.root_manifest(), manifest.to_string().as_bytes())?;

Ok(())
}

/// Get whether or not a dependency is depended upon in a workspace.
fn dep_in_workspace(dep: &str, members: &[LocalManifest]) -> bool {
members.iter().any(|manifest| {
manifest.get_sections().iter().any(|(_, table)| {
table
.as_table_like()
.unwrap()
.get(dep)
.and_then(|t| t.get("workspace"))
.and_then(|v| v.as_bool())
.unwrap_or(false)
})
})
}

/// Remove a dependency from a workspace manifest.
fn remove_workspace_dep(dep: &str, ws_manifest: &mut toml_edit::Document) {
if let Some(toml_edit::Item::Table(table)) = ws_manifest
.get_mut("workspace")
.and_then(|t| t.get_mut("dependencies"))
{
table.set_implicit(true);
table.remove(dep);
}
}
2 changes: 1 addition & 1 deletion src/cargo/util/toml_mut/manifest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ impl std::fmt::Display for Manifest {
}

/// An editable Cargo manifest that is available locally.
#[derive(Debug)]
#[derive(Debug, Clone)]
pub struct LocalManifest {
/// Path to the manifest.
pub path: PathBuf,
Expand Down
3 changes: 3 additions & 0 deletions tests/testsuite/cargo_remove/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ mod target;
mod target_build;
mod target_dev;
mod update_lock_file;
mod workspace;
mod workspace_non_virtual;
mod workspace_preserved;

fn init_registry() {
cargo_test_support::registry::init();
Expand Down
5 changes: 5 additions & 0 deletions tests/testsuite/cargo_remove/workspace/in/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
[workspace]
members = [ "my-package" ]

[workspace.dependencies]
semver = "0.1.0"
24 changes: 24 additions & 0 deletions tests/testsuite/cargo_remove/workspace/in/my-package/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
[package]
name = "my-package"
version = "0.1.0"

[[bin]]
name = "main"
path = "src/main.rs"

[build-dependencies]
semver = { workspace = true }

[dependencies]
docopt = "0.6"
rustc-serialize = "0.4"
semver = "0.1"
toml = "0.1"
clippy = "0.4"

[dev-dependencies]
regex = "0.1.1"
serde = "1.0.90"

[features]
std = ["serde/std", "semver/std"]
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@

25 changes: 25 additions & 0 deletions tests/testsuite/cargo_remove/workspace/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
use cargo_test_support::compare::assert_ui;
use cargo_test_support::curr_dir;
use cargo_test_support::CargoCommand;
use cargo_test_support::Project;

use crate::cargo_remove::init_registry;

#[cargo_test]
fn case() {
init_registry();
let project = Project::from_template(curr_dir!().join("in"));
let project_root = project.root();
let cwd = &project_root;

snapbox::cmd::Command::cargo_ui()
.arg("remove")
.args(["--package", "my-package", "--build", "semver"])
.current_dir(cwd)
.assert()
.success()
.stdout_matches_path(curr_dir!().join("stdout.log"))
.stderr_matches_path(curr_dir!().join("stderr.log"));

assert_ui().subset_matches(curr_dir!().join("out"), &project_root);
}
2 changes: 2 additions & 0 deletions tests/testsuite/cargo_remove/workspace/out/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
[workspace]
members = [ "my-package" ]
21 changes: 21 additions & 0 deletions tests/testsuite/cargo_remove/workspace/out/my-package/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
[package]
name = "my-package"
version = "0.1.0"

[[bin]]
name = "main"
path = "src/main.rs"

[dependencies]
docopt = "0.6"
rustc-serialize = "0.4"
semver = "0.1"
toml = "0.1"
clippy = "0.4"

[dev-dependencies]
regex = "0.1.1"
serde = "1.0.90"

[features]
std = ["serde/std", "semver/std"]
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@

2 changes: 2 additions & 0 deletions tests/testsuite/cargo_remove/workspace/stderr.log
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Removing semver from build-dependencies
Updating `dummy-registry` index
Empty file.
30 changes: 30 additions & 0 deletions tests/testsuite/cargo_remove/workspace_non_virtual/in/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
[workspace]
members = [ "my-member" ]

[workspace.dependencies]
semver = "0.1.0"

[package]
name = "my-package"
version = "0.1.0"

[[bin]]
name = "main"
path = "src/main.rs"

[build-dependencies]
semver = { workspace = true }

[dependencies]
docopt = "0.6"
rustc-serialize = "0.4"
semver = "0.1"
toml = "0.1"
clippy = "0.4"

[dev-dependencies]
regex = "0.1.1"
serde = "1.0.90"

[features]
std = ["serde/std", "semver/std"]
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
[package]
name = "my-member"
version = "0.1.0"
edition = "2021"

[dependencies]
Empty file.
25 changes: 25 additions & 0 deletions tests/testsuite/cargo_remove/workspace_non_virtual/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
use cargo_test_support::compare::assert_ui;
use cargo_test_support::curr_dir;
use cargo_test_support::CargoCommand;
use cargo_test_support::Project;

use crate::cargo_remove::init_registry;

#[cargo_test]
fn case() {
init_registry();
let project = Project::from_template(curr_dir!().join("in"));
let project_root = project.root();
let cwd = &project_root;

snapbox::cmd::Command::cargo_ui()
.arg("remove")
.args(["--package", "my-package", "--build", "semver"])
.current_dir(cwd)
.assert()
.success()
.stdout_matches_path(curr_dir!().join("stdout.log"))
.stderr_matches_path(curr_dir!().join("stderr.log"));

assert_ui().subset_matches(curr_dir!().join("out"), &project_root);
}
24 changes: 24 additions & 0 deletions tests/testsuite/cargo_remove/workspace_non_virtual/out/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
[workspace]
members = [ "my-member" ]

[package]
name = "my-package"
version = "0.1.0"

[[bin]]
name = "main"
path = "src/main.rs"

[dependencies]
docopt = "0.6"
rustc-serialize = "0.4"
semver = "0.1"
toml = "0.1"
clippy = "0.4"

[dev-dependencies]
regex = "0.1.1"
serde = "1.0.90"

[features]
std = ["serde/std", "semver/std"]
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
[package]
name = "my-member"
version = "0.1.0"
edition = "2021"

[dependencies]
Empty file.
2 changes: 2 additions & 0 deletions tests/testsuite/cargo_remove/workspace_non_virtual/stderr.log
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Removing semver from build-dependencies
Updating `dummy-registry` index
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
[workspace]
members = [ "my-package", "my-other-package" ]

[workspace.dependencies]
semver = "0.1.0"
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
[package]
name = "my-other-package"
version = "0.1.0"

[[bin]]
name = "main"
path = "src/main.rs"

[dependencies]
docopt = "0.6"
rustc-serialize = "0.4"
semver = "0.1"
toml = "0.1"
clippy = "0.4"

[dev-dependencies]
regex = "0.1.1"
semver = { workspace = true }
serde = "1.0.90"

[features]
std = ["serde/std", "semver/std"]
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@

Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
[package]
name = "my-package"
version = "0.1.0"

[[bin]]
name = "main"
path = "src/main.rs"

[build-dependencies]
semver = { workspace = true }

[dependencies]
docopt = "0.6"
rustc-serialize = "0.4"
semver = "0.1"
toml = "0.1"
clippy = "0.4"

[dev-dependencies]
regex = "0.1.1"
serde = "1.0.90"

[features]
std = ["serde/std", "semver/std"]
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@

25 changes: 25 additions & 0 deletions tests/testsuite/cargo_remove/workspace_preserved/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
use cargo_test_support::compare::assert_ui;
use cargo_test_support::curr_dir;
use cargo_test_support::CargoCommand;
use cargo_test_support::Project;

use crate::cargo_remove::init_registry;

#[cargo_test]
fn case() {
init_registry();
let project = Project::from_template(curr_dir!().join("in"));
let project_root = project.root();
let cwd = &project_root;

snapbox::cmd::Command::cargo_ui()
.arg("remove")
.args(["--package", "my-package", "--build", "semver"])
.current_dir(cwd)
.assert()
.success()
.stdout_matches_path(curr_dir!().join("stdout.log"))
.stderr_matches_path(curr_dir!().join("stderr.log"));

assert_ui().subset_matches(curr_dir!().join("out"), &project_root);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
[workspace]
members = [ "my-package", "my-other-package" ]

[workspace.dependencies]
semver = "0.1.0"
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
[package]
name = "my-other-package"
version = "0.1.0"

[[bin]]
name = "main"
path = "src/main.rs"

[dependencies]
docopt = "0.6"
rustc-serialize = "0.4"
semver = "0.1"
toml = "0.1"
clippy = "0.4"

[dev-dependencies]
regex = "0.1.1"
semver = { workspace = true }
serde = "1.0.90"

[features]
std = ["serde/std", "semver/std"]
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@

Loading

0 comments on commit 65b2149

Please sign in to comment.