-Zfeatures=host_dep: Support decoupling proc-macro features.

Author: ehuss

crates/cargo-test-support/src/registry.rs

@@ -144,6 +144,7 @@ pub struct Package {
     local: bool,
     alternative: bool,
     invalid_json: bool,
+    proc_macro: bool,
 }
 
 #[derive(Clone)]
@@ -242,6 +243,7 @@ impl Package {
             local: false,
             alternative: false,
             invalid_json: false,
+            proc_macro: false,
         }
     }
 
@@ -345,6 +347,12 @@ impl Package {
         self
     }
 
+    /// Specifies whether or not this is a proc macro.
+    pub fn proc_macro(&mut self, proc_macro: bool) -> &mut Package {
+        self.proc_macro = proc_macro;
+        self
+    }
+
     /// Adds an entry in the `[features]` section.
     pub fn feature(&mut self, name: &str, deps: &[&str]) -> &mut Package {
         let deps = deps.iter().map(|s| s.to_string()).collect();
@@ -413,6 +421,7 @@ impl Package {
             "cksum": cksum,
             "features": self.features,
             "yanked": self.yanked,
+            "pm": self.proc_macro,
         })
         .to_string();
 
@@ -498,6 +507,9 @@ impl Package {
                 manifest.push_str(&format!("registry-index = \"{}\"", alt_registry_url()));
             }
         }
+        if self.proc_macro {
+            manifest.push_str("[lib]\nproc-macro = true\n");
+        }
 
         let dst = self.archive_dst();
         t!(fs::create_dir_all(dst.parent().unwrap()));

crates/resolver-tests/src/lib.rs

@@ -176,6 +176,7 @@ pub fn resolve_with_config_raw(
         &BTreeMap::<String, Vec<String>>::new(),
         None::<&String>,
         false,
+        false,
     )
     .unwrap();
     let opts = ResolveOpts::everything();
@@ -577,6 +578,7 @@ pub fn pkg_dep<T: ToPkgId>(name: T, dep: Vec<Dependency>) -> Summary {
         &BTreeMap::<String, Vec<String>>::new(),
         link,
         false,
+        false,
     )
     .unwrap()
 }
@@ -605,6 +607,7 @@ pub fn pkg_loc(name: &str, loc: &str) -> Summary {
         &BTreeMap::<String, Vec<String>>::new(),
         link,
         false,
+        false,
     )
     .unwrap()
 }
@@ -619,6 +622,7 @@ pub fn remove_dep(sum: &Summary, ind: usize) -> Summary {
         &BTreeMap::<String, Vec<String>>::new(),
         sum.links().map(|a| a.as_str()),
         sum.namespaced_features(),
+        sum.proc_macro(),
     )
     .unwrap()
 }

src/cargo/core/compiler/unit_dependencies.rs

@@ -178,9 +178,11 @@ fn deps_of_roots<'a, 'cfg>(roots: &[Unit<'a>], mut state: &mut State<'a, 'cfg>)
             } else if unit.target.is_custom_build() {
                 // This normally doesn't happen, except `clean` aggressively
                 // generates all units.
-                UnitFor::new_build(false)
+                UnitFor::new_host(false)
+            } else if unit.target.proc_macro() {
+                UnitFor::new_host(true)
             } else if unit.target.for_host() {
-                // Proc macro / plugin should never have panic set.
+                // Plugin should never have panic set.
                 UnitFor::new_compiler()
             } else {
                 UnitFor::new_normal()
@@ -297,7 +299,7 @@ fn compute_deps<'a, 'cfg>(
         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_build_dep(unit.target.is_custom_build());
+            .with_host_features(unit.target.is_custom_build() || lib.proc_macro());
 
         if bcx.config.cli_unstable().dual_proc_macros && lib.proc_macro() && !unit.kind.is_host() {
             let unit_dep = new_unit_dep(state, unit, pkg, lib, dep_unit_for, unit.kind, mode)?;
@@ -388,9 +390,10 @@ fn compute_deps_custom_build<'a, 'cfg>(
             return Ok(Vec::new());
         }
     }
-    // All dependencies of this unit should use profiles for custom
-    // builds.
-    let script_unit_for = UnitFor::new_build(unit_for.is_for_build_dep());
+    // All dependencies of this unit should use profiles for custom builds.
+    // If this is a build script of a proc macro, make sure it uses host
+    // features.
+    let script_unit_for = UnitFor::new_host(unit_for.is_for_host_features());
     // When not overridden, then the dependencies to run a build script are:
     //
     // 1. Compiling the build script itself.
@@ -445,7 +448,9 @@ fn compute_deps_doc<'a, 'cfg>(
         // 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());
+        let dep_unit_for = UnitFor::new_normal()
+            .with_for_host(lib.for_host())
+            .with_host_features(lib.proc_macro());
         let lib_unit_dep = new_unit_dep(
             state,
             unit,
@@ -528,32 +533,32 @@ fn dep_build_script<'a>(
                 .bcx
                 .profiles
                 .get_profile_run_custom_build(&unit.profile);
-            // UnitFor::new_build is used because we want the `host` flag set
+            // UnitFor::new_host is used because we want the `host` flag set
             // for all of our build dependencies (so they all get
             // build-override profiles), including compiling the build.rs
             // script itself.
             //
-            // If `is_for_build_dep` here is `false`, that means we are a
+            // If `is_for_host_features` here is `false`, that means we are a
             // build.rs script for a normal dependency and we want to set the
             // CARGO_FEATURE_* environment variables to the features as a
             // normal dep.
             //
-            // If `is_for_build_dep` here is `true`, that means that this
-            // package is being used as a build dependency, and so we only
-            // want to set CARGO_FEATURE_* variables for the build-dependency
+            // If `is_for_host_features` here is `true`, that means that this
+            // package is being used as a build dependency or proc-macro, and
+            // so we only want to set CARGO_FEATURE_* variables for the host
             // side of the graph.
             //
             // Keep in mind that the RunCustomBuild unit and the Compile
             // build.rs unit use the same features. This is because some
             // people use `cfg!` and `#[cfg]` expressions to check for enabled
             // features instead of just checking `CARGO_FEATURE_*` at runtime.
-            // In the case with `-Zfeatures=build_dep`, and a shared
+            // In the case with `-Zfeatures=host_dep`, and a shared
             // dependency has different features enabled for normal vs. build,
             // then the build.rs script will get compiled twice. I believe it
             // is not feasible to only build it once because it would break a
             // large number of scripts (they would think they have the wrong
             // set of features enabled).
-            let script_unit_for = UnitFor::new_build(unit_for.is_for_build_dep());
+            let script_unit_for = UnitFor::new_host(unit_for.is_for_host_features());
             new_unit_dep_with_profile(
                 state,
                 unit,

src/cargo/core/profiles.rs

@@ -768,7 +768,7 @@ pub struct UnitFor {
     /// any of its dependencies. This enables `build-override` profiles for
     /// these targets.
     ///
-    /// An invariant is that if `build_dep` is true, `host` must be true.
+    /// An invariant is that if `host_features` is true, `host` must be true.
     ///
     /// Note that this is `true` for `RunCustomBuild` units, even though that
     /// unit should *not* use build-override profiles. This is a bit of a
@@ -779,16 +779,16 @@ pub struct UnitFor {
     /// sticky (and forced to `true` for all further dependencies) — which is
     /// the whole point of `UnitFor`.
     host: bool,
-    /// A target for a build dependency (or any of its dependencies). This is
-    /// used for computing features of build dependencies independently of
-    /// other dependency kinds.
+    /// A target for a build dependency or proc-macro (or any of its
+    /// dependencies). This is used for computing features of build
+    /// dependencies and proc-macros independently of other dependency kinds.
     ///
     /// The subtle difference between this and `host` is that the build script
     /// for a non-host package sets this to `false` because it wants the
     /// features of the non-host package (whereas `host` is true because the
-    /// build script is being built for the host). `build_dep` becomes `true`
-    /// for build-dependencies, or any of their dependencies. For example, with
-    /// this dependency tree:
+    /// build script is being built for the host). `host_features` becomes
+    /// `true` for build-dependencies or proc-macros, or any of their
+    /// dependencies. For example, with this dependency tree:
     ///
     /// ```text
     /// foo
@@ -799,17 +799,18 @@ pub struct UnitFor {
     ///     └── shared_dep build.rs
     /// ```
     ///
-    /// In this example, `foo build.rs` is HOST=true, BUILD_DEP=false. This is
-    /// so that `foo build.rs` gets the profile settings for build scripts
-    /// (HOST=true) and features of foo (BUILD_DEP=false) because build scripts
-    /// need to know which features their package is being built with.
+    /// In this example, `foo build.rs` is HOST=true, HOST_FEATURES=false.
+    /// This is so that `foo build.rs` gets the profile settings for build
+    /// scripts (HOST=true) and features of foo (HOST_FEATURES=false) because
+    /// build scripts need to know which features their package is being built
+    /// with.
     ///
     /// But in the case of `shared_dep`, when built as a build dependency,
     /// both flags are true (it only wants the build-dependency features).
     /// When `shared_dep` is built as a normal dependency, then `shared_dep
-    /// build.rs` is HOST=true, BUILD_DEP=false for the same reasons that
+    /// build.rs` is HOST=true, HOST_FEATURES=false for the same reasons that
     /// foo's build script is set that way.
-    build_dep: bool,
+    host_features: bool,
     /// How Cargo processes the `panic` setting or profiles. This is done to
     /// handle test/benches inheriting from dev/release, as well as forcing
     /// `for_host` units to always unwind.
@@ -837,32 +838,35 @@ impl UnitFor {
     pub fn new_normal() -> UnitFor {
         UnitFor {
             host: false,
-            build_dep: false,
+            host_features: false,
             panic_setting: PanicSetting::ReadProfile,
         }
     }
 
-    /// A unit for a custom build script or its dependencies.
+    /// A unit for a custom build script or proc-macro or its dependencies.
     ///
-    /// The `build_dep` parameter is whether or not this is for a build
-    /// dependency. Build scripts for non-host units should use `false`
-    /// because they want to use the features of the package they are running
-    /// for.
-    pub fn new_build(build_dep: bool) -> UnitFor {
+    /// The `host_features` parameter is whether or not this is for a build
+    /// dependency or proc-macro (something that requires being built "on the
+    /// host"). Build scripts for non-host units should use `false` because
+    /// they want to use the features of the package they are running for.
+    pub fn new_host(host_features: bool) -> UnitFor {
         UnitFor {
             host: true,
-            build_dep,
+            host_features,
             // Force build scripts to always use `panic=unwind` for now to
             // maximally share dependencies with procedural macros.
             panic_setting: PanicSetting::AlwaysUnwind,
         }
     }
 
-    /// A unit for a proc macro or compiler plugin or their dependencies.
+    /// A unit for a compiler plugin or their dependencies.
     pub fn new_compiler() -> UnitFor {
         UnitFor {
             host: false,
-            build_dep: false,
+            // The feature resolver doesn't know which dependencies are
+            // plugins, so for now plugins don't split features. Since plugins
+            // are mostly deprecated, just leave this as false.
+            host_features: false,
             // Force plugins to use `panic=abort` so panics in the compiler do
             // not abort the process but instead end with a reasonable error
             // message that involves catching the panic in the compiler.
@@ -879,7 +883,7 @@ impl UnitFor {
     pub fn new_test(config: &Config) -> UnitFor {
         UnitFor {
             host: false,
-            build_dep: false,
+            host_features: false,
             // We're testing out an unstable feature (`-Zpanic-abort-tests`)
             // which inherits the panic setting from the dev/release profile
             // (basically avoid recompiles) but historical defaults required
@@ -902,7 +906,7 @@ impl UnitFor {
     pub fn with_for_host(self, for_host: bool) -> UnitFor {
         UnitFor {
             host: self.host || for_host,
-            build_dep: self.build_dep,
+            host_features: self.host_features,
             panic_setting: if for_host {
                 PanicSetting::AlwaysUnwind
             } else {
@@ -911,15 +915,16 @@ impl UnitFor {
         }
     }
 
-    /// Returns a new copy updating it for a build dependency.
+    /// 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_build_dep(mut self, build_dep: bool) -> UnitFor {
-        if build_dep {
+    pub fn with_host_features(mut self, host_features: bool) -> UnitFor {
+        if host_features {
             assert!(self.host);
         }
-        self.build_dep = self.build_dep || build_dep;
+        self.host_features = self.host_features || host_features;
         self
     }
 
@@ -929,8 +934,8 @@ impl UnitFor {
         self.host
     }
 
-    pub fn is_for_build_dep(&self) -> bool {
-        self.build_dep
+    pub fn is_for_host_features(&self) -> bool {
+        self.host_features
     }
 
     /// Returns how `panic` settings should be handled for this profile
@@ -943,43 +948,43 @@ impl UnitFor {
         static ALL: &[UnitFor] = &[
             UnitFor {
                 host: false,
-                build_dep: false,
+                host_features: false,
                 panic_setting: PanicSetting::ReadProfile,
             },
             UnitFor {
                 host: true,
-                build_dep: false,
+                host_features: false,
                 panic_setting: PanicSetting::AlwaysUnwind,
             },
             UnitFor {
                 host: false,
-                build_dep: false,
+                host_features: false,
                 panic_setting: PanicSetting::AlwaysUnwind,
             },
             UnitFor {
                 host: false,
-                build_dep: false,
+                host_features: false,
                 panic_setting: PanicSetting::Inherit,
             },
-            // build_dep=true must always have host=true
+            // host_features=true must always have host=true
             // `Inherit` is not used in build dependencies.
             UnitFor {
                 host: true,
-                build_dep: true,
+                host_features: true,
                 panic_setting: PanicSetting::ReadProfile,
             },
             UnitFor {
                 host: true,
-                build_dep: true,
+                host_features: true,
                 panic_setting: PanicSetting::AlwaysUnwind,
             },
         ];
         ALL
     }
 
     pub(crate) fn map_to_features_for(&self) -> FeaturesFor {
-        if self.is_for_build_dep() {
-            FeaturesFor::BuildDep
+        if self.is_for_host_features() {
+            FeaturesFor::HostDep
         } else {
             FeaturesFor::NormalOrDev
         }