Skip to content

Commit 91791c4

Browse files
committed
Improve unused patch message when source URLs mismatched
`Resolve.unused_patches` does not contains info about which source URLs they are going to patch. As a result, we cannot provide a precise message but only list all possible URLs of the packages with the same name in the resolved graph. There is a little flaw that if multiple patches are patching the same package, the source URL of the used one would be shown as a possible URL in the warning.
1 parent b85ad15 commit 91791c4

File tree

3 files changed

+185
-64
lines changed

3 files changed

+185
-64
lines changed

src/cargo/core/registry.rs

+6-5
Original file line numberDiff line numberDiff line change
@@ -413,11 +413,12 @@ impl<'cfg> PackageRegistry<'cfg> {
413413
self.patches_locked = true;
414414
}
415415

416-
pub fn patches(&self) -> Vec<Summary> {
417-
self.patches
418-
.values()
419-
.flat_map(|v| v.iter().cloned())
420-
.collect()
416+
/// Gets all patches grouped by the source URLS they are going to patch.
417+
///
418+
/// These patches are mainly collected from [`patch`](Self::patch).
419+
/// They might not be the same as patches actually used during dependency resolving.
420+
pub fn patches(&self) -> &HashMap<CanonicalUrl, Vec<Summary>> {
421+
&self.patches
421422
}
422423

423424
fn load(&mut self, source_id: SourceId, kind: Kind) -> CargoResult<()> {

src/cargo/ops/resolve.rs

+87-19
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ use crate::util::errors::CargoResult;
2929
use crate::util::{profile, CanonicalUrl};
3030
use anyhow::Context as _;
3131
use log::{debug, trace};
32-
use std::collections::HashSet;
32+
use std::collections::{HashMap, HashSet};
3333

3434
/// Result for `resolve_ws_with_opts`.
3535
pub struct WorkspaceResolve<'cfg> {
@@ -467,25 +467,17 @@ pub fn resolve_with_previous<'cfg>(
467467
.require(Feature::public_dependency())
468468
.is_ok(),
469469
)?;
470-
resolved.register_used_patches(&registry.patches());
471-
if register_patches {
472-
// It would be good if this warning was more targeted and helpful
473-
// (such as showing close candidates that failed to match). However,
474-
// that's not terribly easy to do, so just show a general help
475-
// message.
476-
let warnings: Vec<String> = resolved
477-
.unused_patches()
478-
.iter()
479-
.map(|pkgid| format!("Patch `{}` was not used in the crate graph.", pkgid))
480-
.collect();
481-
if !warnings.is_empty() {
482-
ws.config().shell().warn(format!(
483-
"{}\n{}",
484-
warnings.join("\n"),
485-
UNUSED_PATCH_WARNING
486-
))?;
487-
}
470+
let patches: Vec<_> = registry
471+
.patches()
472+
.values()
473+
.flat_map(|v| v.iter().cloned())
474+
.collect();
475+
resolved.register_used_patches(&patches[..]);
476+
477+
if register_patches && !resolved.unused_patches().is_empty() {
478+
emit_warnings_of_unused_patches(ws, &resolved, &registry)?;
488479
}
480+
489481
if let Some(previous) = previous {
490482
resolved.merge_from(previous)?;
491483
}
@@ -757,3 +749,79 @@ fn master_branch_git_source(id: PackageId, resolve: &Resolve) -> Option<PackageI
757749
}
758750
None
759751
}
752+
753+
/// Emits warnings of unused patches case by case.
754+
///
755+
/// This function does its best to provide more targeted and helpful
756+
/// (such as showing close candidates that failed to match). However, that's
757+
/// not terribly easy to do, so just show a general help message if we cannot.
758+
fn emit_warnings_of_unused_patches(
759+
ws: &Workspace<'_>,
760+
resolve: &Resolve,
761+
registry: &PackageRegistry<'_>,
762+
) -> CargoResult<()> {
763+
const MESSAGE: &str = "was not used in the crate graph.";
764+
765+
// Patch package with the source URLs being patch
766+
let mut patch_pkgid_to_urls = HashMap::new();
767+
for (url, summaries) in registry.patches().iter() {
768+
for summary in summaries.iter() {
769+
patch_pkgid_to_urls
770+
.entry(summary.package_id())
771+
.or_insert_with(HashSet::new)
772+
.insert(url);
773+
}
774+
}
775+
776+
// pkg name -> all source IDs of under the same pkg name
777+
let mut source_ids_grouped_by_pkg_name = HashMap::new();
778+
for pkgid in resolve.iter() {
779+
source_ids_grouped_by_pkg_name
780+
.entry(pkgid.name())
781+
.or_insert_with(HashSet::new)
782+
.insert(pkgid.source_id());
783+
}
784+
785+
let mut unemitted_unused_patches = Vec::new();
786+
for unused in resolve.unused_patches().iter() {
787+
// Show alternative source URLs if the source URLs being patch
788+
// cannot not be found in the crate graph.
789+
match (
790+
source_ids_grouped_by_pkg_name.get(&unused.name()),
791+
patch_pkgid_to_urls.get(unused),
792+
) {
793+
(Some(ids), Some(patched_urls))
794+
if ids
795+
.iter()
796+
.all(|id| !patched_urls.contains(id.canonical_url())) =>
797+
{
798+
use std::fmt::Write;
799+
let mut msg = String::new();
800+
writeln!(&mut msg, "Patch `{}` {}", unused, MESSAGE)?;
801+
write!(
802+
&mut msg,
803+
"Perhaps you misspell the source URL being patched.\n\
804+
Possible URLs for `[patch.<URL>]`:",
805+
)?;
806+
for id in ids.iter() {
807+
write!(&mut msg, "\n {}", id.display_registry_name())?;
808+
}
809+
ws.config().shell().warn(msg)?;
810+
}
811+
_ => unemitted_unused_patches.push(unused),
812+
}
813+
}
814+
815+
// Show general help message.
816+
if !unemitted_unused_patches.is_empty() {
817+
let warnings: Vec<_> = unemitted_unused_patches
818+
.iter()
819+
.map(|pkgid| format!("Patch `{}` {}", pkgid, MESSAGE))
820+
.collect();
821+
ws.config()
822+
.shell()
823+
.warn(format!("{}\n{}", warnings.join("\n"), UNUSED_PATCH_WARNING))?;
824+
}
825+
826+
return Ok(());
827+
}

tests/testsuite/patch.rs

+92-40
Original file line numberDiff line numberDiff line change
@@ -358,10 +358,10 @@ fn unused() {
358358
"\
359359
[UPDATING] `dummy-registry` index
360360
[WARNING] Patch `bar v0.2.0 ([CWD]/bar)` was not used in the crate graph.
361-
[..]
362-
[..]
363-
[..]
364-
[..]
361+
Check that [..]
362+
with the [..]
363+
what is [..]
364+
version. [..]
365365
[DOWNLOADING] crates ...
366366
[DOWNLOADED] bar v0.1.0 [..]
367367
[COMPILING] bar v0.1.0
@@ -374,10 +374,10 @@ fn unused() {
374374
.with_stderr(
375375
"\
376376
[WARNING] Patch `bar v0.2.0 ([CWD]/bar)` was not used in the crate graph.
377-
[..]
378-
[..]
379-
[..]
380-
[..]
377+
Check that [..]
378+
with the [..]
379+
what is [..]
380+
version. [..]
381381
[FINISHED] [..]
382382
",
383383
)
@@ -394,6 +394,60 @@ fn unused() {
394394
);
395395
}
396396

397+
#[cargo_test]
398+
fn unused_with_mismatch_source_being_patched() {
399+
registry::alt_init();
400+
Package::new("bar", "0.1.0").publish();
401+
402+
let p = project()
403+
.file(
404+
"Cargo.toml",
405+
r#"
406+
[package]
407+
name = "foo"
408+
version = "0.0.1"
409+
authors = []
410+
411+
[dependencies]
412+
bar = "0.1.0"
413+
414+
[patch.alternative]
415+
bar = { path = "bar" }
416+
417+
[patch.crates-io]
418+
bar = { path = "baz" }
419+
"#,
420+
)
421+
.file("src/lib.rs", "")
422+
.file("bar/Cargo.toml", &basic_manifest("bar", "0.2.0"))
423+
.file("bar/src/lib.rs", "not rust code")
424+
.file("baz/Cargo.toml", &basic_manifest("bar", "0.3.0"))
425+
.file("baz/src/lib.rs", "not rust code")
426+
.build();
427+
428+
p.cargo("build")
429+
.with_stderr(
430+
"\
431+
[UPDATING] `dummy-registry` index
432+
[WARNING] Patch `bar v0.2.0 ([CWD]/bar)` was not used in the crate graph.
433+
Perhaps you misspell the source URL being patched.
434+
Possible URLs for `[patch.<URL>]`:
435+
crates-io
436+
[WARNING] Patch `bar v0.3.0 ([CWD]/baz)` was not used in the crate graph.
437+
Check that [..]
438+
with the [..]
439+
what is [..]
440+
version. [..]
441+
[DOWNLOADING] crates ...
442+
[DOWNLOADED] bar v0.1.0 [..]
443+
[COMPILING] bar v0.1.0
444+
[COMPILING] foo v0.0.1 ([CWD])
445+
[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]
446+
",
447+
)
448+
.run();
449+
}
450+
397451
#[cargo_test]
398452
fn prefer_patch_version() {
399453
Package::new("bar", "0.1.2").publish();
@@ -477,10 +531,10 @@ fn unused_from_config() {
477531
"\
478532
[UPDATING] `dummy-registry` index
479533
[WARNING] Patch `bar v0.2.0 ([CWD]/bar)` was not used in the crate graph.
480-
[..]
481-
[..]
482-
[..]
483-
[..]
534+
Check that [..]
535+
with the [..]
536+
what is [..]
537+
version. [..]
484538
[DOWNLOADING] crates ...
485539
[DOWNLOADED] bar v0.1.0 [..]
486540
[COMPILING] bar v0.1.0
@@ -493,10 +547,10 @@ fn unused_from_config() {
493547
.with_stderr(
494548
"\
495549
[WARNING] Patch `bar v0.2.0 ([CWD]/bar)` was not used in the crate graph.
496-
[..]
497-
[..]
498-
[..]
499-
[..]
550+
Check that [..]
551+
with the [..]
552+
what is [..]
553+
version. [..]
500554
[FINISHED] [..]
501555
",
502556
)
@@ -550,10 +604,10 @@ fn unused_git() {
550604
[UPDATING] git repository `file://[..]`
551605
[UPDATING] `dummy-registry` index
552606
[WARNING] Patch `bar v0.2.0 ([..])` was not used in the crate graph.
553-
[..]
554-
[..]
555-
[..]
556-
[..]
607+
Check that [..]
608+
with the [..]
609+
what is [..]
610+
version. [..]
557611
[DOWNLOADING] crates ...
558612
[DOWNLOADED] bar v0.1.0 [..]
559613
[COMPILING] bar v0.1.0
@@ -566,10 +620,10 @@ fn unused_git() {
566620
.with_stderr(
567621
"\
568622
[WARNING] Patch `bar v0.2.0 ([..])` was not used in the crate graph.
569-
[..]
570-
[..]
571-
[..]
572-
[..]
623+
Check that [..]
624+
with the [..]
625+
what is [..]
626+
version. [..]
573627
[FINISHED] [..]
574628
",
575629
)
@@ -752,21 +806,21 @@ fn add_ignored_patch() {
752806
.with_stderr(
753807
"\
754808
[WARNING] Patch `bar v0.1.1 ([CWD]/bar)` was not used in the crate graph.
755-
[..]
756-
[..]
757-
[..]
758-
[..]
809+
Check that [..]
810+
with the [..]
811+
what is [..]
812+
version. [..]
759813
[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]",
760814
)
761815
.run();
762816
p.cargo("build")
763817
.with_stderr(
764818
"\
765819
[WARNING] Patch `bar v0.1.1 ([CWD]/bar)` was not used in the crate graph.
766-
[..]
767-
[..]
768-
[..]
769-
[..]
820+
Check that [..]
821+
with the [..]
822+
what is [..]
823+
version. [..]
770824
[FINISHED] [..]",
771825
)
772826
.run();
@@ -1714,10 +1768,9 @@ fn two_semver_compatible() {
17141768
.with_stderr(
17151769
"\
17161770
warning: Patch `bar v0.1.1 [..]` was not used in the crate graph.
1717-
Check that [..]
1718-
with the [..]
1719-
what is [..]
1720-
version. [..]
1771+
Perhaps you misspell the source URL being patched.
1772+
Possible URLs for `[patch.<URL>]`:
1773+
[CWD]/bar
17211774
[FINISHED] [..]",
17221775
)
17231776
.run();
@@ -1769,10 +1822,9 @@ fn multipatch_select_big() {
17691822
.with_stderr(
17701823
"\
17711824
warning: Patch `bar v0.1.0 [..]` was not used in the crate graph.
1772-
Check that [..]
1773-
with the [..]
1774-
what is [..]
1775-
version. [..]
1825+
Perhaps you misspell the source URL being patched.
1826+
Possible URLs for `[patch.<URL>]`:
1827+
[CWD]/bar
17761828
[FINISHED] [..]",
17771829
)
17781830
.run();

0 commit comments

Comments
 (0)