Improve message for multiple links to native lib

If a native library is linked multiple times, present the user with a
clear error message, indicating the offending packages and their
dependency-chain.

Fixes #1006

diff --git a/src/cargo/core/resolver/mod.rs b/src/cargo/core/resolver/mod.rs
index b720f5f4de4fed0d85c7222d2f4ea3b9c8cc5eb6..a2f4e11392b737db589f56e5d84d271e882d0d06 100644 (file)
--- a/src/cargo/core/resolver/mod.rs
+++ b/src/cargo/core/resolver/mod.rs
@@ -114,6 +114,31 @@ struct Candidate {
 }
 
 impl Resolve {
+    /// Resolves one of the paths from the given dependent package up to
+    /// the root.
+    pub fn path_to_top(&self, pkg: &PackageId) -> Vec<&PackageId> {
+        let mut result = Vec::new();
+        let mut pkg = pkg;
+        loop {
+            match self.graph
+                  .get_nodes()
+                  .iter()
+                  .filter_map(|(pulling, pulled)|
+                              if pulled.contains(pkg) {
+                                  Some(pulling)
+                              } else {
+                                  None
+                              })
+                  .nth(0) {
+                Some(pulling) => {
+                    result.push(pulling);
+                    pkg = pulling;
+                },
+                None => break
+            }
+        }
+        result
+    }
     pub fn register_used_patches(&mut self,
                                  patches: &HashMap<Url, Vec<Summary>>) {
         for summary in patches.values().flat_map(|v| v) {

diff --git a/src/cargo/ops/cargo_rustc/links.rs b/src/cargo/ops/cargo_rustc/links.rs
index 779cd13739bb34b1686815cd98659e809a31073f..7c8d251a64916b10bf12008b172e56a742c87be2 100644 (file)
--- a/src/cargo/ops/cargo_rustc/links.rs
+++ b/src/cargo/ops/cargo_rustc/links.rs
@@ -1,6 +1,7 @@
 use std::collections::{HashMap, HashSet};
+use std::fmt::Write;
 
-use core::PackageId;
+use core::{Resolve, PackageId};
 use util::CargoResult;
 use super::Unit;
 
@@ -17,7 +18,7 @@ impl<'a> Links<'a> {
         }
     }
 
-    pub fn validate(&mut self, unit: &Unit<'a>) -> CargoResult<()> {
+    pub fn validate(&mut self, resolve: &Resolve, unit: &Unit<'a>) -> CargoResult<()> {
         if !self.validated.insert(unit.pkg.package_id()) {
             return Ok(())
         }
@@ -27,17 +28,41 @@ impl<'a> Links<'a> {
         };
         if let Some(prev) = self.links.get(lib) {
             let pkg = unit.pkg.package_id();
-            if prev.name() == pkg.name() && prev.source_id() == pkg.source_id() {
-                bail!("native library `{}` is being linked to by more \
-                       than one version of the same package, but it can \
-                       only be linked once; try updating or pinning your \
-                       dependencies to ensure that this package only shows \
-                       up once\n\n  {}\n  {}", lib, prev, pkg)
-            } else {
-                bail!("native library `{}` is being linked to by more than \
-                       one package, and can only be linked to by one \
-                       package\n\n  {}\n  {}", lib, prev, pkg)
-            }
+
+            let describe_path = |pkgid: &PackageId| -> String {
+                let dep_path = resolve.path_to_top(pkgid);
+                if dep_path.is_empty() {
+                    String::from("(This is the root-package)")
+                } else {
+                    let mut pkg_path_desc = String::from("(Dependency via ");
+                    let mut dep_path_iter = dep_path.into_iter().peekable();
+                    while let Some(dep) = dep_path_iter.next() {
+                        write!(pkg_path_desc, "{}", dep).unwrap();
+                        if dep_path_iter.peek().is_some() {
+                            pkg_path_desc.push_str(" => ");
+                        }
+                    }
+                    pkg_path_desc.push(')');
+                    pkg_path_desc
+                }
+            };
+
+            bail!("More than one package links to native library `{}`, \
+                   which can only be linked once.\n\n\
+                   Package {} links to native library `{}`.\n\
+                   {}\n\
+                   \n\
+                   Package {} also links to native library `{}`.\n\
+                   {}\n\
+                   \n\
+                   Try updating or pinning your dependencies to ensure that \
+                   native library `{}` is only linked once.",
+                  lib,
+                  prev, lib,
+                  describe_path(prev),
+                  pkg, lib,
+                  describe_path(pkg),
+                  lib)
         }
         if !unit.pkg.manifest().targets().iter().any(|t| t.is_custom_build()) {
             bail!("package `{}` specifies that it links to `{}` but does not \

diff --git a/src/cargo/ops/cargo_rustc/mod.rs b/src/cargo/ops/cargo_rustc/mod.rs
index 02fa67727a55bb3b9aa4dacee69c5febfa70d6f2..77e4542aaaa9c8389572f5321f0304adb00725eb 100644 (file)
--- a/src/cargo/ops/cargo_rustc/mod.rs
+++ b/src/cargo/ops/cargo_rustc/mod.rs
@@ -237,7 +237,7 @@ fn compile<'a, 'cfg: 'a>(cx: &mut Context<'a, 'cfg>,
     let p = profile::start(format!("preparing: {}/{}", unit.pkg,
                                    unit.target.name()));
     fingerprint::prepare_init(cx, unit)?;
-    cx.links.validate(unit)?;
+    cx.links.validate(&cx.resolve, unit)?;
 
     let (dirty, fresh, freshness) = if unit.profile.run_custom_build {
         custom_build::prepare(cx, unit)?

diff --git a/tests/build-script.rs b/tests/build-script.rs
index b9dee48693b51ac5d933265cd1d5152e84d2341b..fd0dc95503efabc2ca481d1ddf64e2124053524b 100644 (file)
--- a/tests/build-script.rs
+++ b/tests/build-script.rs
@@ -220,6 +220,49 @@ not have a custom build script
 
 #[test]
 fn links_duplicates() {
+    let p = project("foo")
+        .file("Cargo.toml", r#"
+            [project]
+            name = "foo"
+            version = "0.5.0"
+            authors = []
+            links = "a"
+            build = "build.rs"
+
+            [dependencies.a-sys]
+            path = "a-sys"
+        "#)
+        .file("src/lib.rs", "")
+        .file("build.rs", "")
+        .file("a-sys/Cargo.toml", r#"
+            [project]
+            name = "a-sys"
+            version = "0.5.0"
+            authors = []
+            links = "a"
+            build = "build.rs"
+        "#)
+        .file("a-sys/src/lib.rs", "")
+        .file("a-sys/build.rs", "");
+
+    assert_that(p.cargo_process("build"),
+                execs().with_status(101)
+                       .with_stderr("\
+error: More than one package links to native library `a`, which can only be \
+linked once.
+
+Package foo v0.5.0 (file://[..]) links to native library `a`.
+(This is the root-package)
+
+Package a-sys v0.5.0 (file://[..]) also links to native library `a`.
+(Dependency via foo v0.5.0 (file://[..]))
+
+Try updating[..]
+"));
+}
+
+#[test]
+fn links_duplicates_deep_dependency() {
     let p = project("foo")
         .file("Cargo.toml", r#"
             [project]
@@ -239,20 +282,37 @@ fn links_duplicates() {
             name = "a"
             version = "0.5.0"
             authors = []
-            links = "a"
             build = "build.rs"
+
+            [dependencies.a-sys]
+            path = "a-sys"
         "#)
         .file("a/src/lib.rs", "")
-        .file("a/build.rs", "");
+        .file("a/build.rs", "")
+        .file("a/a-sys/Cargo.toml", r#"
+            [project]
+            name = "a-sys"
+            version = "0.5.0"
+            authors = []
+            links = "a"
+            build = "build.rs"
+        "#)
+        .file("a/a-sys/src/lib.rs", "")
+        .file("a/a-sys/build.rs", "");
 
     assert_that(p.cargo_process("build"),
                 execs().with_status(101)
                        .with_stderr("\
-[ERROR] native library `a` is being linked to by more than one package, and can only be \
-linked to by one package
+error: More than one package links to native library `a`, which can only be \
+linked once.
+
+Package foo v0.5.0 (file://[..]) links to native library `a`.
+(This is the root-package)
+
+Package a-sys v0.5.0 (file://[..]) also links to native library `a`.
+(Dependency via a v0.5.0 (file://[..]) => foo v0.5.0 (file://[..]))
 
-  [..] v0.5.0 (file://[..])
-  [..] v0.5.0 (file://[..])
+Try updating[..]
 "));
 }