Skip to content

Commit 0360309

Browse files
committed
fix(bindeps): do not propagate artifact dependency to proc macro or build deps
As reproduced in the 2 fixed test cases of this commit, cargo panicked when - 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, with the following 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", }, ), ), ] ``` 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. Signed-off-by: Changyuan Lyu <changyuan.lv@gmail.com>
1 parent a5bedb3 commit 0360309

File tree

3 files changed

+35
-12
lines changed

3 files changed

+35
-12
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: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3395,11 +3395,10 @@ staticlib present: true
33953395
);
33963396
}
33973397

3398-
#[should_panic]
33993398
#[cargo_test]
34003399
fn artifact_dep_target_does_not_propagate_to_deps_of_build_script() {
34013400
if cross_compile_disabled() {
3402-
panic!("panic expected");
3401+
return;
34033402
}
34043403
let bindeps_target = cross_compile::alternate();
34053404
let native_target = cross_compile::native();
@@ -3486,16 +3485,24 @@ fn artifact_dep_target_does_not_propagate_to_deps_of_build_script() {
34863485
)
34873486
.build();
34883487
p.cargo("test -Z bindeps")
3489-
.with_stderr_data(str![[""]])
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+
"#]])
34903498
.masquerade_as_nightly_cargo(&["bindeps"])
34913499
.run();
34923500
}
34933501

3494-
#[should_panic]
34953502
#[cargo_test]
34963503
fn artifact_dep_target_does_not_propagate_to_proc_macro() {
34973504
if cross_compile_disabled() {
3498-
panic!("panic expected");
3505+
return;
34993506
}
35003507
let bindeps_target = cross_compile::alternate();
35013508
let native_target = cross_compile::native();
@@ -3570,7 +3577,16 @@ fn artifact_dep_target_does_not_propagate_to_proc_macro() {
35703577
)
35713578
.build();
35723579
p.cargo("test -Z bindeps")
3573-
.with_stderr_data(str![[""]])
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+
"#]])
35743590
.masquerade_as_nightly_cargo(&["bindeps"])
35753591
.run();
35763592
}

0 commit comments

Comments
 (0)