Do not pass native libraries to rlib compilations #576
Conversation
This file contains 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
hlopko
force-pushed
the
no_native_to_rlib
branch
5 times, most recently
from
483943f
to
5671cd4
Compare
When creating an rlib artifact, rustc will open every native static library and "copy" its contents into the rlib. This results in a quadratic disk usage growth and that's not necessary for rules_rust because we track native dependencies are we can safely pass them to the final linking actions. This fixes the build failure when a native dep is actually a linker script. Rustc will try to open the linker script as if it was a static archive, and then fail the build. Creating shared library artifacts (dylib, cdylib) and binaries is not affected, for those rustc uses the linker, and linker will consume the linker script just fine. Rustc won't try to open/interpret the linker script itself. This also unblocks bazelbuild#562.
hlopko
force-pushed
the
no_native_to_rlib
branch
from
5671cd4
to
757510f
Compare
|
CC @UebelAndre @dfreese feel free to add yourselves as reviewers :) |
illicitonion
approved these changes
Feb 4, 2021
| if ctx.attr.crate: | ||
| # Target is building the crate in `test` config | ||
| # Build the test binary using the dependency's srcs. | ||
| crate = ctx.attr.crate[rust_common.crate_info] | ||
| target = rust_common.crate_info( | ||
| name = crate_name, | ||
| type = crate.type, | ||
| type = crate_type, |
There was a problem hiding this comment.
Should this assert that crate.type == "bin"?
There was a problem hiding this comment.
I don't think so? Crate under test can be a rust_library with crate type rlib or pretty much any other, it's only the test that will be "bin" in any case.
UebelAndre
approved these changes
Feb 4, 2021
|
Oki, will merge now, feel free to review afterwards, I'm happy to address any feedback you might have. 🙇 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
When creating an rlib artifact, rustc will open every
native static library and "copy" its contents into the rlib. This
results in a quadratic disk usage growth and that's not necessary for
rules_rust because we track native dependencies and therefore we can pass
them to the final linking actions.
This fixes the build failure when a native dep is actually a linker
script. Rustc will try to open the linker script as if it was a static
archive, and then fail the build.
Creating shared library artifacts (dylib, cdylib) and binaries is not
affected, for those rustc uses the linker, and linker will consume the
linker script just fine. Rustc won't try to open/interpret the linker
script itself.
This also unblocks #562.