Skip to content

Commit

Permalink
fix: add gc unused patches logic and fix tests
Browse files Browse the repository at this point in the history
  • Loading branch information
alcolmenar committed Aug 9, 2023
1 parent 35d968b commit 87d9c47
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 50 deletions.
92 changes: 42 additions & 50 deletions src/bin/cargo/commands/remove.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use cargo::core::dependency::DepKind;
use cargo::core::PackageIdSpec;
use cargo::core::Resolve;
use cargo::core::Workspace;
use cargo::ops::cargo_remove::remove;
use cargo::ops::cargo_remove::RemoveOptions;
Expand Down Expand Up @@ -109,9 +110,14 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {

// Reload the workspace since we've changed dependencies
let ws = args.workspace(config)?;
resolve_ws(&ws)?;
}
let (_, resolve) = resolve_ws(&ws)?;

// Attempt to gc unused patches and re-resolve if anything is removed
if gc_unused_patches(&workspace, &resolve)? {
let ws = args.workspace(config)?;
resolve_ws(&ws)?;
}
}
Ok(())
}

Expand Down Expand Up @@ -229,31 +235,6 @@ fn gc_workspace(workspace: &Workspace<'_>) -> CargoResult<()> {
}
}

// 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);
Expand Down Expand Up @@ -310,35 +291,46 @@ fn spec_has_match(
Ok(false)
}

/// 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<bool> {
for dep in dependencies {
if &dep.name != name {
continue;
}
/// Removes unused patches from the manifest
fn gc_unused_patches(workspace: &Workspace<'_>, resolve: &Resolve) -> CargoResult<bool> {
let mut manifest: toml_edit::Document =
cargo_util::paths::read(workspace.root_manifest())?.parse()?;
let mut modified = false;

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

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() {
let dep = Dependency::from_toml(&workspace.root_manifest(), key.get(), item)?;

// Generate a PackageIdSpec url for querying
let url = if let MaybeWorkspace::Other(source_id) =
dep.source_id(workspace.config())?
{
return Ok(true);
}
} else if source_id.is_git() {
if source_id.url().as_str() == source {
return Ok(true);
format!("{}#{}", source_id.url(), dep.name)
} else {
continue;
};

if PackageIdSpec::query_str(&url, resolve.unused_patches().iter().cloned())
.is_ok()
{
*item = toml_edit::Item::None;
modified = true;
}
}
}
MaybeWorkspace::Workspace(_) => {}
}
}

Ok(false)
if modified {
cargo_util::paths::write(workspace.root_manifest(), manifest.to_string().as_bytes())?;
}

Ok(modified)
}
5 changes: 5 additions & 0 deletions tests/testsuite/cargo_remove/gc_patch/stderr.log
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
Removing bar from dependencies
Updating git repository `[ROOTURL]/bar[..]`
Updating git repository `[ROOTURL]/bar[..]`
Updating `dummy-registry` index
warning: Patch `bar v0.1.0 ([..])` was not used in the crate graph.
Perhaps you misspelled the source URL being patched.
Possible URLs for `[patch.<URL>]`:
[ROOT]/bar2
3 changes: 3 additions & 0 deletions tests/testsuite/cargo_remove/invalid_gc_patch/out/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,6 @@
[workspace]
members = ["serde", "serde_derive"]

[patch.crates-io]
serde = { path = "serde" }

0 comments on commit 87d9c47

Please sign in to comment.