Add support for Rust ABI dynamic libraries

[buntonj]

Currently, there is not explicit stable Rust ABI for dynamic libraries, but Rust library may still be compiled as one. This can be useful if a large project wants to compile some core portion of its code as a shared object that can be dynamically linked against several different binaries. Bevy, for example, offers this capability as a way to allow customers of Bevy to tweak their own code without having to statically re-link the central Bevy engine code.

This PR exposes the Rust ABI dylib capabilities with a new rule, rust_dylib_library. For clarity, it also creates a rust_cylib_library rule, and updates methods to this name, with rust_shared_library just aliasing to the rust_cdylib_library implementation.

Building a Rust ABI dylib may cause the standard library to be linked dynamically against the generated dylib. When depended on, the dynamic standard library needs to be included to get the binary to execute. Currently, rules_rust offers this as a toolchain-level flag, but applying a toolchain transition at the dylib's interface imposes a new toolchain on its deps, which then forces any upstream rlibs, for example, to recompile.

To fix this, this PR exposes a link_std_dylib attr to rust_binary, rust_dylib_library and rust_test to allow these targets to optionally bundle themselves with the dynamically-linked stdlib.

[google-cla]

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

[UebelAndre]

I'd love @krasimirgg to take a look since I think there's some google uses of experimental_link_std_dylib. The change makes sense to me but I wonder if link_std_dylib should have more impact outside of just the allocators and codegen flag. The attribute sounds like it would directly link the dylibs but I don't think there's any wiring for that. Maybe @krasimirgg can correct me if I'm wrong

[UebelAndre]

Maybe rename these attributes on the toolchain as well?

[buntonj]

Yeah, totally, done!

[krasimirgg]

Hey, I'm not an expert on dynamic linking, so please bear with me...
I don't see how this composes with the rest of the rust ruleset. Two high-level questions:

1. What's the expected way for clients (rust_binary) depending on a rust_dylib_library to work? Are folks running them expected to supply the .so-s manually? Would bazel run do some work to ensure the dylibs are correctly provided?
2. What happens if we have a mix of rust_dylib_library and rust_library dependencies? How do we avoid duplicating shared dependencies in dylibs, rendering them incompatible? For instance, if you have:
   For example, if I read the idea correctly, if you have:

rust_library(name = "d")
rust_dylib_library(name = "c", deps = ["d"])
rust_dylib_library(name = "b", deps = ["d"])
rust_library(name = "a", deps = ["c", "b"])

, you'll likely get rustc error: cannot satisfy dependencies so d only shows up once while compiling a. Example rustc commands I'm thinking about -- https://gist.github.com/krasimirgg/66c52fbebee7af4813e1e71af495bd4a.

Tangent 1: On a high-level, I see a way to set up rust dynamic libraries support in a way similar to how rules_cc does dynamic libraries -- the rust_library itself has actions that produce both the rlib and the dylib, and clients pick up the appropriate dependency based on the --dynamic_mode build setting. This is composable, in particular any transitive dependencies of a dylib are automatically dylibs, avoiding the case where duplicated common dependencies render intermediate dylibs incompatible. But in a world like this, having a separate rust_dylib_library rule becomes just something almost equivalent to a rust_library.

Tangent 2: The reason why I feel this is different than rust_shared_library (cdylib) is that that one is designed to act like a final artifact -- you collect all of your rust into a single such target, which acts more like a final binary; standard rust libraries cannot direclty depend on it and you just hook it up as a special dependency of the final binary.

[UebelAndre]

1. What's the expected way for clients (rust_binary) depending on a rust_dylib_library to work? Are folks running them expected to supply the .so-s manually? Would bazel run do some work to ensure the dylibs are correctly provided?

I would expect Bazel to bundle the .so files as runfiles and have appropriate loader paths to support that, similar to how rules_cc does.

2. What happens if we have a mix of rust_dylib_library and rust_library dependencies? How do we avoid duplicating shared dependencies in dylibs, rendering them incompatible? For instance, if you have:
   For example, if I read the idea correctly, if you have:

rust_library(name = "d")
rust_dylib_library(name = "c", deps = ["d"])
rust_dylib_library(name = "b", deps = ["d"])
rust_library(name = "a", deps = ["c", "b"])

, you'll likely get rustc error: cannot satisfy dependencies so d only shows up once while compiling a. Example rustc commands I'm thinking about -- https://gist.github.com/krasimirgg/66c52fbebee7af4813e1e71af495bd4a.

What makes the dylib unique in this case compared to rlib? Wouldn't the dylib be a runfile that needs to be bubbled up along with the top level rlib?

Tangent 1: On a high-level, I see a way to set up rust dynamic libraries support in a way similar to how rules_cc does dynamic libraries -- the rust_library itself has actions that produce both the rlib and the dylib, and clients pick up the appropriate dependency based on the --dynamic_mode build setting. This is composable, in particular any transitive dependencies of a dylib are automatically dylibs, avoiding the case where duplicated common dependencies render intermediate dylibs incompatible. But in a world like this, having a separate rust_dylib_library rule becomes just something almost equivalent to a rust_library.

I'm not sure it's always desirable to turn everything into dynamic libraries globally like this. Lets say you had a rust_binary with 3 rust_library_group dependencies, would it be reasonable to turn a number of those into rust_dylib_library to avoid the more expensive linking of rust_binary? That sounds like a nice optimization on paper.

Tangent 2: The reason why I feel this is different than rust_shared_library (cdylib) is that that one is designed to act like a final artifact -- you collect all of your rust into a single such target, which acts more like a final binary; standard rust libraries cannot direclty depend on it and you just hook it up as a special dependency of the final binary.

I agree rust_cdylib_library/rust_shared_library can be thought of as an output, akin to rust_binary. I think the open question is should a conceptual new dylib (not cdylib) rule behave the same?

[buntonj]

Thanks for thinking about this @krasimirgg ! Some thoughts:

What's the expected way for clients (rust_binary) depending on a rust_dylib_library to work? Are folks running them expected to supply the .so-s manually? Would bazel run do some work to ensure the dylibs are correctly provided?

Bazel should provide the .so files for dylibs that are in deps--that's in this PR (they have CcInfo that gets collected and bundled with the runfiles).

What happens if we have a mix of rust_dylib_library and rust_library dependencies? How do we avoid duplicating shared dependencies in dylibs, rendering them incompatible?

I feel like this is a limitation of dynamic linking with rustc than something we have to solve in rules_rust...which is why for this PR, I opted to expose the tools to manually walk back through the dep graph and convert things to dylibs that must be for their target to compile.

This is definitely a possible footgun though--asking for dynamic linking at some point in the chain might imply other libraries should become dynamically linked. It's not required if no dependency diamonds like this occur, though.

Tangent 1: On a high-level, I see a way to set up rust dynamic libraries support in a way similar to how rules_cc does dynamic libraries -- the rust_library itself has actions that produce both the rlib and the dylib, and clients pick up the appropriate dependency based on the --dynamic_mode build setting. This is composable, in particular any transitive dependencies of a dylib are automatically dylibs, avoiding the case where duplicated common dependencies render intermediate dylibs incompatible. But in a world like this, having a separate rust_dylib_library rule becomes just something almost equivalent to a rust_library.

I think this is very cool, and could be something to build toward. In my use case, I wanted to avoid forcing every transitive dep to be unconditionally built as both an rlib and a dylib, when for many steps in that graph, only the rlib version was required (in my simpler use case, I had a deep dependencies with a dylib at the focal interface point). Instead, this lets me mark only the required targets as dylibs to save compilation time.

[UebelAndre]

@krasimirgg friendly ping here

[krasimirgg]

Hey sorry, was a bit out of office, replies below.

cc @UebelAndre @buntonj

What makes the dylib unique in this case compared to rlib? Wouldn't the dylib be a runfile that needs to be bubbled up along with the top level rlib?

rust_library(name = "d")
rust_dylib_library(name = "c", deps = ["d"])
rust_dylib_library(name = "b", deps = ["d"])
rust_library(name = "a", deps = ["c", "b"])

Here, both of the dylibs-s c.so and b.so will include the definitions for symbols from the target :d -- there is a linking step in both, so both will statically link in d.rlib (because there is no d.so available). Then later, when you're trying to build a.so, you run into duplicate symbol errors -- there are two copies of the symbols from d.rlib -- one embedded in c.so and another in b.so.
If :c and :b were rust_library-es, their .rlib-s only contain references to the symbols from :d, not definitions. That avoids the potential for duplicate symbols. That's what I mean by saying rust_dylib_library does not compose with the rest of the rulesets -- the end-user needs to ensure something like: for every target A, for every transitive rust_library dependency D of A, if there exist a dependency chain from A to D that passes trough an intermediate rust_dylib_library L, then ALL possible paths from A to D must pass through L (because D will be statically linked into L.so).

What happens if we have a mix of rust_dylib_library and rust_library dependencies? How do we avoid duplicating shared dependencies in dylibs, rendering them incompatible?
I feel like this is a limitation of dynamic linking with rustc than something we have to solve in rules_rust...which is why for this PR, I opted to expose the tools to manually walk back through the dep graph and convert things to dylibs that must be for their target to compile.

I think the introduction of this rule is OK as long as we put some big disclaimers around these limitations and document this rule so whenever some user tries using it and linking fails, we can point to that.

About the tangent, just informationally:

I wanted to avoid forcing every transitive dep to be unconditionally built as both an rlib and a dylib, when for many steps in that graph, only the rlib version was required...

We've started internally poking around adding full --dynamic_mode support (so a cc_test that (transitively) depends on a rust_library gets it as a pair of .so/.ifso-s similarly to cc_library dependencies). We're exploring some approaches around rustc building both an rlib and dylib, vs., having cc_common.link produce the .so from the .rlib (and the .ifso from the .so), etc. I'm hopeful that when we have something satisfactory, we upstream it.

[buntonj]

Hey, no problem--thanks again for the time!

I think the introduction of this rule is OK as long as we put some big disclaimers around these limitations and document this rule so whenever some user tries using it and linking fails, we can point to that.

I'm 100% with you here. I added some language in the docs about this, let me know if it needs revising though.

I made a few other improvements to help support this PR:

• Added integration tests that have a rust_binary dep on a rust_dylib_library
• Fixed some issues with RPATH computation for different execution locations (ran into these when road-testing this PR on my use case). I took a page out of rules_cc's book and opted to supply all of the relevant RPATHs when linking a dylib (see here for my shameless rip).
• Added support for Windows, where RPATH isn't a concept--again took a page out of rules_cc and opted to symlink the dylibs adjacent to the binary so it's found on Windows. Added a test for this.

Also for your tangent:

We've started internally poking around adding full --dynamic_mode support (so a cc_test that (transitively) depends on a rust_library gets it as a pair of .so/.ifso-s similarly to cc_library dependencies). We're exploring some approaches around rustc building both an rlib and dylib, vs., having cc_common.link produce the .so from the .rlib (and the .ifso from the .so), etc. I'm hopeful that when we have something satisfactory, we upstream it.

This sounds awesome. If I can support at all in the future I'd be stoked to!

[buntonj]

cc @krasimirgg

[krasimirgg]

Hey @buntonj , we chatted a bit with @UebelAndre last week about this, who would also follow-up on this review. The whole thing is because of the above concerns so we should tread carefully with the whole feature.

[krasimirgg]

don't we also need this for rust_binary?

[buntonj]

Yep, it's there!

[krasimirgg]

Do we have use cases at all for rust_dylib_library where we don't wanna link the stdlibs dynamically? Naively, for this rule the default should be true?

[buntonj]

Yeah, that's totally fair. AFAICT, Rust ABI dylibs will just be depended on by Rust targets, which would require link_std_dylib anywho, so setting the default to true is harmless. Done!

[krasimirgg]

I think we're losing something with this change -- previously, it was possible to have 2 rust_toolchain-s where one has this enabled and the other doesn't. With the new version, all possible toolchains use same default. I think we should switch to a pattern where _link_std_dylib points to a label as in the new version, but there is an optional per-toolchain override with values [-1, 0, 1] as in experimental_use_allocator_libraries_with_mangled_symbols.

[buntonj]

Ah, great catch and suggestion. Migrated to using the same [-1, 0, 1] pattern, which leaves us with the ability to toolchain-level or target-level change this flag.

[krasimirgg]

(just informational): I personally am not familiar with the RPATH feature and ways it's working in bazel. I'm aiming to read up on this, but currently I can't vouch for those parts.

[UebelAndre]

Looking good so far! Can you also make sure we add a section for rust_dylib_library and rust_cdylib_library to docs with this change?

[UebelAndre]

Is this flag no longer experimental now?

[buntonj]

Ah, less that it's not experimental and more that it's now a computed quantity/property as a function of the toolchain-level experimental_link_std_dylib and the target-level link_std_dylib.

I opted to try and rename things so that the experimental_link_std_dylib hook and name are the same, but all internal customers/byproducts of the new link_std_dylib are not experimental_ anymore.

[UebelAndre]

I'm concerned this will lead to conflicting action outputs. Do you know what rules_cc does here?

[buntonj]

I actually borrowed this pattern from rules_cc's _create_dynamic_libraries_copy_actions. See here and here.

Bazel handles the deduplication of targets in the relevant cases also.

[buntonj]

(I added a comment in this spot to this effect so we remember! For those who come after...)