Upgrade third_party/protobuf to v3.14.0 (for v1.35.x) by jtattermusch · Pull Request #25131 · grpc/grpc

jtattermusch · 2021-01-11T10:18:10Z

Same as #24855, but for the already-cut v1.35.x branch (and including third_party/protobuf commit that hopefully fixes the build problem encountered in #24855)

jtattermusch · 2021-01-11T10:18:23Z

CC @stanley-cheung

jtattermusch · 2021-01-11T10:19:52Z

Lets wait for the tests to see if all problems encountered in #24855 have been fixed.

jtattermusch · 2021-01-12T15:15:02Z

Still seeing problems:

Python artifact build on Windows
https://source.cloud.google.com/results/invocations/82b67290-2e1a-4882-b3b0-bd026e6c8814/targets/github%2Fgrpc/tests;group=tasks;test=build_artifact.python_windows_x86_Python27_32bit;row=2

third_party\protobuf\src/google/protobuf/stubs/mutex.h:119:13: error: explicitly defaulted function 'constexpr google::protobuf::internal::WrappedMutex::WrappedMutex()' cannot be declared as constexpr because the implicit declaration is not constexpr:
   constexpr WrappedMutex() = default;
             ^~~~~~~~~~~~

Python basic bazel tests
https://source.cloud.google.com/results/invocations/fa5b00dd-ab16-47bd-ae0b-328bbaaef574/log

FAIL: //src/python/grpcio_tests/tests_aio/interop:local_interop_test (see /root/.cache/bazel/_bazel_root/d2dc70c3d9da3fab488ba0dcbbd35051/execroot/com_github_grpc_grpc/bazel-out/k8-fastbuild/testlogs/src/python/grpcio_tests/tests_aio/interop/local_interop_test/test.log)
INFO: From Testing //src/python/grpcio_tests/tests_aio/interop:local_interop_test:
==================== Test output for //src/python/grpcio_tests/tests_aio/interop:local_interop_test:
Traceback (most recent call last):
  File "/root/.cache/bazel/_bazel_root/d2dc70c3d9da3fab488ba0dcbbd35051/sandbox/processwrapper-sandbox/2698/execroot/com_github_grpc_grpc/bazel-out/k8-fastbuild/bin/src/python/grpcio_tests/tests_aio/interop/local_interop_test.runfiles/com_github_grpc_grpc/src/python/grpcio_tests/tests_aio/interop/local_interop_test.py", line 24, in <module>
    from tests_aio.interop import methods
  File "/root/.cache/bazel/_bazel_root/d2dc70c3d9da3fab488ba0dcbbd35051/sandbox/processwrapper-sandbox/2698/execroot/com_github_grpc_grpc/bazel-out/k8-fastbuild/bin/src/python/grpcio_tests/tests_aio/interop/local_interop_test.runfiles/com_github_grpc_grpc/src/python/grpcio_tests/tests_aio/interop/methods.py", line 29, in <module>
    from google import auth as google_auth
ImportError: cannot import name 'auth'
================================================================================
[3,626 / 3,642] 71 / 138 tests, 1 failed; Testing //src/python/grpcio_tests/tests/unit/_cython:_channel_test.python2; 13s processwrapper-sandbox ... (16 actions running)
FAIL: //src/python/grpcio_tests/tests_aio/status:grpc_status_test (see /root/.cache/bazel/_bazel_root/d2dc70c3d9da3fab488ba0dcbbd35051/execroot/com_github_grpc_grpc/bazel-out/k8-fastbuild/testlogs/src/python/grpcio_tests/tests_aio/status/grpc_status_test/test.log)
INFO: From Testing //src/python/grpcio_tests/tests_aio/status:grpc_status_test:
==================== Test output for //src/python/grpcio_tests/tests_aio/status:grpc_status_test:
Traceback (most recent call last):
  File "/root/.cache/bazel/_bazel_root/d2dc70c3d9da3fab488ba0dcbbd35051/sandbox/processwrapper-sandbox/2873/execroot/com_github_grpc_grpc/bazel-out/k8-fastbuild/bin/src/python/grpcio_tests/tests_aio/status/grpc_status_test.runfiles/com_github_grpc_grpc/src/python/grpcio_tests/tests_aio/status/grpc_status_test.py", line 22, in <module>
    from google.rpc import code_pb2, error_details_pb2, status_pb2
ModuleNotFoundError: No module named 'google.rpc'
================================================================================
[3,913 / 3,915] 136 / 138 tests, 2 failed; Testing //src/python/grpcio_tests/tests/unit/_cython:_channel_test.python3; 30s processwrapper-sandbox ... (2 actions running)

jtattermusch · 2021-01-12T15:19:27Z

@gnossen @acozzette looks like the backport protocolbuffers/protobuf#8115 doesn't work 100%:

windows python artifact build:

third_party\protobuf\src/google/protobuf/stubs/mutex.h:119:13: error: explicitly defaulted function 'constexpr google::protobuf::internal::WrappedMutex::WrappedMutex()' cannot be declared as constexpr because the implicit declaration is not constexpr:
   constexpr WrappedMutex() = default;
             ^~~~~~~~~~~~
In file included from C:/tools/msys64_win7/mingw32/include/c++/6.3.0/mutex:44:0,
                 from third_party\protobuf\src/google/protobuf/stubs/mutex.h:33,
                 from third_party\protobuf\src/google/protobuf/descriptor.h:65,
                 from third_party\protobuf\src/google/protobuf/compiler/scc.h:38,
                 from third_party\protobuf\src/google/protobuf/compiler/js/js_generator.h:41,
                 from third_party\protobuf\src\google\protobuf\compiler\js\js_generator.cc:31:
C:/tools/msys64_win7/mingw32/include/c++/6.3.0/bits/std_mutex.h:94:5: note: defaulted constructor calls non-constexpr 'std::mutex::mutex()'
     mutex() noexcept = default;
     ^~~~~

jtattermusch · 2021-01-12T15:20:30Z

@gnossen @lidizheng how can we fix the other issue?

Python basic bazel tests
https://source.cloud.google.com/results/invocations/fa5b00dd-ab16-47bd-ae0b-328bbaaef574/log

FAIL: //src/python/grpcio_tests/tests_aio/interop:local_interop_test (see /root/.cache/bazel/_bazel_root/d2dc70c3d9da3fab488ba0dcbbd35051/execroot/com_github_grpc_grpc/bazel-out/k8-fastbuild/testlogs/src/python/grpcio_tests/tests_aio/interop/local_interop_test/test.log)
INFO: From Testing //src/python/grpcio_tests/tests_aio/interop:local_interop_test:
==================== Test output for //src/python/grpcio_tests/tests_aio/interop:local_interop_test:
Traceback (most recent call last):
  File "/root/.cache/bazel/_bazel_root/d2dc70c3d9da3fab488ba0dcbbd35051/sandbox/processwrapper-sandbox/2698/execroot/com_github_grpc_grpc/bazel-out/k8-fastbuild/bin/src/python/grpcio_tests/tests_aio/interop/local_interop_test.runfiles/com_github_grpc_grpc/src/python/grpcio_tests/tests_aio/interop/local_interop_test.py", line 24, in <module>
    from tests_aio.interop import methods
  File "/root/.cache/bazel/_bazel_root/d2dc70c3d9da3fab488ba0dcbbd35051/sandbox/processwrapper-sandbox/2698/execroot/com_github_grpc_grpc/bazel-out/k8-fastbuild/bin/src/python/grpcio_tests/tests_aio/interop/local_interop_test.runfiles/com_github_grpc_grpc/src/python/grpcio_tests/tests_aio/interop/methods.py", line 29, in <module>
    from google import auth as google_auth
ImportError: cannot import name 'auth'
================================================================================
[3,626 / 3,642] 71 / 138 tests, 1 failed; Testing //src/python/grpcio_tests/tests/unit/_cython:_channel_test.python2; 13s processwrapper-sandbox ... (16 actions running)
FAIL: //src/python/grpcio_tests/tests_aio/status:grpc_status_test (see /root/.cache/bazel/_bazel_root/d2dc70c3d9da3fab488ba0dcbbd35051/execroot/com_github_grpc_grpc/bazel-out/k8-fastbuild/testlogs/src/python/grpcio_tests/tests_aio/status/grpc_status_test/test.log)
INFO: From Testing //src/python/grpcio_tests/tests_aio/status:grpc_status_test:
==================== Test output for //src/python/grpcio_tests/tests_aio/status:grpc_status_test:
Traceback (most recent call last):
  File "/root/.cache/bazel/_bazel_root/d2dc70c3d9da3fab488ba0dcbbd35051/sandbox/processwrapper-sandbox/2873/execroot/com_github_grpc_grpc/bazel-out/k8-fastbuild/bin/src/python/grpcio_tests/tests_aio/status/grpc_status_test.runfiles/com_github_grpc_grpc/src/python/grpcio_tests/tests_aio/status/grpc_status_test.py", line 22, in <module>
    from google.rpc import code_pb2, error_details_pb2, status_pb2
ModuleNotFoundError: No module named 'google.rpc'
================================================================================
[3,913 / 3,915] 136 / 138 tests, 2 failed; Testing //src/python/grpcio_tests/tests/unit/_cython:_channel_test.python3; 30s processwrapper-sandbox ... (2 actions running)

acozzette · 2021-01-12T22:38:18Z

I think this pull request might fix the WrappedMutex error.

jtattermusch · 2021-01-14T11:34:43Z

@gnossen @lidizheng how can we fix the other issue?

Python basic bazel tests
https://source.cloud.google.com/results/invocations/fa5b00dd-ab16-47bd-ae0b-328bbaaef574/log

FAIL: //src/python/grpcio_tests/tests_aio/interop:local_interop_test (see /root/.cache/bazel/_bazel_root/d2dc70c3d9da3fab488ba0dcbbd35051/execroot/com_github_grpc_grpc/bazel-out/k8-fastbuild/testlogs/src/python/grpcio_tests/tests_aio/interop/local_interop_test/test.log)
INFO: From Testing //src/python/grpcio_tests/tests_aio/interop:local_interop_test:
==================== Test output for //src/python/grpcio_tests/tests_aio/interop:local_interop_test:
Traceback (most recent call last):
  File "/root/.cache/bazel/_bazel_root/d2dc70c3d9da3fab488ba0dcbbd35051/sandbox/processwrapper-sandbox/2698/execroot/com_github_grpc_grpc/bazel-out/k8-fastbuild/bin/src/python/grpcio_tests/tests_aio/interop/local_interop_test.runfiles/com_github_grpc_grpc/src/python/grpcio_tests/tests_aio/interop/local_interop_test.py", line 24, in <module>
    from tests_aio.interop import methods
  File "/root/.cache/bazel/_bazel_root/d2dc70c3d9da3fab488ba0dcbbd35051/sandbox/processwrapper-sandbox/2698/execroot/com_github_grpc_grpc/bazel-out/k8-fastbuild/bin/src/python/grpcio_tests/tests_aio/interop/local_interop_test.runfiles/com_github_grpc_grpc/src/python/grpcio_tests/tests_aio/interop/methods.py", line 29, in <module>
    from google import auth as google_auth
ImportError: cannot import name 'auth'
================================================================================
[3,626 / 3,642] 71 / 138 tests, 1 failed; Testing //src/python/grpcio_tests/tests/unit/_cython:_channel_test.python2; 13s processwrapper-sandbox ... (16 actions running)
FAIL: //src/python/grpcio_tests/tests_aio/status:grpc_status_test (see /root/.cache/bazel/_bazel_root/d2dc70c3d9da3fab488ba0dcbbd35051/execroot/com_github_grpc_grpc/bazel-out/k8-fastbuild/testlogs/src/python/grpcio_tests/tests_aio/status/grpc_status_test/test.log)
INFO: From Testing //src/python/grpcio_tests/tests_aio/status:grpc_status_test:
==================== Test output for //src/python/grpcio_tests/tests_aio/status:grpc_status_test:
Traceback (most recent call last):
  File "/root/.cache/bazel/_bazel_root/d2dc70c3d9da3fab488ba0dcbbd35051/sandbox/processwrapper-sandbox/2873/execroot/com_github_grpc_grpc/bazel-out/k8-fastbuild/bin/src/python/grpcio_tests/tests_aio/status/grpc_status_test.runfiles/com_github_grpc_grpc/src/python/grpcio_tests/tests_aio/status/grpc_status_test.py", line 22, in <module>
    from google.rpc import code_pb2, error_details_pb2, status_pb2
ModuleNotFoundError: No module named 'google.rpc'
================================================================================
[3,913 / 3,915] 136 / 138 tests, 2 failed; Testing //src/python/grpcio_tests/tests/unit/_cython:_channel_test.python3; 30s processwrapper-sandbox ... (2 actions running)

@gnossen @lidizheng it looks like this is the last issue that's preventing us from upgrading third_party/protobuf.
I tried to fix it by upgrading rules_python to 0.1.0 but that seems to lead to other test failures (see #25143). Can you take a look and suggest how to fix this issue (a hotfix is fine)?
Since we're trying to backport this into 1.35.x branch, we don't want to any major changes, so I'd prefer to fix this in some simple way and then perform the rules_python upgrade properly on the master branch.

lidizheng · 2021-01-14T22:54:17Z

@jtattermusch Sorry, I missed the last notification. Richard and I discussed this problem, and it can be resolved by an existing workaround in our repo: https://github.com/grpc/grpc/blob/master/src/python/grpcio_tests/tests/bazel_namespace_package_hack.py.

gnossen · 2021-01-14T23:02:21Z

And it looks like it was caused by this PR. I'm still investigating to determine the best way to resolve the issue.

gnossen · 2021-01-14T23:33:09Z

So it turns out that the Envoy project dealt with this problem before us. The Protobuf team has intentionally removed their namespace package declarations from the __init__.py file, instead declaring them in their setup.py file, which we do not consume at all. It seems that this solution is only necessary until all other packages using the google namespace package (google-auth) do the same. I've implemented that solution in our repo here.

jtattermusch · 2021-01-16T13:13:08Z

I cherry-picked the patch from #25171. After that, all tests should be green.

jtattermusch · 2021-01-18T08:04:16Z

The interop toprod tests are also failing on master: https://source.cloud.google.com/results/invocations/09ff4cb1-e4af-4841-a86e-3e48b2c99952/targets/github%2Fgrpc%2Finterop_test/tests

jtattermusch · 2021-01-18T08:07:44Z

@gnossen @veblush @stanley-cheung I checked all the test results and they are looking good, and this PR is now ready to merge. Please review so I can merge.

After this is merged, I'll create a PR to upport this change to the master branch.

stanley-cheung

LGTM. I was also following this and came to the same conclusion. Tests are looking good. Thanks for taking care of this!

jtattermusch · 2021-01-18T08:10:53Z

LGTM. I was also following this and came to the same conclusion. Tests are looking good. Thanks for taking care of this!

That was quick! Happy Martin Luther King day!

jtattermusch added 7 commits January 11, 2021 09:58

update third_party/protobuf to v3.14.0

8bb5593

regenerate proto files for csharp, php, ruby

9c8818d

run tools/distrib/python/make_grpcio_tools.py

6c99a03

upgrade ruby, C# python and ObC to protobuf 3.14

9bb2aef

run tools/codegen/core/gen_upb_api.sh

c26961b

include a bugfix in third_party/protobuf

7fc2828

re-run tools/distrib/python/make_grpcio_tools.py

3fcf662

jtattermusch requested review from gnossen, nicolasnoble and veblush as code owners January 11, 2021 10:18

jtattermusch added release notes: yes Indicates if PR needs to be in release notes release notes: no Indicates if PR should not be in release notes and removed release notes: yes Indicates if PR needs to be in release notes labels Jan 11, 2021

jtattermusch assigned stanley-cheung Jan 11, 2021

jtattermusch added the priority/P0/RELEASE BLOCKER label Jan 11, 2021

jtattermusch mentioned this pull request Jan 11, 2021

Upgrade third_party/protobuf to v3.14.0 #24855

Closed

This was referenced Jan 13, 2021

Update bazel io_bazel_rules_python to 0.1.0 #25143

Closed

stubs/mutex: Fix compilation by initializing variable in WrappedMutex class (backport to 3.14.x) protocolbuffers/protobuf#8205

Merged

jtattermusch mentioned this pull request Jan 15, 2021

DO NOT MERGE #25171

Closed

include a bugfix in third_party/protobuf

8d8dda4

jtattermusch and others added 3 commits January 16, 2021 13:12

re-run tools/distrib/python/make_grpcio_tools.py

ded080e

Add patch to protobuf

666e603

Make patch work for downstream dependencies

849c9b0

jtattermusch force-pushed the protobuf_3_14_upgrade_for_1_35 branch from 273ed1f to 849c9b0 Compare January 16, 2021 12:14

stanley-cheung approved these changes Jan 18, 2021

View reviewed changes

jtattermusch merged commit 5a00d68 into grpc:v1.35.x Jan 18, 2021

This was referenced Jan 18, 2021

Try upgrading third_party/protobuf version to 3.14.0-rc2 #24723

Closed

Merge changes from v1.35.x branch into master #25183

Merged

vam-google mentioned this pull request Jan 26, 2021

Remove uses of pkg_resources in non-namespace packages. protocolbuffers/protobuf#7902

Merged

jtattermusch mentioned this pull request Feb 24, 2021

Use Google.Protobuf protoc 3.12 in Grpc.Tools and expose --experimental_allow_proto3_optional switch #22975

Closed

Conversation

jtattermusch commented Jan 11, 2021

Uh oh!

jtattermusch commented Jan 11, 2021

Uh oh!

jtattermusch commented Jan 11, 2021

Uh oh!

jtattermusch commented Jan 12, 2021

Uh oh!

jtattermusch commented Jan 12, 2021

Uh oh!

jtattermusch commented Jan 12, 2021

Uh oh!

acozzette commented Jan 12, 2021

Uh oh!

jtattermusch commented Jan 14, 2021

Uh oh!

lidizheng commented Jan 14, 2021

Uh oh!

gnossen commented Jan 14, 2021

Uh oh!

gnossen commented Jan 14, 2021

Uh oh!

jtattermusch commented Jan 16, 2021

Uh oh!

jtattermusch commented Jan 18, 2021

Uh oh!

jtattermusch commented Jan 18, 2021

Uh oh!

stanley-cheung left a comment

Choose a reason for hiding this comment

Uh oh!

jtattermusch commented Jan 18, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

jtattermusch commented Jan 18, 2021 •

edited

Loading