From 235d9580a28f2548844e8b0c8904dcd0192cf96c Mon Sep 17 00:00:00 2001
From: bors <bors@rust-lang.org>
Date: Thu, 11 Mar 2021 19:57:49 +0000
Subject: [PATCH] Auto merge of #9255 - ehuss:fix-deps-filtering, r=Eh2406

Fix issue with filtering exclusive target dependencies.

#8777 incorrectly changed the filtering logic for dependencies.  Essentially it split `filter(any(A && B && C && D))` into two parts `filter(any(A && B)).filter(any(C && D))` which doesn't have the same meaning. The solution here is to pass a closure so that the conditions are joined again.

Fixes #9216
---
 src/cargo/core/compiler/unit_dependencies.rs | 60 ++++++++++----------
 tests/testsuite/cfg.rs                       | 33 +++++++++++
 2 files changed, 63 insertions(+), 30 deletions(-)

diff --git a/src/cargo/core/compiler/unit_dependencies.rs b/src/cargo/core/compiler/unit_dependencies.rs
index 9445bb8861e..24c683337de 100644
--- a/src/cargo/core/compiler/unit_dependencies.rs
+++ b/src/cargo/core/compiler/unit_dependencies.rs
@@ -235,33 +235,28 @@ fn compute_deps(
     }
 
     let id = unit.pkg.package_id();
-    let filtered_deps = state
-        .deps(unit, unit_for)
-        .into_iter()
-        .filter(|&(_id, deps)| {
-            deps.iter().any(|dep| {
-                // If this target is a build command, then we only want build
-                // dependencies, otherwise we want everything *other than* build
-                // dependencies.
-                if unit.target.is_custom_build() != dep.is_build() {
-                    return false;
-                }
+    let filtered_deps = state.deps(unit, unit_for, &|dep| {
+        // If this target is a build command, then we only want build
+        // dependencies, otherwise we want everything *other than* build
+        // dependencies.
+        if unit.target.is_custom_build() != dep.is_build() {
+            return false;
+        }
 
-                // If this dependency is **not** a transitive dependency, then it
-                // only applies to test/example targets.
-                if !dep.is_transitive()
-                    && !unit.target.is_test()
-                    && !unit.target.is_example()
-                    && !unit.mode.is_any_test()
-                {
-                    return false;
-                }
+        // If this dependency is **not** a transitive dependency, then it
+        // only applies to test/example targets.
+        if !dep.is_transitive()
+            && !unit.target.is_test()
+            && !unit.target.is_example()
+            && !unit.mode.is_any_test()
+        {
+            return false;
+        }
 
-                // If we've gotten past all that, then this dependency is
-                // actually used!
-                true
-            })
-        });
+        // If we've gotten past all that, then this dependency is
+        // actually used!
+        true
+    });
 
     let mut ret = Vec::new();
     let mut dev_deps = Vec::new();
@@ -419,10 +414,7 @@ fn compute_deps_doc(
     state: &mut State<'_, '_>,
     unit_for: UnitFor,
 ) -> CargoResult<Vec<UnitDep>> {
-    let deps = state
-        .deps(unit, unit_for)
-        .into_iter()
-        .filter(|&(_id, deps)| deps.iter().any(|dep| dep.kind() == DepKind::Normal));
+    let deps = state.deps(unit, unit_for, &|dep| dep.kind() == DepKind::Normal);
 
     // To document a library, we depend on dependencies actually being
     // built. If we're documenting *all* libraries, then we also depend on
@@ -780,7 +772,12 @@ impl<'a, 'cfg> State<'a, 'cfg> {
     }
 
     /// Returns a filtered set of dependencies for the given unit.
-    fn deps(&self, unit: &Unit, unit_for: UnitFor) -> Vec<(PackageId, &HashSet<Dependency>)> {
+    fn deps(
+        &self,
+        unit: &Unit,
+        unit_for: UnitFor,
+        filter: &dyn Fn(&Dependency) -> bool,
+    ) -> Vec<(PackageId, &HashSet<Dependency>)> {
         let pkg_id = unit.pkg.package_id();
         let kind = unit.kind;
         self.resolve()
@@ -788,6 +785,9 @@ impl<'a, 'cfg> State<'a, 'cfg> {
             .filter(|&(_id, deps)| {
                 assert!(!deps.is_empty());
                 deps.iter().any(|dep| {
+                    if !filter(dep) {
+                        return false;
+                    }
                     // If this dependency is only available for certain platforms,
                     // make sure we're only enabling it for that platform.
                     if !self.target_data.dep_platform_activated(dep, kind) {
diff --git a/tests/testsuite/cfg.rs b/tests/testsuite/cfg.rs
index c8c1559824e..3f79db77275 100644
--- a/tests/testsuite/cfg.rs
+++ b/tests/testsuite/cfg.rs
@@ -440,3 +440,36 @@ Caused by:
         )
         .run();
 }
+
+#[cargo_test]
+fn exclusive_dep_kinds() {
+    // Checks for a bug where the same package with different cfg expressions
+    // was not being filtered correctly.
+    Package::new("bar", "1.0.0").publish();
+    let p = project()
+        .file(
+            "Cargo.toml",
+            r#"
+                [package]
+                name = "foo"
+                version = "0.1.0"
+
+                [target.'cfg(abc)'.dependencies]
+                bar = "1.0"
+
+                [target.'cfg(not(abc))'.build-dependencies]
+                bar = "1.0"
+            "#,
+        )
+        .file("src/lib.rs", "")
+        .file("build.rs", "extern crate bar; fn main() {}")
+        .build();
+
+    p.cargo("check").run();
+    p.change_file("src/lib.rs", "extern crate bar;");
+    p.cargo("check")
+        .with_status(101)
+        // can't find crate for `bar`
+        .with_stderr_contains("[..]E0463[..]")
+        .run();
+}