From 7c766e2a726c57ace6dd33b63de4cefe743d8714 Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" Date: Mon, 14 Oct 2024 16:14:30 -0700 Subject: [PATCH] [xDS unit test] remove need for `grpc_cc_proto_library` BUILD rule (#37902) This eliminates the need for the `grpc_cc_proto_library` bazel BUILD rule introduced in #37863. To make this work, I had to upgrade several bazel dependencies and apply a patch to rules_go to work around https://github.com/bazelbuild/bazel/issues/11636. Closes #37902 PiperOrigin-RevId: 685868647 --- BUILD | 11 ----- CMakeLists.txt | 6 +-- bazel/grpc_build_system.bzl | 4 -- bazel/grpc_deps.bzl | 47 +++++++++++++++---- bazel/grpc_extra_deps.bzl | 7 ++- bazel/rules_go.patch | 19 ++++++++ build_autogenerated.yaml | 7 +-- test/core/xds/BUILD | 4 +- third_party/protoc-gen-validate | 2 +- third_party/protoc-gen-validate.patch | 14 ------ .../run_tests/sanity/check_bazel_workspace.py | 4 ++ tools/run_tests/sanity/check_submodules.sh | 2 +- 12 files changed, 77 insertions(+), 50 deletions(-) create mode 100644 bazel/rules_go.patch delete mode 100644 third_party/protoc-gen-validate.patch diff --git a/BUILD b/BUILD index bf0dbd6d81b..9953d1e301b 100644 --- a/BUILD +++ b/BUILD @@ -19,7 +19,6 @@ load("@bazel_skylib//rules:common_settings.bzl", "bool_flag") load( "//bazel:grpc_build_system.bzl", "grpc_cc_library", - "grpc_cc_proto_library", "grpc_clang_cl_settings", "grpc_filegroup", "grpc_generate_one_off_targets", @@ -4857,11 +4856,6 @@ grpc_upb_proto_reflection_library( deps = ["@envoy_api//envoy/config/cluster/v3:pkg"], ) -grpc_cc_proto_library( - name = "envoy_config_core_cc_proto", - deps = ["@envoy_api//envoy/config/core/v3:pkg"], -) - grpc_upb_proto_library( name = "envoy_config_core_upb", deps = ["@envoy_api//envoy/config/core/v3:pkg"], @@ -5047,11 +5041,6 @@ grpc_upb_proto_reflection_library( deps = ["@envoy_api//envoy/extensions/upstreams/http/v3:pkg"], ) -grpc_cc_proto_library( - name = "envoy_service_discovery_cc_proto", - deps = ["@envoy_api//envoy/service/discovery/v3:pkg"], -) - grpc_upb_proto_library( name = "envoy_service_discovery_upb", deps = ["@envoy_api//envoy/service/discovery/v3:pkg"], diff --git a/CMakeLists.txt b/CMakeLists.txt index 1c5396adf58..ee57af1eb5c 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -460,9 +460,9 @@ if (NOT EXISTS ${CMAKE_CURRENT_SOURCE_DIR}/third_party/protoc-gen-validate AND g # Download the archive via HTTP, validate the checksum, and extract to third_party/protoc-gen-validate. download_archive( ${CMAKE_CURRENT_SOURCE_DIR}/third_party/protoc-gen-validate - https://github.com/envoyproxy/protoc-gen-validate/archive/4694024279bdac52b77e22dc87808bd0fd732b69.tar.gz - 1e490b98005664d149b379a9529a6aa05932b8a11b76b4cd86f3d22d76346f47 - protoc-gen-validate-4694024279bdac52b77e22dc87808bd0fd732b69 + https://github.com/bufbuild/protoc-gen-validate/archive/refs/tags/v1.0.4.zip + 9372f9ecde8fbadf83c8c7de3dbb49b11067aa26fb608c501106d0b4bf06c28f + protoc-gen-validate-1.0.4 ) endif() # Setup external proto library at third_party/xds with 2 download URLs diff --git a/bazel/grpc_build_system.bzl b/bazel/grpc_build_system.bzl index fbb1be2b3c0..37098ee3048 100644 --- a/bazel/grpc_build_system.bzl +++ b/bazel/grpc_build_system.bzl @@ -29,7 +29,6 @@ Contains macros used throughout the repo. load("@build_bazel_rules_apple//apple:ios.bzl", "ios_unit_test") load("@build_bazel_rules_apple//apple/testing/default_runner:ios_test_runner.bzl", "ios_test_runner") -load("@com_google_protobuf//bazel:cc_proto_library.bzl", "cc_proto_library") load("@com_google_protobuf//bazel:upb_proto_library.bzl", "upb_proto_library", "upb_proto_reflection_library") load("//bazel:cc_grpc_library.bzl", "cc_grpc_library") load("//bazel:copts.bzl", "GRPC_DEFAULT_COPTS") @@ -795,9 +794,6 @@ def grpc_objc_library( visibility = visibility, ) -def grpc_cc_proto_library(name, deps): - cc_proto_library(name = name, deps = deps) - def grpc_upb_proto_library(name, deps): upb_proto_library(name = name, deps = deps) diff --git a/bazel/grpc_deps.bzl b/bazel/grpc_deps.bzl index b3d416aae9d..b54f9cee778 100644 --- a/bazel/grpc_deps.bzl +++ b/bazel/grpc_deps.bzl @@ -206,14 +206,45 @@ def grpc_deps(): ], ) + # TODO(roth): Need to override version of rules_proto, because + # we're using a newer version of rules_go than the xDS protos are, + # and that requires a newer version of rules_proto. In turn, the + # newer version of rules_proto requires a newer version of + # bazel_features. So these two entries can go away once the xDS + # protos upgrade to the newer version of rules_go. + if "bazel_features" not in native.existing_rules(): + http_archive( + name = "bazel_features", + sha256 = "5ac743bf5f05d88e84962e978811f2524df09602b789c92cf7ae2111ecdeda94", + strip_prefix = "bazel_features-1.14.0", + urls = [ + "https://storage.googleapis.com/grpc-bazel-mirror/github.com/bazel-contrib/bazel_features/releases/download/v1.14.0/bazel_features-v1.14.0.tar.gz", + "https://github.com/bazel-contrib/bazel_features/releases/download/v1.14.0/bazel_features-v1.14.0.tar.gz", + ], + ) + if "rules_proto" not in native.existing_rules(): + http_archive( + name = "rules_proto", + sha256 = "6fb6767d1bef535310547e03247f7518b03487740c11b6c6adb7952033fe1295", + strip_prefix = "rules_proto-6.0.2", + urls = [ + "https://storage.googleapis.com/grpc-bazel-mirror/github.com/bazelbuild/rules_proto/archive/refs/tags/6.0.2.tar.gz", + "https://github.com/bazelbuild/rules_proto/archive/refs/tags/6.0.2.tar.gz", + ], + ) + if "io_bazel_rules_go" not in native.existing_rules(): http_archive( name = "io_bazel_rules_go", - sha256 = "69de5c704a05ff37862f7e0f5534d4f479418afc21806c887db544a316f3cb6b", + sha256 = "d93ef02f1e72c82d8bb3d5169519b36167b33cf68c252525e3b9d3d5dd143de7", urls = [ - "https://mirror.bazel.build/github.com/bazelbuild/rules_go/releases/download/v0.27.0/rules_go-v0.27.0.tar.gz", - "https://github.com/bazelbuild/rules_go/releases/download/v0.27.0/rules_go-v0.27.0.tar.gz", + "https://mirror.bazel.build/github.com/bazelbuild/rules_go/releases/download/v0.49.0/rules_go-v0.49.0.zip", + "https://github.com/bazelbuild/rules_go/releases/download/v0.49.0/rules_go-v0.49.0.zip", + ], + patches = [ + "@com_github_grpc_grpc//bazel:rules_go.patch", ], + patch_args = ["-p1"], ) if "build_bazel_rules_apple" not in native.existing_rules(): @@ -272,13 +303,9 @@ def grpc_deps(): if "com_envoyproxy_protoc_gen_validate" not in native.existing_rules(): http_archive( name = "com_envoyproxy_protoc_gen_validate", - strip_prefix = "protoc-gen-validate-4694024279bdac52b77e22dc87808bd0fd732b69", - sha256 = "1e490b98005664d149b379a9529a6aa05932b8a11b76b4cd86f3d22d76346f47", - urls = [ - "https://github.com/envoyproxy/protoc-gen-validate/archive/4694024279bdac52b77e22dc87808bd0fd732b69.tar.gz", - ], - patches = ["@com_github_grpc_grpc//third_party:protoc-gen-validate.patch"], - patch_args = ["-p1"], + sha256 = "9372f9ecde8fbadf83c8c7de3dbb49b11067aa26fb608c501106d0b4bf06c28f", + strip_prefix = "protoc-gen-validate-1.0.4", + urls = ["https://github.com/bufbuild/protoc-gen-validate/archive/refs/tags/v1.0.4.zip"], ) if "com_github_cncf_xds" not in native.existing_rules(): diff --git a/bazel/grpc_extra_deps.bzl b/bazel/grpc_extra_deps.bzl index a370fe44966..6ac6093a5fe 100644 --- a/bazel/grpc_extra_deps.bzl +++ b/bazel/grpc_extra_deps.bzl @@ -13,6 +13,7 @@ # limitations under the License. """Loads the dependencies necessary for the external repositories defined in grpc_deps.bzl.""" +load("@bazel_features//:deps.bzl", "bazel_features_deps") load("@bazel_gazelle//:deps.bzl", "gazelle_dependencies") load("@build_bazel_apple_support//lib:repositories.bzl", "apple_support_dependencies") load("@build_bazel_rules_apple//apple:repositories.bzl", "apple_rules_dependencies") @@ -22,6 +23,7 @@ load("@com_google_protobuf//:protobuf_deps.bzl", "protobuf_deps") load("@envoy_api//bazel:repositories.bzl", "api_dependencies") load("@google_cloud_cpp//bazel:google_cloud_cpp_deps.bzl", "google_cloud_cpp_deps") load("@io_bazel_rules_go//go:deps.bzl", "go_register_toolchains", "go_rules_dependencies") +load("@rules_proto//proto:repositories.bzl", "rules_proto_dependencies") load("@rules_python//python:repositories.bzl", "py_repositories") def grpc_extra_deps(ignore_version_differences = False): @@ -49,10 +51,13 @@ def grpc_extra_deps(ignore_version_differences = False): """ protobuf_deps() + rules_proto_dependencies() + bazel_features_deps() + api_dependencies() go_rules_dependencies() - go_register_toolchains(version = "1.20") + go_register_toolchains(version = "1.22.5") gazelle_dependencies() # Pull-in the go 3rd party dependencies for protoc_gen_validate, which is diff --git a/bazel/rules_go.patch b/bazel/rules_go.patch new file mode 100644 index 00000000000..bfd6b058a39 --- /dev/null +++ b/bazel/rules_go.patch @@ -0,0 +1,19 @@ +# This patch works around a problem with Windows RBE described in +# https://github.com/bazelbuild/bazel/issues/11636. It can be removed +# once that issue is resolved. +diff --git a/go/private/rules/binary.bzl b/go/private/rules/binary.bzl +index 40a17f4d..2741ad71 100644 +--- a/go/private/rules/binary.bzl ++++ b/go/private/rules/binary.bzl +@@ -462,8 +462,9 @@ exit /b %GO_EXIT_CODE% + content = cmd, + ) + ctx.actions.run( +- executable = bat, +- inputs = sdk.headers + sdk.tools + sdk.srcs + ctx.files.srcs + [sdk.go], ++ executable = "cmd.exe", ++ arguments = ["/S", "/C", bat.path.replace("/", "\\")], ++ inputs = sdk.headers + sdk.tools + sdk.srcs + ctx.files.srcs + [sdk.go, bat], + outputs = [out, gotmp], + mnemonic = "GoToolchainBinaryBuild", + ) diff --git a/build_autogenerated.yaml b/build_autogenerated.yaml index c3b23bf49eb..a2a368a109c 100644 --- a/build_autogenerated.yaml +++ b/build_autogenerated.yaml @@ -20811,6 +20811,7 @@ targets: - test/core/test_util/scoped_env_var.h - test/core/xds/xds_client_test_peer.h - test/core/xds/xds_transport_fake.h + - third_party/protoc-gen-validate/validate/validate.h src: - test/core/event_engine/fuzzing_event_engine/fuzzing_event_engine.proto - third_party/envoy-api/envoy/annotations/deprecation.proto @@ -22271,11 +22272,11 @@ external_proto_libraries: - https://storage.googleapis.com/grpc-bazel-mirror/github.com/census-instrumentation/opencensus-proto/archive/v0.3.0.tar.gz - https://github.com/census-instrumentation/opencensus-proto/archive/v0.3.0.tar.gz - destination: third_party/protoc-gen-validate - hash: 1e490b98005664d149b379a9529a6aa05932b8a11b76b4cd86f3d22d76346f47 + hash: 9372f9ecde8fbadf83c8c7de3dbb49b11067aa26fb608c501106d0b4bf06c28f proto_prefix: third_party/protoc-gen-validate/ - strip_prefix: protoc-gen-validate-4694024279bdac52b77e22dc87808bd0fd732b69 + strip_prefix: protoc-gen-validate-1.0.4 urls: - - https://github.com/envoyproxy/protoc-gen-validate/archive/4694024279bdac52b77e22dc87808bd0fd732b69.tar.gz + - https://github.com/bufbuild/protoc-gen-validate/archive/refs/tags/v1.0.4.zip - destination: third_party/xds hash: dc305e20c9fa80822322271b50aa2ffa917bf4fd3973bcec52bfc28dc32c5927 proto_prefix: third_party/xds/ diff --git a/test/core/xds/BUILD b/test/core/xds/BUILD index bf3bee9a753..748416db9ab 100644 --- a/test/core/xds/BUILD +++ b/test/core/xds/BUILD @@ -172,8 +172,6 @@ grpc_cc_test( deps = [ ":xds_client_test_peer", ":xds_transport_fake", - "//:envoy_config_core_cc_proto", - "//:envoy_service_discovery_cc_proto", "//:grpc++_codegen_proto", "//:grpc++_config_proto", "//:xds_client", @@ -181,6 +179,8 @@ grpc_cc_test( "//test/core/event_engine/fuzzing_event_engine", "//test/core/test_util:grpc_test_util", "//test/core/test_util:scoped_env_var", + "@envoy_api//envoy/config/core/v3:pkg_cc_proto", + "@envoy_api//envoy/service/discovery/v3:pkg_cc_proto", ], ) diff --git a/third_party/protoc-gen-validate b/third_party/protoc-gen-validate index fab737efbb4..32c2415389a 160000 --- a/third_party/protoc-gen-validate +++ b/third_party/protoc-gen-validate @@ -1 +1 @@ -Subproject commit fab737efbb4b4d03e7c771393708f75594b121e4 +Subproject commit 32c2415389a3538082507ae537e7edd9578c64ed diff --git a/third_party/protoc-gen-validate.patch b/third_party/protoc-gen-validate.patch deleted file mode 100644 index bea7b87cec0..00000000000 --- a/third_party/protoc-gen-validate.patch +++ /dev/null @@ -1,14 +0,0 @@ -index 5b2443d..c26a9dd 100644 ---- a/dependencies.bzl -+++ b/dependencies.bzl -@@ -104,8 +104,8 @@ def go_third_party(): - go_repository( - name = "org_golang_google_protobuf", - importpath = "google.golang.org/protobuf", -- sum = "h1:bxAC2xTBsZGibn2RTntX0oH50xLsqy1OxA9tTL3p/lk=", -- version = "v1.27.1", -+ sum = "h1:w43yiav+6bVFTBQFZX0r7ipe9JQ1QsbMgHwbBziscLw=", -+ version = "v1.28.0", - ) - go_repository( - name = "org_golang_x_crypto", diff --git a/tools/run_tests/sanity/check_bazel_workspace.py b/tools/run_tests/sanity/check_bazel_workspace.py index 245db8546aa..cc825901aae 100755 --- a/tools/run_tests/sanity/check_bazel_workspace.py +++ b/tools/run_tests/sanity/check_bazel_workspace.py @@ -70,6 +70,8 @@ _GRPC_DEP_NAMES = [ _TWISTED_INCREMENTAL_DEP_NAME, _ZOPEFOUNDATION_ZOPE_INTERFACE_DEP_NAME, _TWISTED_CONSTANTLY_DEP_NAME, + "bazel_features", + "rules_proto", "io_bazel_rules_go", "build_bazel_rules_apple", "build_bazel_apple_support", @@ -99,6 +101,8 @@ _GRPC_BAZEL_ONLY_DEPS = [ _TWISTED_INCREMENTAL_DEP_NAME, _ZOPEFOUNDATION_ZOPE_INTERFACE_DEP_NAME, _TWISTED_CONSTANTLY_DEP_NAME, + "bazel_features", + "rules_proto", "io_bazel_rules_go", "build_bazel_rules_apple", "build_bazel_apple_support", diff --git a/tools/run_tests/sanity/check_submodules.sh b/tools/run_tests/sanity/check_submodules.sh index 9796fb593cc..6b7765cbb41 100755 --- a/tools/run_tests/sanity/check_submodules.sh +++ b/tools/run_tests/sanity/check_submodules.sh @@ -37,7 +37,7 @@ third_party/opencensus-proto 4aa53e15cbf1a47bc9087e6cfdca214c1eea4e89 third_party/opentelemetry 60fa8754d890b5c55949a8c68dcfd7ab5c2395df third_party/opentelemetry-cpp 4bd64c9a336fd438d6c4c9dad2e6b61b0585311f third_party/protobuf 10ef3f77683f77fb3c059bf47725c27b3ff41e63 -third_party/protoc-gen-validate fab737efbb4b4d03e7c771393708f75594b121e4 +third_party/protoc-gen-validate 32c2415389a3538082507ae537e7edd9578c64ed third_party/re2 0c5616df9c0aaa44c9440d87422012423d91c7d1 third_party/xds 3a472e524827f72d1ad621c4983dd5af54c46776 third_party/zlib 09155eaa2f9270dc4ed1fa13e2b4b2613e6e4851