Skip to content

Commit

Permalink
Link in UPB by default for Bazel's python build rules.
Browse files Browse the repository at this point in the history
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
  • Loading branch information
tonyliaoss authored and copybara-github committed Jan 10, 2025
1 parent 5b0fa1d commit 50c1915
Show file tree
Hide file tree
Showing 7 changed files with 94 additions and 19 deletions.
6 changes: 5 additions & 1 deletion .github/workflows/test_cpp.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
35 changes: 25 additions & 10 deletions .github/workflows/test_python.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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

Expand Down
33 changes: 28 additions & 5 deletions .github/workflows/test_upb.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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:
Expand All @@ -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

Expand Down Expand Up @@ -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:
Expand All @@ -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
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand Down
3 changes: 1 addition & 2 deletions ci/common.bazelrc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -88,4 +87,4 @@ build --verbose_failures
# check.
build --features=layering_check

build --enable_platform_specific_config
build --enable_platform_specific_config
1 change: 0 additions & 1 deletion python/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
7 changes: 7 additions & 0 deletions python/build_targets.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ def build_targets(name):
}),
visibility = ["//:__pkg__"],
deps = [
":_message", # Enables UPB
":python_srcs",
":well_known_types_py_pb2",
],
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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"},
Expand Down
28 changes: 28 additions & 0 deletions python/python_api_test.py
Original file line number Diff line number Diff line change
@@ -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()

0 comments on commit 50c1915

Please sign in to comment.