fix(bindeps): do not propagate artifact dependency to proc macro or build deps #15788
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add 2 test cases to reproduce a bug of `-Zbindeps`.
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.
Signed-off-by: Changyuan Lyu <[email protected]>
…uild 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 follwing 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 <[email protected]>
|
r? @weihanglo rustbot has assigned @weihanglo. Use |
rustbot
added
A-dependency-resolution
|
This PR should fix #12358 too. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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, iffoohas an artifact dependencyartifactwith a specified target TARGET_ARTIFACT different from host TARGET_HOST,artifactdepends on proc macromacro,macroconditionally depends onarchon TARGET_HOST,cargo build would panic on TARGET_HOST with the following error message:
Similar panic happens when
foohas an artifact dependencyartifactwith a specified target TARGET_ARTIFACT different from host TARGET_HOST,artifacthas a build dependencybuilder,builderconditionally depends onarchon TARGET_HOST.From the above message, it is clear that proc macro
macrowas wrongly associated withFeaturesFor::ArtifactDepinstead ofFeaturesFor::HostDep. Packagearchwas later ignored because cargo thoughtmacroshould be built for TARGET_ARTIFACT (x86_64-apple-darwin), whilearchwas conditionally needed on TARGET_HOST (aarch64-apple-darwin).Similar analyses apply to the other test case.
This commit fixes 2 paths:
FeaturesFor::HostDep.How to test and review this PR?
This PR contains 2 commits
#[should_panic]#[should_panic]from the 2 test cases.cargo testcan pass on each commit.