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

DO_NOT_SUBMIT: Do not provide CrateInfo from staticlib/cdylib #1219

Closed
wants to merge 1 commit into from

Conversation

hlopko
Copy link
Member

@hlopko hlopko commented Mar 24, 2022

This draft should be split into smaller PRs and needs tests.

This draft addresses #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.

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.
@scentini
Copy link
Collaborator

Closing this draft PR:

  • 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

@scentini scentini closed this Apr 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants