From 4761ada30fde61ff7df4cf56e310da43df15265c Mon Sep 17 00:00:00 2001
From: Alex Crichton <alex@alexcrichton.com>
Date: Fri, 11 Dec 2020 09:57:49 -0800
Subject: [PATCH] Fix the unit dependency graph with dev-dependency `links`

This commit fixes #8966 by updating the unit generation logic to avoid
generating an erroneous circular dependency between the execution of two
build scripts. This has been present for Cargo in a long time since #5651
(an accidental regression), but the situation appears rare enough that
we didn't get to it until now!

Closes #8966
---
 src/cargo/core/compiler/unit_dependencies.rs | 14 ++++++--
 tests/testsuite/build_script.rs              | 37 ++++++++++++++++++++
 2 files changed, 49 insertions(+), 2 deletions(-)

diff --git a/src/cargo/core/compiler/unit_dependencies.rs b/src/cargo/core/compiler/unit_dependencies.rs
index f0dd340cc99..ee184dace36 100644
--- a/src/cargo/core/compiler/unit_dependencies.rs
+++ b/src/cargo/core/compiler/unit_dependencies.rs
@@ -630,8 +630,17 @@ fn connect_run_custom_build_deps(unit_dependencies: &mut UnitGraph) {
         // example a library might depend on a build script, so this map will
         // have the build script as the key and the library would be in the
         // value's set.
+        //
+        // Note that as an important part here we're skipping "test" units. Test
+        // units depend on the execution of a build script, but
+        // links-dependencies only propagate through `[dependencies]`, nothing
+        // else. We don't want to pull in a links-dependency through a
+        // dev-dependency since that could create a cycle.
         let mut reverse_deps_map = HashMap::new();
         for (unit, deps) in unit_dependencies.iter() {
+            if unit.mode.is_any_test() {
+                continue;
+            }
             for dep in deps {
                 if dep.unit.mode == CompileMode::RunCustomBuild {
                     reverse_deps_map
@@ -655,7 +664,8 @@ fn connect_run_custom_build_deps(unit_dependencies: &mut UnitGraph) {
             .keys()
             .filter(|k| k.mode == CompileMode::RunCustomBuild)
         {
-            // This is the lib that runs this custom build.
+            // This list of dependencies all depend on `unit`, an execution of
+            // the build script.
             let reverse_deps = match reverse_deps_map.get(unit) {
                 Some(set) => set,
                 None => continue,
@@ -663,7 +673,7 @@ fn connect_run_custom_build_deps(unit_dependencies: &mut UnitGraph) {
 
             let to_add = reverse_deps
                 .iter()
-                // Get all deps for lib.
+                // Get all sibling dependencies of `unit`
                 .flat_map(|reverse_dep| unit_dependencies[reverse_dep].iter())
                 // Only deps with `links`.
                 .filter(|other| {
diff --git a/tests/testsuite/build_script.rs b/tests/testsuite/build_script.rs
index ff759ccb40f..40b61b90b87 100644
--- a/tests/testsuite/build_script.rs
+++ b/tests/testsuite/build_script.rs
@@ -4034,3 +4034,40 @@ Caused by:
     // Restore permissions so that the directory can be deleted.
     fs::set_permissions(&path, fs::Permissions::from_mode(0o755)).unwrap();
 }
+
+#[cargo_test]
+fn dev_dep_with_links() {
+    let p = project()
+        .file(
+            "Cargo.toml",
+            r#"
+                [package]
+                name = "foo"
+                version = "0.1.0"
+                authors = []
+                links = "x"
+
+                [dev-dependencies]
+                bar = { path = "./bar" }
+            "#,
+        )
+        .file("build.rs", "fn main() {}")
+        .file("src/lib.rs", "")
+        .file(
+            "bar/Cargo.toml",
+            r#"
+                [package]
+                name = "bar"
+                version = "0.1.0"
+                authors = []
+                links = "y"
+
+                [dependencies]
+                foo = { path = ".." }
+            "#,
+        )
+        .file("bar/build.rs", "fn main() {}")
+        .file("bar/src/lib.rs", "")
+        .build();
+    p.cargo("check --tests").run()
+}