From 69828aa58a82ef9f896e961f3cfaaa4ade2ad8d3 Mon Sep 17 00:00:00 2001 From: Cassaundra Smith Date: Sat, 15 Oct 2022 15:31:27 -0700 Subject: [PATCH] Clean up workspace dependencies after cargo remove --- src/bin/cargo/commands/remove.rs | 1 + src/cargo/ops/cargo_remove.rs | 101 ++++++++++++++++++ tests/testsuite/cargo_remove/mod.rs | 2 + .../virtual_workspace/in/Cargo.toml | 5 + .../in/my-package/Cargo.toml | 24 +++++ .../in/my-package/src/lib.rs | 1 + .../cargo_remove/virtual_workspace/mod.rs | 25 +++++ .../virtual_workspace/out/Cargo.toml | 2 + .../out/my-package/Cargo.toml | 21 ++++ .../out/my-package/src/lib.rs | 1 + .../cargo_remove/virtual_workspace/stderr.log | 3 + .../cargo_remove/virtual_workspace/stdout.log | 0 .../cargo_remove/workspace/in/Cargo.toml | 30 ++++++ .../workspace/in/my-member/Cargo.toml | 6 ++ .../workspace/in/my-member/src/main.rs | 0 tests/testsuite/cargo_remove/workspace/mod.rs | 25 +++++ .../cargo_remove/workspace/out/Cargo.toml | 24 +++++ .../workspace/out/my-member/Cargo.toml | 6 ++ .../workspace/out/my-member/src/main.rs | 0 .../cargo_remove/workspace/stderr.log | 3 + .../cargo_remove/workspace/stdout.log | 0 21 files changed, 280 insertions(+) create mode 100644 tests/testsuite/cargo_remove/virtual_workspace/in/Cargo.toml create mode 100644 tests/testsuite/cargo_remove/virtual_workspace/in/my-package/Cargo.toml create mode 100644 tests/testsuite/cargo_remove/virtual_workspace/in/my-package/src/lib.rs create mode 100644 tests/testsuite/cargo_remove/virtual_workspace/mod.rs create mode 100644 tests/testsuite/cargo_remove/virtual_workspace/out/Cargo.toml create mode 100644 tests/testsuite/cargo_remove/virtual_workspace/out/my-package/Cargo.toml create mode 100644 tests/testsuite/cargo_remove/virtual_workspace/out/my-package/src/lib.rs create mode 100644 tests/testsuite/cargo_remove/virtual_workspace/stderr.log create mode 100644 tests/testsuite/cargo_remove/virtual_workspace/stdout.log create mode 100644 tests/testsuite/cargo_remove/workspace/in/Cargo.toml create mode 100644 tests/testsuite/cargo_remove/workspace/in/my-member/Cargo.toml create mode 100644 tests/testsuite/cargo_remove/workspace/in/my-member/src/main.rs create mode 100644 tests/testsuite/cargo_remove/workspace/mod.rs create mode 100644 tests/testsuite/cargo_remove/workspace/out/Cargo.toml create mode 100644 tests/testsuite/cargo_remove/workspace/out/my-member/Cargo.toml create mode 100644 tests/testsuite/cargo_remove/workspace/out/my-member/src/main.rs create mode 100644 tests/testsuite/cargo_remove/workspace/stderr.log create mode 100644 tests/testsuite/cargo_remove/workspace/stdout.log diff --git a/src/bin/cargo/commands/remove.rs b/src/bin/cargo/commands/remove.rs index 56d60c8b161e..89dd11fe9b26 100644 --- a/src/bin/cargo/commands/remove.rs +++ b/src/bin/cargo/commands/remove.rs @@ -77,6 +77,7 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult { let options = RemoveOptions { config, + workspace: &workspace, spec, dependencies, section, diff --git a/src/cargo/ops/cargo_remove.rs b/src/cargo/ops/cargo_remove.rs index 4866caeddace..f1a7f24115b2 100644 --- a/src/cargo/ops/cargo_remove.rs +++ b/src/cargo/ops/cargo_remove.rs @@ -1,6 +1,8 @@ //! Core of cargo-remove command +use crate::core::Dependency; use crate::core::Package; +use crate::core::Workspace; use crate::util::toml_mut::manifest::DepTable; use crate::util::toml_mut::manifest::LocalManifest; use crate::CargoResult; @@ -11,6 +13,8 @@ use crate::Config; pub struct RemoveOptions<'a> { /// Configuration information for Cargo operations pub config: &'a Config, + /// Workspace in which the operation is occurring. + pub workspace: &'a Workspace<'a>, /// Package to remove dependencies from pub spec: &'a Package, /// Dependencies to remove @@ -33,7 +37,18 @@ pub fn remove(options: &RemoveOptions<'_>) -> CargoResult<()> { let manifest_path = options.spec.manifest_path().to_path_buf(); let mut manifest = LocalManifest::try_new(&manifest_path)?; + let is_root = options.spec.manifest_path() == options.workspace.root_manifest(); + let mut parent_manifest = if !is_root { + let data = cargo_util::paths::read(options.workspace.root_manifest())?; + let manifest: toml_edit::Document = data.parse()?; + Some(manifest) + } else { + None + }; + for dep in &options.dependencies { + let is_workspace_dep = dependency_is_workspace(dep, &dep_table, &manifest); + let section = if dep_table.len() >= 3 { format!("{} for target `{}`", &dep_table[2], &dep_table[1]) } else { @@ -50,6 +65,23 @@ pub fn remove(options: &RemoveOptions<'_>) -> CargoResult<()> { // crate, then we need to drop any explicitly activated features on // that crate. manifest.gc_dep(dep); + + // If this was the last instance of a workspace dependency, remove it + // from the workspace dependencies. + if is_workspace_dep + && !dependency_in_workspace(dep, &options.section, options.spec, options.workspace) + { + let workspace_manifest = match &mut parent_manifest { + Some(parent_manifest) => parent_manifest, + None => &mut manifest.data, + }; + remove_dependency_from_workspace(dep, workspace_manifest); + + options + .config + .shell() + .status("Removing", format!("{dep} from workspace dependencies"))?; + } } if options.dry_run { @@ -59,7 +91,76 @@ pub fn remove(options: &RemoveOptions<'_>) -> CargoResult<()> { .warn("aborting remove due to dry run")?; } else { manifest.write()?; + + if let Some(parent_manifest) = parent_manifest { + cargo_util::paths::write( + options.workspace.root_manifest(), + parent_manifest.to_string().as_bytes(), + )?; + } } Ok(()) } + +/// Get whether or not a dependency is marked as `workspace`. +fn dependency_is_workspace(dep: &str, dep_table: &[String], manifest: &LocalManifest) -> bool { + if let Ok(toml_edit::Item::Table(table)) = manifest.get_table(dep_table) { + let value = table.get(dep).and_then(|i| i.get("workspace")); + if let Some(toml_edit::Item::Value(value)) = value { + return value.as_bool() == Some(true); + } + } + + false +} + +/// Get whether or not a dependency is depended upon in a workspace. +fn dependency_in_workspace( + dep: &str, + dep_table: &DepTable, + exclude: &Package, + workspace: &Workspace<'_>, +) -> bool { + workspace.members().any(|p| { + let manifest = LocalManifest::try_new(p.manifest_path()).unwrap(); // TODO handle error + p.dependencies() + .iter() + .filter(|d| d.package_name().as_str() == dep) + .map(|d| (d, dependency_to_table(d))) + // ignore the dep we could have just removed + .filter(|(_, t)| !(p == exclude && t == dep_table)) + // only dependencies marked workspace + .filter(|(d, t)| { + // information about workspace dependencies is not preserved at + // this stage, so we have to manually read the TOML file + let dep_table = t + .to_table() + .into_iter() + .map(String::from) + .collect::>(); + dependency_is_workspace(&d.package_name(), &dep_table, &manifest) + }) + .next() + .is_some() + }) +} + +fn dependency_to_table(dep: &Dependency) -> DepTable { + let mut dep_table = DepTable::new().set_kind(dep.kind()); + if let Some(target) = dep.platform().map(|p| format!("{p}")) { + dep_table = dep_table.set_target(target); + } + dep_table +} + +/// Remove a dependency from a virtual workspace. +fn remove_dependency_from_workspace(dep: &str, virtual_manifest: &mut toml_edit::Document) { + if let Some(toml_edit::Item::Table(table)) = virtual_manifest + .get_mut("workspace") + .and_then(|t| t.get_mut("dependencies")) + { + table.set_implicit(true); + table.remove(dep); + } +} diff --git a/tests/testsuite/cargo_remove/mod.rs b/tests/testsuite/cargo_remove/mod.rs index 75178531ee33..5bd1adfb7349 100644 --- a/tests/testsuite/cargo_remove/mod.rs +++ b/tests/testsuite/cargo_remove/mod.rs @@ -22,6 +22,8 @@ mod target; mod target_build; mod target_dev; mod update_lock_file; +mod virtual_workspace; +mod workspace; fn init_registry() { cargo_test_support::registry::init(); diff --git a/tests/testsuite/cargo_remove/virtual_workspace/in/Cargo.toml b/tests/testsuite/cargo_remove/virtual_workspace/in/Cargo.toml new file mode 100644 index 000000000000..fd5e80a8ba1f --- /dev/null +++ b/tests/testsuite/cargo_remove/virtual_workspace/in/Cargo.toml @@ -0,0 +1,5 @@ +[workspace] +members = [ "my-package" ] + +[workspace.dependencies] +semver = "0.1.0" diff --git a/tests/testsuite/cargo_remove/virtual_workspace/in/my-package/Cargo.toml b/tests/testsuite/cargo_remove/virtual_workspace/in/my-package/Cargo.toml new file mode 100644 index 000000000000..6690d593ba19 --- /dev/null +++ b/tests/testsuite/cargo_remove/virtual_workspace/in/my-package/Cargo.toml @@ -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"] diff --git a/tests/testsuite/cargo_remove/virtual_workspace/in/my-package/src/lib.rs b/tests/testsuite/cargo_remove/virtual_workspace/in/my-package/src/lib.rs new file mode 100644 index 000000000000..8b137891791f --- /dev/null +++ b/tests/testsuite/cargo_remove/virtual_workspace/in/my-package/src/lib.rs @@ -0,0 +1 @@ + diff --git a/tests/testsuite/cargo_remove/virtual_workspace/mod.rs b/tests/testsuite/cargo_remove/virtual_workspace/mod.rs new file mode 100644 index 000000000000..225fbec00e88 --- /dev/null +++ b/tests/testsuite/cargo_remove/virtual_workspace/mod.rs @@ -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); +} diff --git a/tests/testsuite/cargo_remove/virtual_workspace/out/Cargo.toml b/tests/testsuite/cargo_remove/virtual_workspace/out/Cargo.toml new file mode 100644 index 000000000000..83a6a04d0716 --- /dev/null +++ b/tests/testsuite/cargo_remove/virtual_workspace/out/Cargo.toml @@ -0,0 +1,2 @@ +[workspace] +members = [ "my-package" ] diff --git a/tests/testsuite/cargo_remove/virtual_workspace/out/my-package/Cargo.toml b/tests/testsuite/cargo_remove/virtual_workspace/out/my-package/Cargo.toml new file mode 100644 index 000000000000..402780535e2a --- /dev/null +++ b/tests/testsuite/cargo_remove/virtual_workspace/out/my-package/Cargo.toml @@ -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"] diff --git a/tests/testsuite/cargo_remove/virtual_workspace/out/my-package/src/lib.rs b/tests/testsuite/cargo_remove/virtual_workspace/out/my-package/src/lib.rs new file mode 100644 index 000000000000..8b137891791f --- /dev/null +++ b/tests/testsuite/cargo_remove/virtual_workspace/out/my-package/src/lib.rs @@ -0,0 +1 @@ + diff --git a/tests/testsuite/cargo_remove/virtual_workspace/stderr.log b/tests/testsuite/cargo_remove/virtual_workspace/stderr.log new file mode 100644 index 000000000000..5d237cb35495 --- /dev/null +++ b/tests/testsuite/cargo_remove/virtual_workspace/stderr.log @@ -0,0 +1,3 @@ + Removing semver from build-dependencies + Removing semver from workspace dependencies + Updating `dummy-registry` index diff --git a/tests/testsuite/cargo_remove/virtual_workspace/stdout.log b/tests/testsuite/cargo_remove/virtual_workspace/stdout.log new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/tests/testsuite/cargo_remove/workspace/in/Cargo.toml b/tests/testsuite/cargo_remove/workspace/in/Cargo.toml new file mode 100644 index 000000000000..dbac8ab442ea --- /dev/null +++ b/tests/testsuite/cargo_remove/workspace/in/Cargo.toml @@ -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"] diff --git a/tests/testsuite/cargo_remove/workspace/in/my-member/Cargo.toml b/tests/testsuite/cargo_remove/workspace/in/my-member/Cargo.toml new file mode 100644 index 000000000000..bb78904ffe6a --- /dev/null +++ b/tests/testsuite/cargo_remove/workspace/in/my-member/Cargo.toml @@ -0,0 +1,6 @@ +[package] +name = "my-member" +version = "0.1.0" +edition = "2021" + +[dependencies] diff --git a/tests/testsuite/cargo_remove/workspace/in/my-member/src/main.rs b/tests/testsuite/cargo_remove/workspace/in/my-member/src/main.rs new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/tests/testsuite/cargo_remove/workspace/mod.rs b/tests/testsuite/cargo_remove/workspace/mod.rs new file mode 100644 index 000000000000..225fbec00e88 --- /dev/null +++ b/tests/testsuite/cargo_remove/workspace/mod.rs @@ -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); +} diff --git a/tests/testsuite/cargo_remove/workspace/out/Cargo.toml b/tests/testsuite/cargo_remove/workspace/out/Cargo.toml new file mode 100644 index 000000000000..9a3261484d12 --- /dev/null +++ b/tests/testsuite/cargo_remove/workspace/out/Cargo.toml @@ -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"] diff --git a/tests/testsuite/cargo_remove/workspace/out/my-member/Cargo.toml b/tests/testsuite/cargo_remove/workspace/out/my-member/Cargo.toml new file mode 100644 index 000000000000..bb78904ffe6a --- /dev/null +++ b/tests/testsuite/cargo_remove/workspace/out/my-member/Cargo.toml @@ -0,0 +1,6 @@ +[package] +name = "my-member" +version = "0.1.0" +edition = "2021" + +[dependencies] diff --git a/tests/testsuite/cargo_remove/workspace/out/my-member/src/main.rs b/tests/testsuite/cargo_remove/workspace/out/my-member/src/main.rs new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/tests/testsuite/cargo_remove/workspace/stderr.log b/tests/testsuite/cargo_remove/workspace/stderr.log new file mode 100644 index 000000000000..5d237cb35495 --- /dev/null +++ b/tests/testsuite/cargo_remove/workspace/stderr.log @@ -0,0 +1,3 @@ + Removing semver from build-dependencies + Removing semver from workspace dependencies + Updating `dummy-registry` index diff --git a/tests/testsuite/cargo_remove/workspace/stdout.log b/tests/testsuite/cargo_remove/workspace/stdout.log new file mode 100644 index 000000000000..e69de29bb2d1