Remove prior hack and replace it with a proper solution

This is the diff from
https://github.com/rust-lang/cargo/pull/10433#issuecomment-1064577586
applied to the codebase with the previous hack removed.

Thanks a million to @ehuss who has the insight to find fixes like this.

diff --git a/src/cargo/core/compiler/unit_dependencies.rs b/src/cargo/core/compiler/unit_dependencies.rs
index 857436e9f9093a56fe5e2d054702d078e772c657..f93d4a375296de92bf2219d2a2ccfa2ac78edaea 100644 (file)
--- a/src/cargo/core/compiler/unit_dependencies.rs
+++ b/src/cargo/core/compiler/unit_dependencies.rs
@@ -464,10 +464,7 @@ fn compute_deps_custom_build(
     // 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(),
-        unit_for.root_compile_kind(),
-    );
+    let script_unit_for = unit_for.for_custom_build();
     // When not overridden, then the dependencies to run a build script are:
     //
     // 1. Compiling the build script itself.
@@ -782,7 +779,7 @@ fn dep_build_script(
             // The profile stored in the Unit is the profile for the thing
             // the custom build script is running for.
             let profile = state.profiles.get_profile_run_custom_build(&unit.profile);
-            // UnitFor::new_host is used because we want the `host` flag set
+            // UnitFor::for_custom_build 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.
@@ -807,10 +804,7 @@ fn dep_build_script(
             // 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_host(
-                unit_for.is_for_host_features(),
-                unit_for.root_compile_kind(),
-            );
+            let script_unit_for = unit_for.for_custom_build();
             new_unit_dep_with_profile(
                 state,
                 unit,

diff --git a/src/cargo/core/profiles.rs b/src/cargo/core/profiles.rs
index bb354abdc82becd2edaf9656f3ef7797f50a8622..b4019745fba94e6c8b69e0eccb118065eafe8214 100644 (file)
--- a/src/cargo/core/profiles.rs
+++ b/src/cargo/core/profiles.rs
@@ -1097,6 +1097,18 @@ impl UnitFor {
         }
     }
 
+    pub fn for_custom_build(self) -> UnitFor {
+        UnitFor {
+            host: true,
+            host_features: self.host_features,
+            // Force build scripts to always use `panic=unwind` for now to
+            // maximally share dependencies with procedural macros.
+            panic_setting: PanicSetting::AlwaysUnwind,
+            root_compile_kind: self.root_compile_kind,
+            artifact_target_for_features: self.artifact_target_for_features,
+        }
+    }
+
     /// Set the artifact compile target for use in features using the given `artifact`.
     pub(crate) fn with_artifact_features(mut self, artifact: &Artifact) -> UnitFor {
         self.artifact_target_for_features = artifact.target().and_then(|t| t.to_compile_target());

diff --git a/src/cargo/core/resolver/features.rs b/src/cargo/core/resolver/features.rs
index 8b2cb86efa8693fb936f42f7072482b676c2e6ea..9d9277f27ab7a9c2bff2ce8fa31815d1353be77f 100644 (file)
--- a/src/cargo/core/resolver/features.rs
+++ b/src/cargo/core/resolver/features.rs
@@ -761,29 +761,25 @@ impl<'a, 'cfg> FeatureResolver<'a, 'cfg> {
     ) -> Vec<(PackageId, Vec<(&'a Dependency, FeaturesFor)>)> {
         // Helper for determining if a platform is activated.
         let platform_activated = |dep: &Dependency| -> bool {
-            // We always care about build-dependencies, and they are always
-            // Host. If we are computing dependencies "for a build script",
-            // even normal dependencies are host-only.
-            if fk == FeaturesFor::HostDep || dep.is_build() {
-                return self
-                    .target_data
-                    .dep_platform_activated(dep, CompileKind::Host);
-            }
             // We always count platforms as activated if the target stems from an artifact
             // dependency's target specification. This triggers in conjunction with
             // `[target.'cfg(…)'.dependencies]` manifest sections.
-            if let FeaturesFor::NormalOrDevOrArtifactTarget(Some(target)) = fk {
-                if self
-                    .target_data
-                    .dep_platform_activated(dep, CompileKind::Target(target))
-                {
-                    return true;
+            match (dep.is_build(), fk) {
+                (true, _) | (_, FeaturesFor::HostDep) => {
+                    // We always care about build-dependencies, and they are always
+                    // Host. If we are computing dependencies "for a build script",
+                    // even normal dependencies are host-only.
+                    self.target_data
+                        .dep_platform_activated(dep, CompileKind::Host)
                 }
+                (_, FeaturesFor::NormalOrDevOrArtifactTarget(None)) => self
+                    .requested_targets
+                    .iter()
+                    .any(|kind| self.target_data.dep_platform_activated(dep, *kind)),
+                (_, FeaturesFor::NormalOrDevOrArtifactTarget(Some(target))) => self
+                    .target_data
+                    .dep_platform_activated(dep, CompileKind::Target(target)),
             }
-            // Not a build dependency, and not for a build script, so must be Target.
-            self.requested_targets
-                .iter()
-                .any(|kind| self.target_data.dep_platform_activated(dep, *kind))
         };
         self.resolve
             .deps(pkg_id)
@@ -857,7 +853,7 @@ impl<'a, 'cfg> FeatureResolver<'a, 'cfg> {
                             )
                         });
 
-                        let mut dep_fks = match artifact_target_keys {
+                        let dep_fks = match artifact_target_keys {
                             // The artifact is also a library and does specify custom
                             // targets.
                             // The library's feature key needs to be used alongside
@@ -873,16 +869,6 @@ impl<'a, 'cfg> FeatureResolver<'a, 'cfg> {
                             // Use the standard feature key without any alteration.
                             Some((_, None)) | None => vec![lib_fk],
                         };
-
-                        // This is more of a hack to fix a particular issue with platform-gated
-                        // dependencies' build scripts, which unfortunately we can't determine
-                        // here any better than checking for a platform and blindly adding the
-                        // feature key that it will later query.
-                        // If it matters, the dependency that actually should add this key
-                        // drops out in line 798.
-                        if dep.platform().is_some() {
-                            dep_fks.push(FeaturesFor::NormalOrDevOrArtifactTarget(None));
-                        }
                         dep_fks.into_iter().map(move |dep_fk| (dep, dep_fk))
                     })
                     .collect::<Vec<_>>();

diff --git a/tests/testsuite/artifact_dep.rs b/tests/testsuite/artifact_dep.rs
index 0d22e24c59607ea4a582d5cfd9b2bd71fea6d2cb..3799c8b274a1f570b852679a4ac255db9afa6b0d 100644 (file)
--- a/tests/testsuite/artifact_dep.rs
+++ b/tests/testsuite/artifact_dep.rs
@@ -431,7 +431,6 @@ fn feature_resolution_works_for_cfg_target_specification() {
         return;
     }
     let target = cross_compile::alternate();
-    let target_arch = cross_compile::alternate_arch();
     let p = project()
         .file(
             "Cargo.toml",
@@ -465,10 +464,10 @@ fn feature_resolution_works_for_cfg_target_specification() {
                 version = "0.0.1"
                 authors = []
 
-                [target.'cfg(target_arch = "$ARCH")'.dependencies]
+                [target.'$TARGET'.dependencies]
                 d2 = { path = "../d2" }
             "#
-            .replace("$ARCH", target_arch),
+            .replace("$TARGET", target),
         )
         .file(
             "d1/src/main.rs",
@@ -480,11 +479,11 @@ fn feature_resolution_works_for_cfg_target_specification() {
         .file(
             "d1/src/lib.rs",
             &r#"pub fn f() {
-                #[cfg(target_arch = "$ARCH")]
+                #[cfg(target = "$TARGET")]
                 d2::f();
             }
             "#
-            .replace("$ARCH", target_arch),
+            .replace("$TARGET", target),
         )
         .file(
             "d2/Cargo.toml",