Auto merge of #8020 - xiongmao86:issue7853, r=ehuss

Checking for binary that is built as an implicit dependency of an integration test.

When a project containing binary with required-features, the binary will only be built with the given dependency's feature is enabled. But this feature syntax is not checked when the binary is built as an implicit dependency of an integration test. This pr addresses this issue.

More info is in the issue page: this pr fixes #7853.

Author: bors

src/cargo/core/compiler/unit_dependencies.rs

@@ -23,6 +23,7 @@ use crate::core::profiles::{Profile, UnitFor};
 use crate::core::resolver::features::{FeaturesFor, ResolvedFeatures};
 use crate::core::resolver::Resolve;
 use crate::core::{InternedString, Package, PackageId, Target};
+use crate::ops::resolve_all_features;
 use crate::CargoResult;
 use log::trace;
 use std::collections::{HashMap, HashSet};
@@ -309,13 +310,20 @@ fn compute_deps<'a, 'cfg>(
                 .targets()
                 .iter()
                 .filter(|t| {
-                    let no_required_features = Vec::new();
-
-                    t.is_bin() &&
-                        // Skip binaries with required features that have not been selected.
-                        t.required_features().unwrap_or(&no_required_features).iter().all(|f| {
-                            unit.features.contains(&InternedString::new(f.as_str()))
-                        })
+                    // Skip binaries with required features that have not been selected.
+                    match t.required_features() {
+                        Some(rf) if t.is_bin() => {
+                            let features = resolve_all_features(
+                                state.resolve(),
+                                state.features(),
+                                bcx.packages,
+                                id,
+                            );
+                            rf.iter().all(|f| features.contains(f))
+                        }
+                        None if t.is_bin() => true,
+                        _ => false,
+                    }
                 })
                 .map(|t| {
                     new_unit_dep(
@@ -692,16 +700,20 @@ impl<'a, 'cfg> State<'a, 'cfg> {
         }
     }
 
+    fn features(&self) -> &'a ResolvedFeatures {
+        if self.is_std {
+            self.std_features.unwrap()
+        } else {
+            self.usr_features
+        }
+    }
+
     fn activated_features(
         &self,
         pkg_id: PackageId,
         features_for: FeaturesFor,
     ) -> Vec<InternedString> {
-        let features = if self.is_std {
-            self.std_features.unwrap()
-        } else {
-            self.usr_features
-        };
+        let features = self.features();
         features.activated_features(pkg_id, features_for)
     }
 

src/cargo/ops/cargo_compile.rs

@@ -949,7 +949,7 @@ fn generate_targets<'a>(
 ///
 /// Dependencies are added as `dep_name/feat_name` because `required-features`
 /// wants to support that syntax.
-fn resolve_all_features(
+pub fn resolve_all_features(
     resolve_with_overrides: &Resolve,
     resolved_features: &features::ResolvedFeatures,
     package_set: &PackageSet<'_>,

src/cargo/ops/mod.rs

@@ -1,5 +1,7 @@
 pub use self::cargo_clean::{clean, CleanOptions};
-pub use self::cargo_compile::{compile, compile_with_exec, compile_ws, CompileOptions};
+pub use self::cargo_compile::{
+    compile, compile_with_exec, compile_ws, resolve_all_features, CompileOptions,
+};
 pub use self::cargo_compile::{CompileFilter, FilterRule, LibRule, Packages};
 pub use self::cargo_doc::{doc, DocOptions};
 pub use self::cargo_fetch::{fetch, FetchOptions};

tests/testsuite/required_features.rs

@@ -4,6 +4,7 @@ use cargo_test_support::install::{
     assert_has_installed_exe, assert_has_not_installed_exe, cargo_home,
 };
 use cargo_test_support::is_nightly;
+use cargo_test_support::paths::CargoPathExt;
 use cargo_test_support::project;
 
 #[cargo_test]
@@ -910,7 +911,17 @@ fn dep_feature_in_cmd_line() {
         )
         .file("src/main.rs", "fn main() {}")
         .file("examples/foo.rs", "fn main() {}")
-        .file("tests/foo.rs", "#[test]\nfn test() {}")
+        .file(
+            "tests/foo.rs",
+            r#"
+            #[test]
+            fn bin_is_built() {
+                let s = format!("target/debug/foo{}", std::env::consts::EXE_SUFFIX);
+                let p = std::path::Path::new(&s);
+                assert!(p.exists(), "foo does not exist");
+            }
+            "#,
+        )
         .file(
             "benches/foo.rs",
             r#"
@@ -936,7 +947,9 @@ fn dep_feature_in_cmd_line() {
         .file("bar/src/lib.rs", "")
         .build();
 
-    p.cargo("build").run();
+    // This is a no-op
+    p.cargo("build").with_stderr("[FINISHED] dev [..]").run();
+    assert!(!p.bin("foo").is_file());
 
     // bin
     p.cargo("build --bin=foo")
@@ -967,19 +980,23 @@ Consider enabling them by passing, e.g., `--features=\"bar/a\"`
     assert!(p.bin("examples/foo").is_file());
 
     // test
+    // This is a no-op, since no tests are enabled
     p.cargo("test")
         .with_stderr("[FINISHED] test [unoptimized + debuginfo] target(s) in [..]")
         .with_stdout("")
         .run();
 
+    // Delete the target directory so this can check if the main.rs gets built.
+    p.build_dir().rm_rf();
     p.cargo("test --test=foo --features bar/a")
         .with_stderr(
             "\
+[COMPILING] bar v0.0.1 ([CWD]/bar)
 [COMPILING] foo v0.0.1 ([CWD])
 [FINISHED] test [unoptimized + debuginfo] target(s) in [..]
 [RUNNING] target/debug/deps/foo-[..][EXE]",
         )
-        .with_stdout_contains("test test ... ok")
+        .with_stdout_contains("test bin_is_built ... ok")
         .run();
 
     // bench