Skip to content

Commit

Permalink
Clean profile, patch, and replace in cargo remove
Browse files Browse the repository at this point in the history
After a successful removal of a dependency, clean up the profile, patch, and
replace sections to remove all references to the dependency.
  • Loading branch information
cassaundra committed Nov 10, 2022
1 parent 65b2149 commit b292a75
Show file tree
Hide file tree
Showing 24 changed files with 331 additions and 37 deletions.
124 changes: 90 additions & 34 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::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::toml_mut::dependency::Dependency;
use cargo::util::toml_mut::dependency::Source;
use cargo::util::toml_mut::manifest::DepTable;
use cargo::util::toml_mut::manifest::LocalManifest;
use cargo::CargoResult;
Expand Down Expand Up @@ -74,22 +77,22 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
.get_many::<String>("dependencies")
.expect("required(true)")
.cloned()
.collect();
.collect::<Vec<_>>();

let section = parse_section(args);

let options = RemoveOptions {
config,
spec,
dependencies,
dependencies: &dependencies,
section,
dry_run,
};
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)?;
Expand Down Expand Up @@ -121,8 +124,9 @@ 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()?;

Expand All @@ -131,39 +135,91 @@ fn gc_workspace(workspace: &Workspace<'_>, dependencies: &[String]) -> CargoResu
.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)
let 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::<Vec<_>>()
})
})
})
}
.collect::<CargoResult<Vec<_>>>()?;

/// 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
// clean up workspace.dependencies
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() {
if !dependencies.iter().any(|d| {
d.toml_key() == key.get() && matches!(d.source(), Some(Source::Workspace(_)))
}) {
*item = toml_edit::Item::None;
}
}
}

// clean up the profile section
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 let Ok(spec) = PackageIdSpec::parse(key.get()) {
if !dependencies.iter().any(|d| d.name == spec.name().as_str()) {
*item = toml_edit::Item::None;
}
}
}
}
}
}
}

// clean up patch section
if let Some(toml_edit::Item::Table(patch_section_table)) = manifest.get_mut("patch") {
patch_section_table.set_implicit(true);
for (_, 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() {
if !dependencies.iter().any(|d| d.name == key.get()) {
*item = toml_edit::Item::None;
}
}
}
}
}

// clean up replace section
if let Some(toml_edit::Item::Table(table)) = manifest.get_mut("replace") {
table.set_implicit(true);
table.remove(dep);
for (key, item) in table.iter_mut() {
if let Ok(spec) = PackageIdSpec::parse(key.get()) {
if !dependencies.iter().any(|d| {
d.name == spec.name().as_str()
&& d.version()
.and_then(|v| semver::VersionReq::parse(v).ok())
.and_then(|vq| spec.version().map(|v| vq.matches(v)))
.unwrap_or(true)
}) {
*item = toml_edit::Item::None;
}
}
}
}

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

Ok(())
}
4 changes: 2 additions & 2 deletions src/cargo/ops/cargo_remove.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ pub struct RemoveOptions<'a> {
/// Package to remove dependencies from
pub spec: &'a Package,
/// Dependencies to remove
pub dependencies: Vec<String>,
pub dependencies: &'a [String],
/// Which dependency section to remove these from
pub section: DepTable,
/// Whether or not to actually write the manifest
Expand All @@ -33,7 +33,7 @@ pub fn remove(options: &RemoveOptions<'_>) -> CargoResult<()> {
let manifest_path = options.spec.manifest_path().to_path_buf();
let mut manifest = LocalManifest::try_new(&manifest_path)?;

for dep in &options.dependencies {
for dep in options.dependencies {
let section = if dep_table.len() >= 3 {
format!("{} for target `{}`", &dep_table[2], &dep_table[1])
} else {
Expand Down
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 @@ -500,7 +500,7 @@ fn fix_feature_activations(
})
.collect();

// Remove found idx in revers order so we don't invalidate the idx.
// Remove found idx in reverse order so we don't invalidate the idx.
for idx in remove_list.iter().rev() {
feature_values.remove(*idx);
}
Expand Down
56 changes: 56 additions & 0 deletions tests/testsuite/cargo_remove/gc_patch/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
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!(
"[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",
),
)
.file("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());
}
3 changes: 3 additions & 0 deletions tests/testsuite/cargo_remove/gc_patch/out/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
[package]
name = "my-project"
version = "0.1.0"
Empty file.
1 change: 1 addition & 0 deletions tests/testsuite/cargo_remove/gc_patch/stderr.log
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Removing bar from dependencies
Empty file.
35 changes: 35 additions & 0 deletions tests/testsuite/cargo_remove/gc_profile/in/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
[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"
docopt = "0.6"

[features]
std = ["serde/std", "semver/std"]

[profile.dev.package.docopt]
opt-level = 3

[profile.dev.package.toml]
opt-level = 3

[profile.release.package.toml]
opt-level = 1
overflow-checks = false
1 change: 1 addition & 0 deletions tests/testsuite/cargo_remove/gc_profile/in/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@

25 changes: 25 additions & 0 deletions tests/testsuite/cargo_remove/gc_profile/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(["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);
}
27 changes: 27 additions & 0 deletions tests/testsuite/cargo_remove/gc_profile/out/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
[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"
docopt = "0.6"

[features]
std = ["serde/std", "semver/std"]

[profile.dev.package.docopt]
opt-level = 3
2 changes: 2 additions & 0 deletions tests/testsuite/cargo_remove/gc_profile/stderr.log
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Removing toml from dependencies
Updating `dummy-registry` index
Empty file.
5 changes: 5 additions & 0 deletions tests/testsuite/cargo_remove/gc_replace/in/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
[workspace]
members = [ "my-package" ]

[replace]
"toml:0.1.0" = { path = "../toml" }
25 changes: 25 additions & 0 deletions tests/testsuite/cargo_remove/gc_replace/in/my-package/Cargo.toml
Original file line number Diff line number Diff line change
@@ -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"
toml = "0.1"
clippy = "0.4"

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

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

Loading

0 comments on commit b292a75

Please sign in to comment.