From 20cfb41e42938a84a69c67cb83483c1f18ccb80b Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Sat, 29 Sep 2018 14:37:03 -0700 Subject: [PATCH] Try to improve "version not found" error This commit tweaks the wording for when versions aren't found in a source, notably including more location information other than the source id but rather also taking into account replaced sources and such. It's hoped that this gives more information to make it a bit more clear what's going on. Closes #6111 --- src/cargo/core/registry.rs | 17 ++++++++++++ src/cargo/core/resolver/mod.rs | 16 ++++++----- src/cargo/core/source/mod.rs | 25 +++++++++++++++++ src/cargo/sources/directory.rs | 4 +++ src/cargo/sources/git/source.rs | 4 +++ src/cargo/sources/path.rs | 7 +++++ src/cargo/sources/registry/mod.rs | 4 +++ src/cargo/sources/replaced.rs | 8 ++++++ tests/testsuite/directory.rs | 42 +++++++++++++++++++++++++++++ tests/testsuite/registry.rs | 19 ++++++------- tests/testsuite/resolve.rs | 12 ++++----- tests/testsuite/support/resolver.rs | 8 ++++++ 12 files changed, 145 insertions(+), 21 deletions(-) diff --git a/src/cargo/core/registry.rs b/src/cargo/core/registry.rs index 86cf40dee9b..e5c975ef5fe 100644 --- a/src/cargo/core/registry.rs +++ b/src/cargo/core/registry.rs @@ -21,6 +21,9 @@ pub trait Registry { self.query(dep, &mut |s| ret.push(s), fuzzy)?; Ok(ret) } + + fn describe_source(&self, source: &SourceId) -> String; + fn is_replaced(&self, source: &SourceId) -> bool; } /// This structure represents a registry of known packages. It internally @@ -528,6 +531,20 @@ impl<'cfg> Registry for PackageRegistry<'cfg> { f(self.lock(override_summary)); Ok(()) } + + fn describe_source(&self, id: &SourceId) -> String { + match self.sources.get(id) { + Some(src) => src.describe(), + None => id.to_string(), + } + } + + fn is_replaced(&self, id: &SourceId) -> bool { + match self.sources.get(id) { + Some(src) => src.is_replaced(), + None => false, + } + } } fn lock(locked: &LockedMap, patches: &HashMap>, summary: Summary) -> Summary { diff --git a/src/cargo/core/resolver/mod.rs b/src/cargo/core/resolver/mod.rs index f34b4cf23e3..ed3cc20526d 100644 --- a/src/cargo/core/resolver/mod.rs +++ b/src/cargo/core/resolver/mod.rs @@ -933,13 +933,13 @@ fn activation_error( }; let mut msg = format!( - "no matching version `{}` found for package `{}`\n\ - location searched: {}\n\ - versions found: {}\n", - dep.version_req(), + "failed to select a version for the requirement `{} = \"{}\"`\n \ + candidate versions found which didn't match: {}\n \ + location searched: {}\n", dep.package_name(), - dep.source_id(), - versions + dep.version_req(), + versions, + registry.describe_source(dep.source_id()), ); msg.push_str("required by "); msg.push_str(&describe_path(&graph.path_to_top(parent.package_id()))); @@ -954,6 +954,10 @@ fn activation_error( ); } + if registry.is_replaced(dep.source_id()) { + msg.push_str("\nperhaps a crate was updated and forgotten to be re-vendored?"); + } + msg } else { // Maybe the user mistyped the name? Like `dep-thing` when `Dep_Thing` diff --git a/src/cargo/core/source/mod.rs b/src/cargo/core/source/mod.rs index 71eab5c2857..406ebd12c17 100644 --- a/src/cargo/core/source/mod.rs +++ b/src/cargo/core/source/mod.rs @@ -75,6 +75,15 @@ pub trait Source { fn verify(&self, _pkg: &PackageId) -> CargoResult<()> { Ok(()) } + + /// Describes this source in a human readable fashion, used for display in + /// resolver error messages currently. + fn describe(&self) -> String; + + /// Returns whether a source is being replaced by another here + fn is_replaced(&self) -> bool { + false + } } pub enum MaybePackage { @@ -139,6 +148,14 @@ impl<'a, T: Source + ?Sized + 'a> Source for Box { fn verify(&self, pkg: &PackageId) -> CargoResult<()> { (**self).verify(pkg) } + + fn describe(&self) -> String { + (**self).describe() + } + + fn is_replaced(&self) -> bool { + (**self).is_replaced() + } } impl<'a, T: Source + ?Sized + 'a> Source for &'a mut T { @@ -185,6 +202,14 @@ impl<'a, T: Source + ?Sized + 'a> Source for &'a mut T { fn verify(&self, pkg: &PackageId) -> CargoResult<()> { (**self).verify(pkg) } + + fn describe(&self) -> String { + (**self).describe() + } + + fn is_replaced(&self) -> bool { + (**self).is_replaced() + } } /// A `HashMap` of `SourceId` -> `Box` diff --git a/src/cargo/sources/directory.rs b/src/cargo/sources/directory.rs index c4918e1b82e..0076b75d278 100644 --- a/src/cargo/sources/directory.rs +++ b/src/cargo/sources/directory.rs @@ -212,4 +212,8 @@ impl<'cfg> Source for DirectorySource<'cfg> { Ok(()) } + + fn describe(&self) -> String { + format!("directory source `{}`", self.root.display()) + } } diff --git a/src/cargo/sources/git/source.rs b/src/cargo/sources/git/source.rs index 2633be285e6..4a899598558 100644 --- a/src/cargo/sources/git/source.rs +++ b/src/cargo/sources/git/source.rs @@ -229,6 +229,10 @@ impl<'cfg> Source for GitSource<'cfg> { fn fingerprint(&self, _pkg: &Package) -> CargoResult { Ok(self.rev.as_ref().unwrap().to_string()) } + + fn describe(&self) -> String { + format!("git repository {}", self.source_id) + } } #[cfg(test)] diff --git a/src/cargo/sources/path.rs b/src/cargo/sources/path.rs index f16201a4f52..6115b626c20 100644 --- a/src/cargo/sources/path.rs +++ b/src/cargo/sources/path.rs @@ -558,4 +558,11 @@ impl<'cfg> Source for PathSource<'cfg> { let (max, max_path) = self.last_modified_file(pkg)?; Ok(format!("{} ({})", max, max_path.display())) } + + fn describe(&self) -> String { + match self.source_id.url().to_file_path() { + Ok(path) => path.display().to_string(), + Err(_) => self.source_id.to_string(), + } + } } diff --git a/src/cargo/sources/registry/mod.rs b/src/cargo/sources/registry/mod.rs index d53dcd1080a..76afec1c427 100644 --- a/src/cargo/sources/registry/mod.rs +++ b/src/cargo/sources/registry/mod.rs @@ -585,4 +585,8 @@ impl<'cfg> Source for RegistrySource<'cfg> { fn fingerprint(&self, pkg: &Package) -> CargoResult { Ok(pkg.package_id().version().to_string()) } + + fn describe(&self) -> String { + self.source_id.display_registry() + } } diff --git a/src/cargo/sources/replaced.rs b/src/cargo/sources/replaced.rs index 3575f67ca45..e413de2d17b 100644 --- a/src/cargo/sources/replaced.rs +++ b/src/cargo/sources/replaced.rs @@ -103,4 +103,12 @@ impl<'cfg> Source for ReplacedSource<'cfg> { let id = id.with_source_id(&self.replace_with); self.inner.verify(&id) } + + fn describe(&self) -> String { + format!("{} (which is replacing {})", self.inner.describe(), self.to_replace) + } + + fn is_replaced(&self) -> bool { + true + } } diff --git a/tests/testsuite/directory.rs b/tests/testsuite/directory.rs index 32cd776b095..a2ce591a59d 100644 --- a/tests/testsuite/directory.rs +++ b/tests/testsuite/directory.rs @@ -683,3 +683,45 @@ fn workspace_different_locations() { ", ).run(); } + +#[test] +fn version_missing() { + setup(); + + VendorPackage::new("foo") + .file("src/lib.rs", "pub fn foo() {}") + .build(); + + VendorPackage::new("bar") + .file( + "Cargo.toml", + r#" + [package] + name = "bar" + version = "0.1.0" + authors = [] + + [dependencies] + foo = "2" + "#, + ) + .file("src/main.rs", "fn main() {}") + .build(); + + cargo_process("install bar") + .with_stderr( + "\ +[INSTALLING] bar v0.1.0 +error: failed to compile [..] + +Caused by: + failed to select a version for the requirement `foo = \"^2\"` + candidate versions found which didn't match: 0.0.1 + location searched: directory source `[..] (which is replacing registry `[..]`) +required by package `bar v0.1.0` +perhaps a crate was updated and forgotten to be re-vendored? +", + ) + .with_status(101) + .run(); +} diff --git a/tests/testsuite/registry.rs b/tests/testsuite/registry.rs index 0e0222504f0..804eba4bccb 100644 --- a/tests/testsuite/registry.rs +++ b/tests/testsuite/registry.rs @@ -219,9 +219,9 @@ fn wrong_version() { .with_status(101) .with_stderr_contains( "\ -error: no matching version `>= 1.0.0` found for package `foo` -location searched: registry [..] -versions found: 0.0.2, 0.0.1 +error: failed to select a version for the requirement `foo = \">= 1.0.0\"` + candidate versions found which didn't match: 0.0.2, 0.0.1 + location searched: `[..]` index (which is replacing registry `[..]`) required by package `foo v0.0.1 ([..])` ", ).run(); @@ -233,9 +233,9 @@ required by package `foo v0.0.1 ([..])` .with_status(101) .with_stderr_contains( "\ -error: no matching version `>= 1.0.0` found for package `foo` -location searched: registry [..] -versions found: 0.0.4, 0.0.3, 0.0.2, ... +error: failed to select a version for the requirement `foo = \">= 1.0.0\"` + candidate versions found which didn't match: 0.0.4, 0.0.3, 0.0.2, ... + location searched: `[..]` index (which is replacing registry `[..]`) required by package `foo v0.0.1 ([..])` ", ).run(); @@ -520,10 +520,11 @@ fn relying_on_a_yank_is_bad() { .with_status(101) .with_stderr_contains( "\ -error: no matching version `= 0.0.2` found for package `baz` -location searched: registry `[..]` -versions found: 0.0.1 +error: failed to select a version for the requirement `baz = \"= 0.0.2\"` + candidate versions found which didn't match: 0.0.1 + location searched: `[..]` index (which is replacing registry `[..]`) required by package `bar v0.0.1` + ... which is depended on by `foo [..]` ", ).run(); } diff --git a/tests/testsuite/resolve.rs b/tests/testsuite/resolve.rs index eb8713094e9..6d8cbb8912e 100644 --- a/tests/testsuite/resolve.rs +++ b/tests/testsuite/resolve.rs @@ -17,12 +17,12 @@ use proptest::prelude::*; proptest! { #![proptest_config(ProptestConfig { - cases: - if env::var("CI").is_ok() { - 256 // we have a lot of builds in CI so one or another of them will find problems - } else { - 1024 // but locally try and find it in the one build - }, + // Note that this is a little low in terms of cases we'd like to test, + // but this number affects how long this function takes. It can be + // increased locally to execute more tests and try to find more bugs, + // but for now it's semi-low to run in a small-ish amount of time on CI + // and locally. + cases: 256, max_shrink_iters: if env::var("CI").is_ok() { // This attempts to make sure that CI will fail fast, diff --git a/tests/testsuite/support/resolver.rs b/tests/testsuite/support/resolver.rs index 98b7b0f54b2..25d162816d2 100644 --- a/tests/testsuite/support/resolver.rs +++ b/tests/testsuite/support/resolver.rs @@ -46,6 +46,14 @@ pub fn resolve_with_config( } Ok(()) } + + fn describe_source(&self, _src: &SourceId) -> String { + String::new() + } + + fn is_replaced(&self, _src: &SourceId) -> bool { + false + } } let mut registry = MyRegistry(registry); let summary = Summary::new(