diff --git a/src/cargo/core/registry.rs b/src/cargo/core/registry.rs index b1259a56806..2dccec54c4d 100644 --- a/src/cargo/core/registry.rs +++ b/src/cargo/core/registry.rs @@ -296,7 +296,7 @@ impl<'cfg> PackageRegistry<'cfg> { let (summary, should_unlock) = summary_for_patch(orig_patch, &locked, summaries, source).chain_err(|| { format!( - "patch for `{}` in `{}` did not resolve to any crates", + "patch for `{}` in `{}` failed to resolve", orig_patch.package_name(), url, ) @@ -754,21 +754,28 @@ fn summary_for_patch( if summaries.len() == 1 { return Ok((summaries.pop().unwrap(), None)); } - let best_summary = |summaries: &mut Vec| -> Summary { - // TODO: This could maybe honor -Zminimal-versions? - summaries.sort_by(|a, b| a.version().cmp(b.version())); - summaries.pop().unwrap() - }; if summaries.len() > 1 { - let summary = best_summary(&mut summaries); - if let Some((_dep, lock_id)) = locked { - // I can't think of a scenario where this might happen (locked by - // definition should only match at most one summary). Maybe if the - // source is broken? - return Ok((summary, Some(*lock_id))); - } else { - return Ok((summary, None)); - } + // TODO: In the future, it might be nice to add all of these + // candidates so that version selection would just pick the + // appropriate one. However, as this is currently structured, if we + // added these all as patches, the unselected versions would end up in + // the "unused patch" listing, and trigger a warning. It might take a + // fair bit of restructuring to make that work cleanly, and there + // isn't any demand at this time to support that. + let mut vers: Vec<_> = summaries.iter().map(|summary| summary.version()).collect(); + vers.sort(); + let versions: Vec<_> = vers.into_iter().map(|v| v.to_string()).collect(); + anyhow::bail!( + "patch for `{}` in `{}` resolved to more than one candidate\n\ + Found versions: {}\n\ + Update the patch definition to select only one package.\n\ + For example, add an `=` version requirement to the patch definition, \ + such as `version = \"={}\"`.", + orig_patch.package_name(), + orig_patch.source_id(), + versions.join(", "), + versions.last().unwrap() + ); } assert!(summaries.is_empty()); // No summaries found, try to help the user figure out what is wrong. diff --git a/tests/testsuite/patch.rs b/tests/testsuite/patch.rs index 2ed61b3d40e..d80bd9b6da5 100644 --- a/tests/testsuite/patch.rs +++ b/tests/testsuite/patch.rs @@ -1567,15 +1567,20 @@ fn too_many_matches() { // Picks 0.1.1, the most recent version. p.cargo("check") + .with_status(101) .with_stderr( "\ [UPDATING] `[..]/alternative-registry` index -[UPDATING] `[..]/registry` index -[DOWNLOADING] crates ... -[DOWNLOADED] bar v0.1.1 (registry `[..]/alternative-registry`) -[CHECKING] bar v0.1.1 (registry `[..]/alternative-registry`) -[CHECKING] foo v0.1.0 ([..]/foo) -[FINISHED] [..] +[ERROR] failed to resolve patches for `https://github.com/rust-lang/crates.io-index` + +Caused by: + patch for `bar` in `https://github.com/rust-lang/crates.io-index` failed to resolve + +Caused by: + patch for `bar` in `registry `[..]/alternative-registry`` resolved to more than one candidate +Found versions: 0.1.0, 0.1.1 +Update the patch definition to select only one package. +For example, add an `=` version requirement to the patch definition, such as `version = \"=0.1.1\"`. ", ) .run(); @@ -1611,7 +1616,7 @@ fn no_matches() { error: failed to resolve patches for `https://github.com/rust-lang/crates.io-index` Caused by: - patch for `bar` in `https://github.com/rust-lang/crates.io-index` did not resolve to any crates + patch for `bar` in `https://github.com/rust-lang/crates.io-index` failed to resolve Caused by: The patch location `[..]/foo/bar` does not appear to contain any packages matching the name `bar`. @@ -1650,7 +1655,7 @@ fn mismatched_version() { [ERROR] failed to resolve patches for `https://github.com/rust-lang/crates.io-index` Caused by: - patch for `bar` in `https://github.com/rust-lang/crates.io-index` did not resolve to any crates + patch for `bar` in `https://github.com/rust-lang/crates.io-index` failed to resolve Caused by: The patch location `[..]/foo/bar` contains a `bar` package with version `0.1.0`, \ @@ -1760,7 +1765,7 @@ fn patch_walks_backwards_restricted() { error: failed to resolve patches for `https://github.com/rust-lang/crates.io-index` Caused by: - patch for `bar` in `https://github.com/rust-lang/crates.io-index` did not resolve to any crates + patch for `bar` in `https://github.com/rust-lang/crates.io-index` failed to resolve Caused by: The patch location `[..]/foo/bar` contains a `bar` package with version `0.1.0`, but the patch definition requires `^0.1.1`. @@ -1942,7 +1947,7 @@ fn can_update_with_alt_reg() { bar = "0.1" [patch.crates-io] - bar = { version = "0.1.1", registry = "alternative" } + bar = { version = "=0.1.1", registry = "alternative" } "#, ) .file("src/lib.rs", "") @@ -1967,12 +1972,42 @@ fn can_update_with_alt_reg() { // Should remain locked. p.cargo("check").with_stderr("[FINISHED] [..]").run(); + // This does nothing, due to `=` requirement. p.cargo("update -p bar") .with_stderr( "\ [UPDATING] `[..]/alternative-registry` index [UPDATING] `[..]/registry` index -[UPDATING] bar v0.1.1 (registry `[..]/alternative-registry`) -> v0.1.2 +", + ) + .run(); + + // Bump to 0.1.2. + p.change_file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.1.0" + + [dependencies] + bar = "0.1" + + [patch.crates-io] + bar = { version = "=0.1.2", registry = "alternative" } + "#, + ); + + p.cargo("check") + .with_stderr( + "\ +[UPDATING] `[..]/alternative-registry` index +[UPDATING] `[..]/registry` index +[DOWNLOADING] crates ... +[DOWNLOADED] bar v0.1.2 (registry `[..]/alternative-registry`) +[CHECKING] bar v0.1.2 (registry `[..]/alternative-registry`) +[CHECKING] foo v0.1.0 ([..]/foo) +[FINISHED] [..] ", ) .run();