From db48abbef49312d76d442c0cde6a551961726b28 Mon Sep 17 00:00:00 2001 From: jchadwick-buf <116005195+jchadwick-buf@users.noreply.github.com> Date: Thu, 1 Aug 2024 19:35:20 -0700 Subject: [PATCH] Add breaking change detection for well-known types (#17513) Adds new tests in `//compatibility` for detecting undesired breaking changes in the schema of the well-known types and `descriptor.proto` files, using the Buf breaking change detector, via [`rules_buf`](https://github.com/bufbuild/rules_buf/). In order to keep things light-touch as far as maintenance goes, I have chosen to keep the integration as small and simple as possible. Some notes: - Breaking change behavior can be granularly controlled via [`buf.yaml`](https://buf.build/docs/configuration/v1/buf-yaml#breaking). - Bazel sandboxes us away from meaningful VCS information, so in order to pick a target to check for breaking changes against, a new variable is added to `protobuf_versions.bzl` that needs to be updated with changes in the release version. - Breaking change detection is performed on a file-level, not a package-level, so migrating types across WKT files would constitute a breaking change. If this is not desired the behavior can be made to work on a package-level, though we need to do some more work as `buf_breaking_test` currently only accepts a single file descriptor set target for the `against` attribute. Closes #17513 COPYBARA_INTEGRATE_REVIEW=https://github.com/protocolbuffers/protobuf/pull/17513 from jchadwick-buf:buf-breaking cd46bb2c1e0c768238455e5c2e5faa4b08a55434 PiperOrigin-RevId: 658624708 --- MODULE.bazel | 3 +- WORKSPACE | 24 ++++++++ WORKSPACE.bzlmod | 8 +++ compatibility/BUILD.bazel | 124 +++++++++++++++++++++++++++++++++++++- compatibility/buf.yaml | 1 + protobuf_version.bzl | 1 + 6 files changed, 158 insertions(+), 3 deletions(-) create mode 100644 compatibility/buf.yaml diff --git a/MODULE.bazel b/MODULE.bazel index 05128b355c..fa8c7c5a6d 100644 --- a/MODULE.bazel +++ b/MODULE.bazel @@ -95,6 +95,7 @@ use_repo(maven, "maven") # Development dependencies bazel_dep(name = "googletest", version = "1.14.0", repo_name = "com_google_googletest", dev_dependency = True) +bazel_dep(name = "rules_buf", version = "0.3.0", dev_dependency = True) bazel_dep(name = "rules_testing", version = "0.6.0", dev_dependency = True) # rules_proto are needed for @com_google_protobuf_v25.0 used in //compatibility/... tests -bazel_dep(name = "rules_proto", version = "4.0.0", dev_dependency = True) \ No newline at end of file +bazel_dep(name = "rules_proto", version = "4.0.0", dev_dependency = True) diff --git a/WORKSPACE b/WORKSPACE index 74a96c7455..0459aff61a 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -254,3 +254,27 @@ http_archive( strip_prefix = "rules_testing-0.6.0", url = "https://github.com/bazelbuild/rules_testing/releases/download/v0.6.0/rules_testing-v0.6.0.tar.gz", ) + +# For checking breaking changes to well-known types from the previous release version. +load("//:protobuf_version.bzl", "PROTOBUF_PREVIOUS_RELEASE") + +http_archive( + name = "com_google_protobuf_previous_release", + strip_prefix = "protobuf-" + PROTOBUF_PREVIOUS_RELEASE, + url = "https://github.com/protocolbuffers/protobuf/releases/download/v{0}/protobuf-{0}.tar.gz".format(PROTOBUF_PREVIOUS_RELEASE), +) + +http_archive( + name = "rules_buf", + integrity = "sha256-Hr64Q/CaYr0E3ptAjEOgdZd1yc+cBjp7OG1wzuf3DIs=", + strip_prefix = "rules_buf-0.3.0", + urls = [ + "https://github.com/bufbuild/rules_buf/archive/refs/tags/v0.3.0.zip", + ], +) + +load("@rules_buf//buf:repositories.bzl", "rules_buf_dependencies", "rules_buf_toolchains") + +rules_buf_dependencies() + +rules_buf_toolchains(version = "v1.32.1") diff --git a/WORKSPACE.bzlmod b/WORKSPACE.bzlmod index d6e152777b..a2b3d3935f 100644 --- a/WORKSPACE.bzlmod +++ b/WORKSPACE.bzlmod @@ -38,3 +38,11 @@ http_archive( url = "https://github.com/protocolbuffers/utf8_range/archive/d863bc33e15cba6d873c878dcca9e6fe52b2f8cb.zip", ) +# Needed for checking breaking changes from the previous release version. +load("//:protobuf_version.bzl", "PROTOBUF_PREVIOUS_RELEASE") + +http_archive( + name = "com_google_protobuf_previous_release", + strip_prefix = "protobuf-" + PROTOBUF_PREVIOUS_RELEASE, + url = "https://github.com/protocolbuffers/protobuf/releases/download/v{0}/protobuf-{0}.tar.gz".format(PROTOBUF_PREVIOUS_RELEASE), +) diff --git a/compatibility/BUILD.bazel b/compatibility/BUILD.bazel index d55f61956e..91bf221d58 100644 --- a/compatibility/BUILD.bazel +++ b/compatibility/BUILD.bazel @@ -1,11 +1,12 @@ +load("@rules_buf//buf:defs.bzl", "buf_breaking_test") +load("//compatibility:runtime_conformance.bzl", "java_runtime_conformance") + # Simple build tests for compatibility of gencode from previous major versions # with the current runtime. # # To add more test cases in Java, use java_runtime_conformance as below, and add # the corresponding http_archive in the WORKSPACE file for the version. -load("//compatibility:runtime_conformance.bzl", "java_runtime_conformance") - # main gencode builds with main runtime as a proof of concept. java_runtime_conformance( name = "java_conformance_main", @@ -17,3 +18,122 @@ java_runtime_conformance( name = "java_conformance_v3.25.0", gencode_version = "3.25.0", ) + +# Breaking change detection for well-known types and descriptor.proto. +buf_breaking_test( + name = "any_proto_breaking", + against = "@com_google_protobuf_previous_release//:any_proto", + config = ":buf.yaml", + targets = ["//:any_proto"], +) + +buf_breaking_test( + name = "api_proto_breaking", + against = "@com_google_protobuf_previous_release//:api_proto", + config = ":buf.yaml", + targets = ["//:api_proto"], +) + +buf_breaking_test( + name = "descriptor_proto_breaking", + against = "@com_google_protobuf_previous_release//:descriptor_proto", + config = ":buf.yaml", + targets = ["//:descriptor_proto"], +) + +buf_breaking_test( + name = "duration_proto_breaking", + against = "@com_google_protobuf_previous_release//:duration_proto", + config = ":buf.yaml", + targets = ["//:duration_proto"], +) + +buf_breaking_test( + name = "empty_proto_breaking", + against = "@com_google_protobuf_previous_release//:empty_proto", + config = ":buf.yaml", + targets = ["//:empty_proto"], +) + +buf_breaking_test( + name = "field_mask_proto_breaking", + against = "@com_google_protobuf_previous_release//:field_mask_proto", + config = ":buf.yaml", + targets = ["//:field_mask_proto"], +) + +buf_breaking_test( + name = "source_context_proto_breaking", + against = "@com_google_protobuf_previous_release//:source_context_proto", + config = ":buf.yaml", + targets = ["//:source_context_proto"], +) + +buf_breaking_test( + name = "struct_proto_breaking", + against = "@com_google_protobuf_previous_release//:struct_proto", + config = ":buf.yaml", + targets = ["//:struct_proto"], +) + +buf_breaking_test( + name = "timestamp_proto_breaking", + against = "@com_google_protobuf_previous_release//:timestamp_proto", + config = ":buf.yaml", + targets = ["//:timestamp_proto"], +) + +buf_breaking_test( + name = "type_proto_breaking", + against = "@com_google_protobuf_previous_release//:type_proto", + config = ":buf.yaml", + targets = ["//:type_proto"], +) + +buf_breaking_test( + name = "wrappers_proto_breaking", + against = "@com_google_protobuf_previous_release//:wrappers_proto", + config = ":buf.yaml", + targets = ["//:wrappers_proto"], +) + +buf_breaking_test( + name = "compiler_plugin_proto_breaking", + against = "@com_google_protobuf_previous_release//:compiler_plugin_proto", + config = ":buf.yaml", + targets = ["//:compiler_plugin_proto"], +) + +buf_breaking_test( + name = "cpp_features_proto_breaking", + against = "@com_google_protobuf_previous_release//:cpp_features_proto", + config = ":buf.yaml", + targets = ["//:cpp_features_proto"], +) + +buf_breaking_test( + name = "java_features_proto_breaking", + against = "@com_google_protobuf_previous_release//:java_features_proto", + config = ":buf.yaml", + targets = ["//:java_features_proto"], +) + +test_suite( + name = "proto_breaking", + tests = [ + "any_proto_breaking", + "api_proto_breaking", + "compiler_plugin_proto_breaking", + "cpp_features_proto_breaking", + "descriptor_proto_breaking", + "duration_proto_breaking", + "empty_proto_breaking", + "field_mask_proto_breaking", + "java_features_proto_breaking", + "source_context_proto_breaking", + "struct_proto_breaking", + "timestamp_proto_breaking", + "type_proto_breaking", + "wrappers_proto_breaking", + ], +) diff --git a/compatibility/buf.yaml b/compatibility/buf.yaml new file mode 100644 index 0000000000..c126332f30 --- /dev/null +++ b/compatibility/buf.yaml @@ -0,0 +1 @@ +version: v1 diff --git a/protobuf_version.bzl b/protobuf_version.bzl index 41dabece73..b7c794893e 100644 --- a/protobuf_version.bzl +++ b/protobuf_version.bzl @@ -4,3 +4,4 @@ PROTOBUF_JAVA_VERSION = "4.29.0" PROTOBUF_PYTHON_VERSION = "5.29.0" PROTOBUF_PHP_VERSION = "4.29.0" PROTOBUF_RUBY_VERSION = "4.29.0" +PROTOBUF_PREVIOUS_RELEASE = "28.0-rc1"