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 15, 2022
1 parent 16b0978 commit c05bd27
Show file tree
Hide file tree
Showing 23 changed files with 444 additions and 30 deletions.
202 changes: 173 additions & 29 deletions src/bin/cargo/commands/remove.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -86,7 +90,7 @@ 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);

Expand All @@ -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)?;
Expand Down Expand Up @@ -133,8 +137,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 @@ -143,9 +148,104 @@ 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);
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<_>>>()?;

// Clean up the workspace.dependencies section
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
//
// 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;
}
}
}
}
}
}

// 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;
}
}
}

Expand All @@ -154,28 +254,72 @@ fn gc_workspace(workspace: &Workspace<'_>, dependencies: &[String]) -> CargoResu
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 dependencies.
fn spec_has_match(
spec: &PackageIdSpec,
dependencies: &[Dependency],
config: &Config,
) -> CargoResult<bool> {
for dep in dependencies {
match dep.source_id(config)? {
MaybeWorkspace::Other(source_id) => {
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, _) => true,
};
if !version_matches {
continue;
}

let source_matches = match spec.url() {
Some(u) => u == source_id.url(),
None => true,
};
if source_matches {
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 dependencies.
fn source_has_match(
name: &str,
source: &str,
dependencies: &[Dependency],
config: &Config,
) -> CargoResult<bool> {
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)
}
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
72 changes: 72 additions & 0 deletions tests/testsuite/cargo_remove/gc_patch/mod.rs
Original file line number Diff line number Diff line change
@@ -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());
}
9 changes: 9 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,9 @@
[workspace]
members = [ "my-member" ]

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

[patch.crates-io]
bar = { git = "[ROOTURL]/bar2" }
Empty file.
3 changes: 3 additions & 0 deletions tests/testsuite/cargo_remove/gc_patch/stderr.log
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Removing bar from dependencies
Updating git repository `[ROOTURL]/bar2`
Updating `dummy-registry` index
Empty file.
36 changes: 36 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,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
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 @@

Loading

0 comments on commit c05bd27

Please sign in to comment.