Depend on a copy of rustfmt for the target #2685
Conversation
These rules don't use this target as if it were a toolchain. Only as a data argument.
EdSchouten
force-pushed
the
eschouten/20240607-rustfmt
branch
3 times, most recently
from
16c8856 to
a4397db
Compare
The toolchain_files() and current_rust_toolchain() rules return files for the current 'exec' platform. This is perfecly reasonable if you want to use these tools as part of ctx.actions.run(). But if you want to use these as part of 'data = []' (runfiles), they must be built for the target. This change adds a helper rule, toolchain_files_for_target(), which can take the aforementioned rules as an input, transitioning it so that it yields files for the target platform. I'm not happy with how this rule is implemented, but there is little we can do about that until bazelbuild/bazel#19645 is addressed.
EdSchouten
force-pushed
the
eschouten/20240607-rustfmt
branch
from
a4397db to
51c26b1
Compare
There was a problem hiding this comment.
Thanks!
transition_to_target feels like it should maybe be built in to Bazel itself? @katre any idea where would be a good place for this transition to live for real?
(Happy to host a fork of it here in the mean time!)
|
@illicitonion I just read over the code, and I have a lot of questions about I kind of feel like this is a lot of indirection: how many rustfmt toolchain implementations are there? I'm happy to continue this discussion: if we made this a standard transition, it would belong in https://github.com/bazelbuild/rules_platform. Can you open a feature request there with a clear problem statement and any background context? Is this related to bazelbuild/bazel#19645? It feels similar but I'm not sure. |
|
Hey @katre,
Exactly!
As far as I know, it's equal to the number of Rust toolchains being registered. There could be multiple versions registered, but also for multiple operating systems/hardware architectures.
This is indeed related to bazelbuild/bazel#19645. Note that the approach we just merged uses a transition, but there's absolutely no hard requirement for that. If bazelbuild/bazel#19645 would result in some kind of direct syntax for doing platform transitions, or having a special 'target' exec group or something, that would also be fine. I will file an issue at https://github.com/bazelbuild/rules_platform regardless. |
[](https://renovatebot.com) This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | rules_rust | bazel_dep | minor | `0.45.1` -> `0.46.0` | | [rules_rust](https://togithub.com/bazelbuild/rules_rust) | http_archive | minor | `0.45.1` -> `0.46.0` | --- ### Release Notes <details> <summary>bazelbuild/rules_rust (rules_rust)</summary> ### [`v0.46.0`](https://togithub.com/bazelbuild/rules_rust/releases/tag/0.46.0) [Compare Source](https://togithub.com/bazelbuild/rules_rust/compare/0.45.1...0.46.0) ##### 0.46.0 ```python load("@​bazel_tools//tools/build_defs/repo:http.bzl", "http_archive") http_archive( name = "rules_rust", integrity = "sha256-F8U7+AC5MvMtPKGdLLnorVM84cDXKfDRgwd7/dq3rUY=", urls = ["https://github.com/bazelbuild/rules_rust/releases/download/0.46.0/rules_rust-v0.46.0.tar.gz"], ) ``` Additional documentation can be found at: https://bazelbuild.github.io/rules_rust/#setup ##### What's Changed - fix: default rustfmt version to supplied rust version by [@​mattem](https://togithub.com/mattem) in [https://github.com/bazelbuild/rules_rust/pull/2660](https://togithub.com/bazelbuild/rules_rust/pull/2660) - Use repo-mapping-aware runfiles API in rust-analyzer by [@​dzbarsky](https://togithub.com/dzbarsky) in [https://github.com/bazelbuild/rules_rust/pull/2666](https://togithub.com/bazelbuild/rules_rust/pull/2666) - Fix protobuf generated srcs to include all sources by [@​matts1](https://togithub.com/matts1) in [https://github.com/bazelbuild/rules_rust/pull/2676](https://togithub.com/bazelbuild/rules_rust/pull/2676) - Fix cargo_build_script executables. by [@​matts1](https://togithub.com/matts1) in [https://github.com/bazelbuild/rules_rust/pull/2675](https://togithub.com/bazelbuild/rules_rust/pull/2675) - crate_universe: Don't include crate name in build script path. by [@​criemen](https://togithub.com/criemen) in [https://github.com/bazelbuild/rules_rust/pull/2663](https://togithub.com/bazelbuild/rules_rust/pull/2663) - Ensure dynamic library dependencies end up in the runfiles directory by [@​EdSchouten](https://togithub.com/EdSchouten) in [https://github.com/bazelbuild/rules_rust/pull/2671](https://togithub.com/bazelbuild/rules_rust/pull/2671) - use bazel_ci_rules bazel_dep instead of http_archive by [@​mmorel-35](https://togithub.com/mmorel-35) in [https://github.com/bazelbuild/rules_rust/pull/2678](https://togithub.com/bazelbuild/rules_rust/pull/2678) - Ensure that running `bazel build` on a cargo_build_script target actually runs the build script by [@​matts1](https://togithub.com/matts1) in [https://github.com/bazelbuild/rules_rust/pull/2680](https://togithub.com/bazelbuild/rules_rust/pull/2680) - Allow overriding crate universe packages to local packages by [@​ograff](https://togithub.com/ograff) in [https://github.com/bazelbuild/rules_rust/pull/2674](https://togithub.com/bazelbuild/rules_rust/pull/2674) - Depend on a copy of rustfmt for the target by [@​EdSchouten](https://togithub.com/EdSchouten) in [https://github.com/bazelbuild/rules_rust/pull/2685](https://togithub.com/bazelbuild/rules_rust/pull/2685) - \[Bugfix] Convert metadata keys to screaming snake case instead of screaming kebab case in `cargo_build_script_runner` by [@​Pagten](https://togithub.com/Pagten) in [https://github.com/bazelbuild/rules_rust/pull/2682](https://togithub.com/bazelbuild/rules_rust/pull/2682) - Add support for override_target when using bzlmod by [@​AmeliasCode](https://togithub.com/AmeliasCode) in [https://github.com/bazelbuild/rules_rust/pull/2683](https://togithub.com/bazelbuild/rules_rust/pull/2683) - Release 0.46.0 by [@​illicitonion](https://togithub.com/illicitonion) in [https://github.com/bazelbuild/rules_rust/pull/2686](https://togithub.com/bazelbuild/rules_rust/pull/2686) ##### New Contributors - [@​mattem](https://togithub.com/mattem) made their first contribution in [https://github.com/bazelbuild/rules_rust/pull/2660](https://togithub.com/bazelbuild/rules_rust/pull/2660) - [@​EdSchouten](https://togithub.com/EdSchouten) made their first contribution in [https://github.com/bazelbuild/rules_rust/pull/2671](https://togithub.com/bazelbuild/rules_rust/pull/2671) - [@​mmorel-35](https://togithub.com/mmorel-35) made their first contribution in [https://github.com/bazelbuild/rules_rust/pull/2678](https://togithub.com/bazelbuild/rules_rust/pull/2678) - [@​ograff](https://togithub.com/ograff) made their first contribution in [https://github.com/bazelbuild/rules_rust/pull/2674](https://togithub.com/bazelbuild/rules_rust/pull/2674) - [@​Pagten](https://togithub.com/Pagten) made their first contribution in [https://github.com/bazelbuild/rules_rust/pull/2682](https://togithub.com/bazelbuild/rules_rust/pull/2682) **Full Changelog**: bazelbuild/rules_rust@0.45.1...0.46.0 </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 these updates again. --- - [ ] <!-- rebase-check -->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). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4zOTMuMCIsInVwZGF0ZWRJblZlciI6IjM3LjM5My4wIiwidGFyZ2V0QnJhbmNoIjoibWFzdGVyIiwibGFiZWxzIjpbXX0=--> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
](https://renovatebot.com){kind=link}
The toolchain_files() and current_rust_toolchain() rules return files for the current 'exec' platform. This is perfecly reasonable if you want to use these tools as part of ctx.actions.run(). But if you want to use these as part of 'data = []' (runfiles), they must be built for the target.
Fixes: #2684