bazelbuild
Registering toolchains via :all does not work with aliases.
Description of the bug:
Using the :all target with register_toolchain should register all the toolchains in the package. However, when the package contains aliases to toolchains this no longer works.
What's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.
A minimal example can be found here: https://github.com/cameron-martin/bazel-register-all-toolchains-repro
This builds fine when using register_toolchains("//toolchains:all"), which directly references the toolchains. However, if this is changed to register_toolchains("//:all"), which indirectly references the toolchain via an alias, no toolchains are registered and the following error occurs when building:
ERROR: /home/cameron/Repos/bazel-register-all-toolchains-repro/toolchains/BUILD:5:11: While resolving toolchains for target //toolchains:bar_binary: No matching toolchains found for types //toolchains:toolchain_type.
Which operating system are you running Bazel on?
Ubuntu 22.04
What is the output of bazel info release?
release 5.3.0/development version
If bazel info release returns development version or (@non-git), tell us how you built Bazel.
Using bazelisk last_green with commit fae86178fc94f28e509c3e1ed221e63b9fa433d9. The bug appears in both this version and 5.3.0.
What's the output of git remote get-url origin; git rev-parse master; git rev-parse HEAD ?
No response
Have you found anything relevant by searching the web?
No response
Any other information, logs, or outputs that you want to share?
No response
Hmm, this makes sense, the code in RegisteredToolchainsFunction is looking explicitly for toolchain rules, not aliases to toolchains.
The main reason for this is to reduce the number of targets being considered: many rules mix their toolchain targets with other targets (such as rust_toolchain), and so we want to load as few targets as possible to reduce the amount of memory and recursive dependencies analyzed.
It may be possible to change the filtering and make it also handle aliases, but I'm not sure when we can get to that. How big a problem is this for rules_rust? Is this something someone on your team could investigate? The relevant code change would be to use a different FilteringPolicy, possibly checking for the presence an alias specifically, or checking for a specific provider (`DeclaredToolchainInfo), except I don't know offhand whether aliases are capable of forwarding declared providers.
This makes sense. I think it is a blocker for rules_rust adopting bzlmod, since I don't think all the toolchains could easily be defined in one repository. I can take a look at implementing this, although I can't make any promises since I am not at all familiar with the codebase.
I tried fixing this by using a FilteringPolicy allowing both toolchain and alias targets. However, this caused undesired side effects for the Java and C++ toolchains shipped with Bazel since they are located in the same package as some of the artifacts they reference, causing eager fetches of e.g. JDKs for all platforms. I would expect third-party rulesets to be affected by this as well.
@katre What do you think of introducing a dedicated toolchain_alias rule that simply forwards DeclaredToolchainInfo from a dependency? That rule could safely be included in the filter without a potential for regressions. If DeclaredToolchainInfo (not the construcotr, just they key) could be exposed to Starlark, this rule could even be part of skylib rather than core Bazel. Alternatively, it could be a Starlark built-in rule.
I'll have a go at putting the toolchain proxies into a single repository in rules_rust, since this would likely be preferable to introducing new concepts/API surface into Bazel.
@cameron-martin Let me know if that should turn out to be infeasible.
@Wyverald Do you think we should still pursue this even if rules_rust can do without it?
i had a similar thought to Cameron's actually. The toolchain rules (not the rust_toolchain rules) are kind of already "pointer" rules. Maybe we can afford to have those in the hub repo instead? It's slightly clunky, but if it works, it does avoid the API surface increase (toolchain_alias) and the big churn migrating Java toolchains and the like.
If this ends up encouraging putting toolchains lang_toolchains in separate repos, that is actually a desirable side effect as it names eager fetches less likely. Sounds good to me, hope it works out for rules_rust.
Sorry, I've been out with covid for several days.
Just allowing alias won't work, for the reasons you point out. That's unfortunate, but not unexpected. If alias could somehow also report what providers it can provide (in a sane way, this needs some serious thought) then we could potentially do something, but I'm not sure that Bazel's analysis phase is currently robust enough to handle that.
Splitting toolchains into a separate repo makes a lot of sense here.
Putting all the toolchain proxies into one repository seems to work. Shall I close this issue?
I'd prefer to leave the issue open but low priority. While there's a workaround, it's good to track that there's a gap in our functionality and I would like to see if it can be addressed.