Fix unit_for computation on proc-macros in shared workspace.

Author: ehuss

src/cargo/core/compiler/unit_dependencies.rs

@@ -265,10 +265,7 @@ fn compute_deps(
             None => continue,
         };
         let mode = check_or_build_mode(unit.mode, lib);
-        let dep_unit_for = unit_for
-            .with_for_host(lib.for_host())
-            // If it is a custom build script, then it *only* has build dependencies.
-            .with_host_features(unit.target.is_custom_build() || lib.proc_macro());
+        let dep_unit_for = unit_for.with_dependency(unit, lib);
 
         if state.config.cli_unstable().dual_proc_macros && lib.proc_macro() && !unit.kind.is_host()
         {
@@ -417,9 +414,7 @@ fn compute_deps_doc(unit: &Unit, state: &mut State<'_, '_>) -> CargoResult<Vec<U
         // Rustdoc only needs rmeta files for regular dependencies.
         // However, for plugins/proc macros, deps should be built like normal.
         let mode = check_or_build_mode(unit.mode, lib);
-        let dep_unit_for = UnitFor::new_normal()
-            .with_for_host(lib.for_host())
-            .with_host_features(lib.proc_macro());
+        let dep_unit_for = UnitFor::new_normal().with_dependency(unit, lib);
         let lib_unit_dep = new_unit_dep(
             state,
             unit,
@@ -466,12 +461,13 @@ fn maybe_lib(
         .find(|t| t.is_linkable())
         .map(|t| {
             let mode = check_or_build_mode(unit.mode, t);
+            let dep_unit_for = unit_for.with_dependency(unit, t);
             new_unit_dep(
                 state,
                 unit,
                 &unit.pkg,
                 t,
-                unit_for,
+                dep_unit_for,
                 unit.kind.for_target(t),
                 mode,
             )

src/cargo/core/profiles.rs

@@ -1,6 +1,6 @@
-use crate::core::compiler::CompileMode;
+use crate::core::compiler::{CompileMode, Unit};
 use crate::core::resolver::features::FeaturesFor;
-use crate::core::{Feature, Features, PackageId, PackageIdSpec, Resolve, Shell};
+use crate::core::{Feature, Features, PackageId, PackageIdSpec, Resolve, Shell, Target};
 use crate::util::errors::CargoResultExt;
 use crate::util::interning::InternedString;
 use crate::util::toml::{ProfilePackageSpec, StringOrBool, TomlProfile, TomlProfiles, U32OrBool};
@@ -976,36 +976,38 @@ impl UnitFor {
         unit_for
     }
 
-    /// Returns a new copy based on `for_host` setting.
+    /// Returns a new copy updated based on the target dependency.
     ///
-    /// When `for_host` is true, this clears `panic_abort_ok` in a sticky
-    /// fashion so that all its dependencies also have `panic_abort_ok=false`.
-    /// This'll help ensure that once we start compiling for the host platform
-    /// (build scripts, plugins, proc macros, etc) we'll share the same build
-    /// graph where everything is `panic=unwind`.
-    pub fn with_for_host(self, for_host: bool) -> UnitFor {
+    /// This is where the magic happens that the host/host_features settings
+    /// transition in a sticky fashion. As the dependency graph is being
+    /// built, once those flags are set, they stay set for the duration of
+    /// that portion of tree.
+    pub fn with_dependency(self, parent: &Unit, dep_target: &Target) -> UnitFor {
+        // A build script or proc-macro transitions this to being built for the host.
+        let dep_for_host = dep_target.for_host();
+        // This is where feature decoupling of host versus target happens.
+        //
+        // Once host features are desired, they are always desired.
+        //
+        // A proc-macro should always use host features.
+        //
+        // Dependencies of a build script should use host features (subtle
+        // point: the build script itself does *not* use host features, that's
+        // why the parent is checked here, and not the dependency).
+        let host_features =
+            self.host_features || parent.target.is_custom_build() || dep_target.proc_macro();
+        // Build scripts and proc macros, and all of their dependencies are
+        // AlwaysUnwind.
+        let panic_setting = if dep_for_host {
+            PanicSetting::AlwaysUnwind
+        } else {
+            self.panic_setting
+        };
         UnitFor {
-            host: self.host || for_host,
-            host_features: self.host_features,
-            panic_setting: if for_host {
-                PanicSetting::AlwaysUnwind
-            } else {
-                self.panic_setting
-            },
-        }
-    }
-
-    /// Returns a new copy updating it whether or not it should use features
-    /// for build dependencies and proc-macros.
-    ///
-    /// This is part of the machinery responsible for handling feature
-    /// decoupling for build dependencies in the new feature resolver.
-    pub fn with_host_features(mut self, host_features: bool) -> UnitFor {
-        if host_features {
-            assert!(self.host);
+            host: self.host || dep_for_host,
+            host_features,
+            panic_setting,
         }
-        self.host_features = self.host_features || host_features;
-        self
     }
 
     /// Returns `true` if this unit is for a build script or any of its

tests/testsuite/features2.rs

@@ -2147,3 +2147,133 @@ foo v0.1.0 ([ROOT]/foo)
         .run();
     clear();
 }
+
+#[cargo_test]
+fn pm_with_int_shared() {
+    // This is a somewhat complex scenario of a proc-macro in a workspace with
+    // an integration test where the proc-macro is used for other things, and
+    // *everything* is built at once (`--workspace --all-targets
+    // --all-features`). There was a bug where the UnitFor settings were being
+    // incorrectly computed based on the order that the graph was traversed.
+    //
+    // There are some uncertainties about exactly how proc-macros should behave
+    // with `--workspace`, see https://github.com/rust-lang/cargo/issues/8312.
+    //
+    // This uses a const-eval hack to do compile-time feature checking.
+    let p = project()
+        .file(
+            "Cargo.toml",
+            r#"
+                [workspace]
+                members = ["foo", "pm", "shared"]
+                resolver = "2"
+            "#,
+        )
+        .file(
+            "foo/Cargo.toml",
+            r#"
+                [package]
+                name = "foo"
+                version = "0.1.0"
+                edition = "2018"
+
+                [dependencies]
+                pm = { path = "../pm" }
+                shared = { path = "../shared", features = ["norm-feat"] }
+            "#,
+        )
+        .file(
+            "foo/src/lib.rs",
+            r#"
+                // foo->shared always has both features set
+                const _CHECK: [(); 0] = [(); 0-!(shared::FEATS==3) as usize];
+            "#,
+        )
+        .file(
+            "pm/Cargo.toml",
+            r#"
+                [package]
+                name = "pm"
+                version = "0.1.0"
+
+                [lib]
+                proc-macro = true
+
+                [dependencies]
+                shared = { path = "../shared", features = ["host-feat"] }
+            "#,
+        )
+        .file(
+            "pm/src/lib.rs",
+            r#"
+                // pm->shared always has just host
+                const _CHECK: [(); 0] = [(); 0-!(shared::FEATS==1) as usize];
+            "#,
+        )
+        .file(
+            "pm/tests/pm_test.rs",
+            r#"
+                // integration test gets both set
+                const _CHECK: [(); 0] = [(); 0-!(shared::FEATS==3) as usize];
+            "#,
+        )
+        .file(
+            "shared/Cargo.toml",
+            r#"
+                [package]
+                name = "shared"
+                version = "0.1.0"
+
+                [features]
+                norm-feat = []
+                host-feat = []
+            "#,
+        )
+        .file(
+            "shared/src/lib.rs",
+            r#"
+                pub const FEATS: u32 = {
+                    if cfg!(feature="norm-feat") && cfg!(feature="host-feat") {
+                        3
+                    } else if cfg!(feature="norm-feat") {
+                        2
+                    } else if cfg!(feature="host-feat") {
+                        1
+                    } else {
+                        0
+                    }
+                };
+            "#,
+        )
+        .build();
+
+    p.cargo("build --workspace --all-targets --all-features -v")
+        .with_stderr_unordered(
+            "\
+[COMPILING] shared [..]
+[RUNNING] `rustc --crate-name shared [..]--crate-type lib [..]
+[RUNNING] `rustc --crate-name shared [..]--crate-type lib [..]
+[RUNNING] `rustc --crate-name shared [..]--test[..]
+[COMPILING] pm [..]
+[RUNNING] `rustc --crate-name pm [..]--crate-type proc-macro[..]
+[RUNNING] `rustc --crate-name pm [..]--test[..]
+[COMPILING] foo [..]
+[RUNNING] `rustc --crate-name foo [..]--test[..]
+[RUNNING] `rustc --crate-name pm_test [..]--test[..]
+[RUNNING] `rustc --crate-name foo [..]--crate-type lib[..]
+[FINISHED] [..]
+",
+        )
+        .run();
+
+    // And again, should stay fresh.
+    p.cargo("build --workspace --all-targets --all-features -v")
+        .with_stderr_unordered(
+            "\
+[FRESH] shared [..]
+[FRESH] pm [..]
+[FRESH] foo [..]
+[FINISHED] [..]",
+        )
+        .run();
+}