Support multiple artifact dependency aliases to the same package#17067
Support multiple artifact dependency aliases to the same package#17067npmccallum wants to merge 5 commits into
Conversation
rustbot
added
A-dependency-resolution
|
r? @weihanglo rustbot has assigned @weihanglo. Use Why was this reviewer chosen?The reviewer was selected based on:
|
|
@weihanglo are you able to review this? If not, maybe @ehuss could? |
|
@rustbot reroll |
|
Note that in general, there is a lot less review bandwidth at this time. Weihang has the most bandwidth and he said he wouldn't be available. We'll see when/if I have time. |
| authors = [] | ||
| resolver = "2" | ||
|
|
||
| [dependencies.bar] |
There was a problem hiding this comment.
Is this mimicking existing tests? I at least would prefer we only use inline tables for deps, not standard tables.
|
|
||
| p.cargo("build --unit-graph -Zunstable-options -Z bindeps") | ||
| .masquerade_as_nightly_cargo(&["unit-graph", "bindeps"]) | ||
| .with_stdout_contains(r#"[..]"index":0,"extern_crate_name":"bar"[..]"#) |
There was a problem hiding this comment.
Is this mimicking an existing pattern? We try to avoid the _contains functions
This comment has been minimized.
This comment has been minimized.
|
I split this into smaller commits:
I also adjusted the tests from the inline comments to use inline dependency tables and a structured unit-graph stdout assertion. |
|
May have time for this, though let me pick up the RFC first. |
Add regression coverage for the current invariant that multiple dependency declarations to the same package are rejected when they would require distinct dependency names. This makes the existing behavior explicit before relaxing the artifact-dependency cases tracked in rust-lang#12374.
Extract dependency-name calculation so artifact alias handling can use the same extern crate naming rules in build graph lowering and metadata output.
Permit all-artifact aliases to the same package to keep their dependency-specific names when lowering build and unit graph edges. Mixed artifact and non-artifact dependencies still use the existing duplicate-name check so library dependencies have one extern crate name.
Use the shared dependency-name rules when emitting cargo metadata for artifact aliases to the same package. Bin-only artifact aliases emit dependency-specific extern names, while lib aliases continue to require a single library dependency name.
Document declaring separate aliases when building artifacts from the same package for multiple target platforms. Also call out that multiple lib=true aliases must agree on one library extern crate name.
|
This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
|
@joshtriplett This is my next highest priority if you have cycles. With this feature, arma can generate PMIs cross-architecture. |
4 participants
This fills in a behavior described by RFC 3176 that was not implemented when artifact dependencies were initially added.
The RFC allows a package to be declared multiple times under different dependency names, with each declaration selecting a different artifact target. Cargo rejected several of those same-package artifact dependency forms as duplicate renamed dependencies. This PR allows the all-artifact cases while keeping the existing duplicate-name checks for mixed artifact/non-artifact dependency cases.
The series is split into reviewable commits:
cargo metadata.lib = truerestriction.The
lib = truecase still requires Cargo to refer to the package library with one Rust extern crate name. Multiple artifact aliases that would give the library different extern names continue to be rejected.Closes #12374
FYI @joshtriplett @Byron