Auto merge of #8048 - ehuss:fix-features-for-host, r=alexcrichton

Fix -Zfeatures=itarget with certain host dependencies

In some cases, `-Zfeatures=itarget` can panic because an entry is missing in the activation map. This happens because the way "for_host" was tracked when building the activation map. Previously "for_host" was only set when `-Zfeatures=host_dep` was specified. However, if you don't specify `host_dep`, then "for_host" was always false.

This is a problem because `itarget` needs to also be able to detect if something is enabled for the host or target. If you have a proc-macro ("for_host"), and it has a dependency with a `[target]` specification that matched the host, then the activation map would fail to include it (because the "for_host" flag was not "sticky" and didn't get passed down).

The solution is to always carry the "for_host" setting around while building the activation map. Only when inserting a feature into the map will it check if `opts.decouple_host_deps` is set. This allows it to handle the "for_host" setting correctly, even if the `host_dep` option isn't enabled.

This was discovered at #7914 (comment) where a dependency on `hashbrown` would fail if you pass `--target something_not_unix` because it has a unix-only dependency for `libc`.

(Sorry, long-winded explanation. Please ask if that is confusing.)

Author: bors

src/cargo/core/resolver/features.rs

@@ -298,7 +298,7 @@ impl<'a, 'cfg> FeatureResolver<'a, 'cfg> {
         let member_features = self.ws.members_with_features(specs, requested_features)?;
         for (member, requested_features) in &member_features {
             let fvs = self.fvs_from_requested(member.package_id(), requested_features);
-            let for_host = self.opts.decouple_host_deps && self.is_proc_macro(member.package_id());
+            let for_host = self.is_proc_macro(member.package_id());
             self.activate_pkg(member.package_id(), &fvs, for_host)?;
             if for_host {
                 // Also activate without for_host. This is needed if the
@@ -324,7 +324,7 @@ impl<'a, 'cfg> FeatureResolver<'a, 'cfg> {
         // finding bugs where the resolver missed something it should have visited.
         // Remove this in the future if `activated_features` uses an empty default.
         self.activated_features
-            .entry((pkg_id, for_host))
+            .entry((pkg_id, self.opts.decouple_host_deps && for_host))
             .or_insert_with(BTreeSet::new);
         for fv in fvs {
             self.activate_fv(pkg_id, fv, for_host)?;
@@ -418,7 +418,7 @@ impl<'a, 'cfg> FeatureResolver<'a, 'cfg> {
     ) -> CargoResult<()> {
         let enabled = self
             .activated_features
-            .entry((pkg_id, for_host))
+            .entry((pkg_id, self.opts.decouple_host_deps && for_host))
             .or_insert_with(BTreeSet::new);
         if !enabled.insert(feature_to_enable) {
             // Already enabled.
@@ -541,8 +541,7 @@ impl<'a, 'cfg> FeatureResolver<'a, 'cfg> {
                         true
                     })
                     .map(|dep| {
-                        let dep_for_host = for_host
-                            || (self.opts.decouple_host_deps && (dep.is_build() || is_proc_macro));
+                        let dep_for_host = for_host || dep.is_build() || is_proc_macro;
                         (dep, dep_for_host)
                     })
                     .collect::<Vec<_>>();

tests/testsuite/features2.rs

@@ -1,8 +1,9 @@
 //! Tests for the new feature resolver.
 
+use cargo_test_support::cross_compile::{self, alternate};
 use cargo_test_support::paths::CargoPathExt;
 use cargo_test_support::registry::{Dependency, Package};
-use cargo_test_support::{basic_manifest, project};
+use cargo_test_support::{basic_manifest, project, rustc_host};
 
 #[cargo_test]
 fn inactivate_targets() {
@@ -183,6 +184,49 @@ fn inactive_target_optional() {
         .run();
 }
 
+#[cargo_test]
+fn itarget_proc_macro() {
+    // itarget inside a proc-macro while cross-compiling
+    if cross_compile::disabled() {
+        return;
+    }
+    Package::new("hostdep", "1.0.0").publish();
+    Package::new("pm", "1.0.0")
+        .proc_macro(true)
+        .target_dep("hostdep", "1.0", &rustc_host())
+        .file("src/lib.rs", "extern crate hostdep;")
+        .publish();
+    let p = project()
+        .file(
+            "Cargo.toml",
+            r#"
+            [package]
+            name = "foo"
+            version = "0.1.0"
+
+            [dependencies]
+            pm = "1.0"
+            "#,
+        )
+        .file("src/lib.rs", "")
+        .build();
+
+    p.cargo("check").run();
+    p.cargo("check -Zfeatures=itarget")
+        .masquerade_as_nightly_cargo()
+        .run();
+    p.cargo("check --target").arg(alternate()).run();
+    p.cargo("check -Zfeatures=itarget --target")
+        .arg(alternate())
+        .masquerade_as_nightly_cargo()
+        .run();
+    // For good measure, just make sure things don't break.
+    p.cargo("check -Zfeatures=all --target")
+        .arg(alternate())
+        .masquerade_as_nightly_cargo()
+        .run();
+}
+
 #[cargo_test]
 fn decouple_host_deps() {
     // Basic test for `host_dep` decouple.
@@ -1185,3 +1229,63 @@ fn has_dev_dep_for_test() {
         )
         .run();
 }
+
+#[cargo_test]
+fn build_dep_activated() {
+    // Build dependencies always match the host for [target.*.build-dependencies].
+    if cross_compile::disabled() {
+        return;
+    }
+    Package::new("somedep", "1.0.0")
+        .file("src/lib.rs", "")
+        .publish();
+    Package::new("targetdep", "1.0.0").publish();
+    Package::new("hostdep", "1.0.0")
+        // Check that "for_host" is sticky.
+        .target_dep("somedep", "1.0", &rustc_host())
+        .feature("feat1", &[])
+        .file(
+            "src/lib.rs",
+            r#"
+            extern crate somedep;
+
+            #[cfg(not(feature="feat1"))]
+            compile_error!{"feat1 missing"}
+            "#,
+        )
+        .publish();
+
+    let p = project()
+        .file(
+            "Cargo.toml",
+            &format!(
+                r#"
+            [package]
+            name = "foo"
+            version = "0.1.0"
+
+            # This should never be selected.
+            [target.'{}'.build-dependencies]
+            targetdep = "1.0"
+
+            [target.'{}'.build-dependencies]
+            hostdep = {{version="1.0", features=["feat1"]}}
+            "#,
+                alternate(),
+                rustc_host()
+            ),
+        )
+        .file("src/lib.rs", "")
+        .file("build.rs", "fn main() {}")
+        .build();
+
+    p.cargo("check").run();
+    p.cargo("check -Zfeatures=all")
+        .masquerade_as_nightly_cargo()
+        .run();
+    p.cargo("check --target").arg(alternate()).run();
+    p.cargo("check -Zfeatures=all --target")
+        .arg(alternate())
+        .masquerade_as_nightly_cargo()
+        .run();
+}