Skip to content
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

Cannot link rust_static_library in a rust_binary target #1063

Closed
boxdot opened this issue Dec 10, 2021 · 8 comments
Closed

Cannot link rust_static_library in a rust_binary target #1063

boxdot opened this issue Dec 10, 2021 · 8 comments
Labels

Comments

@boxdot
Copy link

boxdot commented Dec 10, 2021

The symbols from rust_static_library are not found when linking it to the rust_binary target via deps. In the invocation of the linker the path to the library directory is added via -L, however there is not library parameter with -l. Also note that the library has a numeric suffix generated from its name.

A workaround is possible via defining a cc_library target, linking the rust_static_library to it, and then using it in the rust_binary target.

Example BUILD file and repository to reproduce:

https://github.com/boxdot/bazel-rust-linking-issue/blob/fc92fb2b6384c78f9a5df95035c5dd882ad11694/BUILD.bazel

@UebelAndre
Copy link
Collaborator

UebelAndre commented Mar 23, 2022

@hlopko curious what your thoughts are on this interaction. Not quite sure how to triage this.

hlopko added a commit to hlopko/rules_rust that referenced this issue Mar 24, 2022
This draft should be split into smaller PRs and needs tests.

This draft addresses bazelbuild#1063.
There are multiple small fixes:

* currently we don't include the output hash for cdylib, but we do for
  staticlib. It's more accurate to not include the output hash for
  staticlib as well.
* currently rust_static_library and rust_shared_library announce they
  provide CrateInfo, DepInfo, and DefaultInfo. They should announce
  providing CcInfo (and maybe DefaultInfo - I thought that is implied by
  default so there's no need to explicitly mention that)
* currently rust_static_library and rust_shared_library actually
  provide CrateInfo and DepInfo. That is incorrect - outputs of these
  rules are not meant for consumption by Rust rules. These rules are
  meant to be used when leaving the world of Rust rules and entering the
  world of other language that wants to depend on native library (and it
  doesn't matter to this user if the library is written in Rust, C, C++,
  or other)

Lastly, this PR makes https://github.com/boxdot/bazel-rust-linking-issue build. Surprising plot twist - it is actually not supported to link rust_static_library into a rust_binary. From rustc docs:

> --crate-type=staticlib, #[crate_type = "staticlib"] - A static system
> library will be produced. This is different from other library outputs
> in that the compiler will never attempt to link to staticlib outputs.
> The purpose of this output type is to create a static library containing
> all of the local crate's code along with all upstream dependencies. This
> output type will create *.a files on Linux, macOS and Windows (MinGW),
> and *.lib files on Windows (MSVC). This format is recommended for use in
> situations such as linking Rust code into an existing non-Rust
> application because it will not have dynamic dependencies on other Rust
> code.

https://doc.rust-lang.org/reference/linkage.html#linkage.

The `bazel-rust-linking-issue` fails at linktime since there are
multiple definitions of allocator stubs. We may want to consider
detecting this situation and failing the build with a nice error message
saying this is not how these rules are intended to be used.
@hlopko
Copy link
Member

hlopko commented Mar 24, 2022

Hey @boxdot thank you for the repro, it was super helpful. There are indeed various bugs that I think I hacked out in #1219 (however that draft is not ready for submission, it needs tests and potentially polishing - feel free to take over :). See the draft description for details.

On a different level though a question begs to be asked - why are you using rust_static_library if you are using rust_binary for linking? I believe that rustc doesn't support this scenario. Why not use rust_library?

@boxdot
Copy link
Author

boxdot commented Mar 24, 2022

Thank you for providing a possible fix.

why are you using rust_static_library

As in my example, the dependency is on the C-level. It is not a usual Rust crate dependency. The rust lib implements a C-API. The final rust binary uses the C-API loosely via linking the needed symbols. So, we need to link the library as static library into the Rust binary. C-API is important here, because in a real-world scenario, the rust binary might link different implementations of the C-API. Or, the Rust implementation of the C-API can be linked into different binaries (Rust or not).

@hlopko
Copy link
Member

hlopko commented Mar 24, 2022

I see, then I'm afraid rustc doesn't really support your use case right now. The staticlib crate output contains allocators shims generated, and bin crate output also generates them. The linking fails with duplicated symbols (on linux, I haven't tested on mac, but even if it passed there it wouldn't be a robust solution). There is nothing we can do on rules_rust side to fix it.

Is it an option to use cc_binary instead of rust_binary? That is fully supported by rules_rust and by rustc.

scentini pushed a commit that referenced this issue Mar 24, 2022
…y and rust_shared_library (#1222)

Extracted from https://github.com/bazelbuild/rules_rust/pull/1219/files, addressing this part:
* currently we don't include the output hash for cdylib, but we do for staticlib. It's more accurate to not include the output hash for staticlib as well.


Related to, but does not address #1063.

@scentini suggested to not even emit --codegen flags in these cases.

Co-authored-by: Marcel Hlopko <hlopko@google.com>
@scentini
Copy link
Collaborator

#1298 and #1222 took care of rules_rust issues uncovered here, however as @hlopko notes in #1219:

it is actually not supported to link rust_static_library into a rust_binary. From rustc docs:

--crate-type=staticlib, #[crate_type = "staticlib"] - A static system
library will be produced. This is different from other library outputs
in that the compiler will never attempt to link to staticlib outputs.
The purpose of this output type is to create a static library containing
all of the local crate's code along with all upstream dependencies. This
output type will create *.a files on Linux, macOS and Windows (MinGW),
and *.lib files on Windows (MSVC). This format is recommended for use in
situations such as linking Rust code into an existing non-Rust
application because it will not have dynamic dependencies on other Rust
code.

https://doc.rust-lang.org/reference/linkage.html#linkage.

At HEAD, bazel-rust-linking-issue now fails with duplicate symbols:

note: duplicate symbol '___rust_alloc_error_handler' in:
              bazel-out/darwin-fastbuild/bin/rust_binary.46d3zwq56yl4ofng.rcgu.o
              bazel-out/darwin-fastbuild/bin/librust_c_lib.a(rust_c_lib.46voediov2mf69ue.rcgu.o)
...
ld: 5 duplicate symbols for architecture x86_64

As there's nothing we can do about this, should we close this issue?

@keith
Copy link
Member

keith commented Apr 28, 2022

In other rule sets with rules that are explicitly only used for archiving targets to use outside of bazel, they sometimes don't propagate the right providers to be included up the dependency tree. rules_apple has a few of these. Maybe rust_static_library should do the same? and just propagate the files via DefaultInfo?

@hlopko
Copy link
Member

hlopko commented Apr 29, 2022

Hi Keith, yeah this is exactly what @scentini did in #1298.

@hlopko
Copy link
Member

hlopko commented Jun 3, 2022

I believe we can close this issue now. Please let me know if there's something I missed.

@hlopko hlopko closed this as completed Jun 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants