Skip to content

Commit

Permalink
Add breaking change detection for well-known types (#17513)
Browse files Browse the repository at this point in the history
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=#17513 from jchadwick-buf:buf-breaking cd46bb2
PiperOrigin-RevId: 658624708
  • Loading branch information
jchadwick-buf authored and copybara-github committed Aug 2, 2024
1 parent 621f2a2 commit db48abb
Show file tree
Hide file tree
Showing 6 changed files with 158 additions and 3 deletions.
3 changes: 2 additions & 1 deletion MODULE.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -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)
bazel_dep(name = "rules_proto", version = "4.0.0", dev_dependency = True)
24 changes: 24 additions & 0 deletions WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -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")
8 changes: 8 additions & 0 deletions WORKSPACE.bzlmod
Original file line number Diff line number Diff line change
Expand Up @@ -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),
)
124 changes: 122 additions & 2 deletions compatibility/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -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",
Expand All @@ -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",
],
)
1 change: 1 addition & 0 deletions compatibility/buf.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
version: v1
1 change: 1 addition & 0 deletions protobuf_version.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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"

0 comments on commit db48abb

Please sign in to comment.