Improve resolver message for validate_links

Author: weihanglo

src/cargo/core/compiler/links.rs

@@ -1,8 +1,8 @@
 use super::unit_graph::UnitGraph;
+use crate::core::resolver::errors::describe_path;
 use crate::core::{PackageId, Resolve};
 use crate::util::errors::CargoResult;
 use std::collections::{HashMap, HashSet};
-use std::fmt::Write;
 
 /// Validate `links` field does not conflict between packages.
 pub fn validate_links(resolve: &Resolve, unit_graph: &UnitGraph) -> CargoResult<()> {
@@ -28,17 +28,15 @@ pub fn validate_links(resolve: &Resolve, unit_graph: &UnitGraph) -> CargoResult<
             None => continue,
         };
         if let Some(&prev) = links.get(lib) {
+            let prev_path = resolve
+                .path_to_top(&prev)
+                .into_iter()
+                .map(|(p, d)| (p, d.and_then(|d| d.iter().next())));
             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();
-                }
-                dep_path_desc
-            };
-
+            let path = resolve
+                .path_to_top(&pkg)
+                .into_iter()
+                .map(|(p, d)| (p, d.and_then(|d| d.iter().next())));
             anyhow::bail!(
                 "multiple packages link to native library `{}`, \
                  but a native library can be linked only once\n\
@@ -47,9 +45,9 @@ pub fn validate_links(resolve: &Resolve, unit_graph: &UnitGraph) -> CargoResult<
                  \n\
                  {}\nalso links to native library `{}`",
                 lib,
-                describe_path(prev),
+                describe_path(prev_path),
                 lib,
-                describe_path(pkg),
+                describe_path(path),
                 lib
             )
         }

src/cargo/core/resolver/errors.rs

@@ -366,7 +366,7 @@ pub(super) fn describe_path_in_context(cx: &Context, id: &PackageId) -> String {
 /// -> (pkg1, dep from pkg1 satisfied by pkg0)
 /// -> (pkg2, dep from pkg2 satisfied by pkg1)
 /// -> ...
-pub(super) fn describe_path<'a>(
+pub(crate) fn describe_path<'a>(
     mut path: impl Iterator<Item = (&'a PackageId, Option<&'a Dependency>)>,
 ) -> String {
     use std::fmt::Write;

src/cargo/core/resolver/mod.rs

@@ -78,7 +78,7 @@ mod conflict_cache;
 mod context;
 mod dep_cache;
 mod encode;
-mod errors;
+pub(crate) mod errors;
 pub mod features;
 mod resolve;
 mod types;

src/cargo/core/resolver/resolve.rs

@@ -117,7 +117,10 @@ 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> {
+    pub fn path_to_top<'a>(
+        &'a self,
+        pkg: &'a PackageId,
+    ) -> Vec<(&'a PackageId, Option<&'a HashSet<Dependency>>)> {
         self.graph.path_to_top(pkg)
     }
 

src/cargo/util/graph.rs

@@ -111,23 +111,28 @@ impl<N: Eq + Ord + Clone, E: Default + Clone> 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> {
+    ///
+    /// Each element contains a node along with an edge except the first one.
+    /// The representation would look like:
+    ///
+    /// (Node0,) -> (Node1, Edge01) -> (Node2, Edge12)...
+    pub fn path_to_top<'a>(&'a self, mut pkg: &'a N) -> Vec<(&'a N, Option<&'a 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![(pkg, None)];
+        let first_pkg_depending_on = |pkg, res: &[(&N, Option<&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)
+                .find(|&(node, _)| !res.iter().any(|p| p.0 == node))
+                .map(|(p, adjacent)| (p, adjacent.get(pkg)))
         };
         while let Some(p) = first_pkg_depending_on(pkg, &result) {
             result.push(p);
-            pkg = p;
+            pkg = p.0;
         }
         result
     }

tests/testsuite/build_script.rs

@@ -1000,7 +1000,7 @@ fn links_duplicates_old_registry() {
     but a native library can be linked only once
 
 package `bar v0.1.0`
-    ... which is depended on by `foo v0.1.0 ([..]foo)`
+    ... which satisfies dependency `bar = \"=0.1.0\"` of package `foo v0.1.0 ([..]foo)`
 links to native library `a`
 
 package `foo v0.1.0 ([..]foo)`