Extend gencode compatibility support back to 3.20.0 (#22423)

mkruskal-google · web-flow · commit a532f5226aff · 2025-06-27T11:30:29.000-07:00
* Extend gencode compatibility support back to 3.20.0

#test-continuous

This adds 2 shims for some previous breakages, and compatibility tests to avoid further accidents.  This will be backported to 25.x and 29.x.

PiperOrigin-RevId: 776414924

* Remove 30.x parameterization

* Fix requirements install

* Desynchronize upb/pure tests

* Update comment
diff --git a/.github/workflows/test_upb.yml b/.github/workflows/test_upb.yml
@@ -306,3 +306,63 @@ jobs:
           for test in $TESTS; do
             python -m unittest -v $test
           done
+
+  test_python_compatibility:
+    name: ${{ inputs.continuous-prefix }} Test Python Compatibility (${{ matrix.implementation }} - ${{ matrix.version }})
+    needs: build_wheels
+    strategy:
+      fail-fast: false   # Don't cancel all jobs if one fails.
+      matrix:
+        # Test the first release of each python major version, plus some extra coverage for 3.x.y
+        # since it predates our breaking change policies.
+        # Major versions: 4.21.0, 5.26.0
+        version: ["3.18.0", "3.20.0", "21.0", "26.0"]
+        implementation: ["Pure", "upb"]
+        include:
+          # We don't support 3.18.0 anymore, so our infra should show a failure.
+          - version: "3.18.0"
+            fail: true
+    runs-on: ubuntu-latest
+    if: ${{ github.event_name != 'pull_request_target' }}
+    steps:
+      - name: Checkout pending changes
+        if: ${{ inputs.continuous-run }}
+        uses: protocolbuffers/protobuf-ci/checkout@v4
+        with:
+          ref: ${{ inputs.safe-checkout }}
+      - name: Download Wheels
+        if: ${{ inputs.continuous-run }}
+        uses: actions/download-artifact@fa0a91b85d4f404e444e00e005971372dc801d16 #4.1.8
+        with:
+          name: python-wheels
+          path: wheels
+      - name: Delete Binary Wheels (pure python only)
+        if: ${{ inputs.continuous-run && matrix.implementation == 'Pure' }}
+        run: find wheels -type f | grep -v none-any | xargs rm
+      - name: Setup Python
+        if: ${{ inputs.continuous-run }}
+        uses: actions/setup-python@61a6322f88396a6271a6ee3565807d608ecaddd1 # v4.7.0
+        with:
+          python-version: 3.13
+      - name: Setup Python venv
+        if: ${{ inputs.continuous-run }}
+        run: |
+          python -m pip install --upgrade pip
+          python -m venv env
+          source env/bin/activate
+          echo "VIRTUAL ENV:" $VIRTUAL_ENV
+      - name: Install numpy
+        if: ${{ inputs.continuous-run }}
+        run: pip install numpy
+      - name: Install Protobuf Wheels
+        if: ${{ inputs.continuous-run }}
+        run: pip install -vvv --no-index --find-links wheels protobuf protobuftests
+      - name: Make the test script executable
+        if: ${{ inputs.continuous-run }}
+        run: chmod +x ci/python_compatibility.sh
+      - name: Run compatibility tests
+        if: ${{ inputs.continuous-run && !matrix.fail }}
+        run: ci/python_compatibility.sh ${{ matrix.version }} ${{ matrix.implementation == 'upb' && 'pb_unit_tests.*py$' || '_test.py' }}
+      - name: Run failing compatibility tests
+        if: ${{ inputs.continuous-run && matrix.fail }}
+        run: (! ci/python_compatibility.sh ${{ matrix.version }} ${{ matrix.implementation == 'upb' && 'pb_unit_tests.*py$' || '_test.py' }})
diff --git a/ci/python_compatibility.sh b/ci/python_compatibility.sh
@@ -0,0 +1,79 @@
+#!/bin/bash
+
+set -ex
+
+PROTOC_VERSION=$1
+TEST_FILTER=$2
+PROTOC_DOWNLOAD=https://github.com/protocolbuffers/protobuf/releases/download/v$PROTOC_VERSION/protoc-$PROTOC_VERSION-linux-x86_64.zip
+PY_SITE_PACKAGES=$(python -c 'import site; print(site.getsitepackages()[0])')
+
+rm -rf protoc-old
+mkdir protoc-old
+pushd protoc-old
+wget $PROTOC_DOWNLOAD
+unzip *.zip
+PROTOC=$(pwd)/bin/protoc
+popd
+
+# protoc prior to 28.0 doesn't support inf/nan option values.
+sed -i 's/\(inf\|nan\)/0/g' src/google/protobuf/unittest_custom_options.proto
+
+bazel build //python:copied_test_proto_files //python:copied_wkt_proto_files
+
+COMPAT_COPIED_PROTOS=(
+  # Well-known types give good build coverage
+  any
+  api
+  duration
+  empty
+  field_mask
+  source_context
+  struct
+  timestamp
+  type
+  wrappers
+  # These protos are used in tests of custom options handling.
+  unittest_custom_options
+  unittest_import
+)
+
+for proto in ${COMPAT_COPIED_PROTOS[@]}; do
+  $PROTOC --python_out=$PY_SITE_PACKAGES \
+    bazel-bin/python/google/protobuf/$proto.proto \
+    -Ibazel-bin/python
+done
+
+COMPAT_PROTOS=(
+  # All protos without transitive dependencies on edition 2023+.
+  descriptor_pool_test1
+  descriptor_pool_test2
+  factory_test1
+  factory_test2
+  file_options_test
+  import_test_package/import_public
+  import_test_package/import_public_nested
+  import_test_package/inner
+  import_test_package/outer
+  message_set_extensions
+  missing_enum_values
+  more_extensions
+  more_extensions_dynamic
+  more_messages
+  no_package
+  packed_field_test
+  test_bad_identifiers
+  test_proto2
+  test_proto3_optional
+)
+
+for proto in ${COMPAT_PROTOS[@]}; do
+  $PROTOC --python_out=$PY_SITE_PACKAGES \
+    python/google/protobuf/internal/$proto.proto \
+    -Ipython
+done
+
+# Exclude pybind11 tests because they require C++ code that doesn't ship with
+# our test wheels.
+TEST_EXCLUSIONS="_pybind11_test.py"
+TESTS=$(pip show -f protobuftests | grep -i $TEST_FILTER | grep --invert-match $TEST_EXCLUSIONS | sed 's,/,.,g' | sed -E 's,.py$,,g')
+python -m unittest -v ${TESTS[@]}
diff --git a/python/google/protobuf/descriptor.py b/python/google/protobuf/descriptor.py
@@ -128,13 +128,16 @@ class of the options message. The name of the class is required in case
     """
     self._features = None
     self.file = file
-    self._options = options
-    self._loaded_options = None
+    self._original_options = options
+    # These two fields are duplicated as a compatibility shim for old gencode
+    # that resets them.  In 26.x (cl/580304039) we renamed _options to,
+    # _loaded_options breaking backwards compatibility.
+    self._options = self._loaded_options = None
     self._options_class_name = options_class_name
     self._serialized_options = serialized_options
 
     # Does this descriptor have non-default options?
-    self.has_options = (self._options is not None) or (
+    self.has_options = (self._original_options is not None) or (
         self._serialized_options is not None
     )
 
@@ -186,7 +189,8 @@ def _ResolveFeatures(self, edition, raw_options):
 
   def _LazyLoadOptions(self):
     """Lazily initializes descriptor options towards the end of the build."""
-    if self._loaded_options:
+    if self._options and self._loaded_options == self._options:
+      # If neither has been reset by gencode, use the cache.
       return
 
     # pylint: disable=g-import-not-at-top
@@ -206,12 +210,12 @@ def _LazyLoadOptions(self):
             descriptor_pb2.Edition.Value(edition), options_class()
         )
       with _lock:
-        self._loaded_options = options_class()
+        self._options = self._loaded_options = options_class()
         if not self._features:
           self._features = features
     else:
       if not self._serialized_options:
-        options = self._options
+        options = self._original_options
       else:
         options = _ParseOptions(options_class(), self._serialized_options)
 
@@ -220,13 +224,13 @@ def _LazyLoadOptions(self):
             descriptor_pb2.Edition.Value(edition), options
         )
       with _lock:
-        self._loaded_options = options
+        self._options = self._loaded_options = options
         if not self._features:
           self._features = features
         if options.HasField('features'):
           options.ClearField('features')
           if not options.SerializeToString():
-            self._loaded_options = options_class()
+            self._options = self._loaded_options = options_class()
             self.has_options = False
 
   def GetOptions(self):
@@ -235,9 +239,10 @@ def GetOptions(self):
     Returns:
       The options set on this descriptor.
     """
-    if not self._loaded_options:
+    # If either has been reset by gencode, reload options.
+    if not self._options or not self._loaded_options:
       self._LazyLoadOptions()
-    return self._loaded_options
+    return self._options
 
 
 class _NestedDescriptorBase(DescriptorBase):
diff --git a/python/google/protobuf/internal/python_message.py b/python/google/protobuf/internal/python_message.py
@@ -822,6 +822,13 @@ def _AddPropertiesForExtensions(descriptor, cls):
     pool = descriptor.file.pool
 
 def _AddStaticMethods(cls):
+
+  def RegisterExtension(_):
+    """no-op to keep generated code <=4.23 working with new runtimes."""
+    # This was originally removed in 5.26 (cl/595989309).
+    pass
+
+  cls.RegisterExtension = staticmethod(RegisterExtension)
   def FromString(s):
     message = cls()
     message.MergeFromString(s)
diff --git a/python/google/protobuf/internal/runtime_version_test.py b/python/google/protobuf/internal/runtime_version_test.py
@@ -36,24 +36,6 @@ def test_cross_domain_disallowed(self):
           gencode_domain, 1, 2, 3, '', 'foo.proto'
       )
 
-  def test_mismatched_major_disallowed(self):
-    gencode_version_string = f'{1}.{runtime_version.MINOR}.{runtime_version.PATCH}{runtime_version.SUFFIX}'
-    runtime_version_string = f'{runtime_version.MAJOR}.{runtime_version.MINOR}.{runtime_version.PATCH}{runtime_version.SUFFIX}'
-    with self.assertRaisesRegex(
-        runtime_version.VersionError,
-        'Detected mismatched Protobuf Gencode/Runtime major versions when'
-        f' loading foo.proto: gencode {gencode_version_string} runtime'
-        f' {runtime_version_string}',
-    ):
-      runtime_version.ValidateProtobufRuntimeVersion(
-          runtime_version.DOMAIN,
-          1,
-          runtime_version.MINOR,
-          runtime_version.PATCH,
-          runtime_version.SUFFIX,
-          'foo.proto',
-      )
-
   def test_same_version_allowed(self):
     runtime_version.ValidateProtobufRuntimeVersion(
         runtime_version.DOMAIN,
@@ -103,34 +85,6 @@ def test_older_runtime_version_disallowed(self):
           'foo.proto',
       )
 
-  def test_different_suffix_disallowed(self):
-    with self.assertRaisesRegex(
-        runtime_version.VersionError,
-        'Detected mismatched Protobuf Gencode/Runtime version suffixes when'
-        ' loading foo.proto',
-    ):
-      runtime_version.ValidateProtobufRuntimeVersion(
-          runtime_version.DOMAIN,
-          runtime_version.MAJOR,
-          runtime_version.MINOR,
-          runtime_version.PATCH,
-          '-noflag',
-          'foo.proto',
-      )
-
-  def test_gencode_one_major_version_older_warning(self):
-    with self.assertWarnsRegex(
-        Warning, expected_regex='is exactly one major version older'
-    ):
-      runtime_version.ValidateProtobufRuntimeVersion(
-          runtime_version.DOMAIN,
-          runtime_version.MAJOR - 1,
-          runtime_version.MINOR,
-          runtime_version.PATCH,
-          runtime_version.SUFFIX,
-          'foo.proto',
-      )
-
 
 if __name__ == '__main__':
   unittest.main()
diff --git a/python/google/protobuf/runtime_version.py b/python/google/protobuf/runtime_version.py
@@ -92,33 +92,13 @@ def ValidateProtobufRuntimeVersion(
         ' Cross-domain usage of Protobuf is not supported.'
     )
 
-  if gen_major != MAJOR:
-    if gen_major == MAJOR - 1:
-      if _warning_count < _MAX_WARNING_COUNT:
-        warnings.warn(
-            'Protobuf gencode version %s is exactly one major version older'
-            ' than the runtime version %s at %s. Please update the gencode to'
-            ' avoid compatibility violations in the next runtime release.'
-            % (gen_version, version, location)
-        )
-        _warning_count += 1
-    else:
-      _ReportVersionError(
-          'Detected mismatched Protobuf Gencode/Runtime major versions when'
-          f' loading {location}: gencode {gen_version} runtime {version}.'
-          f' Same major version is required. {error_prompt}'
-      )
-
-  if MINOR < gen_minor or (MINOR == gen_minor and PATCH < gen_patch):
+  if (
+      MAJOR < gen_major
+      or (MAJOR == gen_major and MINOR < gen_minor)
+      or (MAJOR == gen_major and MINOR == gen_minor and PATCH < gen_patch)
+  ):
     _ReportVersionError(
         'Detected incompatible Protobuf Gencode/Runtime versions when loading'
         f' {location}: gencode {gen_version} runtime {version}. Runtime version'
         f' cannot be older than the linked gencode version. {error_prompt}'
     )
-
-  if gen_suffix != SUFFIX:
-    _ReportVersionError(
-        'Detected mismatched Protobuf Gencode/Runtime version suffixes when'
-        f' loading {location}: gencode {gen_version} runtime {version}.'
-        f' Version suffixes must be the same. {error_prompt}'
-    )
