chore: expose toolchains helper as public API#207
Conversation
It's meant to be used in toolchainization of language rules, for example in rules_python: https://github.com/bazelbuild/rules_python/pull/1577/files#diff-9cb07a0e8453ad8b4f429bee2ccf9de818a78cfb6e5d9bb908d8518ea189e2a7R18
Relies on bazelbuild/rules_proto#207 landing and being available in a rules_proto release.
|
The other answer might be that this helper is intended to be private API, and we should copy-paste logic from it into each ruleset? |
| # Modules | ||
| proto_common = _proto_common | ||
| ProtoInfo = _ProtoInfo | ||
| toolchains = _toolchains |
There was a problem hiding this comment.
I'd prefer not to expose additional symbols in defs.bzl. Each symbol should be in a separate bzl file. That reduces transitively loaded bzl files and prevents repo, macro, and rule code from needlessly loading each other.
Also, exposing symbol toolchains seems to be a very general name.
Also toolchains contains a temporary transitional functionality, doesn't it?
There was a problem hiding this comment.
Let me try to explain more clearly:
In language ruleset implementations like py_proto_library, do you expect these three helpers to be available?
https://github.com/bazelbuild/rules_proto/blob/main/proto/proto_common.bzl#L51-L53
use_toolchainfind_toolchainif_legacy_toolchain
Currently I'm forced to copy-paste some of that logic instead.
There was a problem hiding this comment.
I expect those 3 helpers to be temporary. After toolchain resolution is flipped on and well tested, the helpers may be removed. Since they need to be removed from wherever they are used I believe that copy-pasting is a better approach in this case.
There was a problem hiding this comment.
Note @fmeum as you recommended them yesterday
Relies on bazelbuild/rules_proto#207 landing and being available in a rules_proto release.
2 participants
It's meant to be used in toolchainization of language rules, for example in rules_python: https://github.com/bazelbuild/rules_python/pull/1577/files#diff-9cb07a0e8453ad8b4f429bee2ccf9de818a78cfb6e5d9bb908d8518ea189e2a7R18
Fixes that PR following the recent breaking change landed here: d4c3498#commitcomment-139420930