From 130238ca32994748275f15650757d1a39d017e97 Mon Sep 17 00:00:00 2001 From: Raushan kumar Date: Mon, 11 May 2026 05:14:03 +0000 Subject: [PATCH 1/2] test(resolve): add baseline test for unused patch with same dependency --- tests/testsuite/patch.rs | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/tests/testsuite/patch.rs b/tests/testsuite/patch.rs index bd14ef67dd6..71815392142 100644 --- a/tests/testsuite/patch.rs +++ b/tests/testsuite/patch.rs @@ -3430,3 +3430,43 @@ foo v0.5.0 ([ROOT]/foo/foo) "#]]) .run(); } + +#[cargo_test] +fn unused_patch_same_dependency_different_sources() { + Package::new("bar", "0.1.0").publish(); + + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.0.1" + edition = "2015" + + [dependencies] + bar = "0.1.0" + + [patch.crates-io] + bar = { path = "my-bar" } + + [patch.'https://github.com/example/bar'] + bar = { path = "my-bar" } + "#, + ) + .file("src/lib.rs", "") + .file("my-bar/Cargo.toml", &basic_manifest("bar", "0.1.0")) + .file("my-bar/src/lib.rs", "") + .build(); + + p.cargo("check") + .with_stderr_data(str![[r#" +[UPDATING] `dummy-registry` index +[LOCKING] 1 package to latest compatible version +[CHECKING] bar v0.1.0 ([ROOT]/foo/my-bar) +[CHECKING] foo v0.0.1 ([ROOT]/foo) +[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s + +"#]]) + .run(); +} From 43e378c9789d65bb763e75cc6df65c066dbc12fe Mon Sep 17 00:00:00 2001 From: Raushan kumar Date: Mon, 11 May 2026 06:17:17 +0000 Subject: [PATCH 2/2] fix(resolve): detect unused patches when multiple patches share a PackageId Previously, Cargo tracked used patches solely by PackageId. If multiple sources patched the same dependency, unused patches were incorrectly marked as used. This updates the logic to track usage by both source URL and PackageId in-memory, ensuring accurate warnings while keeping Cargo.lock backward-compatible. Fixes #12471 --- src/cargo/core/resolver/resolve.rs | 24 ++++++++--- src/cargo/ops/resolve.rs | 67 +++++++++++++++--------------- tests/testsuite/patch.rs | 4 ++ 3 files changed, 57 insertions(+), 38 deletions(-) diff --git a/src/cargo/core/resolver/resolve.rs b/src/cargo/core/resolver/resolve.rs index fb472d99d2b..43931f38fe2 100644 --- a/src/cargo/core/resolver/resolve.rs +++ b/src/cargo/core/resolver/resolve.rs @@ -3,6 +3,7 @@ use cargo_util_schemas::manifest::RustVersion; use crate::core::dependency::DepKind; use crate::core::{Dependency, PackageId, PackageIdSpec, PackageIdSpecQuery, Summary, Target}; +use crate::util::CanonicalUrl; use crate::util::Graph; use crate::util::errors::CargoResult; use crate::util::interning::InternedString; @@ -201,11 +202,24 @@ impl Resolve { self.graph.path_to_top(pkg) } - pub fn register_used_patches<'a>(&mut self, patches: impl Iterator) { - for summary in patches { - if !self.graph.contains(&summary.package_id()) { - self.unused_patches.push(summary.package_id()) - }; + pub fn register_used_patches(&mut self, patches: &HashMap>) { + let mut used_patches = HashSet::new(); + for pkg in self.iter() { + for (dep_pkg_id, deps) in self.deps_not_replaced(pkg) { + for dep in deps { + used_patches.insert((dep.source_id().canonical_url().clone(), dep_pkg_id)); + } + } + } + + for (url, summaries) in patches { + for summary in summaries { + if !used_patches.contains(&(url.clone(), summary.package_id())) + && !self.unused_patches.contains(&summary.package_id()) + { + self.unused_patches.push(summary.package_id()); + } + } } } diff --git a/src/cargo/ops/resolve.rs b/src/cargo/ops/resolve.rs index 722cefe5713..b38138db750 100644 --- a/src/cargo/ops/resolve.rs +++ b/src/cargo/ops/resolve.rs @@ -511,8 +511,7 @@ pub fn resolve_with_previous<'gctx>( Some(ws.gctx()), )?; - let patches = registry.patches().values().flat_map(|v| v.iter()); - resolved.register_used_patches(patches); + resolved.register_used_patches(registry.patches()); if register_patches && !resolved.unused_patches().is_empty() { emit_warnings_of_unused_patches(ws, &resolved, registry)?; @@ -818,14 +817,12 @@ fn emit_warnings_of_unused_patches( ) -> CargoResult<()> { const MESSAGE: &str = "was not used in the crate graph"; - // Patch package with the source URLs being patch - let mut patch_pkgid_to_urls = HashMap::new(); - for (url, summaries) in registry.patches().iter() { - for summary in summaries.iter() { - patch_pkgid_to_urls - .entry(summary.package_id()) - .or_insert_with(HashSet::new) - .insert(url); + let mut used_patches = HashSet::new(); + for pkg in resolve.iter() { + for (dep_pkg_id, deps) in resolve.deps_not_replaced(pkg) { + for dep in deps { + used_patches.insert((dep.source_id().canonical_url().clone(), dep_pkg_id)); + } } } @@ -839,34 +836,38 @@ fn emit_warnings_of_unused_patches( } let mut unemitted_unused_patches = Vec::new(); - for unused in resolve.unused_patches().iter() { - // Show alternative source URLs if the source URLs being patched - // cannot be found in the crate graph. - match ( - source_ids_grouped_by_pkg_name.get(&unused.name()), - patch_pkgid_to_urls.get(unused), - ) { - (Some(ids), Some(patched_urls)) - if ids - .iter() - .all(|id| !patched_urls.contains(id.canonical_url())) => - { - let mut help = "perhaps you meant one of the following:".to_owned(); - for id in ids { - help.push_str("\n\t"); - help.push_str(&id.display_registry_name()); + for (url, summaries) in registry.patches().iter() { + for summary in summaries.iter() { + let unused = summary.package_id(); + if !used_patches.contains(&(url.clone(), unused)) { + // Show alternative source URLs if the source URLs being patched + // cannot be found in the crate graph. + match source_ids_grouped_by_pkg_name.get(&unused.name()) { + Some(ids) if ids.iter().all(|id| id.canonical_url() != url) => { + let mut ids: Vec<_> = ids.into_iter().collect(); + ids.sort_by_key(|id| id.url()); // SourceId implements Ord but url() is safe + let mut help = "perhaps you meant one of the following:".to_owned(); + for id in ids { + help.push_str("\n\t"); + help.push_str(&id.display_registry_name()); + } + ws.gctx().shell().print_report( + &[Level::WARNING + .secondary_title(format!("patch `{unused}` {MESSAGE}")) + .element(Level::HELP.message(help))], + false, + )?; + } + _ => unemitted_unused_patches.push(unused), } - ws.gctx().shell().print_report( - &[Level::WARNING - .secondary_title(format!("patch `{unused}` {MESSAGE}")) - .element(Level::HELP.message(help))], - false, - )?; } - _ => unemitted_unused_patches.push(unused), } } + // Deduplicate general warnings by PackageId + unemitted_unused_patches.sort(); + unemitted_unused_patches.dedup(); + // Show general help message. if !unemitted_unused_patches.is_empty() { let mut warnings: Vec<_> = unemitted_unused_patches diff --git a/tests/testsuite/patch.rs b/tests/testsuite/patch.rs index 71815392142..d41eecf1bf4 100644 --- a/tests/testsuite/patch.rs +++ b/tests/testsuite/patch.rs @@ -3462,6 +3462,10 @@ fn unused_patch_same_dependency_different_sources() { p.cargo("check") .with_stderr_data(str![[r#" [UPDATING] `dummy-registry` index +[WARNING] patch `bar v0.1.0 ([ROOT]/foo/my-bar)` was not used in the crate graph + | + = [HELP] perhaps you meant one of the following: + [ROOT]/foo/my-bar [LOCKING] 1 package to latest compatible version [CHECKING] bar v0.1.0 ([ROOT]/foo/my-bar) [CHECKING] foo v0.0.1 ([ROOT]/foo)