diff --git a/.bazelci/presubmit.yml b/.bazelci/presubmit.yml index fc0102748d..87d9d61db0 100644 --- a/.bazelci/presubmit.yml +++ b/.bazelci/presubmit.yml @@ -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 diff --git a/.bazelrc b/.bazelrc index 2cc995f10c..931cacc20a 100644 --- a/.bazelrc +++ b/.bazelrc @@ -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' diff --git a/MODULE.bazel b/MODULE.bazel index 0077a5ae7e..34e72c7f3b 100644 --- a/MODULE.bazel +++ b/MODULE.bazel @@ -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") diff --git a/WORKSPACE b/WORKSPACE index 39e2a61b04..67f537e599 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -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", +) + +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", diff --git a/go/tools/bazel_testing/bazel_testing.go b/go/tools/bazel_testing/bazel_testing.go index 78820c8a4a..30bc8f0581 100644 --- a/go/tools/bazel_testing/bazel_testing.go +++ b/go/tools/bazel_testing/bazel_testing.go @@ -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 @@ -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 @@ -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, @@ -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 @@ -538,6 +534,7 @@ type workspaceTemplateInfo struct { Nogo string NogoIncludes []string NogoExcludes []string + Prefix string Suffix string } @@ -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") diff --git a/proto/BUILD.bazel b/proto/BUILD.bazel index 4c47c31cc7..1d6b3dd054 100644 --- a/proto/BUILD.bazel +++ b/proto/BUILD.bazel @@ -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__"], ) diff --git a/proto/compiler.bzl b/proto/compiler.bzl index b51242271b..1fa3931d71 100644 --- a/proto/compiler.bzl +++ b/proto/compiler.bzl @@ -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", @@ -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" + GoProtoCompiler = provider( doc = "Information and dependencies needed to generate Go code from protos", fields = { @@ -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) @@ -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], @@ -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) @@ -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, @@ -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, @@ -193,7 +211,7 @@ def _go_proto_compiler_impl(ctx): _go_proto_compiler = rule( implementation = _go_proto_compiler_impl, - attrs = { + attrs = dict({ "deps": attr.label_list(providers = [GoLibrary]), "options": attr.string_list(), "suffix": attr.string(default = ".pb.go"), @@ -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): diff --git a/proto/private/BUILD.bazel b/proto/private/BUILD.bazel new file mode 100644 index 0000000000..abb3f9bba0 --- /dev/null +++ b/proto/private/BUILD.bazel @@ -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__"], +) diff --git a/proto/private/toolchain.bzl b/proto/private/toolchain.bzl new file mode 100644 index 0000000000..503fdb0d80 --- /dev/null +++ b/proto/private/toolchain.bzl @@ -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"], +) diff --git a/tests/bcr/MODULE.bazel b/tests/bcr/MODULE.bazel index 5a230c6e31..0944cc6425 100644 --- a/tests/bcr/MODULE.bazel +++ b/tests/bcr/MODULE.bazel @@ -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") + go_sdk = use_extension("@my_rules_go//go:extensions.bzl", "go_sdk") go_sdk.download( patch_strip = 1, diff --git a/tests/core/cross/proto_test.go b/tests/core/cross/proto_test.go index cd9e6c6058..c8372ffb8c 100644 --- a/tests/core/cross/proto_test.go +++ b/tests/core/cross/proto_test.go @@ -21,6 +21,25 @@ import ( ) var testArgs = bazel_testing.Args{ + ModuleFileSuffix: ` +bazel_dep(name = "rules_proto", version = "6.0.0") +bazel_dep(name = "toolchains_protoc", version = "0.2.4") +`, + WorkspacePrefix: ` +load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive") + +# 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() +`, WorkspaceSuffix: ` load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive") @@ -41,14 +60,16 @@ protobuf_deps() http_archive( name = "rules_proto", - sha256 = "4d421d51f9ecfe9bf96ab23b55c6f2b809cbaf0eea24952683e397decfbd0dd0", - strip_prefix = "rules_proto-f6b8d89b90a7956f6782a4a3609b2f0eee3ce965", - # master, as of 2020-01-06 - urls = [ - "https://mirror.bazel.build/github.com/bazelbuild/rules_proto/archive/f6b8d89b90a7956f6782a4a3609b2f0eee3ce965.tar.gz", - "https://github.com/bazelbuild/rules_proto/archive/f6b8d89b90a7956f6782a4a3609b2f0eee3ce965.tar.gz", - ], + 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", ) + +load("@rules_proto//proto:repositories.bzl", "rules_proto_dependencies") +rules_proto_dependencies() + +load("@rules_proto//proto:toolchains.bzl", "rules_proto_toolchains") +rules_proto_toolchains() `, Main: ` -- BUILD.bazel -- diff --git a/tests/core/transition/hermeticity_test.go b/tests/core/transition/hermeticity_test.go index 3650ce16b6..a121f90037 100644 --- a/tests/core/transition/hermeticity_test.go +++ b/tests/core/transition/hermeticity_test.go @@ -120,6 +120,26 @@ option go_package = "github.com/bazelbuild/rules_go/tests/core/transition/foo"; message Foo { int64 value = 1; } +`, + ModuleFileSuffix: ` +bazel_dep(name = "protobuf", version = "21.7", repo_name = "com_google_protobuf") +bazel_dep(name = "rules_proto", version = "6.0.0") +bazel_dep(name = "toolchains_protoc", version = "0.2.4") +`, + WorkspacePrefix: ` +load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive") + +# 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() `, WorkspaceSuffix: ` load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive") @@ -141,14 +161,16 @@ protobuf_deps() http_archive( name = "rules_proto", - sha256 = "4d421d51f9ecfe9bf96ab23b55c6f2b809cbaf0eea24952683e397decfbd0dd0", - strip_prefix = "rules_proto-f6b8d89b90a7956f6782a4a3609b2f0eee3ce965", - # master, as of 2020-01-06 - urls = [ - "https://mirror.bazel.build/github.com/bazelbuild/rules_proto/archive/f6b8d89b90a7956f6782a4a3609b2f0eee3ce965.tar.gz", - "https://github.com/bazelbuild/rules_proto/archive/f6b8d89b90a7956f6782a4a3609b2f0eee3ce965.tar.gz", - ], + 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", ) + +load("@rules_proto//proto:repositories.bzl", "rules_proto_dependencies") +rules_proto_dependencies() + +load("@rules_proto//proto:toolchains.bzl", "rules_proto_toolchains") +rules_proto_toolchains() `, }) } @@ -157,21 +179,24 @@ func TestGoBinaryNonGoAttrsAreReset(t *testing.T) { assertDependsCleanlyOnWithFlags( t, "//:main", - "//:helper") + "//:helper", + false) } func TestGoLibraryNonGoAttrsAreReset(t *testing.T) { assertDependsCleanlyOnWithFlags( t, "//:main", - "//:indirect_helper") + "//:indirect_helper", + false) } func TestGoTestNonGoAttrsAreReset(t *testing.T) { assertDependsCleanlyOnWithFlags( t, "//:main_test", - "//:helper") + "//:helper", + false) } func TestGoProtoLibraryToolAttrsAreReset(t *testing.T) { @@ -179,6 +204,8 @@ func TestGoProtoLibraryToolAttrsAreReset(t *testing.T) { t, "//:foo_go_proto", "@com_google_protobuf//:protoc", + // No dep with --incompatible_enable_proto_toolchain_resolution. + true, "--@io_bazel_rules_go//go/config:static", "--@io_bazel_rules_go//go/config:msan", "--@io_bazel_rules_go//go/config:race", @@ -190,11 +217,12 @@ func TestGoProtoLibraryToolAttrsAreReset(t *testing.T) { t, "//:foo_go_proto", "@com_google_protobuf//:protoc", + true, "--@io_bazel_rules_go//go/config:pure", ) } -func assertDependsCleanlyOnWithFlags(t *testing.T, targetA, targetB string, flags ...string) { +func assertDependsCleanlyOnWithFlags(t *testing.T, targetA, targetB string, allowNoDep bool, flags ...string) { // Analyze the targets to ensure that MODULE.bazel.lock has been created, otherwise bazel config // will fail after the cquery command due to the Skyframe invalidation caused by a changed file. err := bazel_testing.RunBazel(append([]string{"build", targetA, targetB, "--nobuild"}, flags...)...) @@ -217,6 +245,12 @@ func assertDependsCleanlyOnWithFlags(t *testing.T, targetA, targetB string, flag } cqueryOut := bytes.TrimSpace(out) configHashes := extractConfigHashes(t, cqueryOut) + if len(configHashes) == 0 { + if allowNoDep { + return + } + t.Fatalf("%s does not depend on %s", targetA, targetB) + } if len(configHashes) != 1 { differingGoOptions := getGoOptions(t, configHashes...) if len(differingGoOptions) != 0 {