Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for --incompatible_enable_proto_toolchain_resolution #3919

Merged
merged 12 commits into from
May 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions .bazelci/presubmit.yml
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,22 @@ tasks:
- "@go_default_sdk//..."
test_targets:
- "//..."
bcr_tests_proto:
name: BCR test module (--incompatible_enable_proto_toolchain_resolution)
platform: ${{ platform }}
bazel: 7.1.1
working_directory: tests/bcr
build_flags:
- "--allow_yanked_versions=all"
- "--incompatible_enable_proto_toolchain_resolution"
test_flags:
- "--allow_yanked_versions=all"
- "--incompatible_enable_proto_toolchain_resolution"
build_targets:
- "//..."
- "@go_default_sdk//..."
test_targets:
- "//..."
macos:
shell_commands:
- tests/core/cgo/generate_imported_dylib.sh
Expand Down
3 changes: 2 additions & 1 deletion .bazelrc
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ build:incompatible --incompatible_enforce_config_setting_visibility
build:incompatible --incompatible_disallow_empty_glob
build:incompatible --incompatible_disable_starlark_host_transitions
build:incompatible --nolegacy_external_runfiles
build:incompatible --incompatible_enable_proto_toolchain_resolution
# Also enable all incompatible flags in go_bazel_test by default.
# TODO: Add --incompatible_disallow_empty_glob once
# https://github.com/bazelbuild/bazel-gazelle/pull/1405 has been released.
test:incompatible --test_env=GO_BAZEL_TEST_BAZELFLAGS='--incompatible_load_proto_rules_from_bzl --incompatible_enable_cc_toolchain_resolution --incompatible_config_setting_private_default_visibility --incompatible_enforce_config_setting_visibility --incompatible_disable_starlark_host_transitions --nolegacy_external_runfiles'
test:incompatible --test_env=GO_BAZEL_TEST_BAZELFLAGS='--incompatible_load_proto_rules_from_bzl --incompatible_enable_cc_toolchain_resolution --incompatible_config_setting_private_default_visibility --incompatible_enforce_config_setting_visibility --incompatible_disable_starlark_host_transitions --nolegacy_external_runfiles --incompatible_enable_proto_toolchain_resolution'
2 changes: 1 addition & 1 deletion MODULE.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ module(
bazel_dep(name = "bazel_features", version = "1.9.1", repo_name = "io_bazel_rules_go_bazel_features")
bazel_dep(name = "bazel_skylib", version = "1.2.0")
bazel_dep(name = "platforms", version = "0.0.4")
bazel_dep(name = "rules_proto", version = "4.0.0")
bazel_dep(name = "rules_proto", version = "6.0.0")
bazel_dep(name = "protobuf", version = "3.19.2", repo_name = "com_google_protobuf")

go_sdk = use_extension("//go:extensions.bzl", "go_sdk")
Expand Down
51 changes: 51 additions & 0 deletions WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,61 @@ workspace(name = "io_bazel_rules_go")
load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")
load("@io_bazel_rules_go//go:deps.bzl", "go_register_toolchains", "go_rules_dependencies")

# Required by toolchains_protoc.
http_archive(
name = "platforms",
sha256 = "218efe8ee736d26a3572663b374a253c012b716d8af0c07e842e82f238a0a7ee",
urls = [
"https://mirror.bazel.build/github.com/bazelbuild/platforms/releases/download/0.0.10/platforms-0.0.10.tar.gz",
"https://github.com/bazelbuild/platforms/releases/download/0.0.10/platforms-0.0.10.tar.gz",
],
)

# The non-polyfill version of this is needed by rules_proto below.
http_archive(
name = "bazel_features",
sha256 = "d7787da289a7fb497352211ad200ec9f698822a9e0757a4976fd9f713ff372b3",
strip_prefix = "bazel_features-1.9.1",
url = "https://github.com/bazel-contrib/bazel_features/releases/download/v1.9.1/bazel_features-v1.9.1.tar.gz",
)

load("@bazel_features//:deps.bzl", "bazel_features_deps")

bazel_features_deps()

go_rules_dependencies()

go_register_toolchains(version = "1.21.8")

http_archive(
name = "rules_proto",
sha256 = "303e86e722a520f6f326a50b41cfc16b98fe6d1955ce46642a5b7a67c11c0f5d",
strip_prefix = "rules_proto-6.0.0",
url = "https://github.com/bazelbuild/rules_proto/releases/download/6.0.0/rules_proto-6.0.0.tar.gz",
)
fmeum marked this conversation as resolved.
Show resolved Hide resolved

load("@rules_proto//proto:repositories.bzl", "rules_proto_dependencies")

rules_proto_dependencies()

load("@rules_proto//proto:toolchains.bzl", "rules_proto_toolchains")

rules_proto_toolchains()

http_archive(
name = "toolchains_protoc",
sha256 = "1f3cd768bbb92164952301228bac5e5079743843488598f2b17fecd41163cadb",
strip_prefix = "toolchains_protoc-0.2.4",
url = "https://github.com/aspect-build/toolchains_protoc/releases/download/v0.2.4/toolchains_protoc-v0.2.4.tar.gz",
)

load("@toolchains_protoc//protoc:toolchain.bzl", "protoc_toolchains")

protoc_toolchains(
name = "protoc_toolchains",
version = "v25.3",
)

http_archive(
name = "com_google_protobuf",
sha256 = "75be42bd736f4df6d702a0e4e4d30de9ee40eac024c4b845d17ae4cc831fe4ae",
Expand Down
19 changes: 9 additions & 10 deletions go/tools/bazel_testing/bazel_testing.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,10 @@ type Args struct {
// NogoExcludes is the list of targets to include for Nogo linting.
NogoExcludes []string

// WorkspacePrefix is a string that should be inserted at the beginning
// of the default generated WORKSPACE file.
WorkspacePrefix string

// WorkspaceSuffix is a string that should be appended to the end
// of the default generated WORKSPACE file.
WorkspaceSuffix string
Expand All @@ -90,7 +94,7 @@ type Args struct {
// workspace. It is executed once and only once before the beginning of
// all tests. If SetUp returns a non-nil error, execution is halted and
// tests cases are not executed.
SetUp func() error
SetUp func() error
}

// debug may be set to make the test print the test workspace path and stop
Expand Down Expand Up @@ -409,6 +413,7 @@ func setupWorkspace(args Args, files []string) (dir string, cleanup func() error
}
}()
info := workspaceTemplateInfo{
Prefix: args.WorkspacePrefix,
Suffix: args.WorkspaceSuffix,
Nogo: args.Nogo,
NogoIncludes: args.NogoIncludes,
Expand Down Expand Up @@ -461,15 +466,6 @@ func setupWorkspace(args Args, files []string) (dir string, cleanup func() error
if err := defaultModuleBazelTpl.Execute(w, info); err != nil {
return "", cleanup, err
}

// Enable Bzlmod.
bazelrcPath := filepath.Join(mainDir, ".bazelrc")
if _, err = os.Stat(bazelrcPath); os.IsNotExist(err) {
err = os.WriteFile(bazelrcPath, []byte("common --enable_bzlmod"), 0666)
if err != nil {
return "", cleanup, err
}
}
}

return mainDir, cleanup, nil
Expand Down Expand Up @@ -538,6 +534,7 @@ type workspaceTemplateInfo struct {
Nogo string
NogoIncludes []string
NogoExcludes []string
Prefix string
Suffix string
}

Expand All @@ -549,6 +546,8 @@ local_repository(
)
{{end}}

{{.Prefix}}

{{if not .GoSDKPath}}
load("@io_bazel_rules_go//go:deps.bzl", "go_rules_dependencies", "go_register_toolchains")

Expand Down
11 changes: 9 additions & 2 deletions proto/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -127,20 +127,27 @@ go_proto_compiler(
non_go_reset_target(
name = "protoc",
dep = "@com_google_protobuf//:protoc",
deprecation = "No longer used by rules_go, will be removed in a future release.",
visibility = ["//visibility:public"],
)

filegroup(
name = "all_rules",
testonly = True,
srcs = glob(["*.bzl"]) + ["//proto/wkt:all_rules"],
srcs = glob(["*.bzl"]) + [
"//proto/private:all_files",
"//proto/wkt:all_rules",
],
visibility = ["//:__subpackages__"],
)

filegroup(
name = "all_files",
testonly = True,
srcs = glob(["**"]) + ["//proto/wkt:all_files"],
srcs = glob(["**"]) + [
"//proto/private:all_files",
"//proto/wkt:all_files",
],
visibility = ["//:__subpackages__"],
)

Expand Down
44 changes: 33 additions & 11 deletions proto/compiler.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ load(
"@bazel_skylib//lib:paths.bzl",
"paths",
)
load(
"@rules_proto//proto:proto_common.bzl",
proto_toolchains = "toolchains",
)
load(
"//go:def.bzl",
"GoLibrary",
Expand All @@ -31,6 +35,15 @@ load(
"go_reset_target",
)

# This is actually a misuse of Proto toolchains: The proper way to use `protoc` would be to go
# through a Go-specific `proto_lang_toolchain` and use the methods on `proto_common` to interact
# with `protoc`. Since rules_go has a very bespoke setup with customizable compilers and the need
# to apply reset transitions in case `protoc` *is* built from source, this would require major
# changes.
# TODO: Revisit this after --incompatible_enable_proto_toolchain_resolution has been enabled by
# default.
_PROTO_TOOLCHAIN_TYPE = "@rules_proto//proto:toolchain_type"
fmeum marked this conversation as resolved.
Show resolved Hide resolved

GoProtoCompiler = provider(
doc = "Information and dependencies needed to generate Go code from protos",
fields = {
Expand Down Expand Up @@ -104,7 +117,7 @@ def go_proto_compile(go, compiler, protos, imports, importpath):
transitive_descriptor_sets = depset(direct = [], transitive = desc_sets)

args = go.actions.args()
args.add("-protoc", compiler.internal.protoc)
args.add("-protoc", compiler.internal.protoc.executable)
args.add("-importpath", importpath)
args.add("-out_path", outpath)
args.add("-plugin", compiler.internal.plugin)
Expand All @@ -122,7 +135,6 @@ def go_proto_compile(go, compiler, protos, imports, importpath):
inputs = depset(
direct = [
compiler.internal.go_protoc,
compiler.internal.protoc,
compiler.internal.plugin,
],
transitive = [transitive_descriptor_sets],
Expand All @@ -132,6 +144,7 @@ def go_proto_compile(go, compiler, protos, imports, importpath):
mnemonic = "GoProtocGen",
executable = compiler.internal.go_protoc,
toolchain = GO_TOOLCHAIN_LABEL,
tools = [compiler.internal.protoc],
arguments = [args],
env = go.env,
# We may need the shell environment (potentially augmented with --action_env)
Expand Down Expand Up @@ -172,6 +185,11 @@ def _go_proto_compiler_impl(ctx):
go = go_context(ctx)
library = go.new_library(go)
source = go.library_to_source(go, ctx.attr, library, ctx.coverage_instrumented())
proto_toolchain = proto_toolchains.find_toolchain(
ctx,
legacy_attr = "_legacy_proto_toolchain",
toolchain_type = _PROTO_TOOLCHAIN_TYPE,
)
return [
GoProtoCompiler(
deps = ctx.attr.deps,
Expand All @@ -181,7 +199,7 @@ def _go_proto_compiler_impl(ctx):
options = ctx.attr.options,
suffix = ctx.attr.suffix,
suffixes = ctx.attr.suffixes,
protoc = ctx.executable._protoc,
protoc = proto_toolchain.proto_compiler,
go_protoc = ctx.executable._go_protoc,
plugin = ctx.executable.plugin,
import_path_option = ctx.attr.import_path_option,
Expand All @@ -193,7 +211,7 @@ def _go_proto_compiler_impl(ctx):

_go_proto_compiler = rule(
implementation = _go_proto_compiler_impl,
attrs = {
attrs = dict({
fmeum marked this conversation as resolved.
Show resolved Hide resolved
"deps": attr.label_list(providers = [GoLibrary]),
"options": attr.string_list(),
"suffix": attr.string(default = ".pb.go"),
Expand All @@ -210,16 +228,20 @@ _go_proto_compiler = rule(
cfg = "exec",
default = "//go/tools/builders:go-protoc",
),
"_protoc": attr.label(
executable = True,
cfg = "exec",
default = "//proto:protoc",
),
"_go_context_data": attr.label(
default = "//:go_context_data",
),
},
toolchains = [GO_TOOLCHAIN],
}, **proto_toolchains.if_legacy_toolchain({
"_legacy_proto_toolchain": attr.label(
# Setting cfg = "exec" here as the legacy_proto_toolchain target
# already needs to apply the non_go_tool_transition. Flipping the
# two would be more idiomatic, but proto_toolchains.find_toolchain
# doesn't support split transitions.
cfg = "exec",
default = "//proto/private:legacy_proto_toolchain",
),
})),
toolchains = [GO_TOOLCHAIN] + proto_toolchains.use_toolchain(_PROTO_TOOLCHAIN_TYPE),
)

def go_proto_compiler(name, **kwargs):
Expand Down
20 changes: 20 additions & 0 deletions proto/private/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
load(":toolchain.bzl", "legacy_proto_toolchain")

legacy_proto_toolchain(
name = "legacy_proto_toolchain",
visibility = ["//visibility:public"],
)

filegroup(
name = "all_rules",
testonly = True,
srcs = glob(["*.bzl"]),
visibility = ["//:__subpackages__"],
)

filegroup(
name = "all_files",
testonly = True,
srcs = glob(["**"]),
visibility = ["//:__subpackages__"],
)
46 changes: 46 additions & 0 deletions proto/private/toolchain.bzl
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
# Copyright 2024 The Bazel Authors. All rights reserved.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

# Helper that wraps --proto_compiler into a ProtoLangToolchainInfo for backwards
# compatibility with --noincompatible_enable_proto_toolchain_resolution.

load(
"@rules_proto//proto:proto_common.bzl",
"ProtoLangToolchainInfo",
)
load(
"//go/private/rules:transition.bzl",
"non_go_tool_transition",
)

def _legacy_proto_toolchain_impl(ctx):
return [
ProtoLangToolchainInfo(
proto_compiler = ctx.attr._protoc.files_to_run,
),
]

legacy_proto_toolchain = rule(
implementation = _legacy_proto_toolchain_impl,
cfg = non_go_tool_transition,
attrs = {
"_protoc": attr.label(
default = configuration_field(fragment = "proto", name = "proto_compiler"),
),
"_allowlist_function_transition": attr.label(
default = "@bazel_tools//tools/allowlists/function_transition_allowlist",
),
},
fragments = ["proto"],
)
4 changes: 4 additions & 0 deletions tests/bcr/MODULE.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@ local_path_override(
bazel_dep(name = "gazelle", version = "0.33.0")
bazel_dep(name = "protobuf", version = "3.19.6")

# Required with --incompatible_enable_proto_toolchain_resolution.
# Avoids building protoc from source, which speeds up CI runs.
bazel_dep(name = "toolchains_protoc", version = "0.2.1")
fmeum marked this conversation as resolved.
Show resolved Hide resolved

go_sdk = use_extension("@my_rules_go//go:extensions.bzl", "go_sdk")
go_sdk.download(
patch_strip = 1,
Expand Down
Loading