From 180b448e0cda42e730ce5f8a0a03a67107969c75 Mon Sep 17 00:00:00 2001 From: Lukas Lueg Date: Wed, 20 Sep 2017 21:28:30 +0200 Subject: [PATCH 1/2] 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 --- src/cargo/core/resolver/mod.rs | 25 +++++++++++ src/cargo/ops/cargo_rustc/links.rs | 51 +++++++++++++++------ src/cargo/ops/cargo_rustc/mod.rs | 2 +- tests/build-script.rs | 72 +++++++++++++++++++++++++++--- 4 files changed, 130 insertions(+), 20 deletions(-) diff --git a/src/cargo/core/resolver/mod.rs b/src/cargo/core/resolver/mod.rs index b720f5f4de4..a2f4e11392b 100644 --- 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>) { 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 779cd13739b..7c8d251a649 100644 --- 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 02fa67727a5..77e4542aaaa 100644 --- 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 b9dee48693b..fd0dc95503e 100644 --- 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[..] ")); } From 45b4a55ffc2f9fd9d5f63513f2f35fdeb21fbf48 Mon Sep 17 00:00:00 2001 From: Lukas Lueg Date: Thu, 21 Sep 2017 17:08:51 +0200 Subject: [PATCH 2/2] Further improve doubly-linked error-msg --- src/cargo/ops/cargo_rustc/links.rs | 36 +++++++++++------------------- tests/build-script.rs | 29 +++++++++++------------- 2 files changed, 26 insertions(+), 39 deletions(-) diff --git a/src/cargo/ops/cargo_rustc/links.rs b/src/cargo/ops/cargo_rustc/links.rs index 7c8d251a649..79bb240cf82 100644 --- a/src/cargo/ops/cargo_rustc/links.rs +++ b/src/cargo/ops/cargo_rustc/links.rs @@ -32,37 +32,27 @@ impl<'a> Links<'a> { 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)") + String::from("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(" => "); - } + let mut dep_path_desc = format!("Package `{}`\n", pkgid); + for dep in dep_path { + write!(dep_path_desc, + " ... which is depended on by `{}`\n", + dep).unwrap(); } - pkg_path_desc.push(')'); - pkg_path_desc + dep_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\ + bail!("Multiple packages link to native library `{}`. \ + A native library can be linked only once.\n\ \n\ - Package {} also links to native library `{}`.\n\ - {}\n\ + {}links to native library `{}`.\n\ \n\ - Try updating or pinning your dependencies to ensure that \ - native library `{}` is only linked once.", + {}also links to native library `{}`.", lib, - prev, lib, - describe_path(prev), - pkg, lib, - describe_path(pkg), - lib) + describe_path(prev), 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/tests/build-script.rs b/tests/build-script.rs index fd0dc95503e..4e7a5816f69 100644 --- a/tests/build-script.rs +++ b/tests/build-script.rs @@ -248,16 +248,14 @@ fn links_duplicates() { 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. +[ERROR] Multiple packages link to native library `a`. A native library can be \ +linked only once. -Package foo v0.5.0 (file://[..]) links to native library `a`. -(This is the root-package) +The root-package links to native library `a`. -Package a-sys v0.5.0 (file://[..]) also links to native library `a`. -(Dependency via foo v0.5.0 (file://[..])) - -Try updating[..] +Package `a-sys v0.5.0 (file://[..])` + ... which is depended on by `foo v0.5.0 (file://[..])` +also links to native library `a`. ")); } @@ -303,16 +301,15 @@ fn links_duplicates_deep_dependency() { 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) +[ERROR] Multiple packages link to native library `a`. A native library can be \ +linked only once. -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://[..])) +The root-package links to native library `a`. -Try updating[..] +Package `a-sys v0.5.0 (file://[..])` + ... which is depended on by `a v0.5.0 (file://[..])` + ... which is depended on by `foo v0.5.0 (file://[..])` +also links to native library `a`. ")); }