-
Notifications
You must be signed in to change notification settings - Fork 426
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Pass C++ runtime lib when C++ toolchain declares it #562
Conversation
f1a6c6d
to
6408822
Compare
cc @MForster |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One last question, and then generally good. Would appreciate a second set of eyes from someone with more of a bazel background.
Update: I discovered that when creating an rlib artifact, rustc will open every static library and "copy" its contents into the rlib. This results in a quadratic disk usage growth and that's not acceptable in our workloads. I'll create a separate PR to stop passing native libraries when producing rlibs and wait with this one. |
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.
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.
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.
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.
f88c3ce
to
b0d91e2
Compare
#576 has to be submitted first. |
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 #562.
37d83d0
to
ff2d7f2
Compare
Currently `rules_rust` won't link against the C++ standard library in case C++ toolchain is the `static_link_cpp_runtimes` feature (therefore when it uses `cc_toolchain.static_runtime_lib` and `cc_toolchain.dynamic_runtime_lib` attributes to declare which version of C++ static library should be linked into the final binary). This PR modifies `get_libs_for_static_executable` to also look into the C++ toolchain and depend on C++ standard library from there. Once we implement the support for dynamically linked rust binaries we will likely have an equivalend `get_libs_for_dynamic_executable`, which would then look into `cc_toolchain.dynamic_runtime_lib`. While there, I moved the `get_cc_toolchain` function to the `utils.rs`, to be consistent with the handling of the Rust toolchain.
ff2d7f2
to
8ac6e3a
Compare
This PR is now ready for review. Blocking PRs have been merged, and I've tested it internally with a C++ toolchain that uses static/dynamic_runtime_lib. Please take another look :) |
Currently
rules_rust
won't link against the C++ standard library in caseC++ toolchain is the
static_link_cpp_runtimes
feature (therefore whenit uses
cc_toolchain.static_runtime_lib
andcc_toolchain.dynamic_runtime_lib
attributes to declare which version of C++ static library should be
linked into the final binary).
This PR modifies
add_native_link_flags
to also look into theC++ toolchain and depend on C++ standard library from there.
While there, I moved the
get_cc_toolchain
function to theutils.rs
,to be consistent with the handling of the Rust toolchain.