Binary compatibility shims for GeneratedMessageV3, SingleFieldBuilder…

…V3, RepeatedFieldBuilderV3, and their nested classes to restore binary compatibility with <=v3.x.x generated code built against v3.x.x prior to v4.26.0 breaking release. 3.x.x descriptor.proto generated code is *not* supported with 4.x.x runtime, since this results in an ODR violation with the descriptor.proto built into the 4.x.x runtime. This is expected to result in undefined behavior / failures. Tested against //java/core:v25_generated_message_test_jar (binary compatibility) and //java/core:v25_generated_message_test_srcjar (source compatibility) PiperOrigin-RevId: 666329342
protocolbuffers · Aug 22, 2024 · df8a11e · df8a11e
1 parent e9140a1
commit df8a11e
Show file tree

Hide file tree

Showing 12 changed files with 1,504 additions and 64 deletions.
diff --git a/WORKSPACE b/WORKSPACE
@@ -238,13 +238,9 @@ http_archive(
     url = "https://github.com/protocolbuffers/protobuf/releases/download/v25.0/protobuf-25.0.tar.gz",
 )
 
-# Needed as a dependency of @com_google_protobuf_v25.x, which was before
-# utf8_range was merged in.
-http_archive(
-    name = "utf8_range",
-    strip_prefix = "utf8_range-d863bc33e15cba6d873c878dcca9e6fe52b2f8cb",
-    url = "https://github.com/protocolbuffers/utf8_range/archive/d863bc33e15cba6d873c878dcca9e6fe52b2f8cb.zip",
-)
+# Needed as a dependency of @com_google_protobuf_v25.0
+load("@com_google_protobuf_v25.0//:protobuf_deps.bzl", protobuf_v25_deps="protobuf_deps")
+protobuf_v25_deps()
 
 # Needed for testing only
 load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")

diff --git a/WORKSPACE.bzlmod b/WORKSPACE.bzlmod
@@ -30,13 +30,10 @@ http_archive(
     url = "https://github.com/protocolbuffers/protobuf/releases/download/v25.0/protobuf-25.0.tar.gz",
 )
 
-# Needed as a dependency of @com_google_protobuf_v25.x, which was before
-# utf8_range was merged in.
-http_archive(
-    name = "utf8_range",
-    strip_prefix = "utf8_range-d863bc33e15cba6d873c878dcca9e6fe52b2f8cb",
-    url = "https://github.com/protocolbuffers/utf8_range/archive/d863bc33e15cba6d873c878dcca9e6fe52b2f8cb.zip",
-)
+# Needed as a dependency of @com_google_protobuf_v25.0
+load("@com_google_protobuf_v25.0//:protobuf_deps.bzl", protobuf_v25_deps="protobuf_deps")
+protobuf_v25_deps()
+
 
 # Needed for checking breaking changes from the previous release version.
 load("//:protobuf_version.bzl", "PROTOBUF_PREVIOUS_RELEASE")

diff --git a/compatibility/BUILD.bazel b/compatibility/BUILD.bazel
@@ -7,6 +7,26 @@ load("//compatibility:runtime_conformance.bzl", "java_runtime_conformance")
 # 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.
 
+java_library(
+    name = "v25_test_protos_srcjar",
+    testonly = True,
+    srcs = glob([
+        "v3.25.0/*.srcjar",
+    ]),
+    visibility = ["//java/core:__pkg__"],
+    deps = ["//java/core"],
+)
+
+java_library(
+    name = "v25_test_protos_jar",
+    testonly = True,
+    srcs = glob([
+        "v3.25.0/*.srcjar",
+    ]),
+    visibility = ["//java/core:__pkg__"],
+    deps = ["@com_google_protobuf_v25.0//java/core"],
+)
+
 # main gencode builds with main runtime as a proof of concept.
 java_runtime_conformance(
     name = "java_conformance_main",

diff --git a/compatibility/v3.25.0/generic_test_protos-speed.srcjar b/compatibility/v3.25.0/generic_test_protos-speed.srcjar
diff --git a/compatibility/v3.25.0/java_test_protos-speed.srcjar b/compatibility/v3.25.0/java_test_protos-speed.srcjar
diff --git a/compatibility/v3.25.0/lite_test_protos-speed.srcjar b/compatibility/v3.25.0/lite_test_protos-speed.srcjar
diff --git a/java/core/BUILD.bazel b/java/core/BUILD.bazel
@@ -590,6 +590,102 @@ junit_tests(
     ],
 )
 
+protobuf_java_library(
+    name = "v25_test_util_srcjar",
+    testonly = True,
+    srcs = [
+        "src/test/java/com/google/protobuf/TestUtil.java",
+        "src/test/java/com/google/protobuf/TestUtilLite.java",
+    ],
+    deps = [
+        ":core",
+        "//compatibility:v25_test_protos_srcjar",
+        "@maven//:com_google_guava_guava",
+        "@maven//:junit_junit",
+    ],
+)
+
+# Tests source compatibility against v25 gencode jar compiled against current runtime
+junit_tests(
+    name = "v25_core_tests_srcjar",
+    size = "small",
+    srcs = glob(
+        ["src/test/java/**/*.java"],
+        exclude = [
+            # Depends on test protos or API changes added in v4.x.x (e.g. editions)
+            "src/test/java/com/google/protobuf/TextFormatTest.java",
+            "src/test/java/com/google/protobuf/DescriptorsTest.java",
+            "src/test/java/com/google/protobuf/DebugFormatTest.java",
+            "src/test/java/com/google/protobuf/CodedOutputStreamTest.java",
+            "src/test/java/com/google/protobuf/ProtobufToStringOutputTest.java",
+            # Excluded in core_tests
+            "src/test/java/com/google/protobuf/DecodeUtf8Test.java",
+            "src/test/java/com/google/protobuf/IsValidUtf8Test.java",
+            "src/test/java/com/google/protobuf/TestUtil.java",
+            "src/test/java/com/google/protobuf/TestUtilLite.java",
+            "src/test/java/com/google/protobuf/RuntimeVersionTest.java",
+        ],
+    ),
+    test_prefix = "v25SrcJar",
+    deps = [
+        ":core",
+        ":v25_test_util_srcjar",
+        "//compatibility:v25_test_protos_srcjar",
+        "@maven//:com_google_guava_guava",
+        "@maven//:com_google_truth_truth",
+        "@maven//:junit_junit",
+        "@maven//:org_mockito_mockito_core",
+    ],
+)
+
+protobuf_java_library(
+    name = "v25_test_util_jar",
+    testonly = True,
+    srcs = [
+        "src/test/java/com/google/protobuf/TestUtil.java",
+        "src/test/java/com/google/protobuf/TestUtilLite.java",
+    ],
+    deps = [
+        ":core",
+        "//compatibility:v25_test_protos_jar",
+        "@maven//:com_google_guava_guava",
+        "@maven//:junit_junit",
+    ],
+)
+
+# Tests binary compatibility against v25 gencode ja compiled against v25 runtime
+junit_tests(
+    name = "v25_core_tests_jar",
+    size = "small",
+    srcs = glob(
+        ["src/test/java/**/*.java"],
+        exclude = [
+            # Depends on test protos or API changes added in v4.x.x (e.g. editions)
+            "src/test/java/com/google/protobuf/TextFormatTest.java",
+            "src/test/java/com/google/protobuf/DescriptorsTest.java",
+            "src/test/java/com/google/protobuf/DebugFormatTest.java",
+            "src/test/java/com/google/protobuf/CodedOutputStreamTest.java",
+            "src/test/java/com/google/protobuf/ProtobufToStringOutputTest.java",
+            # Excluded in core_tests
+            "src/test/java/com/google/protobuf/DecodeUtf8Test.java",
+            "src/test/java/com/google/protobuf/IsValidUtf8Test.java",
+            "src/test/java/com/google/protobuf/TestUtil.java",
+            "src/test/java/com/google/protobuf/TestUtilLite.java",
+            "src/test/java/com/google/protobuf/RuntimeVersionTest.java",
+        ],
+    ),
+    test_prefix = "v25Jar",
+    deps = [
+        ":core",
+        ":v25_test_util_jar",
+        "//compatibility:v25_test_protos_jar",
+        "@maven//:com_google_guava_guava",
+        "@maven//:com_google_truth_truth",
+        "@maven//:junit_junit",
+        "@maven//:org_mockito_mockito_core",
+    ],
+)
+
 pkg_files(
     name = "dist_files",
     srcs = glob([

diff --git a/java/core/src/main/java/com/google/protobuf/GeneratedMessage.java b/java/core/src/main/java/com/google/protobuf/GeneratedMessage.java
@@ -884,16 +884,16 @@ public interface ExtendableMessageOrBuilder<MessageT extends ExtendableMessage<M
     Message getDefaultInstanceForType();
 
     /** Check if a singular extension is present. */
-    <T> boolean hasExtension(ExtensionLite<MessageT, T> extension);
+    <T> boolean hasExtension(ExtensionLite<? extends MessageT, T> extension);
 
     /** Get the number of elements in a repeated extension. */
-    <T> int getExtensionCount(ExtensionLite<MessageT, List<T>> extension);
+    <T> int getExtensionCount(ExtensionLite<? extends MessageT, List<T>> extension);
 
     /** Get the value of an extension. */
-    <T> T getExtension(ExtensionLite<MessageT, T> extension);
+    <T> T getExtension(ExtensionLite<? extends MessageT, T> extension);
 
     /** Get one element of a repeated extension. */
-    <T> T getExtension(ExtensionLite<MessageT, List<T>> extension, int index);
+    <T> T getExtension(ExtensionLite<? extends MessageT, List<T>> extension, int index);
   }
 
   /**
@@ -944,7 +944,7 @@ protected ExtendableMessage(ExtendableBuilder<MessageT, ?> builder) {
       this.extensions = builder.buildExtensions();
     }
 
-    private void verifyExtensionContainingType(final Extension<MessageT, ?> extension) {
+    private void verifyExtensionContainingType(final Extension<? extends MessageT, ?> extension) {
       if (extension.getDescriptor().getContainingType() != getDescriptorForType()) {
         // This can only happen if someone uses unchecked operations.
         throw new IllegalArgumentException(
@@ -958,7 +958,8 @@ private void verifyExtensionContainingType(final Extension<MessageT, ?> extensio
 
     /** Check if a singular extension is present. */
     @Override
-    public final <T> boolean hasExtension(final ExtensionLite<MessageT, T> extensionLite) {
+    public final <T> boolean hasExtension(
+        final ExtensionLite<? extends MessageT, T> extensionLite) {
       Extension<MessageT, T> extension = checkNotLite(extensionLite);
 
       verifyExtensionContainingType(extension);
@@ -967,7 +968,8 @@ public final <T> boolean hasExtension(final ExtensionLite<MessageT, T> extension
 
     /** Get the number of elements in a repeated extension. */
     @Override
-    public final <T> int getExtensionCount(final ExtensionLite<MessageT, List<T>> extensionLite) {
+    public final <T> int getExtensionCount(
+        final ExtensionLite<? extends MessageT, List<T>> extensionLite) {
       Extension<MessageT, List<T>> extension = checkNotLite(extensionLite);
 
       verifyExtensionContainingType(extension);
@@ -978,7 +980,7 @@ public final <T> int getExtensionCount(final ExtensionLite<MessageT, List<T>> ex
     /** Get the value of an extension. */
     @Override
     @SuppressWarnings("unchecked")
-    public final <T> T getExtension(final ExtensionLite<MessageT, T> extensionLite) {
+    public final <T> T getExtension(final ExtensionLite<? extends MessageT, T> extensionLite) {
       Extension<MessageT, T> extension = checkNotLite(extensionLite);
 
       verifyExtensionContainingType(extension);
@@ -1001,7 +1003,7 @@ public final <T> T getExtension(final ExtensionLite<MessageT, T> extensionLite)
     @Override
     @SuppressWarnings("unchecked")
     public final <T> T getExtension(
-        final ExtensionLite<MessageT, List<T>> extensionLite, final int index) {
+        final ExtensionLite<? extends MessageT, List<T>> extensionLite, final int index) {
       Extension<MessageT, List<T>> extension = checkNotLite(extensionLite);
 
       verifyExtensionContainingType(extension);
@@ -1052,7 +1054,8 @@ protected class ExtensionWriter implements ExtensionSerializer {
       private Map.Entry<FieldDescriptor, Object> next;
       private final boolean messageSetWireFormat;
 
-      private ExtensionWriter(final boolean messageSetWireFormat) {
+      // TODO: Should be marked private in v5.x.x once GeneratedMessageV3 is removed.
+      protected ExtensionWriter(final boolean messageSetWireFormat) {
         if (iter.hasNext()) {
           next = iter.next();
         }
@@ -1296,7 +1299,8 @@ private void verifyExtensionContainingType(final Extension<MessageT, ?> extensio
 
     /** Check if a singular extension is present. */
     @Override
-    public final <T> boolean hasExtension(final ExtensionLite<MessageT, T> extensionLite) {
+    public final <T> boolean hasExtension(
+        final ExtensionLite<? extends MessageT, T> extensionLite) {
       Extension<MessageT, T> extension = checkNotLite(extensionLite);
 
       verifyExtensionContainingType(extension);
@@ -1305,7 +1309,8 @@ public final <T> boolean hasExtension(final ExtensionLite<MessageT, T> extension
 
     /** Get the number of elements in a repeated extension. */
     @Override
-    public final <T> int getExtensionCount(final ExtensionLite<MessageT, List<T>> extensionLite) {
+    public final <T> int getExtensionCount(
+        final ExtensionLite<? extends MessageT, List<T>> extensionLite) {
       Extension<MessageT, List<T>> extension = checkNotLite(extensionLite);
 
       verifyExtensionContainingType(extension);
@@ -1315,7 +1320,7 @@ public final <T> int getExtensionCount(final ExtensionLite<MessageT, List<T>> ex
 
     /** Get the value of an extension. */
     @Override
-    public final <T> T getExtension(final ExtensionLite<MessageT, T> extensionLite) {
+    public final <T> T getExtension(final ExtensionLite<? extends MessageT, T> extensionLite) {
       Extension<MessageT, T> extension = checkNotLite(extensionLite);
 
       verifyExtensionContainingType(extension);
@@ -1337,7 +1342,7 @@ public final <T> T getExtension(final ExtensionLite<MessageT, T> extensionLite)
     /** Get one element of a repeated extension. */
     @Override
     public final <T> T getExtension(
-        final ExtensionLite<MessageT, List<T>> extensionLite, final int index) {
+        final ExtensionLite<? extends MessageT, List<T>> extensionLite, final int index) {
       Extension<MessageT, List<T>> extension = checkNotLite(extensionLite);
 
       verifyExtensionContainingType(extension);
@@ -1351,7 +1356,7 @@ public final <T> T getExtension(
 
     /** Set the value of an extension. */
     public final <T> BuilderT setExtension(
-        final ExtensionLite<MessageT, T> extensionLite, final T value) {
+        final ExtensionLite<? extends MessageT, T> extensionLite, final T value) {
       Extension<MessageT, T> extension = checkNotLite(extensionLite);
 
       verifyExtensionContainingType(extension);
@@ -1364,7 +1369,9 @@ public final <T> BuilderT setExtension(
 
     /** Set the value of one element of a repeated extension. */
     public final <T> BuilderT setExtension(
-        final ExtensionLite<MessageT, List<T>> extensionLite, final int index, final T value) {
+        final ExtensionLite<? extends MessageT, List<T>> extensionLite,
+        final int index,
+        final T value) {
       Extension<MessageT, List<T>> extension = checkNotLite(extensionLite);
 
       verifyExtensionContainingType(extension);
@@ -1377,7 +1384,7 @@ public final <T> BuilderT setExtension(
 
     /** Append a value to a repeated extension. */
     public final <T> BuilderT addExtension(
-        final ExtensionLite<MessageT, List<T>> extensionLite, final T value) {
+        final ExtensionLite<? extends MessageT, List<T>> extensionLite, final T value) {
       Extension<MessageT, List<T>> extension = checkNotLite(extensionLite);
 
       verifyExtensionContainingType(extension);
@@ -1389,7 +1396,8 @@ public final <T> BuilderT addExtension(
     }
 
     /** Clear an extension. */
-    public final <T> BuilderT clearExtension(final ExtensionLite<MessageT, T> extensionLite) {
+    public final <T> BuilderT clearExtension(
+        final ExtensionLite<? extends MessageT, T> extensionLite) {
       Extension<MessageT, T> extension = checkNotLite(extensionLite);
 
       verifyExtensionContainingType(extension);
@@ -1605,7 +1613,8 @@ public Message.Builder newBuilderForField(final FieldDescriptor field) {
       }
     }
 
-    protected final void mergeExtensionFields(final ExtendableMessage<?> other) {
+    // TODO: Should be marked final in v5.x.x once GeneratedMessageV3 is removed.
+    protected void mergeExtensionFields(final ExtendableMessage<?> other) {
       if (other.extensions != null) {
         ensureExtensionsIsMutable();
         extensions.mergeFrom(other.extensions);
@@ -1981,7 +1990,8 @@ protected MapField internalGetMapField(int fieldNumber) {
    * Users should ignore this class. This class provides the implementation with access to the
    * fields of a message object using Java reflection.
    */
-  public static final class FieldAccessorTable {
+  // TODO: Should be marked final in v5.x.x once GeneratedMessageV3 is removed.
+  public static class FieldAccessorTable {
 
     /**
      * Construct a FieldAccessorTable for a particular message class. Only one FieldAccessorTable
@@ -3172,7 +3182,7 @@ protected Object writeReplace() throws ObjectStreamException {
    * Checks that the {@link Extension} is non-Lite and returns it as a {@link GeneratedExtension}.
    */
   private static <MessageT extends ExtendableMessage<MessageT>, T>
-      Extension<MessageT, T> checkNotLite(ExtensionLite<MessageT, T> extension) {
+      Extension<MessageT, T> checkNotLite(ExtensionLite<? extends MessageT, T> extension) {
     if (extension.isLite()) {
       throw new IllegalArgumentException("Expected non-lite extension.");
     }