Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve message for multiple links to native lib #4515

Merged
merged 2 commits into from
Sep 21, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions src/cargo/core/resolver/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
41 changes: 28 additions & 13 deletions src/cargo/ops/cargo_rustc/links.rs
Original file line number Diff line number Diff line change
@@ -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;

Expand All @@ -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(())
}
Expand All @@ -27,17 +28,31 @@ 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("The root-package ")
} else {
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();
}
dep_path_desc
}
};

bail!("Multiple packages link to native library `{}`. \
A native library can be linked only once.\n\
\n\
{}links to native library `{}`.\n\
\n\
{}also links to native library `{}`.",
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 \
Expand Down
2 changes: 1 addition & 1 deletion src/cargo/ops/cargo_rustc/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)?
Expand Down
69 changes: 63 additions & 6 deletions tests/build-script.rs
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,47 @@ 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] Multiple packages link to native library `a`. A native library can be \
linked only once.

The root-package links to native library `a`.

Package `a-sys v0.5.0 (file://[..])`
... which is depended on by `foo v0.5.0 (file://[..])`
also links to native library `a`.
"));
}

#[test]
fn links_duplicates_deep_dependency() {
let p = project("foo")
.file("Cargo.toml", r#"
[project]
Expand All @@ -239,20 +280,36 @@ 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] Multiple packages link to native library `a`. A native library can be \
linked only once.

The root-package links to native library `a`.

[..] v0.5.0 (file://[..])
[..] v0.5.0 (file://[..])
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`.
"));
}

Expand Down