diff --git a/src/bin/cargo/commands/remove.rs b/src/bin/cargo/commands/remove.rs index 66be50024ad6..fc09ec480491 100644 --- a/src/bin/cargo/commands/remove.rs +++ b/src/bin/cargo/commands/remove.rs @@ -1,10 +1,14 @@ use cargo::core::dependency::DepKind; +use cargo::core::PackageIdSpec; 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::print_available_packages; +use cargo::util::toml_mut::dependency::Dependency; +use cargo::util::toml_mut::dependency::MaybeWorkspace; +use cargo::util::toml_mut::dependency::Source; use cargo::util::toml_mut::manifest::DepTable; use cargo::util::toml_mut::manifest::LocalManifest; use cargo::CargoResult; @@ -86,7 +90,7 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult { .get_many::("dependencies") .expect("required(true)") .cloned() - .collect(); + .collect::>(); let section = parse_section(args); @@ -100,8 +104,8 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult { remove(&options)?; if !dry_run { - // Clean up workspace dependencies - gc_workspace(&workspace, &options.dependencies)?; + // Clean up the workspace + gc_workspace(&workspace)?; // Reload the workspace since we've changed dependencies let ws = args.workspace(config)?; @@ -133,49 +137,203 @@ 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<()> { +/// Clean up the workspace.dependencies, profile, patch, and replace sections of the root manifest +/// by removing dependencies which no longer have a reference to them. +fn gc_workspace(workspace: &Workspace<'_>) -> CargoResult<()> { let mut manifest: toml_edit::Document = cargo_util::paths::read(workspace.root_manifest())?.parse()?; + let mut is_modified = true; let members = workspace .members() .map(|p| LocalManifest::try_new(p.manifest_path())) .collect::>>()?; - for dep in dependencies { - if !dep_in_workspace(dep, &members) { - remove_workspace_dep(dep, &mut manifest); + let mut dependencies = members + .iter() + .flat_map(|manifest| { + manifest.get_sections().into_iter().flat_map(|(_, table)| { + table + .as_table_like() + .unwrap() + .iter() + .map(|(key, item)| Dependency::from_toml(&manifest.path, key, item)) + .collect::>() + }) + }) + .collect::>>()?; + + // Clean up the workspace.dependencies section and replace instances of + // workspace dependencies with their definitions + if let Some(toml_edit::Item::Table(deps_table)) = manifest + .get_mut("workspace") + .and_then(|t| t.get_mut("dependencies")) + { + deps_table.set_implicit(true); + for (key, item) in deps_table.iter_mut() { + let ws_dep = Dependency::from_toml(&workspace.root(), key.get(), item)?; + + // search for uses of this workspace dependency + let mut is_used = false; + for dep in dependencies.iter_mut().filter(|d| { + d.toml_key() == key.get() && matches!(d.source(), Some(Source::Workspace(_))) + }) { + // replace the reference with the definition + *dep = ws_dep.clone(); + + is_used = true; + } + + if !is_used { + *item = toml_edit::Item::None; + is_modified = true; + } } } - cargo_util::paths::write(workspace.root_manifest(), manifest.to_string().as_bytes())?; + // Clean up the profile section + // + // Example tables: + // - profile.dev.package.foo + // - profile.release.package."*" + // - profile.release.package."foo:2.1.0" + if let Some(toml_edit::Item::Table(profile_section_table)) = manifest.get_mut("profile") { + profile_section_table.set_implicit(true); + + for (_, item) in profile_section_table.iter_mut() { + if let toml_edit::Item::Table(profile_table) = item { + profile_table.set_implicit(true); + + if let Some(toml_edit::Item::Table(package_table)) = + profile_table.get_mut("package") + { + package_table.set_implicit(true); + + for (key, item) in package_table.iter_mut() { + if !spec_has_match( + &PackageIdSpec::parse(key.get())?, + &dependencies, + workspace.config(), + )? { + *item = toml_edit::Item::None; + is_modified = true; + } + } + } + } + } + } + + // Clean up the patch section + if let Some(toml_edit::Item::Table(patch_section_table)) = manifest.get_mut("patch") { + patch_section_table.set_implicit(true); + + // The key in each of the subtables is a source (either a registry or a URL) + for (source, item) in patch_section_table.iter_mut() { + if let toml_edit::Item::Table(patch_table) = item { + patch_table.set_implicit(true); + + for (key, item) in patch_table.iter_mut() { + let package_name = + Dependency::from_toml(&workspace.root_manifest(), key.get(), item)?.name; + if !source_has_match( + &package_name, + source.get(), + &dependencies, + workspace.config(), + )? { + *item = toml_edit::Item::None; + } + } + } + } + } + + // Clean up the replace section + if let Some(toml_edit::Item::Table(table)) = manifest.get_mut("replace") { + table.set_implicit(true); + + for (key, item) in table.iter_mut() { + if !spec_has_match( + &PackageIdSpec::parse(key.get())?, + &dependencies, + workspace.config(), + )? { + *item = toml_edit::Item::None; + is_modified = true; + } + } + } + + if is_modified { + 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) - }) - }) +/// Check whether or not a package ID spec matches any non-workspace dependencies. +fn spec_has_match( + spec: &PackageIdSpec, + dependencies: &[Dependency], + config: &Config, +) -> CargoResult { + for dep in dependencies { + if spec.name().as_str() != &dep.name { + continue; + } + + let version_matches = match (spec.version(), dep.version()) { + (Some(v), Some(vq)) => semver::VersionReq::parse(vq)?.matches(v), + (Some(_), None) => false, + (None, None | Some(_)) => true, + }; + if !version_matches { + continue; + } + + match dep.source_id(config)? { + MaybeWorkspace::Other(source_id) => { + if spec.url().map(|u| u == source_id.url()).unwrap_or(true) { + return Ok(true); + } + } + MaybeWorkspace::Workspace(_) => {} + } + } + + Ok(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); +/// Check whether or not a source (URL or registry name) matches any non-workspace dependencies. +fn source_has_match( + name: &str, + source: &str, + dependencies: &[Dependency], + config: &Config, +) -> CargoResult { + for dep in dependencies { + if &dep.name != name { + continue; + } + + match dep.source_id(config)? { + MaybeWorkspace::Other(source_id) => { + if source_id.is_registry() { + if source_id.display_registry_name() == source + || source_id.url().as_str() == source + { + return Ok(true); + } + } else if source_id.is_git() { + if source_id.url().as_str() == source { + return Ok(true); + } + } + } + MaybeWorkspace::Workspace(_) => {} + } } + + Ok(false) } diff --git a/tests/testsuite/cargo_remove/gc_patch/mod.rs b/tests/testsuite/cargo_remove/gc_patch/mod.rs new file mode 100644 index 000000000000..2c1d592fb338 --- /dev/null +++ b/tests/testsuite/cargo_remove/gc_patch/mod.rs @@ -0,0 +1,72 @@ +use cargo_test_support::basic_manifest; +use cargo_test_support::compare::assert_ui; +use cargo_test_support::curr_dir; +use cargo_test_support::git; +use cargo_test_support::project; +use cargo_test_support::CargoCommand; + +use crate::cargo_remove::init_registry; + +#[cargo_test] +fn case() { + init_registry(); + + let git_project1 = git::new("bar1", |project| { + project + .file("Cargo.toml", &basic_manifest("bar", "0.1.0")) + .file("src/lib.rs", "") + }) + .url(); + + let git_project2 = git::new("bar2", |project| { + project + .file("Cargo.toml", &basic_manifest("bar", "0.1.0")) + .file("src/lib.rs", "") + }) + .url(); + + let in_project = project() + .file( + "Cargo.toml", + &format!( + "[workspace]\n\ + members = [ \"my-member\" ]\n\ + \n\ + [package]\n\ + name = \"my-project\"\n\ + version = \"0.1.0\"\n\ + \n\ + [dependencies]\n\ + bar = {{ git = \"{git_project1}\" }}\n\ + \n\ + [patch.\"{git_project1}\"]\n\ + bar = {{ git = \"{git_project2}\" }}\n\ + \n\ + [patch.crates-io]\n\ + bar = {{ git = \"{git_project2}\" }}\n", + ), + ) + .file("src/lib.rs", "") + .file( + "my-member/Cargo.toml", + "[package]\n\ + name = \"my-member\"\n\ + version = \"0.1.0\"\n\ + \n\ + [dependencies]\n\ + bar = \"0.1.0\"\n", + ) + .file("my-member/src/lib.rs", "") + .build(); + + snapbox::cmd::Command::cargo_ui() + .arg("remove") + .args(["bar"]) + .current_dir(&in_project.root()) + .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"), &in_project.root()); +} diff --git a/tests/testsuite/cargo_remove/gc_patch/out/Cargo.toml b/tests/testsuite/cargo_remove/gc_patch/out/Cargo.toml new file mode 100644 index 000000000000..2d8c22115236 --- /dev/null +++ b/tests/testsuite/cargo_remove/gc_patch/out/Cargo.toml @@ -0,0 +1,9 @@ +[workspace] +members = [ "my-member" ] + +[package] +name = "my-project" +version = "0.1.0" + +[patch.crates-io] +bar = { git = "[ROOTURL]/bar2" } diff --git a/tests/testsuite/cargo_remove/gc_patch/out/src/lib.rs b/tests/testsuite/cargo_remove/gc_patch/out/src/lib.rs new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/tests/testsuite/cargo_remove/gc_patch/stderr.log b/tests/testsuite/cargo_remove/gc_patch/stderr.log new file mode 100644 index 000000000000..1dd2e775712d --- /dev/null +++ b/tests/testsuite/cargo_remove/gc_patch/stderr.log @@ -0,0 +1,3 @@ + Removing bar from dependencies + Updating git repository `[ROOTURL]/bar2` + Updating `dummy-registry` index diff --git a/tests/testsuite/cargo_remove/gc_patch/stdout.log b/tests/testsuite/cargo_remove/gc_patch/stdout.log new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/tests/testsuite/cargo_remove/gc_profile/in/Cargo.toml b/tests/testsuite/cargo_remove/gc_profile/in/Cargo.toml new file mode 100644 index 000000000000..d781ad5a5f26 --- /dev/null +++ b/tests/testsuite/cargo_remove/gc_profile/in/Cargo.toml @@ -0,0 +1,36 @@ +[package] +name = "cargo-remove-test-fixture" +version = "0.1.0" + +[[bin]] +name = "main" +path = "src/main.rs" + +[build-dependencies] +semver = "0.1.0" + +[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" +toml = "0.2.3" +docopt = "0.6" + +[features] +std = ["serde/std", "semver/std"] + +[profile.dev.package.docopt] +opt-level = 3 + +[profile.dev.package."toml@0.1.0"] +opt-level = 3 + +[profile.release.package.toml] +opt-level = 1 +overflow-checks = false diff --git a/tests/testsuite/cargo_remove/gc_profile/in/src/lib.rs b/tests/testsuite/cargo_remove/gc_profile/in/src/lib.rs new file mode 100644 index 000000000000..8b137891791f --- /dev/null +++ b/tests/testsuite/cargo_remove/gc_profile/in/src/lib.rs @@ -0,0 +1 @@ + diff --git a/tests/testsuite/cargo_remove/gc_profile/mod.rs b/tests/testsuite/cargo_remove/gc_profile/mod.rs new file mode 100644 index 000000000000..7047c92e2f38 --- /dev/null +++ b/tests/testsuite/cargo_remove/gc_profile/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(["toml"]) + .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/gc_profile/out/Cargo.toml b/tests/testsuite/cargo_remove/gc_profile/out/Cargo.toml new file mode 100644 index 000000000000..21b43fe68263 --- /dev/null +++ b/tests/testsuite/cargo_remove/gc_profile/out/Cargo.toml @@ -0,0 +1,32 @@ +[package] +name = "cargo-remove-test-fixture" +version = "0.1.0" + +[[bin]] +name = "main" +path = "src/main.rs" + +[build-dependencies] +semver = "0.1.0" + +[dependencies] +docopt = "0.6" +rustc-serialize = "0.4" +semver = "0.1" +clippy = "0.4" + +[dev-dependencies] +regex = "0.1.1" +serde = "1.0.90" +toml = "0.2.3" +docopt = "0.6" + +[features] +std = ["serde/std", "semver/std"] + +[profile.dev.package.docopt] +opt-level = 3 + +[profile.release.package.toml] +opt-level = 1 +overflow-checks = false diff --git a/tests/testsuite/cargo_remove/gc_profile/stderr.log b/tests/testsuite/cargo_remove/gc_profile/stderr.log new file mode 100644 index 000000000000..0e2e38f266e0 --- /dev/null +++ b/tests/testsuite/cargo_remove/gc_profile/stderr.log @@ -0,0 +1,2 @@ + Removing toml from dependencies + Updating `dummy-registry` index diff --git a/tests/testsuite/cargo_remove/gc_profile/stdout.log b/tests/testsuite/cargo_remove/gc_profile/stdout.log new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/tests/testsuite/cargo_remove/gc_replace/in/Cargo.toml b/tests/testsuite/cargo_remove/gc_replace/in/Cargo.toml new file mode 100644 index 000000000000..48242c2d381e --- /dev/null +++ b/tests/testsuite/cargo_remove/gc_replace/in/Cargo.toml @@ -0,0 +1,5 @@ +[workspace] +members = [ "my-package" ] + +[replace] +"toml:0.1.0" = { path = "../toml" } diff --git a/tests/testsuite/cargo_remove/gc_replace/in/my-package/Cargo.toml b/tests/testsuite/cargo_remove/gc_replace/in/my-package/Cargo.toml new file mode 100644 index 000000000000..bee343a8b15c --- /dev/null +++ b/tests/testsuite/cargo_remove/gc_replace/in/my-package/Cargo.toml @@ -0,0 +1,26 @@ +[package] +name = "my-package" +version = "0.1.0" + +[[bin]] +name = "main" +path = "src/main.rs" + +[build-dependencies] +semver = "0.1.0" + +[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" +toml = "0.2.3" +docopt = "0.6" + +[features] +std = ["serde/std", "semver/std"] diff --git a/tests/testsuite/cargo_remove/gc_replace/in/my-package/src/main.rs b/tests/testsuite/cargo_remove/gc_replace/in/my-package/src/main.rs new file mode 100644 index 000000000000..8b137891791f --- /dev/null +++ b/tests/testsuite/cargo_remove/gc_replace/in/my-package/src/main.rs @@ -0,0 +1 @@ + diff --git a/tests/testsuite/cargo_remove/gc_replace/mod.rs b/tests/testsuite/cargo_remove/gc_replace/mod.rs new file mode 100644 index 000000000000..717adef3e56d --- /dev/null +++ b/tests/testsuite/cargo_remove/gc_replace/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", "toml"]) + .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/gc_replace/out/Cargo.toml b/tests/testsuite/cargo_remove/gc_replace/out/Cargo.toml new file mode 100644 index 000000000000..83a6a04d0716 --- /dev/null +++ b/tests/testsuite/cargo_remove/gc_replace/out/Cargo.toml @@ -0,0 +1,2 @@ +[workspace] +members = [ "my-package" ] diff --git a/tests/testsuite/cargo_remove/gc_replace/out/my-package/Cargo.toml b/tests/testsuite/cargo_remove/gc_replace/out/my-package/Cargo.toml new file mode 100644 index 000000000000..36ddf7a04997 --- /dev/null +++ b/tests/testsuite/cargo_remove/gc_replace/out/my-package/Cargo.toml @@ -0,0 +1,25 @@ +[package] +name = "my-package" +version = "0.1.0" + +[[bin]] +name = "main" +path = "src/main.rs" + +[build-dependencies] +semver = "0.1.0" + +[dependencies] +docopt = "0.6" +rustc-serialize = "0.4" +semver = "0.1" +clippy = "0.4" + +[dev-dependencies] +regex = "0.1.1" +serde = "1.0.90" +toml = "0.2.3" +docopt = "0.6" + +[features] +std = ["serde/std", "semver/std"] diff --git a/tests/testsuite/cargo_remove/gc_replace/out/my-package/src/main.rs b/tests/testsuite/cargo_remove/gc_replace/out/my-package/src/main.rs new file mode 100644 index 000000000000..8b137891791f --- /dev/null +++ b/tests/testsuite/cargo_remove/gc_replace/out/my-package/src/main.rs @@ -0,0 +1 @@ + diff --git a/tests/testsuite/cargo_remove/gc_replace/stderr.log b/tests/testsuite/cargo_remove/gc_replace/stderr.log new file mode 100644 index 000000000000..0e2e38f266e0 --- /dev/null +++ b/tests/testsuite/cargo_remove/gc_replace/stderr.log @@ -0,0 +1,2 @@ + Removing toml from dependencies + Updating `dummy-registry` index diff --git a/tests/testsuite/cargo_remove/gc_replace/stdout.log b/tests/testsuite/cargo_remove/gc_replace/stdout.log new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/tests/testsuite/cargo_remove/mod.rs b/tests/testsuite/cargo_remove/mod.rs index 4aa2dbe585b6..fd8b4a233194 100644 --- a/tests/testsuite/cargo_remove/mod.rs +++ b/tests/testsuite/cargo_remove/mod.rs @@ -2,6 +2,9 @@ mod avoid_empty_tables; mod build; mod dev; mod dry_run; +mod gc_patch; +mod gc_profile; +mod gc_replace; mod invalid_arg; mod invalid_dep; mod invalid_package;