Fix: Build only the specified artifact library when multiple types are available#13842
Merged
bors merged 2 commits intorust-lang:masterfrom May 7, 2024
Merged
Conversation
Collaborator
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @weihanglo (or someone else) some time within the next two weeks. Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (
|
weihanglo
reviewed
May 7, 2024
Member
weihanglo
left a comment
There was a problem hiding this comment.
Overall looks good! I added some minor suggestions.
Thanks for your contribution!
a0e5a36 to
9a5cfbc
Compare
weihanglo
approved these changes
May 7, 2024
Member
|
Thank you for the patch! @bors r+ |
Contributor
Contributor
Contributor
|
☀️ Test successful - checks-actions |
bors
added a commit
that referenced
this pull request
May 7, 2024
test: clean up unnecessary uses of `match_exact` It was found during the review of <#13842 (comment)> We should be happy using `assert_match_exact` directly. The extra arguments to `match_exact` are not necessary for those test cases.
bors
added a commit
that referenced
this pull request
May 7, 2024
test: clean up unnecessary uses of `match_exact` It was found during the review of <#13842 (comment)> We should be happy using `assert_match_exact` directly. The extra arguments to `match_exact` are not necessary for those test cases.
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
May 8, 2024
Update cargo 10 commits in 05364cb2f61a2c2b091e061c1f42b207dfb5f81f..0ca60e940821c311c9b25a6423b59ccdbcea218f 2024-05-03 16:48:59 +0000 to 2024-05-08 01:54:25 +0000 - chore: Add cargo-lints to unstable docs (rust-lang/cargo#13881) - test: clean up unnecessary uses of `match_exact` (rust-lang/cargo#13879) - docs(ref): Correct heading level of `[lints]` documentation (rust-lang/cargo#13878) - Fix: Build only the specified artifact library when multiple types are available (rust-lang/cargo#13842) - docs: add missing CARGO_MAKEFLAGS env for plugins (rust-lang/cargo#13872) - Add more documentation to `cargo::rustc-check-cfg` (rust-lang/cargo#13869) - fix(toml): Remove unstable rejrected frontmatter syntax for cargo script (rust-lang/cargo#13861) - Update UI example code in contributor guide (rust-lang/cargo#13864) - style(test): Remove check-cfg warning (rust-lang/cargo#13865) - Fix global_cache_tracker::max_download_size test flakiness (rust-lang/cargo#13860) r? ghost
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 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.
What does this PR try to resolve?
Fixes #12109.
TL;DR:
A crate
barexposes it's library as both astaticliband acdylib. A second cratefoospecifies an artifact dependency of typestaticlibonbar, meaning cargo should buildbaras astaticliband expose certain environment variables (e.g.CARGO_STATICLIB_FILE_BAR). However, due to a bug, cargo ends up building (and exposing the associated environment variables) for both thestaticlibandcdylib.The same happens if
foospecifies an artifact dependency of typecdylib; both artifact types are built.How should we test and review this PR?
The first commit introduces a test which reproduces the issue, the second commit introduces the fix. This setup was recommended by @ehuss.
Additional information
Artifact dependencies: https://rust-lang.github.io/rfcs/3028-cargo-binary-dependencies.html
TL;DR
If a crate
foorequests an artifact dependency of kind <artifact_kind> from a cratebarthen the following happens:baris built with crate-type <artifact_kind> and a directory is created attarget/<profile>/deps/artifact/bar-<build_hash_or_something>/<artifact_kind>. The binary artifact is placed in this directory.CARGO_<artifact_kind>_FILE_BAR, which points to the binary artifact that was specified. This environment variable is available at compile time for normal dependencies and at runtime for build-dependencies.If multiple artifact-kinds are requested cargo will create a unit for each, and so they will all be built separately.