Skip to content

Commit

Permalink
Support automatic exec groups in proto_common.compile
Browse files Browse the repository at this point in the history
Pass toolchain_type through ProtoLangToolchainInfo into proto_common.compile and use it on ctx.actions.run. Automatic exec groups require that toolchain type is set on the `ctx.actions.run`. This information is used to select correct execution platform.

Expose INCOMPATIBLE_ENABLE_PROTO_TOOLCHAIN_RESOLUTION in proto_common. This will be needed to support lang_proto_libraries that are not part of Bazel. For example py_proto_library. Other methods in `toolchains` struct in proto_common.bzl, are both temporary and can be written in Starlark, so don't expose them. It's possible to access the value in backward compatible manner (that is with `getattr(proto_common, ...)`).

Expose INCOMPATIBLE_PASS_TOOLCHAIN_TYPE in proto_common. Second "flag" is here to mark, that builtin `proto_lang_toolchain` rule has a `toolchain_type` attribute. This way `proto_lang_toolchain` macro can pass the value in a compatible fashion with older Bazel. This should make toolchainisation work with older versions of Bazel that don't know about automatic exec groups and don't need to pass in the value.

Issue: bazelbuild/rules_proto#179
PiperOrigin-RevId: 571876657
Change-Id: I543ab862b318c9062d40430160e33ad197973094
  • Loading branch information
comius authored and copybara-github committed Oct 9, 2023
1 parent 10169bb commit 5a24c8a
Show file tree
Hide file tree
Showing 4 changed files with 12 additions and 7 deletions.
6 changes: 4 additions & 2 deletions src/main/starlark/builtins_bzl/common/proto/proto_common.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ ProtoLangToolchainInfo = provider(
mnemonic = "(str) Mnemonic to set on the proto compiler action.",
allowlist_different_package = """(Target) Allowlist to create lang_proto_library in a
different package than proto_library""",
toolchain_type = """(Label) Toolchain type that was used to obtain this info""",
),
)

Expand Down Expand Up @@ -238,7 +239,7 @@ def _compile(
use_default_shell_env = True,
resource_set = resource_set,
exec_group = experimental_exec_group,
toolchain = None,
toolchain = getattr(proto_lang_toolchain_info, "toolchain_type", None),
)

_BAZEL_TOOLS_PREFIX = "external/bazel_tools/"
Expand Down Expand Up @@ -375,7 +376,6 @@ toolchains = struct(
use_toolchain = _use_toolchain,
find_toolchain = _find_toolchain,
if_legacy_toolchain = _if_legacy_toolchain,
INCOMPATIBLE_ENABLE_PROTO_TOOLCHAIN_RESOLUTION = _builtins.toplevel.proto_common.incompatible_enable_proto_toolchain_resolution(),
)

proto_common_do_not_use = struct(
Expand All @@ -386,4 +386,6 @@ proto_common_do_not_use = struct(
experimental_filter_sources = _experimental_filter_sources,
get_import_path = _get_import_path,
ProtoLangToolchainInfo = ProtoLangToolchainInfo,
INCOMPATIBLE_ENABLE_PROTO_TOOLCHAIN_RESOLUTION = _builtins.toplevel.proto_common.incompatible_enable_proto_toolchain_resolution(),
INCOMPATIBLE_PASS_TOOLCHAIN_TYPE = True,
)
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

"""A Starlark implementation of the proto_lang_toolchain rule."""

load(":common/proto/proto_common.bzl", "ProtoLangToolchainInfo", "toolchains")
load(":common/proto/proto_common.bzl", "ProtoLangToolchainInfo", "toolchains", proto_common = "proto_common_do_not_use")
load(":common/proto/proto_info.bzl", "ProtoInfo")
load(":common/proto/proto_semantics.bzl", "semantics")

Expand All @@ -32,7 +32,7 @@ def _rule_impl(ctx):
if ctx.attr.plugin != None:
plugin = ctx.attr.plugin[DefaultInfo].files_to_run

if toolchains.INCOMPATIBLE_ENABLE_PROTO_TOOLCHAIN_RESOLUTION:
if proto_common.INCOMPATIBLE_ENABLE_PROTO_TOOLCHAIN_RESOLUTION:
proto_compiler = ctx.toolchains[semantics.PROTO_TOOLCHAIN].proto.proto_compiler
protoc_opts = ctx.toolchains[semantics.PROTO_TOOLCHAIN].proto.protoc_opts
else:
Expand All @@ -51,6 +51,7 @@ def _rule_impl(ctx):
progress_message = ctx.attr.progress_message,
mnemonic = ctx.attr.mnemonic,
allowlist_different_package = ctx.attr.allowlist_different_package,
toolchain_type = ctx.attr.toolchain_type.label if ctx.attr.toolchain_type else None,
)
return [
DefaultInfo(files = depset(), runfiles = ctx.runfiles()),
Expand Down Expand Up @@ -81,7 +82,8 @@ proto_lang_toolchain = rule(
cfg = "exec",
providers = [PackageSpecificationInfo],
),
} | ({} if toolchains.INCOMPATIBLE_ENABLE_PROTO_TOOLCHAIN_RESOLUTION else {
"toolchain_type": attr.label(),
} | ({} if proto_common.INCOMPATIBLE_ENABLE_PROTO_TOOLCHAIN_RESOLUTION else {
"_proto_compiler": attr.label(
cfg = "exec",
executable = True,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ def _write_descriptor_set(ctx, proto_info, deps, exports, descriptor_set):
map_each = proto_common.get_import_path,
join_with = ":",
)
if toolchains.INCOMPATIBLE_ENABLE_PROTO_TOOLCHAIN_RESOLUTION:
if proto_common.INCOMPATIBLE_ENABLE_PROTO_TOOLCHAIN_RESOLUTION:
toolchain = ctx.toolchains[semantics.PROTO_TOOLCHAIN]
if not toolchain:
fail("Protocol compiler toolchain could not be resolved.")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,7 @@ public static void setupWorkspace(MockToolsConfig config) throws IOException {
" progress_message = ctx.attr.progress_message,",
" mnemonic = ctx.attr.mnemonic,",
" allowlist_different_package = None,",
" toolchain_type = '//third_party/bazel_rules/rules_proto/proto:toolchain_type'",
" ),",
" ),",
" ]",
Expand All @@ -288,7 +289,7 @@ public static void setupWorkspace(MockToolsConfig config) throws IOException {
"third_party/bazel_rules/rules_proto/proto/proto_lang_toolchain.bzl",
"def proto_lang_toolchain(*, name, toolchain_type = None, exec_compatible_with = [],",
" target_compatible_with = [], **attrs):",
" native.proto_lang_toolchain(name = name, **attrs)",
" native.proto_lang_toolchain(name = name, toolchain_type = toolchain_type, **attrs)",
" if toolchain_type:",
" native.toolchain(",
" name = name + '_toolchain',",
Expand Down

0 comments on commit 5a24c8a

Please sign in to comment.