rustc: correctly handle alwayslink staticlibs #606
Conversation
durin42
force-pushed
the
alwayslink
branch
2 times, most recently
from
27ed182
to
142134c
Compare
durin42
force-pushed
the
alwayslink
branch
5 times, most recently
from
7eb1c3f
to
12bcb7c
Compare
|
@hlopko I'm closer, all the tests pass, but it looks like I broke something that's only covered by examples and I'm not running those locally? Is that right? (If so, what's the right way to run the examples? I'm not figuring it out...) |
$ bazel build @examples//...(Some don't run on non-Linux platforms, see https://github.com/bazelbuild/rules_rust/blob/main/.bazelci/presubmit.yml for a list of exceptions) |
|
@illicitonion thanks! Found my bug. :) |
Hurrah! (Hi, by the way! Long time no see!) |
|
Hi! It has been a while. Somehow unsurprising this is a corner I'd find
you... :)
…On Thu, Feb 25, 2021 at 7:06 PM Daniel Wagner-Hall ***@***.***> wrote:
@illicitonion <https://github.com/illicitonion> thanks! Found my bug. :)
Hurrah!
(Hi, by the way! Long time no see!)
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#606 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAE6LP4SXKSQTSMFTXIXALTA3QYZANCNFSM4YFUR3SQ>
.
|
This comment has been minimized.
This comment has been minimized.
durin42
force-pushed
the
alwayslink
branch
2 times, most recently
from
ccd3860
to
63e2b26
Compare
|
Alright, this now breaks @examples//ffi/rust_calling_c:matrix_dynamically_linked on windows and darwin, but I'm honestly a little confused if that's my fault. I think this is something that had to be broken all along and I'm merely exposing it? |
|
Okay, those broke in 3ad0e99, but it's not super obvious to me that that change is wrong. :( |
|
Did some other stuff, realized I could just "bake in" the old behavior in _compute_rpaths() with a TODO, so that's what did. |
durin42
force-pushed
the
alwayslink
branch
4 times, most recently
from
496dc81
to
93ff654
Compare
durin42
force-pushed
the
alwayslink
branch
2 times, most recently
from
aca6e17
to
01c5b55
Compare
We then move the function to its only remaining caller in bindgen. I didn't inline it there because it's used in a few different ways and cleaning it up felt like more work than it was immediately worth.
We need this information to implement support for alwayslink, and it cascaded around the codebase more than a little. The next change will implement always link, which is currently left as a stub so that this change has no functional changes.
Instead we just verify we're not going to link anything static. This removes the need for a toolchain in _is_dylib, which will allow further cleanups in a moment.
This allows us to use a more efficient construction to add linker arguments to the command line.
This follows up on the previous commit's refactoring and actually adds alwayslink support. Fixes bazelbuild#325.
Makes sure that cdylibs also correctly pick up their alwayslink dependencies.
This lets us avoid reifying a depset a little longer, which is nice.
| "_macos_constraint": attr.label(default = Label("@platforms//os:macos")), | ||
| "_windows_constraint": attr.label(default = Label("@platforms//os:windows")), | ||
| }) | ||
| cdylib_has_native_dep_and_alwayslink_test = analysistest.make(_cdylib_has_native_libs_test_impl, attrs = { |
There was a problem hiding this comment.
Looks like this this function is using the wrong implementation and correcting this causes a failure. @hlopko @durin42 would either of you be able to take a quick look? 🙏
| cdylib_has_native_dep_and_alwayslink_test = analysistest.make(_cdylib_has_native_libs_test_impl, attrs = { | |
| cdylib_has_native_dep_and_alwayslink_test = analysistest.make(_cdylib_has_native_dep_and_alwayslink_test_impl, attrs = { |
In order to do this, we have to inline get_libs_for_static_executable so we
can interact with the LibraryToLink entity as we're categorizing staticlibs.
Fixes #325.