Auto merge of #10433 - Byron:fix-#10431, r=ehuss

Fix panic when artifact target is used for `[target.'cfg(<target>)'.dependencies`

With an artifact dependency like this in package `a`…

```toml
[dependencies.a]
path = "b"
artifact = "bin"
target = "$TARGET"
```

…and when using `$TARGET` like this in another package `b`…

```toml
[target.'cfg(target_arch = "$ARCHOF_$TARGET")'.dependencies]
c = { path = "../c" }
```

…it panics with `thread 'main' panicked at 'activated_features for invalid package: features did not find PackageId <dbg-info>`, but we would expect this to work normally.

### Tasks

- [x] reproduce issue in new test
   - [x] figure out why the test is fixed but the real-world example isn't
- [x] find a fix

Fixes #10431 and  #10452.

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,

src/cargo/core/profiles.rs

@@ -969,6 +969,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());

src/cargo/core/resolver/features.rs

@@ -761,18 +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
+            // 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.
+            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::Host);
+                    .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)

tests/testsuite/artifact_dep.rs

@@ -425,6 +425,84 @@ fn features_are_not_unified_among_lib_and_bin_dep_of_different_target() {
         .run();
 }
 
+#[cargo_test]
+fn feature_resolution_works_for_cfg_target_specification() {
+    if cross_compile::disabled() {
+        return;
+    }
+    let target = cross_compile::alternate();
+    let p = project()
+        .file(
+            "Cargo.toml",
+            &r#"
+                [project]
+                name = "foo"
+                version = "0.0.1"
+                authors = []
+                resolver = "2"
+
+                [dependencies.d1]
+                path = "d1"
+                artifact = "bin"
+                target = "$TARGET"
+            "#
+            .replace("$TARGET", target),
+        )
+        .file(
+            "src/main.rs",
+            r#"
+                fn main() {
+                    let _b = include_bytes!(env!("CARGO_BIN_FILE_D1"));
+                }
+            "#,
+        )
+        .file(
+            "d1/Cargo.toml",
+            &r#"
+                [package]
+                name = "d1"
+                version = "0.0.1"
+                authors = []
+
+                [target.'$TARGET'.dependencies]
+                d2 = { path = "../d2" }
+            "#
+            .replace("$TARGET", target),
+        )
+        .file(
+            "d1/src/main.rs",
+            r#"fn main() {
+                d1::f();
+            }"#,
+        )
+        .file("d1/build.rs", r#"fn main() { }"#)
+        .file(
+            "d1/src/lib.rs",
+            &r#"pub fn f() {
+                #[cfg(target = "$TARGET")]
+                d2::f();
+            }
+            "#
+            .replace("$TARGET", target),
+        )
+        .file(
+            "d2/Cargo.toml",
+            r#"
+                [package]
+                name = "d2"
+                version = "0.0.1"
+                authors = []
+            "#,
+        )
+        .file("d2/build.rs", r#"fn main() { }"#)
+        .file("d2/src/lib.rs", "pub fn f() {}")
+        .build();
+
+    p.cargo("test -Z bindeps")
+        .masquerade_as_nightly_cargo()
+        .run();
+}
+
 #[cargo_test]
 fn build_script_with_bin_artifacts() {
     let p = project()
@@ -2075,3 +2153,113 @@ fn build_script_output_string(p: &Project, package_name: &str) -> String {
     assert_eq!(paths.len(), 1);
     std::fs::read_to_string(&paths[0]).unwrap()
 }
+
+#[cargo_test]
+fn build_script_features_for_shared_dependency() {
+    // When a build script is built and run, its features should match. Here:
+    //
+    // foo
+    //   -> artifact on d1 with target
+    //   -> common with features f1
+    //
+    // d1
+    //   -> common with features f2
+    //
+    // common has features f1 and f2, with a build script.
+    //
+    // When common is built as a dependency of d1, it should have features
+    // `f2` (for the library and the build script).
+    //
+    // When common is built as a dependency of foo, it should have features
+    // `f1` (for the library and the build script).
+    if cross_compile::disabled() {
+        return;
+    }
+    let target = cross_compile::alternate();
+    let p = project()
+        .file(
+            "Cargo.toml",
+            &r#"
+                [project]
+                name = "foo"
+                version = "0.0.1"
+                resolver = "2"
+
+                [dependencies]
+                d1 = { path = "d1", artifact = "bin", target = "$TARGET" }
+                common = { path = "common", features = ["f1"] }
+            "#
+            .replace("$TARGET", target),
+        )
+        .file(
+            "src/main.rs",
+            r#"
+                fn main() {
+                    let _b = include_bytes!(env!("CARGO_BIN_FILE_D1"));
+                    common::f1();
+                }
+            "#,
+        )
+        .file(
+            "d1/Cargo.toml",
+            r#"
+                [package]
+                name = "d1"
+                version = "0.0.1"
+
+                [dependencies]
+                common = { path = "../common", features = ["f2"] }
+            "#,
+        )
+        .file(
+            "d1/src/main.rs",
+            r#"fn main() {
+                common::f2();
+            }"#,
+        )
+        .file(
+            "common/Cargo.toml",
+            r#"
+                [package]
+                name = "common"
+                version = "0.0.1"
+
+                [features]
+                f1 = []
+                f2 = []
+            "#,
+        )
+        .file(
+            "common/src/lib.rs",
+            r#"
+                #[cfg(feature = "f1")]
+                pub fn f1() {}
+
+                #[cfg(feature = "f2")]
+                pub fn f2() {}
+            "#,
+        )
+        .file(
+            "common/build.rs",
+            &r#"
+                use std::env::var_os;
+                fn main() {
+                    assert_eq!(var_os("CARGO_FEATURE_F1").is_some(), cfg!(feature="f1"));
+                    assert_eq!(var_os("CARGO_FEATURE_F2").is_some(), cfg!(feature="f2"));
+                    if std::env::var("TARGET").unwrap() == "$TARGET" {
+                        assert!(var_os("CARGO_FEATURE_F1").is_none());
+                        assert!(var_os("CARGO_FEATURE_F2").is_some());
+                    } else {
+                        assert!(var_os("CARGO_FEATURE_F1").is_some());
+                        assert!(var_os("CARGO_FEATURE_F2").is_none()); 
+                    }
+                }
+            "#
+            .replace("$TARGET", target),
+        )
+        .build();
+
+    p.cargo("build -Z bindeps -v")
+        .masquerade_as_nightly_cargo()
+        .run();
+}