Improve resolve error messages

src/cargo/core/compiler/context/mod.rs

@@ -580,10 +580,19 @@ impl Links {
  let pkg = unit.pkg.package_id();
 
  let describe_path = |pkgid: PackageId| -> String {
- let dep_path = resolve.path_to_top(&pkgid);
- let mut dep_path_desc = format!("package `{}`", dep_path[0]);
- for dep in dep_path.iter().skip(1) {
- write!(dep_path_desc, "\n ... which is depended on by `{}`", dep).unwrap();
+ let dep_path = resolve.graph().path_to_top(&pkgid);
+ let mut dep_path_desc = format!("package `{}`", pkgid);
+ for &(dep, req) in dep_path.iter() {
+ let req = req.first().unwrap();
+ write!(
+ dep_path_desc,
+ "\n ... selected to satisfy the requirement \
+ `{} = \"{}\"` from package `{}`",
+ req.name_in_toml(),
+ req.version_req(),
+ dep
+ )
+ .unwrap();
  }
  dep_path_desc
  };

src/cargo/core/resolver/errors.rs

@@ -81,9 +81,14 @@ pub(super) fn activation_error(
  let to_resolve_err = |err| {
  ResolveError::new(
  err,
- graph
- .path_to_top(&parent.package_id())
- .into_iter()
+ Some(parent.package_id())
+ .iter()
+ .chain(
+ graph
+ .path_to_top(&parent.package_id())
+ .into_iter()
+ .map(|x| x.0),
+ )
  .cloned()
  .collect(),
  )
@@ -92,7 +97,11 @@ pub(super) fn activation_error(
  if !candidates.is_empty() {
  let mut msg = format!("failed to select a version for `{}`.", dep.package_name());
  msg.push_str("\n ... required by ");
- msg.push_str(&describe_path(&graph.path_to_top(&parent.package_id())));
+ msg.push_str(&describe_path(&graph, parent.package_id()));
+ msg.push_str("which requires ");
+ msg.push_str(&dep.package_name());
+ msg.push_str(" ");
+ msg.push_str(&dep.version_req().to_string());
 
  msg.push_str("\nversions that meet the requirements `");
  msg.push_str(&dep.version_req().to_string());
@@ -113,7 +122,7 @@ pub(super) fn activation_error(
  .rev()
  .partition(|&(_, r)| r.is_links());
 
- for &(p, r) in links_errors.iter() {
+ for &(&p, r) in links_errors.iter() {
  if let ConflictReason::Links(ref link) = *r {
  msg.push_str("\n\nthe package `");
  msg.push_str(&*dep.package_name());
@@ -123,7 +132,7 @@ pub(super) fn activation_error(
  msg.push_str(link);
  msg.push_str("` as well:\n");
  }
- msg.push_str(&describe_path(&graph.path_to_top(p)));
+ msg.push_str(&describe_path(&graph, p));
  }
 
  let (features_errors, other_errors): (Vec<_>, Vec<_>) = other_errors
@@ -152,9 +161,9 @@ pub(super) fn activation_error(
  );
  }
 
- for &(p, _) in other_errors.iter() {
+ for &(&p, _) in other_errors.iter() {
  msg.push_str("\n\n previously selected ");
- msg.push_str(&describe_path(&graph.path_to_top(p)));
+ msg.push_str(&describe_path(&graph, p));
  }
 
  msg.push_str("\n\nfailed to select a version for `");
@@ -204,7 +213,7 @@ pub(super) fn activation_error(
  registry.describe_source(dep.source_id()),
  );
  msg.push_str("required by ");
- msg.push_str(&describe_path(&graph.path_to_top(&parent.package_id())));
+ msg.push_str(&describe_path(&graph, parent.package_id()));
 
  // If we have a path dependency with a locked version, then this may
  // indicate that we updated a sub-package and forgot to run `cargo
@@ -258,7 +267,7 @@ pub(super) fn activation_error(
  msg.push_str("\n");
  }
  msg.push_str("required by ");
- msg.push_str(&describe_path(&graph.path_to_top(&parent.package_id())));
+ msg.push_str(&describe_path(&graph, parent.package_id()));
 
  msg
  };
@@ -278,11 +287,23 @@ pub(super) fn activation_error(
 }
 
 /// Returns String representation of dependency chain for a particular `pkgid`.
-pub(super) fn describe_path(path: &[&PackageId]) -> String {
+pub(super) fn describe_path(
+ graph: &crate::util::graph::Graph<PackageId, Vec<Dependency>>,
+ this: PackageId,
+) -> String {
  use std::fmt::Write;
- let mut dep_path_desc = format!("package `{}`", path[0]);
- for dep in path[1..].iter() {
- write!(dep_path_desc, "\n ... which is depended on by `{}`", dep).unwrap();
+ let path = &graph.path_to_top(&this);
+ let mut dep_path_desc = format!("package `{}`", this);
+ for &(dep, req) in path.iter() {
+ let req = req.first().unwrap();
+ write!(
+ dep_path_desc,
+ "\n ... selected to satisfy the requirement `{} = \"{}\"` from package `{}`",
+ req.name_in_toml(),
+ req.version_req(),
+ dep
+ )
+ .unwrap();
  }
  dep_path_desc
 }

src/cargo/core/resolver/mod.rs

@@ -848,7 +848,7 @@ fn check_cycles(resolve: &Resolve, activations: &Activations) -> CargoResult<()>
  failure::bail!(
  "cyclic package dependency: package `{}` depends on itself. Cycle:\n{}",
  id,
- errors::describe_path(&resolve.path_to_top(&id))
+ errors::describe_path(&resolve.graph(), id)
  );
  }
 

src/cargo/core/resolver/resolve.rs

@@ -54,12 +54,6 @@ impl Resolve {
  }
  }
 
- /// Resolves one of the paths from the given dependent package up to
- /// the root.
- pub fn path_to_top<'a>(&'a self, pkg: &'a PackageId) -> Vec<&'a PackageId> {
- self.graph.path_to_top(pkg)
- }
-
  pub fn register_used_patches(&mut self, patches: &HashMap<Url, Vec<Summary>>) {
  for summary in patches.values().flat_map(|v| v) {
  if self.iter().any(|id| id == summary.package_id()) {
@@ -194,6 +188,10 @@ unable to verify that `{0}` is the same as when the lockfile was generated
  &self.replacements
  }
 
+ pub fn graph(&self) -> &Graph<PackageId, Vec<Dependency>> {
+ &self.graph
+ }
+
  pub fn features(&self, pkg: PackageId) -> &HashSet<String> {
  self.features.get(&pkg).unwrap_or(&self.empty_features)
  }

src/cargo/util/graph.rs

@@ -72,23 +72,24 @@ impl<N: Eq + Hash + Clone, E: Default> Graph<N, E> {
 
  /// Resolves one of the paths from the given dependent package up to
  /// the root.
- pub fn path_to_top<'a>(&'a self, mut pkg: &'a N) -> Vec<&'a N> {
+ pub fn path_to_top<'s: 'q, 'q>(&'s self, mut pkg: &'q N) -> Vec<(&'s N, &'s E)> {
  // Note that this implementation isn't the most robust per se, we'll
  // likely have to tweak this over time. For now though it works for what
  // it's used for!
- let mut result = vec![pkg];
- let first_pkg_depending_on = |pkg: &N, res: &[&N]| {
+ let mut result: Vec<(&'s N, &'s E)> = vec![];
+ let first_pkg_depending_on = |pkg: &N, res: &[(&'s N, &'s E)]| {
  self.nodes
  .iter()
  .filter(|&(_, adjacent)| adjacent.contains_key(pkg))
  // Note that we can have "cycles" introduced through dev-dependency
  // edges, so make sure we don't loop infinitely.
- .find(|&(node, _)| !res.contains(&node))
- .map(|p| p.0)
+ .find(|&(node, _)| res.iter().find(|p| p.0 == node).is_none())
+ // TODO: find_map would be clearer
+ .map(|(node, adjacent)| (node, adjacent.get(pkg).unwrap()))
  };
  while let Some(p) = first_pkg_depending_on(pkg, &result) {
  result.push(p);
- pkg = p;
+ pkg = p.0;
  }
  result
  }

tests/testsuite/build.rs

@@ -1171,15 +1171,15 @@ fn incompatible_dependencies() {
  .with_stderr_contains(
  "\
 error: failed to select a version for `bad`.
- ... required by package `qux v0.1.0`
- ... which is depended on by `foo v0.0.1 ([..])`
+ ... required by package `qux v0.1.0` which requires `bad >=1.0.1`
+ ... selected to satisfy the requirement `qux = \"^0.1.0\"` from package `foo v0.0.1 ([..])`
 versions that meet the requirements `>= 1.0.1` are: 1.0.2, 1.0.1
 
 all possible versions conflict with previously selected packages.
 
  previously selected package `bad v1.0.0`
- ... which is depended on by `baz v0.1.0`
- ... which is depended on by `foo v0.0.1 ([..])`
+ ... selected to satisfy the requirement `bad = \"= 1.0.0\"` from package `baz v0.1.0`
+ ... selected to satisfy the requirement `baz = \"^0.1.0\"` from package `foo v0.0.1 ([..])`
 
 failed to select a version for `bad` which could resolve this conflict",
  )
@@ -1217,18 +1217,18 @@ fn incompatible_dependencies_with_multi_semver() {
  .with_stderr_contains(
  "\
 error: failed to select a version for `bad`.
- ... required by package `foo v0.0.1 ([..])`
+ ... required by package `foo v0.0.1 ([..]) which requires `bad = >=1.0.1, <=2.0.0`
 versions that meet the requirements `>= 1.0.1, <= 2.0.0` are: 2.0.0, 1.0.1
 
 all possible versions conflict with previously selected packages.
 
  previously selected package `bad v2.0.1`
- ... which is depended on by `baz v0.1.0`
- ... which is depended on by `foo v0.0.1 ([..])`
+ ... selected to satisfy the requirement `bad = \">= 2.0.1\"` from package `baz v0.1.0`
+ ... selected to satisfy the requirement `baz = \"^0.1.0\"` from package `foo v0.0.1 ([..])`
 
  previously selected package `bad v1.0.0`
- ... which is depended on by `bar v0.1.0`
- ... which is depended on by `foo v0.0.1 ([..])`
+ ... selected to satisfy the requirement `bad = \"= 1.0.0\"` from package `bar v0.1.0`
+ ... selected to satisfy the requirement `bar = \"^0.1.0\"` from package `foo v0.0.1 ([..])`
 
 failed to select a version for `bad` which could resolve this conflict",
  )
@@ -1278,7 +1278,8 @@ fn compile_offline_while_transitive_dep_not_cached() {
 error: no matching package named `baz` found
 location searched: registry `[..]`
 required by package `bar v0.1.0`
- ... which is depended on by `foo v0.0.1 ([CWD])`
+ ... selected to satisfy the requirement `bar = \"= 0.1.0\"` \
+from package `foo v0.0.1 ([CWD])`
 As a reminder, you're using offline mode (-Z offline) \
 which can sometimes cause surprising resolution failures, \
 if this error is too confusing you may with to retry \
@@ -1688,7 +1689,8 @@ fn self_dependency() {
  .with_stderr(
  "\
 [ERROR] cyclic package dependency: package `test v0.0.0 ([CWD])` depends on itself. Cycle:
-package `test v0.0.0 ([CWD])`",
+package `test v0.0.0 ([CWD])`
+ ... selected to satisfy the requirement `test = \"*\"` from package `test v0.0.0 ([..]foo)`",
  )
  .run();
 }
@@ -2798,7 +2800,8 @@ fn cyclic_deps_rejected() {
  .with_stderr(
 "[ERROR] cyclic package dependency: package `a v0.0.1 ([CWD]/a)` depends on itself. Cycle:
 package `a v0.0.1 ([CWD]/a)`
- ... which is depended on by `foo v0.0.1 ([CWD])`",
+ ... selected to satisfy the requirement `a = \"*\"` from package `foo v0.0.1 ([..]foo)`
+ ... selected to satisfy the requirement `foo = \"*\"` from package `a v0.0.1 ([..]a)`",
  ).run();
 }
 

tests/testsuite/build_script.rs

@@ -315,7 +315,7 @@ fn links_duplicates() {
  p.cargo("build").with_status(101)
  .with_stderr("\
 error: failed to select a version for `a-sys`.
- ... required by package `foo v0.5.0 ([..])`
+ ... required by package `foo v0.5.0 ([..]) which requires `
 versions that meet the requirements `*` are: 0.5.0
 
 the package `a-sys` links to the native library `a`, but it conflicts with a previous package which links to `a` as well:
@@ -378,8 +378,8 @@ fn links_duplicates_deep_dependency() {
  p.cargo("build").with_status(101)
  .with_stderr("\
 error: failed to select a version for `a-sys`.
- ... required by package `a v0.5.0 ([..])`
- ... which is depended on by `foo v0.5.0 ([..])`
+ ... required by package `a v0.5.0 ([..]) which requires a-sys = *`
+ ... selected to fulfill the requirement `a = \"*\"` from package `foo v0.5.0 ([..])`
 versions that meet the requirements `*` are: 0.5.0
 
 the package `a-sys` links to the native library `a`, but it conflicts with a previous package which links to `a` as well:
@@ -3411,7 +3411,7 @@ fn links_duplicates_with_cycle() {
  p.cargo("build").with_status(101)
  .with_stderr("\
 error: failed to select a version for `a`.
- ... required by package `foo v0.5.0 ([..])`
+ ... required by package `foo v0.5.0 ([..])` which requires `a-sys *`
 versions that meet the requirements `*` are: 0.5.0
 
 the package `a` links to the native library `a`, but it conflicts with a previous package which links to `a` as well:

tests/testsuite/features.rs

@@ -131,7 +131,7 @@ fn invalid4() {
  .with_stderr(
  "\
 error: failed to select a version for `bar`.
- ... required by package `foo v0.0.1 ([..])`
+ ... required by package `foo v0.0.1 ([..])` which requires bar *
 versions that meet the requirements `*` are: 0.0.1
 
 the package `foo` depends on `bar`, with features: `bar` but `bar` does not have these features.

tests/testsuite/registry.rs

@@ -553,7 +553,7 @@ 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 [..]`
+ ... which is depended on by `foo [..] which requires bar = *`
 ",
  )
  .run();