New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Upgrade third_party/protobuf to v3.14.0 (for v1.35.x) #25131
Upgrade third_party/protobuf to v3.14.0 (for v1.35.x) #25131
Conversation
Lets wait for the tests to see if all problems encountered in #24855 have been fixed. |
Still seeing problems: Python artifact build on Windows
Python basic bazel tests
|
@gnossen @acozzette looks like the backport protocolbuffers/protobuf#8115 doesn't work 100%: windows python artifact build:
|
@gnossen @lidizheng how can we fix the other issue? Python basic bazel tests
|
I think this pull request might fix the WrappedMutex error. |
@gnossen @lidizheng it looks like this is the last issue that's preventing us from upgrading third_party/protobuf. |
@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. |
And it looks like it was caused by this PR. I'm still investigating to determine the best way to resolve the issue. |
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 |
273ed1f
to
849c9b0
Compare
I cherry-picked the patch from #25171. After that, all tests should be green. |
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 |
@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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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! |
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)