Update third_party/protobuf to v3.11.2 #21590

jtattermusch · 2020-01-06T12:08:54Z

We're currently at protobuf 3.8 (didn't manage to upgrade to protobuf 3.9 and 3.10 due to some issues).

jtattermusch · 2020-01-06T16:04:38Z

Python Bazel Build
https://source.cloud.google.com/results/invocations/da1975c4-f708-49f1-b090-5e86b8c4ccfd/log

DEBUG: /root/.cache/bazel/_bazel_root/d2dc70c3d9da3fab488ba0dcbbd35051/external/bazel_toolchains/rules/rbe_repo/checked_in.bzl:226:13: rbe_msan not using checked in configs; Bazel version 1.0.0 was picked/selected with '["9.0.0", "10.0.0"]' compatible configs but none match the 'env = {"ABI_LIBC_VERSION": "glibc_2.19", "ABI_VERSION": "clang", "BAZEL_COMPILER": "clang", "BAZEL_HOST_SYSTEM": "i686-unknown-linux-gnu", "BAZEL_TARGET_CPU": "k8", "BAZEL_TARGET_LIBC": "glibc_2.19", "BAZEL_TARGET_SYSTEM": "x86_64-unknown-linux-gnu", "CC": "clang", "CC_TOOLCHAIN_NAME": "linux_gnu_x86", "BAZEL_LINKOPTS": "-lc++:-lc++abi:-lm"}', 'config_repos = None',and/or 'create_cc_configs = True' passed as attrs
Analyzing: 214 targets (36 packages loaded, 0 targets configured)
INFO: Call stack for the definition of repository 'go_sdk' which is a _go_download_sdk (rule definition at /root/.cache/bazel/_bazel_root/d2dc70c3d9da3fab488ba0dcbbd35051/external/io_bazel_rules_go/go/private/sdk.bzl:53:20):
 - /root/.cache/bazel/_bazel_root/d2dc70c3d9da3fab488ba0dcbbd35051/external/io_bazel_rules_go/go/private/sdk.bzl:65:5
 - /root/.cache/bazel/_bazel_root/d2dc70c3d9da3fab488ba0dcbbd35051/external/io_bazel_rules_go/go/toolchain/toolchains.bzl:379:13
 - /var/local/git/grpc/bazel/grpc_extra_deps.bzl:36:5
 - /var/local/git/grpc/WORKSPACE:11:1
ERROR: /root/.cache/bazel/_bazel_root/d2dc70c3d9da3fab488ba0dcbbd35051/external/com_google_protobuf/BUILD:874:1: no such package '@six//': The repository '@six' could not be resolved and referenced by '@com_google_protobuf//:protobuf_python'
ERROR: Analysis of target '//src/python/grpcio_reflection/grpc_reflection/v1alpha:reflection_py_pb2' failed; build aborted: no such package '@six//': The repository '@six' could not be resolved

jtattermusch · 2020-01-06T16:09:48Z

A bunch of Macos bazel tests terminating with an odd, likely protobuf related error:

https://source.cloud.google.com/results/invocations/9a21fcab-5195-4f6d-88e4-1780fe99d5a0/targets

[libprotobuf ERROR external/com_google_protobuf/src/google/protobuf/descriptor_database.cc:120] File already exists in database: google/protobuf/any.proto
[libprotobuf FATAL external/com_google_protobuf/src/google/protobuf/descriptor.cc:1356] CHECK failed: GeneratedDatabase()->Add(encoded_file_descriptor, size):
libc++abi.dylib: terminating with uncaught exception of type google::protobuf::FatalException: CHECK failed: GeneratedDatabase()->Add(encoded_file_descriptor, size):

jtattermusch · 2020-01-06T16:23:07Z

ASAN problems:
https://source.cloud.google.com/results/invocations/cfad3c7d-e44b-4853-974a-3b2e8cbb649a/targets

+ export GRPC_POLL_STRATEGY=epoll1
+ shift
+ test/core/channel/channelz_test
=================================================================
==18==ERROR: AddressSanitizer: odr-violation (0x7f3ef8ff53a0):
  [1] size=32 'google::protobuf::_DoubleValue_default_instance_' bazel-out/k8-fastbuild/bin/external/com_google_protobuf/_virtual_imports/wrappers_proto/google/protobuf/wrappers.pb.cc:21:3
  [2] size=32 'google::protobuf::_DoubleValue_default_instance_' external/com_google_protobuf/src/google/protobuf/wrappers.pb.cc:21:3
These globals were registered at these points:
  [1]:
    #0 0x45f0dd in __asan_register_globals /tmp/clang-build/src/compiler-rt/lib/asan/asan_globals.cpp:362:3
    #1 0x7f3ef8fdb0db in asan.module_ctor (/b/f/w/bazel-out/k8-fastbuild/bin/test/core/channel/channelz_test@poller=epoll1.runfiles/com_github_grpc_grpc/test/core/channel/../../../_solib_k8/libexternal_Scom_Ugoogle_Uprotobuf_Slibwrappers_Uproto.so+0x2b0db)

  [2]:
    #0 0x45f0dd in __asan_register_globals /tmp/clang-build/src/compiler-rt/lib/asan/asan_globals.cpp:362:3
    #1 0x7f3ef8dff48b in asan.module_ctor (/b/f/w/bazel-out/k8-fastbuild/bin/test/core/channel/channelz_test@poller=epoll1.runfiles/com_github_grpc_grpc/test/core/channel/../../../_solib_k8/libexternal_Scom_Ugoogle_Uprotobuf_Slibprotobuf.so+0x9af48b)

==18==HINT: if you don't care about these errors you may set ASAN_OPTIONS=detect_odr_violation=0
SUMMARY: AddressSanitizer: odr-violation: global 'google::protobuf::_DoubleValue_default_instance_' at bazel-out/k8-fastbuild/bin/external/com_google_protobuf/_virtual_imports/wrappers_proto/google/protobuf/wrappers.pb.cc:21:3

jtattermusch · 2020-01-06T16:24:48Z

@rafi-kamal looks like there's several problems upgrading gRPC to protobuf 3.11.2 - can you look at them and help us resolve?

jtattermusch · 2020-01-06T17:54:36Z

ASAN problems:
https://source.cloud.google.com/results/invocations/cfad3c7d-e44b-4853-974a-3b2e8cbb649a/targets

+ export GRPC_POLL_STRATEGY=epoll1
+ shift
+ test/core/channel/channelz_test
=================================================================
==18==ERROR: AddressSanitizer: odr-violation (0x7f3ef8ff53a0):
  [1] size=32 'google::protobuf::_DoubleValue_default_instance_' bazel-out/k8-fastbuild/bin/external/com_google_protobuf/_virtual_imports/wrappers_proto/google/protobuf/wrappers.pb.cc:21:3
  [2] size=32 'google::protobuf::_DoubleValue_default_instance_' external/com_google_protobuf/src/google/protobuf/wrappers.pb.cc:21:3
These globals were registered at these points:
  [1]:
    #0 0x45f0dd in __asan_register_globals /tmp/clang-build/src/compiler-rt/lib/asan/asan_globals.cpp:362:3
    #1 0x7f3ef8fdb0db in asan.module_ctor (/b/f/w/bazel-out/k8-fastbuild/bin/test/core/channel/channelz_test@poller=epoll1.runfiles/com_github_grpc_grpc/test/core/channel/../../../_solib_k8/libexternal_Scom_Ugoogle_Uprotobuf_Slibwrappers_Uproto.so+0x2b0db)

  [2]:
    #0 0x45f0dd in __asan_register_globals /tmp/clang-build/src/compiler-rt/lib/asan/asan_globals.cpp:362:3
    #1 0x7f3ef8dff48b in asan.module_ctor (/b/f/w/bazel-out/k8-fastbuild/bin/test/core/channel/channelz_test@poller=epoll1.runfiles/com_github_grpc_grpc/test/core/channel/../../../_solib_k8/libexternal_Scom_Ugoogle_Uprotobuf_Slibprotobuf.so+0x9af48b)

==18==HINT: if you don't care about these errors you may set ASAN_OPTIONS=detect_odr_violation=0
SUMMARY: AddressSanitizer: odr-violation: global 'google::protobuf::_DoubleValue_default_instance_' at bazel-out/k8-fastbuild/bin/external/com_google_protobuf/_virtual_imports/wrappers_proto/google/protobuf/wrappers.pb.cc:21:3

Oh, looks like this is due to #20224 (comment)
@rafi-kamal what is the next step here to fix?

gnossen · 2020-01-06T18:07:57Z

The failure in the Python build is addressed by #20783. We were hesitant to pull it in because it constitutes a breaking change, but I think at this point, we'd better.

rafi-kamal · 2020-01-06T18:18:51Z

There are two potential solutions mentioned in #20224 (comment), but I'm not familiar enough with Protobuf or gRPC C++ to determine which one is better or what exactly the next steps would be in either of these two solutions. @veblush would be able to help here?

I'm happy to help if any changes are needed from the Protobuf side.

jtattermusch · 2020-01-07T15:32:22Z

ObjC regression
https://source.cloud.google.com/results/invocations/b889017f-279c-43ff-9d12-317f0ec660fe/targets/github%2Fgrpc%2Frun_tests%2Fobjc_macos_opt_native%2Fios-cpp-test-cronet/tests

/Volumes/BuildData/tmpfs/src/github/grpc/workspace_objc_macos_opt_native/test/cpp/ios/Pods/Headers/Private/RemoteTestCpp/src/proto/grpc/testing/echo_messages.pb.h:576:111: error: only virtual member functions can be marked 'final'
Tue Jan  7 01:55:51 PST 2020 -       ::PROTOBUF_NAMESPACE_ID::uint8* target, ::PROTOBUF_NAMESPACE_ID::io::EpsCopyOutputStream* stream) const final;
Tue Jan  7 01:55:51 PST 2020 -                                                                                                               ^~~~~
Tue Jan  7 01:55:51 PST 2020 - /Volumes/BuildData/tmpfs/src/github/grpc/workspace_objc_macos_opt_native/test/cpp/ios/Pods/Headers/Private/RemoteTestCpp/src/proto/grpc/testing/echo_messages.pb.h:548:42: error: no member named 'GenericSwap' in namespace 'google::protobuf::internal'
Tue Jan  7 01:55:51 PST 2020 -       ::PROTOBUF_NAMESPACE_ID::internal::GenericSwap(this, other);
Tue Jan  7 01:55:51 PST 2020 -       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^
Tue Jan  7 01:55:51 PST 2020 - /Volumes/BuildData/tmpfs/src/github/grpc/workspace_objc_macos_opt_native/test/cpp/ios/Pods/Headers/Private/RemoteTestCpp/src/proto/grpc/testing/echo_messages.pb.h:978:101: error: only virtual member functions can be marked 'final'
Tue Jan  7 01:55:51 PST 2020 -   const char* _InternalParse(const char* ptr, ::PROTOBUF_NAMESPACE_ID::internal::ParseContext* ctx) final;
Tue Jan  7 01:55:51 PST 2020 -                                                                                                     ^~~~~
Tue Jan  7 01:55:51 PST 2020 - fatal error: too many errors emitted, stopping now [-ferror-limit=]
Tue Jan  7 01:55:51 PST 2020 -

See #20224 (comment) for potential root cause

@muxi any ideas?

muxi · 2020-01-07T16:43:16Z

I see Protobuf-C++ 3.8.0 got installed.

muxi · 2020-01-07T16:48:58Z

The file is here. I'm not sure why it is maintained in gRPC repo though (since I remember discussion with teboring on integrating its release in Protobuf's release process).

jtattermusch · 2020-01-07T16:56:17Z

The file is here. I'm not sure why it is maintained in gRPC repo though (since I remember discussion with teboring on integrating its release in Protobuf's release process).

This is the history of that file:
https://github.com/grpc/grpc/commits/master/src/cpp/Protobuf-C%2B%2B.podspec
(and it also looks that is only affects ios-cpp-test-cronet tests as other objC test are passing?)

Btw, I found:
https://github.com/protocolbuffers/protobuf/blob/3.11.x/Protobuf-C++.podspec

looks like we should get rid of our copy of Protobuf-C++.podspec?

muxi · 2020-01-07T17:01:22Z

If Protobuf has a copy of this file it's probably right to get rid of our copy. Let me check why our copy exist first.

rafi-kamal · 2020-01-07T17:03:07Z

The protobuf version of the podspec doesn't work. Previous internal discussion: https://groups.google.com/a/google.com/g/protobuf-team/c/7kqwEta6q_A/m/8doAs-OlCwAJ

jtattermusch · 2020-01-07T17:03:39Z

@gnossen, looks like well_known_protos conflict problem described here #21590 (comment) is related to a PR that you reviewed in the past
https://github.com/grpc/grpc/pull/18955/files. Can you please take a look and suggest a fix?

jtattermusch · 2020-01-07T17:08:52Z

If Protobuf has a copy of this file it's probably right to get rid of our copy. Let me check why our copy exist first.

Thanks! Btw, Protobuf-C++.podspec being in our repo under src/cpp (why there?) makes no sense to me at all. We should get rid of it.

jtattermusch · 2020-01-07T17:12:22Z

@muxi

grpc/test/cpp/ios/Podfile

Line 15 in b07d7f1

    
           pod 'Protobuf-C++', :podspec => "#{GRPC_LOCAL_SRC}/src/cpp", :inhibit_warnings => true

muxi · 2020-01-07T17:12:56Z

@rafi-kamal - Sorry I did not notice that thread in Sep. Those two files are exactly the same. It's probably an error that happens during Cocoapods lint but not with gRPC test. I'm taking a look.

jtattermusch · 2020-01-07T17:18:55Z

@muxi @rafi-kamal for now I'll try to go with updating our local copy of Protobuf-C++.podspec (406cb5d) to see if that helps fix the failing test

muxi · 2020-01-07T17:50:40Z

@rafi-kamal - I found the reason for the error. Short version: pushing the podspec with additional flags --use-libraries --allow-warnings should make the linter pass.

Protobuf C++ code are using the include style #include <google/protobuf/...>. The style is not compatible with Apple's Frameworks include style, so the podspec cannot be used as frameworks and thus cannot be lint that way either. --use-libraries lints the library as static library.

rafi-kamal · 2020-01-15T01:38:04Z

Nice! I'm happy to merge protocolbuffers/protobuf#7096 when it's ready.

jtattermusch · 2020-01-15T10:00:51Z

I also tested with protocolbuffers/protobuf#7096 locally and it seems to work. As another confirmation, I added a "DO NOT MERGE" commit to this PR that tests protobuf with @Yannic 's changes included a16e793 - let's see what the tests say.

@Yannic any idea when will you be able to finalize the PR? We are in bit of a hurry because we want to cut the release branch for gRPC 1.27.x (with upgraded protobuf) as soon as possible.

Yannic · 2020-01-15T10:06:34Z

The Protobuf PRs are ready now (I only submitted it as a draft so it doesn't get accidentally merged).

jtattermusch · 2020-01-15T12:51:56Z

I also tested with protocolbuffers/protobuf#7096 locally and it seems to work. As another confirmation, I added a "DO NOT MERGE" commit to this PR that tests protobuf with @Yannic 's changes included a16e793 - let's see what the tests say.

Test failures seem unrelated so I'd say let's proceed with merging protocolbuffers/protobuf#7096

rafi-kamal · 2020-01-15T18:24:41Z

Merged protocolbuffers/protobuf#7096.

jtattermusch · 2020-01-16T07:49:04Z

I updated the PR to include changes from protocolbuffers/protobuf#7096 (that have just been merged to protobuf's 3.11.x). I only upgraded the grpc_deps.bzl ((see bf1b849)), which should be enough to fix our bazel problems, the third_party/protobuf stays exactly at the 3.11.2 release

Adhoc package/distribtest build: https://sponge.corp.google.com/invocation?id=6b284726-c9c9-4aa3-8e0f-ee5f19664c1a&searchFor=

Once the tests finish, this PR will be ready to merge. Thanks everyone for the help!

jtattermusch · 2020-01-16T17:20:33Z

Known failures:
#21627
#21674

Two failures I'm puzzled about is the the MacOS artifact build:
https://source.cloud.google.com/results/invocations/0456a690-de68-46ee-8baf-e74ade7d7df1/targets/github%2Fgrpc/tests

and the android build
https://source.cloud.google.com/results/invocations/029bcc7f-0b34-467f-a43b-545d7819a1df/log

AFAIR the two failures weren't there on the last build or this PR, which is strange because the only change in bf1b849 is upgrading the bazel protobuf dependency (which should have nothing to do with those tests).

Edit:

it seems that the MacOS artifact build has only timed out so it could have been a random flake - I have restarted it.
For android build, I found a bunch of similar flakes on master, so perhaps it's also just a random flake - I have restarted the build as well.

jtattermusch · 2020-01-16T17:42:15Z

Another run of adhoc artifact/package/distribtest build: https://sponge.corp.google.com/invocation?tab=Build+Log&id=43bd076b-7395-4fe5-8a56-74d6177a8fba&searchFor=

jtattermusch added lang/all wrapped languages release notes: yes Indicates if PR needs to be in release notes labels Jan 6, 2020

jtattermusch requested review from gnossen, nicolasnoble and veblush as code owners January 6, 2020 12:08

This was referenced Jan 6, 2020

Update Google.Protobuf version in Unity package #21571

Closed

Upgrade third_party/protobuf to v3.11.2 #21562

Closed

jtattermusch force-pushed the protobuf_3_11_2 branch from c1205f2 to 286d56c Compare January 6, 2020 12:27

jtattermusch force-pushed the protobuf_3_11_2 branch from 03f1e33 to c221ff6 Compare January 6, 2020 17:48

jtattermusch mentioned this pull request Jan 6, 2020

Update Protobuf Version to 3.10.0-RC1 #20224

Closed

gnossen mentioned this pull request Jan 7, 2020

Conform to Protobuf's Usage of six Dependency under Bazel #21596

Closed

jtattermusch and others added 12 commits January 15, 2020 04:42

update third_party/protobuf to v3.11.2

b4ed0bf

tools/run_tests/sanity/check_submodules.sh

a6cda9a

regenerate C# protos

433b93e

regenerate ruby protobufs

d536637

run make_grpcio_tools.py

d684931

update ruby protobuf dependency

b3acd7d

update C# protobuf dependency

bfdeafa

update ObjC podspecs to 3.11.2

b67a7ef

Conform to protobuf's usage of six

69f9906

buildifier format

27cdec0

hotfix: update Protobuf-C++.podspec to 3.11.2

6393e17

try incorporate protocolbuffers/protobuf#7069

1f465f5

jtattermusch force-pushed the protobuf_3_11_2 branch from 8d916ab to a16e793 Compare January 15, 2020 09:55

Yannic mentioned this pull request Jan 15, 2020

[bazel] Fix blacklisted_protos in cc_toolchain and add test protocolbuffers/protobuf#7075

Merged

jtattermusch mentioned this pull request Jan 15, 2020

[bazel] Fix blacklisted_protos in cc_toolchain and add test protocolbuffers/protobuf#7096

Merged

incorporate protocolbuffers/protobuf#7096

bf1b849

jtattermusch force-pushed the protobuf_3_11_2 branch from a16e793 to bf1b849 Compare January 16, 2020 07:44

nicolasnoble approved these changes Jan 16, 2020

View reviewed changes

apolcyn merged commit 232fa12 into grpc:master Jan 16, 2020

This was referenced Jan 17, 2020

Fix C# distribtests #21695

Merged

Update embedded copy of protoc to 3.10 in grpc-tools rubygem #21226

Closed

lock bot locked as resolved and limited conversation to collaborators Apr 17, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update third_party/protobuf to v3.11.2 #21590

Update third_party/protobuf to v3.11.2 #21590

jtattermusch commented Jan 6, 2020 •

edited

jtattermusch commented Jan 6, 2020

jtattermusch commented Jan 6, 2020

jtattermusch commented Jan 6, 2020

jtattermusch commented Jan 6, 2020

jtattermusch commented Jan 6, 2020

gnossen commented Jan 6, 2020

rafi-kamal commented Jan 6, 2020 •

edited

jtattermusch commented Jan 7, 2020 •

edited

muxi commented Jan 7, 2020

muxi commented Jan 7, 2020

jtattermusch commented Jan 7, 2020 •

edited

muxi commented Jan 7, 2020

rafi-kamal commented Jan 7, 2020

jtattermusch commented Jan 7, 2020

jtattermusch commented Jan 7, 2020

jtattermusch commented Jan 7, 2020

muxi commented Jan 7, 2020

jtattermusch commented Jan 7, 2020

muxi commented Jan 7, 2020

rafi-kamal commented Jan 15, 2020

jtattermusch commented Jan 15, 2020

Yannic commented Jan 15, 2020 •

edited

jtattermusch commented Jan 15, 2020 •

edited

rafi-kamal commented Jan 15, 2020

jtattermusch commented Jan 16, 2020 •

edited

jtattermusch commented Jan 16, 2020 •

edited

jtattermusch commented Jan 16, 2020

Update third_party/protobuf to v3.11.2 #21590

Update third_party/protobuf to v3.11.2 #21590

Conversation

jtattermusch commented Jan 6, 2020 • edited

jtattermusch commented Jan 6, 2020

jtattermusch commented Jan 6, 2020

jtattermusch commented Jan 6, 2020

jtattermusch commented Jan 6, 2020

jtattermusch commented Jan 6, 2020

gnossen commented Jan 6, 2020

rafi-kamal commented Jan 6, 2020 • edited

jtattermusch commented Jan 7, 2020 • edited

muxi commented Jan 7, 2020

muxi commented Jan 7, 2020

jtattermusch commented Jan 7, 2020 • edited

muxi commented Jan 7, 2020

rafi-kamal commented Jan 7, 2020

jtattermusch commented Jan 7, 2020

jtattermusch commented Jan 7, 2020

jtattermusch commented Jan 7, 2020

muxi commented Jan 7, 2020

jtattermusch commented Jan 7, 2020

muxi commented Jan 7, 2020

rafi-kamal commented Jan 15, 2020

jtattermusch commented Jan 15, 2020

Yannic commented Jan 15, 2020 • edited

jtattermusch commented Jan 15, 2020 • edited

rafi-kamal commented Jan 15, 2020

jtattermusch commented Jan 16, 2020 • edited

jtattermusch commented Jan 16, 2020 • edited

jtattermusch commented Jan 16, 2020

jtattermusch commented Jan 6, 2020 •

edited

rafi-kamal commented Jan 6, 2020 •

edited

jtattermusch commented Jan 7, 2020 •

edited

jtattermusch commented Jan 7, 2020 •

edited

Yannic commented Jan 15, 2020 •

edited

jtattermusch commented Jan 15, 2020 •

edited

jtattermusch commented Jan 16, 2020 •

edited

jtattermusch commented Jan 16, 2020 •

edited