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
Update grpc protoc plugin to be compliant of proto3 field presence #22998
Update grpc protoc plugin to be compliant of proto3 field presence #22998
Conversation
fdf64cb
to
312e70c
Compare
There are some Objective-C failures, that may be due to a new change in Protobuf in 3.12 protocolbuffers/protobuf#7173 The error is
Logged an issue to protobuf: protocolbuffers/protobuf#7532 to see if it's an issue with their [Edit:] Fixed by protocolbuffers/protobuf#7538 |
There seems to be an issue with Python27 + Windows build after the upgrade to Protobuf 3.12 too
Seems to be caused by this PR: protocolbuffers/protobuf#7344, and originally protocolbuffers/protobuf#6535 Logged a separate issue #23011 to track this issue. Current idea is that [Edit:] Fixed by protocolbuffers/protobuf#7539 |
@srini100 note that Stanley is already updating protobuf submodule as part of this PR. |
91eb8de
to
72d9335
Compare
This is looking better. Both the Objective-C and the Python issues have found a fix. We just need protobuf team to merge in my fixes and tag a new release. Will keep monitoring that. |
0dc7df9
to
b8fd8c7
Compare
All the issues are resolved as of this point, after protobuf team merged in my two fixes. All the tests are passing except for this known flake #23027. I will wait for the protobuf team to cut a new release |
5ca45ff
to
a2cba00
Compare
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
a2cba00
to
f70ebf8
Compare
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.
Unfortunately, this PR breaks some build targets internally - https://g3c.corp.google.com/results/invocations/c787c170-933c-4d84-88db-908739d593c1/targets/%2F%2Fgooglex%2Fdaedalus%2Fembedded%2Fbase%2Fnet%2Fngrpc2%2Fclient:channel_test/log I will create a revert PR. |
Fixes #23030
Proto3 introduced the
optional
syntax to support field presence tracking in release 3.12.0. We need to add support in our grpc protoc plugin.See this doc for more details. Basically we need to add a function
GetSupportedFeatures()
to our grpc protoc plugins to mark our compliance.This PR also has to update the
third_party/protobuf
link to thev3.12.2
commit.Internal bug: b/156243646
A couple of recent issues to keep an eye on in terms of interactions with this PR:
Current blockers:
3.12.2
release.