Add extra_rustc_flags_for_crate_types. #2431

granaghan · 2024-01-20T00:07:49Z

This allows extra rustc flags that will only apply to library targets to be set by the toolchain. Cross language LTO needs '-C linker-plugin-lto' on libraries, how passing this flag to rust_binary generated bloat. Adding this attribute resolves this issue.

Change-Id: Iba725fab1b1941e9586ff97cd71ec3bc1dfc1523

daivinhtran

I left a comment. I think macros would be the solution for your use cases. Let me know if macros won't work.

rust/private/rustc.bzl

daivinhtran

This LGTM. Do you mind adding some tests?

granaghan · 2024-03-15T00:53:34Z

I added a simple test. Is there a way to get the CrateInfo provider in the test so I can test with multiple crate types? I poked around a bit and didn't manage to, but I'm pretty new to working with rule implementations.

matts1 · 2024-03-15T01:06:54Z

To get any provider, you just need to do target[Provider] on a target that defines a CrateInfo (cc_binary and cc_library). However, looking at the test, it seems quite confusing, but it only seems to do assertions on the toolchain itself, so doesn't have access to cc_binary and cc_library targets, so there's no CrateInfo to get.

granaghan · 2024-03-20T22:47:53Z

From the CI:

bazel-out/darwin_x86_64-opt-exec-ST-a84ee23024d0/bin/util/process_wrapper/process_wrapper.sh: line 18: /private/var/tmp/_bazel_buildkite/e4140ff87fa9a41eb6ddd3bb20d07fdc/sandbox/darwin-sandbox/22/execroot/rules_rust/bazel-out/darwin_x86_64-opt-exec-ST-a84ee23024d0/bin/test/toolchain/rust_extra_flags_toolchain/bin/mock_rustc.exe: Undefined error: 0

Did I do something to make it try and run the fake binaries? Is it the rust_binary I added?

daivinhtran · 2024-03-21T21:04:01Z

@granaghan I merged the main branch into your branch to rerun CI. Now it's failing with different error.

Can you pull the updated branch, run ./docs/update_docs.sh locally, and then git push again?

granaghan · 2024-03-22T23:37:56Z

It looks like the coverage check is still tying to use the mock compiler. For some reason it's building for OS none on both Ubuntu and Mac. It passes for me locally though.

    CARGO_CFG_TARGET_OS=none \

granaghan · 2024-03-23T00:45:41Z

Okay, I managed to repro it after gathering all the additions flags. It seems that //rust/settings:experimental_use_coverage_metadata_files is causing this, but I'm not sure why yet.

granaghan · 2024-03-23T01:12:18Z

It seems this only happens if I declare a rust_binary in the tests. I switched rust_shared_library and it passes.

This allows extra rustc flags that will only apply to specific library types to be set by the toolchain. Cross language LTO needs '-C linker-plugin-lto' on libraries, how passing this flag to rust_binary generated bloat. Adding this attribute resolves this issue. Change-Id: Iba725fab1b1941e9586ff97cd71ec3bc1dfc1523

https://blog.rust-lang.org/2024/03/21/Rust-1.77.0.html

Change-Id: I75472b1f9bb67d7348ad31a495d59344c4a6edf1

granaghan · 2024-03-27T20:17:17Z

Any other changes here or is this good to go?

daivinhtran

LGTM. Can you also file an issue about the failure with rust_binary? Thanks!

granaghan · 2024-03-28T19:01:27Z

Added #2583. I don't have write access, so you'll need to merge. Thanks!

[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [rules_rust](https://togithub.com/bazelbuild/rules_rust) | http_archive | patch | `0.41.0` -> `0.41.1` | --- ### Release Notes <details> <summary>bazelbuild/rules_rust (rules_rust)</summary> ### [`v0.41.1`](https://togithub.com/bazelbuild/rules_rust/releases/tag/0.41.1) [Compare Source](https://togithub.com/bazelbuild/rules_rust/compare/0.41.0...0.41.1) ### 0.41.1 ```python load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive") http_archive( name = "rules_rust", integrity = "sha256-mUV3N2A8ORVVZbrm3O9yepAe/Kv4MD2ob9YQhB8aOI8=", urls = ["https://github.com/bazelbuild/rules_rust/releases/download/0.41.1/rules_rust-v0.41.1.tar.gz"], ) ``` Additional documentation can be found at: https://bazelbuild.github.io/rules_rust/#setup #### What's Changed - Add extra_rustc_flags_for_crate_types. by [@granaghan](https://togithub.com/granaghan) in [bazelbuild/rules_rust#2431 - Android jobs should be using LTS Bazel releases by [@UebelAndre](https://togithub.com/UebelAndre) in [bazelbuild/rules_rust#2589 - BUG-FIX: host-triple str for bzl mod by [@ericmcbride](https://togithub.com/ericmcbride) in [bazelbuild/rules_rust#2587 - fix(cargo-bazel): ignore example crates when checking if proc-macro by [@qtica](https://togithub.com/qtica) in [bazelbuild/rules_rust#2596 - Deprecated `rust_bindgen.leak_symbols` by [@UebelAndre](https://togithub.com/UebelAndre) in [bazelbuild/rules_rust#2590 - Update test metadata for crate_universe by [@UebelAndre](https://togithub.com/UebelAndre) in [bazelbuild/rules_rust#2599 - Fixed bug where crate_universe could match aliases to bench/example deps by [@UebelAndre](https://togithub.com/UebelAndre) in [bazelbuild/rules_rust#2600 - Cleanup splicing utils by [@dzbarsky](https://togithub.com/dzbarsky) in [bazelbuild/rules_rust#2564 - Updated repository rules to notify users about non-reproducible repos. by [@UebelAndre](https://togithub.com/UebelAndre) in [bazelbuild/rules_rust#2593 - feat: Strip debug info from opt builds by [@matte1](https://togithub.com/matte1) in [bazelbuild/rules_rust#2513 - Release 0.41.1 by [@UebelAndre](https://togithub.com/UebelAndre) in [bazelbuild/rules_rust#2592 #### New Contributors - [@qtica](https://togithub.com/qtica) made their first contribution in [bazelbuild/rules_rust#2596 - [@matte1](https://togithub.com/matte1) made their first contribution in [bazelbuild/rules_rust#2513 **Full Changelog**: bazelbuild/rules_rust@0.41.0...0.41.1 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/bazel-contrib/toolchains_llvm).  Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

daivinhtran suggested changes Jan 20, 2024

View reviewed changes

rust/private/rustc.bzl Outdated Show resolved Hide resolved

granaghan closed this Jan 24, 2024

granaghan reopened this Feb 27, 2024

granaghan force-pushed the rlib_rustc_flags branch from 5707027 to 668d23f Compare March 12, 2024 21:51

daivinhtran reviewed Mar 14, 2024

View reviewed changes

granaghan force-pushed the rlib_rustc_flags branch from 668d23f to 8442d3e Compare March 15, 2024 00:50

granaghan changed the title ~~Add extra_rustlib_rustc_flags.~~ Add extra_rustc_flags_for_crate_types. Mar 15, 2024

granaghan force-pushed the rlib_rustc_flags branch 6 times, most recently from d17f136 to 1c87ac6 Compare March 20, 2024 19:41

granaghan requested a review from daivinhtran March 20, 2024 22:42

granaghan force-pushed the rlib_rustc_flags branch 2 times, most recently from 6222d3f to 5e2943d Compare March 23, 2024 01:11

granaghan and others added 3 commits March 23, 2024 01:14

Added Rust 1.77.0 (bazelbuild#2568)

eca31ac

https://blog.rust-lang.org/2024/03/21/Rust-1.77.0.html

Regenerate documentation

57e9798

Change-Id: I75472b1f9bb67d7348ad31a495d59344c4a6edf1

granaghan force-pushed the rlib_rustc_flags branch from 5e2943d to 57e9798 Compare March 23, 2024 01:14

Merge branch 'main' into rlib_rustc_flags

d25fe40

daivinhtran approved these changes Mar 27, 2024

View reviewed changes

granaghan mentioned this pull request Mar 28, 2024

Adding a rust_binary target to toolchain_test.bzl causes failure. #2583

Open

daivinhtran added this pull request to the merge queue Mar 29, 2024

daivinhtran requested a review from matts1 March 29, 2024 15:56

Merged via the queue into bazelbuild:main with commit 63fbedb Mar 29, 2024
3 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add extra_rustc_flags_for_crate_types. #2431

Add extra_rustc_flags_for_crate_types. #2431

granaghan commented Jan 20, 2024

daivinhtran left a comment

daivinhtran left a comment

granaghan commented Mar 15, 2024

matts1 commented Mar 15, 2024

granaghan commented Mar 20, 2024

daivinhtran commented Mar 21, 2024

granaghan commented Mar 22, 2024

granaghan commented Mar 23, 2024

granaghan commented Mar 23, 2024

granaghan commented Mar 27, 2024

daivinhtran left a comment

granaghan commented Mar 28, 2024

Add extra_rustc_flags_for_crate_types. #2431

Add extra_rustc_flags_for_crate_types. #2431

Conversation

granaghan commented Jan 20, 2024

daivinhtran left a comment

Choose a reason for hiding this comment

daivinhtran left a comment

Choose a reason for hiding this comment

granaghan commented Mar 15, 2024

matts1 commented Mar 15, 2024

granaghan commented Mar 20, 2024

daivinhtran commented Mar 21, 2024

granaghan commented Mar 22, 2024

granaghan commented Mar 23, 2024

granaghan commented Mar 23, 2024

granaghan commented Mar 27, 2024

daivinhtran left a comment

Choose a reason for hiding this comment

granaghan commented Mar 28, 2024