Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 11 commits into from
Mar 16, 2022
31 changes: 25 additions & 6 deletions src/cargo/core/resolver/features.rs
Original file line number Diff line number Diff line change
Expand Up @@ -769,6 +769,17 @@ impl<'a, 'cfg> FeatureResolver<'a, 'cfg> {
.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;
}
}
// Not a build dependency, and not for a build script, so must be Target.
self.requested_targets
.iter()
Expand Down Expand Up @@ -846,24 +857,32 @@ impl<'a, 'cfg> FeatureResolver<'a, 'cfg> {
)
});

let dep_fks = match artifact_target_keys {
let mut 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
// the keys artifact targets.
Some((is_lib, Some(mut dep_fks))) if is_lib => {
// For good measure, we always add the lib_fk as build scripts
// may trigger it to be queried.
Some((_, Some(mut dep_fks))) => {
dep_fks.push(lib_fk);
dep_fks
}
// The artifact is not a library, but does specify
// custom targets.
// Use only these targets feature keys.
Some((_, Some(dep_fks))) => dep_fks,
// There is no artifact in the current dependency
// or there is no target specified on the artifact.
// 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<_>>();
Expand Down
79 changes: 79 additions & 0 deletions tests/testsuite/artifact_dep.rs
Original file line number Diff line number Diff line change
Expand Up @@ -425,6 +425,85 @@ 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 target_arch = cross_compile::alternate_arch();
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.'cfg(target_arch = "$ARCH")'.dependencies]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be something like the following:

Suggested change
[target.'cfg(target_arch = "$ARCH")'.dependencies]
[target.'$TARGET'.dependencies]

On macOS, both the host and alt target arch are x86_64.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the suggestion, it's applied in 4e847a2 as part of following up on the above comment.

d2 = { path = "../d2" }
"#
.replace("$ARCH", target_arch),
)
.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_arch = "$ARCH")]
d2::f();
}
"#
.replace("$ARCH", target_arch),
)
.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()
Expand Down