From 50c19154a449d65cca5cb805176682edff39f5d4 Mon Sep 17 00:00:00 2001 From: Tony Liao Date: Thu, 2 Jan 2025 08:47:29 -0800 Subject: [PATCH] Link in UPB by default for Bazel's python build rules. In v21.x, we announced our intent to flip the default implementation of protobuf python from pure python to upb. - https://github.com/protocolbuffers/protobuf/releases/tag/v21.0 We also documented in docs and readme that PROTOCOL_BUFFERS_PYTHON_IMPLEMENTATION env var can be set to override the python implementation used. - https://protobuf.dev/reference/python/python-generated/#sharing-messages In practice, we only use upb if it can successfully be imported: - https://github.com/protocolbuffers/protobuf/blob/1c29f34b24eafe4a362ce075c12c62c614fad199/python/google/protobuf/internal/api_implementation.py#L54 - https://github.com/protocolbuffers/protobuf/blob/1c29f34b24eafe4a362ce075c12c62c614fad199/python/google/protobuf/internal/api_implementation.py#L100 Otherwise, we fall back to pure python, even if the env var is set to upb. Since we don't currently link in upb for Bazel, this means the default is still pure python in practice and the environment variable is not respected. This change links in UPB by default and ensures that default python builds and unit tests using Bazel will enable running with python-UPB. #test-continuous PiperOrigin-RevId: 711436307 --- .github/workflows/test_cpp.yml | 6 +++++- .github/workflows/test_python.yml | 35 ++++++++++++++++++++++--------- .github/workflows/test_upb.yml | 33 ++++++++++++++++++++++++----- ci/common.bazelrc | 3 +-- python/BUILD.bazel | 1 - python/build_targets.bzl | 7 +++++++ python/python_api_test.py | 28 +++++++++++++++++++++++++ 7 files changed, 94 insertions(+), 19 deletions(-) create mode 100644 python/python_api_test.py diff --git a/.github/workflows/test_cpp.yml b/.github/workflows/test_cpp.yml index 19a1a5149bbf2..ec331c7f5b054 100644 --- a/.github/workflows/test_cpp.yml +++ b/.github/workflows/test_cpp.yml @@ -93,7 +93,11 @@ jobs: image: us-docker.pkg.dev/protobuf-build/containers/test/linux/gcc:7.1.2-${{ matrix.version }}-d9624f2aa83cba3eaf906f751d75b36aacb9aa82 credentials: ${{ secrets.GAR_SERVICE_ACCOUNT }} bazel-cache: cpp_linux/gcc-${{ matrix.version }} - bazel: test //pkg/... //src/... @com_google_protobuf_examples//... //third_party/utf8_range/... //conformance:conformance_framework_tests + bazel: >- + test --copt="-Wno-error=attributes" + //pkg/... //src/... @com_google_protobuf_examples//... + //third_party/utf8_range/... + //conformance:conformance_framework_tests linux-release: strategy: diff --git a/.github/workflows/test_python.yml b/.github/workflows/test_python.yml index 7f0c613e7c339..3c8215899f041 100644 --- a/.github/workflows/test_python.yml +++ b/.github/workflows/test_python.yml @@ -27,20 +27,29 @@ jobs: strategy: fail-fast: false # Don't cancel all jobs if one fails. matrix: - type: [ Pure, C++] + type: [Pure, C++] version: ["3.9", "3.10", "3.11", "3.12", "3.13"] include: - type: Pure - targets: //python/... //python:python_version_test - flags: --define=use_fast_cpp_protos=false + targets: //python/... //python:python_version_test //python:python_api_test + flags: >- + --define=use_fast_cpp_protos=false + --test_env=PROTOCOL_BUFFERS_PYTHON_IMPLEMENTATION=python + --test_env=PROTOCOL_BUFFERS_EXPECTED_PYTHON_BACKEND=python - type: C++ - targets: //python/... //python:python_version_test - flags: --define=use_fast_cpp_protos=true + targets: //python/... //python:python_version_test //python:python_api_test + flags: >- + --define=use_fast_cpp_protos=true + --test_env=PROTOCOL_BUFFERS_PYTHON_IMPLEMENTATION=cpp + --test_env=PROTOCOL_BUFFERS_EXPECTED_PYTHON_BACKEND=cpp - type: C++ version: aarch64 - targets: //python/... //python:aarch64_test + targets: //python/... //python:aarch64_test //python:python_api_test # TODO Enable this once conformance tests are fixed. - flags: --define=use_fast_cpp_protos=true --test_tag_filters=-conformance + flags: >- + --define=use_fast_cpp_protos=true --test_tag_filters=-conformance + --test_env=PROTOCOL_BUFFERS_PYTHON_IMPLEMENTATION=cpp + --test_env=PROTOCOL_BUFFERS_EXPECTED_PYTHON_BACKEND=cpp image: us-docker.pkg.dev/protobuf-build/containers/test/linux/emulation:7.1.2-aarch64-2920199ab0090ed427413a8e422e62695c8392a8 - version: "3.9" - version: "3.10" @@ -77,10 +86,16 @@ jobs: version: [ "3.12", "3.13" ] include: - type: Pure - targets: //python/... //python:python_version_test + targets: //python/... //python:python_version_test //python:python_api_test + flags: >- + --test_env=PROTOCOL_BUFFERS_PYTHON_IMPLEMENTATION=python + --test_env=PROTOCOL_BUFFERS_EXPECTED_PYTHON_BACKEND=python - type: C++ - targets: //python/... //python:python_version_test - flags: --define=use_fast_cpp_protos=true + targets: //python/... //python:python_version_test //python:python_api_test + flags: >- + --define=use_fast_cpp_protos=true + --test_env=PROTOCOL_BUFFERS_PYTHON_IMPLEMENTATION=cpp + --test_env=PROTOCOL_BUFFERS_EXPECTED_PYTHON_BACKEND=cpp - version: "3.13" continuous-only: true diff --git a/.github/workflows/test_upb.yml b/.github/workflows/test_upb.yml index cee38c270314f..51b92a3b3213a 100644 --- a/.github/workflows/test_upb.yml +++ b/.github/workflows/test_upb.yml @@ -52,7 +52,13 @@ jobs: image: us-docker.pkg.dev/protobuf-build/containers/test/linux/sanitize:${{ matrix.config.bazel_version || '7.1.2' }}-d9624f2aa83cba3eaf906f751d75b36aacb9aa82 credentials: ${{ secrets.GAR_SERVICE_ACCOUNT }} bazel-cache: upb-bazel - bazel: test --cxxopt=-std=c++17 --host_cxxopt=-std=c++17 //bazel/... //benchmarks/... //lua/... //python/... //upb/... //upb_generator/... ${{ matrix.config.flags }} + bazel: >- + test --cxxopt=-std=c++17 --host_cxxopt=-std=c++17 + --test_env=PROTOCOL_BUFFERS_PYTHON_IMPLEMENTATION=upb + --test_env=PROTOCOL_BUFFERS_EXPECTED_PYTHON_BACKEND=upb + //bazel/... + //benchmarks/... //lua/... //python/... //upb/... + //upb_generator/... ${{ matrix.config.flags }} exclude-targets: ${{ matrix.config.exclude-targets }} linux-gcc: @@ -74,7 +80,10 @@ jobs: bazel: >- test --cxxopt=-std=c++17 --host_cxxopt=-std=c++17 -c opt --copt="-Wno-error=maybe-uninitialized" --copt="-Wno-error=attributes" - //bazel/... //benchmarks/... //lua/... //python/... //upb/... //upb_generator/... + --test_env=PROTOCOL_BUFFERS_PYTHON_IMPLEMENTATION=upb + --test_env=PROTOCOL_BUFFERS_EXPECTED_PYTHON_BACKEND=upb + //bazel/... //benchmarks/... //lua/... //python/... //upb/... + //upb_generator/... windows: strategy: @@ -96,7 +105,11 @@ jobs: with: credentials: ${{ secrets.GAR_SERVICE_ACCOUNT }} bazel-cache: "upb-bazel-windows" - bazel: test --cxxopt=/std:c++17 --host_cxxopt=/std:c++17 //upb/... //upb_generator/... //python/... + bazel: >- + test --cxxopt=/std:c++17 --host_cxxopt=/std:c++17 + --test_env=PROTOCOL_BUFFERS_PYTHON_IMPLEMENTATION=upb + --test_env=PROTOCOL_BUFFERS_EXPECTED_PYTHON_BACKEND=upb + //upb/... //upb_generator/... //python/... version: 7.1.2 exclude-targets: -//python:conformance_test -//upb/reflection:def_builder_test @@ -125,7 +138,13 @@ jobs: with: credentials: ${{ secrets.GAR_SERVICE_ACCOUNT }} bazel-cache: "upb-bazel-macos" - bazel: ${{ matrix.config.bazel-command }} --cxxopt=-std=c++17 --host_cxxopt=-std=c++17 ${{ matrix.config.flags }} //bazel/... //benchmarks/... //lua/... //python/... //upb/... //upb_generator/... + bazel: >- + ${{ matrix.config.bazel-command }} --cxxopt=-std=c++17 + --host_cxxopt=-std=c++17 ${{ matrix.config.flags }} + --test_env=PROTOCOL_BUFFERS_PYTHON_IMPLEMENTATION=upb + --test_env=PROTOCOL_BUFFERS_EXPECTED_PYTHON_BACKEND=upb + //bazel/... //benchmarks/... //lua/... //python/... //upb/... + //upb_generator/... version: 7.1.2 no-python: @@ -148,7 +167,7 @@ jobs: which python3 && mv `which python3` /tmp && ! which python3 && - bazel test $BAZEL_FLAGS --cxxopt=-std=c++17 --host_cxxopt=-std=c++17 //python/... -- -//python/dist:source_wheel -//python:aarch64_test -//python:x86_64_test -//python:google/protobuf/pyext/_message.so -//python:proto_api + bazel test $BAZEL_FLAGS --cxxopt=-std=c++17 --host_cxxopt=-std=c++17 //python/... -- -//python/dist:source_wheel -//python:aarch64_test -//python:x86_64_test -//python:python_api_test -//python:google/protobuf/pyext/_message.so -//python:proto_api build_wheels: name: Build Wheels @@ -179,6 +198,8 @@ jobs: path: python/requirements.txt test_wheels: + env: + PROTOCOL_BUFFERS_EXPECTED_PYTHON_BACKEND: upb strategy: fail-fast: false # Don't cancel all jobs if one fails. matrix: @@ -278,6 +299,8 @@ jobs: done test_pure_python_wheels: + env: + PROTOCOL_BUFFERS_EXPECTED_PYTHON_BACKEND: python name: Test Pure Python Wheels needs: build_wheels strategy: diff --git a/ci/common.bazelrc b/ci/common.bazelrc index a5f37ae1ccbaa..f51b71c819d5d 100644 --- a/ci/common.bazelrc +++ b/ci/common.bazelrc @@ -43,7 +43,6 @@ build --remote_download_outputs=all # running bazelisk with the --migrate flag and filtering out all flags that # default to true or are deprecated. build --incompatible_check_sharding_support -build --incompatible_default_to_explicit_init_py build --incompatible_disable_native_android_rules build --incompatible_disable_target_provider_fields build --incompatible_disallow_empty_glob @@ -88,4 +87,4 @@ build --verbose_failures # check. build --features=layering_check -build --enable_platform_specific_config \ No newline at end of file +build --enable_platform_specific_config diff --git a/python/BUILD.bazel b/python/BUILD.bazel index 4ccb437f3b5eb..c1abd7dedd15e 100644 --- a/python/BUILD.bazel +++ b/python/BUILD.bazel @@ -145,7 +145,6 @@ py_extension( # casts between function pointers and object pointers. "-Wno-pedantic", ], - target_compatible_with = select(_message_target_compatible_with), deps = [ "//src/google/protobuf:descriptor_upb_reflection_proto", "//third_party/utf8_range", diff --git a/python/build_targets.bzl b/python/build_targets.bzl index d203af33559c5..d003f5cab0a90 100644 --- a/python/build_targets.bzl +++ b/python/build_targets.bzl @@ -37,6 +37,7 @@ def build_targets(name): }), visibility = ["//:__pkg__"], deps = [ + ":_message", # Enables UPB ":python_srcs", ":well_known_types_py_pb2", ], @@ -330,6 +331,7 @@ def build_targets(name): srcs_version = "PY2AND3", visibility = ["//python:__subpackages__"], deps = [ + ":_message", # enables UPB ":protobuf_python", ":python_common_test_protos", ":python_specific_test_protos", @@ -468,6 +470,11 @@ def build_targets(name): srcs = ["python_version_test.py"], ) + internal_py_test( + name = "python_api_test", + srcs = ["python_api_test.py"], + ) + conformance_test( name = "conformance_test", env = {"PROTOCOL_BUFFERS_PYTHON_IMPLEMENTATION": "python"}, diff --git a/python/python_api_test.py b/python/python_api_test.py new file mode 100644 index 0000000000000..ee0b1c5b173e5 --- /dev/null +++ b/python/python_api_test.py @@ -0,0 +1,28 @@ +# Protocol Buffers - Google's data interchange format +# Copyright 2008 Google Inc. All rights reserved. +# +# Use of this source code is governed by a BSD-style +# license that can be found in the LICENSE file or at +# https://developers.google.com/open-source/licenses/bsd +"""Test that Kokoro is using the intended backend implementation of python.""" + +import os +import sys +import unittest + +from google.protobuf.internal import api_implementation + + +class PythonApiTest(unittest.TestCase): + + def testExpectedBackend(self): + """Test that a python test is using the expected backend.""" + expected_api_type = os.getenv( + 'PROTOCOL_BUFFERS_EXPECTED_PYTHON_BACKEND', 'upb' + ) # default is UPB. + + self.assertEqual(expected_api_type, api_implementation.Type()) + + +if __name__ == '__main__': + unittest.main()