Skip to content

Commit b5b8b07

Browse files
authored
fix(bindeps): do not propagate artifact dependency to proc macro or build deps (#15788)
_Thanks for the pull request 🎉!_ _Please read the contribution guide: <https://doc.crates.io/contrib/>._ ### What does this PR try to resolve? This PR fixes a bug for the nightly feature `bindeps`. Basically, if - a package `foo` has an artifact dependency `artifact` with a specified target TARGET_ARTIFACT different from host TARGET_HOST, - `artifact` depends on proc macro `macro`, - `macro` conditionally depends on `arch` on TARGET_HOST, cargo build would panic on TARGET_HOST with the following error message: ``` did not find features for (PackageId { name: "arch", version: "0.0.1", source: "/Users/lencerf/Developer/cargo/target/tmp/cit/t0/foo/arch" }, ArtifactDep(CompileTarget { name: "x86_64-apple-darwin" })) within activated_features: [ ( PackageId { name: "foo", version: "0.0.1", source: "/Users/lencerf/Developer/cargo/target/tmp/cit/t0/foo", }, NormalOrDev, ), ( PackageId { name: "macro", version: "0.0.1", source: "/Users/lencerf/Developer/cargo/target/tmp/cit/t0/foo/macro", }, ArtifactDep( CompileTarget { name: "x86_64-apple-darwin", }, ), ), ( PackageId { name: "artifact", version: "0.0.1", source: "/Users/lencerf/Developer/cargo/target/tmp/cit/t0/foo/artifact", }, ArtifactDep( CompileTarget { name: "x86_64-apple-darwin", }, ), ), ] ``` Similar panic happens when - a package `foo` has an artifact dependency `artifact` with a specified target TARGET_ARTIFACT different from host TARGET_HOST, - `artifact` has a build dependency `builder`, - `builder` conditionally depends on `arch` on TARGET_HOST. From the above message, it is clear that proc macro `macro` was wrongly associated with `FeaturesFor::ArtifactDep` instead of `FeaturesFor::HostDep`. Package `arch` was later ignored because cargo thought `macro` should be built for TARGET_ARTIFACT (x86_64-apple-darwin), while `arch` was conditionally needed on TARGET_HOST (aarch64-apple-darwin). Similar analyses apply to the other test case. This commit fixes 2 paths: - when resolving features, if we encounter build dependencies or proc macros, always associate them with `FeaturesFor::HostDep`. - when deriving UnitFor for dependencies, stop propagating artifact_target_for_features if the the dependency is a build dependency or a proc macro. ### How to test and review this PR? This PR contains 2 commits - the first commit adds 2 test cases to reproduce the issue above. Since they cannot pass now, they are marked with `#[should_panic]` - the second commit fixes the bug and removes the `#[should_panic]` from the 2 test cases. `cargo test` can pass on each commit.
2 parents 5c03433 + 0360309 commit b5b8b07

File tree

3 files changed

+209
-6
lines changed

3 files changed

+209
-6
lines changed

src/cargo/core/profiles.rs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1180,12 +1180,19 @@ impl UnitFor {
11801180
} else {
11811181
self.panic_setting
11821182
};
1183+
let artifact_target_for_features =
1184+
// build.rs and proc-macros are always for host.
1185+
if dep_target.proc_macro() || parent.target.is_custom_build() {
1186+
None
1187+
} else {
1188+
self.artifact_target_for_features
1189+
};
11831190
UnitFor {
11841191
host: self.host || dep_for_host,
11851192
host_features,
11861193
panic_setting,
11871194
root_compile_kind,
1188-
artifact_target_for_features: self.artifact_target_for_features,
1195+
artifact_target_for_features,
11891196
}
11901197
}
11911198

src/cargo/core/resolver/features.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -906,11 +906,11 @@ impl<'a, 'gctx> FeatureResolver<'a, 'gctx> {
906906
// All this may result in a dependency being built multiple times
907907
// for various targets which are either specified in the manifest
908908
// or on the cargo command-line.
909-
let lib_fk = if fk == FeaturesFor::default() {
910-
(self.track_for_host
911-
&& (dep.is_build() || self.has_proc_macro_lib(dep_id)))
912-
.then(|| FeaturesFor::HostDep)
913-
.unwrap_or_default()
909+
let lib_fk = if fk != FeaturesFor::HostDep
910+
&& self.track_for_host
911+
&& (dep.is_build() || self.has_proc_macro_lib(dep_id))
912+
{
913+
FeaturesFor::HostDep
914914
} else {
915915
fk
916916
};

tests/testsuite/artifact_dep.rs

Lines changed: 196 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3394,3 +3394,199 @@ staticlib present: true
33943394
"#]],
33953395
);
33963396
}
3397+
3398+
#[cargo_test]
3399+
fn artifact_dep_target_does_not_propagate_to_deps_of_build_script() {
3400+
if cross_compile_disabled() {
3401+
return;
3402+
}
3403+
let bindeps_target = cross_compile::alternate();
3404+
let native_target = cross_compile::native();
3405+
3406+
let p = project()
3407+
.file(
3408+
"Cargo.toml",
3409+
&r#"
3410+
[package]
3411+
name = "foo"
3412+
version = "0.0.1"
3413+
edition = "2015"
3414+
resolver = "2"
3415+
3416+
[dependencies.artifact]
3417+
path = "artifact"
3418+
artifact = "bin"
3419+
target = "$TARGET"
3420+
"#
3421+
.replace("$TARGET", bindeps_target),
3422+
)
3423+
.file(
3424+
"src/main.rs",
3425+
r#"
3426+
fn main() {
3427+
let _b = include_bytes!(env!("CARGO_BIN_FILE_ARTIFACT"));
3428+
}
3429+
"#,
3430+
)
3431+
.file(
3432+
"artifact/Cargo.toml",
3433+
r#"
3434+
[package]
3435+
name = "artifact"
3436+
version = "0.0.1"
3437+
edition = "2015"
3438+
3439+
[build-dependencies]
3440+
builder = { path = "../builder" }
3441+
"#,
3442+
)
3443+
.file("artifact/src/main.rs", "fn main() { }")
3444+
.file(
3445+
"artifact/build.rs",
3446+
r#"
3447+
extern crate builder;
3448+
fn main() {
3449+
let _ = builder::add(1, 2);
3450+
}
3451+
"#,
3452+
)
3453+
.file(
3454+
"builder/Cargo.toml",
3455+
&r#"
3456+
[package]
3457+
name = "builder"
3458+
version = "0.0.1"
3459+
edition = "2015"
3460+
3461+
[target.'$TARGET'.dependencies]
3462+
arch = { path = "../arch" }
3463+
"#
3464+
.replace("$TARGET", native_target),
3465+
)
3466+
.file(
3467+
"builder/src/lib.rs",
3468+
r#"
3469+
extern crate arch;
3470+
pub fn add(a: i32, b: i32) -> i32 { arch::add(a, b) }
3471+
"#,
3472+
)
3473+
.file(
3474+
"arch/Cargo.toml",
3475+
r#"
3476+
[package]
3477+
name = "arch"
3478+
version = "0.0.1"
3479+
edition = "2015"
3480+
"#,
3481+
)
3482+
.file(
3483+
"arch/src/lib.rs",
3484+
r#"pub fn add(a: i32, b: i32) -> i32 { a + b }"#,
3485+
)
3486+
.build();
3487+
p.cargo("test -Z bindeps")
3488+
.with_stderr_data(str![[r#"
3489+
[LOCKING] 3 packages to latest compatible versions
3490+
[COMPILING] arch v0.0.1 ([ROOT]/foo/arch)
3491+
[COMPILING] builder v0.0.1 ([ROOT]/foo/builder)
3492+
[COMPILING] artifact v0.0.1 ([ROOT]/foo/artifact)
3493+
[COMPILING] foo v0.0.1 ([ROOT]/foo)
3494+
[FINISHED] `test` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
3495+
[RUNNING] unittests src/main.rs (target/debug/deps/foo-[HASH][EXE])
3496+
3497+
"#]])
3498+
.masquerade_as_nightly_cargo(&["bindeps"])
3499+
.run();
3500+
}
3501+
3502+
#[cargo_test]
3503+
fn artifact_dep_target_does_not_propagate_to_proc_macro() {
3504+
if cross_compile_disabled() {
3505+
return;
3506+
}
3507+
let bindeps_target = cross_compile::alternate();
3508+
let native_target = cross_compile::native();
3509+
3510+
let p = project()
3511+
.file(
3512+
"Cargo.toml",
3513+
&r#"
3514+
[package]
3515+
name = "foo"
3516+
version = "0.0.1"
3517+
edition = "2015"
3518+
resolver = "2"
3519+
3520+
[dependencies.artifact]
3521+
path = "artifact"
3522+
artifact = "bin"
3523+
target = "$TARGET"
3524+
"#
3525+
.replace("$TARGET", bindeps_target),
3526+
)
3527+
.file(
3528+
"src/main.rs",
3529+
r#"
3530+
fn main() {
3531+
let _b = include_bytes!(env!("CARGO_BIN_FILE_ARTIFACT"));
3532+
}
3533+
"#,
3534+
)
3535+
.file(
3536+
"artifact/Cargo.toml",
3537+
r#"
3538+
[package]
3539+
name = "artifact"
3540+
version = "0.0.1"
3541+
edition = "2015"
3542+
3543+
[dependencies]
3544+
macro = { path = "../macro" }
3545+
"#,
3546+
)
3547+
.file("artifact/src/main.rs", "fn main() { }")
3548+
.file(
3549+
"macro/Cargo.toml",
3550+
&r#"
3551+
[package]
3552+
name = "macro"
3553+
version = "0.0.1"
3554+
edition = "2015"
3555+
3556+
[lib]
3557+
proc-macro = true
3558+
3559+
[target.'$TARGET'.dependencies]
3560+
arch = { path = "../arch" }
3561+
"#
3562+
.replace("$TARGET", native_target),
3563+
)
3564+
.file("macro/src/lib.rs", "")
3565+
.file(
3566+
"arch/Cargo.toml",
3567+
r#"
3568+
[package]
3569+
name = "arch"
3570+
version = "0.0.1"
3571+
edition = "2015"
3572+
"#,
3573+
)
3574+
.file(
3575+
"arch/src/lib.rs",
3576+
"pub fn add(a: i32, b: i32) -> i32 { a + b }",
3577+
)
3578+
.build();
3579+
p.cargo("test -Z bindeps")
3580+
.with_stderr_data(str![[r#"
3581+
[LOCKING] 3 packages to latest compatible versions
3582+
[COMPILING] arch v0.0.1 ([ROOT]/foo/arch)
3583+
[COMPILING] macro v0.0.1 ([ROOT]/foo/macro)
3584+
[COMPILING] artifact v0.0.1 ([ROOT]/foo/artifact)
3585+
[COMPILING] foo v0.0.1 ([ROOT]/foo)
3586+
[FINISHED] `test` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
3587+
[RUNNING] unittests src/main.rs (target/debug/deps/foo-[HASH][EXE])
3588+
3589+
"#]])
3590+
.masquerade_as_nightly_cargo(&["bindeps"])
3591+
.run();
3592+
}

0 commit comments

Comments
 (0)