From: Alex Crichton Date: Sun, 10 Dec 2017 18:14:23 +0000 (-0800) Subject: Fix an infinite loop in error reporting X-Git-Tag: archive/raspbian/0.35.0-2+rpi1~3^2^2^2^2^2^2^2~22^2~4^2~26^2 X-Git-Url: https://dgit.raspbian.org/?a=commitdiff_plain;h=4f0b8f88085572f4f888fb49780ff0a76ce42ac4;p=cargo.git Fix an infinite loop in error reporting The `path_to_root` function unfortunately didn't account for cycles in the dependency graph introduced through dev-dependencies, so if a cycle was present then the function would infinitely loop pushing items onto a vector. This commit fixes the infinite loop and also touches up the reporting to be a little more consistent with the rest of Cargo --- diff --git a/src/cargo/core/resolver/mod.rs b/src/cargo/core/resolver/mod.rs index 9ce0a9e2f..712808391 100644 --- a/src/cargo/core/resolver/mod.rs +++ b/src/cargo/core/resolver/mod.rs @@ -118,21 +118,26 @@ 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; - while let Some(pulling) = self.graph - .get_nodes() - .iter() - .filter_map(|(pulling, pulled)| - if pulled.contains(pkg) { - Some(pulling) - } else { - None - }) - .nth(0) { - result.push(pulling); - pkg = pulling; + pub fn path_to_top<'a>(&'a self, mut pkg: &'a PackageId) -> Vec<&'a PackageId> { + // 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: &PackageId| { + self.graph.get_nodes() + .iter() + .filter(|&(_node, adjacent)| adjacent.contains(pkg)) + .next() + .map(|p| p.0) + }; + while let Some(p) = first_pkg_depending_on(pkg) { + // Note that we can have "cycles" introduced through dev-dependency + // edges, so make sure we don't loop infinitely. + if result.contains(&p) { + break + } + result.push(p); + pkg = p; } result } diff --git a/src/cargo/ops/cargo_rustc/links.rs b/src/cargo/ops/cargo_rustc/links.rs index 79bb240cf..5484a68b9 100644 --- a/src/cargo/ops/cargo_rustc/links.rs +++ b/src/cargo/ops/cargo_rustc/links.rs @@ -31,25 +31,21 @@ 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("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 + 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 }; - bail!("Multiple packages link to native library `{}`. \ - A native library can be linked only once.\n\ + bail!("multiple packages link to native library `{}`, \ + but a native library can be linked only once\n\ \n\ - {}links to native library `{}`.\n\ + {}\nlinks to native library `{}`\n\ \n\ - {}also links to native library `{}`.", + {}\nalso links to native library `{}`", lib, describe_path(prev), lib, describe_path(pkg), lib) diff --git a/tests/build-script.rs b/tests/build-script.rs index 3644235a3..145db6902 100644 --- a/tests/build-script.rs +++ b/tests/build-script.rs @@ -254,14 +254,15 @@ fn links_duplicates() { assert_that(p.cargo("build"), execs().with_status(101) .with_stderr("\ -[ERROR] Multiple packages link to native library `a`. A native library can be \ -linked only once. +[ERROR] multiple packages link to native library `a`, but a native library can \ +be linked only once -The root-package links to native library `a`. +package `foo v0.5.0 ([..])` +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`. +package `a-sys v0.5.0 ([..])` + ... which is depended on by `foo v0.5.0 ([..])` +also links to native library `a` ")); } @@ -308,15 +309,16 @@ fn links_duplicates_deep_dependency() { assert_that(p.cargo("build"), execs().with_status(101) .with_stderr("\ -[ERROR] Multiple packages link to native library `a`. A native library can be \ -linked only once. +[ERROR] multiple packages link to native library `a`, but a native library can \ +be linked only once -The root-package links to native library `a`. +package `foo v0.5.0 ([..])` +links to native library `a` -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`. +package `a-sys v0.5.0 ([..])` + ... which is depended on by `a v0.5.0 ([..])` + ... which is depended on by `foo v0.5.0 ([..])` +also links to native library `a` ")); } @@ -2734,3 +2736,61 @@ fn deterministic_rustc_dependency_flags() { -L native=test3 -L native=test4` ")); } + +#[test] +fn links_duplicates_with_cycle() { + let p = project("foo") + .file("Cargo.toml", r#" + [project] + name = "foo" + version = "0.5.0" + authors = [] + links = "a" + build = "build.rs" + + [dependencies.a] + path = "a" + + [dev-dependencies] + b = { path = "b" } + "#) + .file("src/lib.rs", "") + .file("build.rs", "") + .file("a/Cargo.toml", r#" + [project] + name = "a" + version = "0.5.0" + authors = [] + links = "a" + build = "build.rs" + "#) + .file("a/src/lib.rs", "") + .file("a/build.rs", "") + .file("b/Cargo.toml", r#" + [project] + name = "b" + version = "0.5.0" + authors = [] + + [dependencies] + foo = { path = ".." } + "#) + .file("b/src/lib.rs", "") + .build(); + + assert_that(p.cargo("build"), + execs().with_status(101) + .with_stderr("\ +[ERROR] multiple packages link to native library `a`, but a native library can \ +be linked only once + +package `foo v0.5.0 ([..])` + ... which is depended on by `b v0.5.0 ([..])` +links to native library `a` + +package `a v0.5.0 (file://[..])` + ... which is depended on by `foo v0.5.0 ([..])` + ... which is depended on by `b v0.5.0 ([..])` +also links to native library `a` +")); +}