Upgrade grpc dependency.

[bogdandrutu]

Signed-off-by: Bogdan Drutu bogdandrutu@gmail.com

[codecov]

Codecov Report

Merging #459 (59446d0) into main (3dfebc7) will increase coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main     #459      +/-   ##
==========================================
+ Coverage   94.39%   94.40%   +0.01%     
==========================================
  Files         200      200              
  Lines        9068     9068              
==========================================
+ Hits         8560     8561       +1     
+ Misses        508      507       -1     

Impacted Files	Coverage Δ
sdk/test/common/circular_buffer_test.cc	100.00% <0.00%> (+1.04%)	⬆️

[g-easy]

Is this new in grpc 1.34? It changes how protoc is pulled in?

(looks like the new grpc release also breaks the gcc-4.8+bazel build)

[bogdandrutu]

Is this new in grpc 1.34? It changes how protoc is pulled in?

I think you added the specific proto dep to work around the bug in the description, but proto is a transitive deps from gRPC, and not sure why we need to redeclare since there is no cc_library that has any deps to the proto library. Am I missing something?

[ThomsonTan]

Is this new in grpc 1.34? It changes how protoc is pulled in?

I think you added the specific proto dep to work around the bug in the description, but proto is a transitive deps from gRPC, and not sure why we need to redeclare since there is no cc_library that has any deps to the proto library. Am I missing something?

I think it is fine to remove protobuf dependency. gRPC dependence was introduced recently which supersedes the protobuf.

[maxgolov]

I think it is fine to remove protobuf dependency.

For CMake we still require protobuf, since its package (both Debian package and vcpkg package) includes the protoc .. We also require at least protobuf3 .. Actually for older Ubuntu-14.xx that comes with protobuf2 we deploy protoc by downloading it from https://github.com/protocolbuffers/protobuf/releases/download/v3.14.0/protoc-3.14.0-linux-x86_64.zip

While unrelated to Bazel PR, I am not sure if (how) we can safely eliminate dependency on protobuf from CMake build at the moment. Can't comment on Bazel specifics.

[jsuereth]

FYI - grpc/grpc#24953

I tried to bump gRPC locally and ran into a bazel issue pulling it in on windows. I don't think this is related to the legacy-bazel CI breakage but it'd be nice to verify if downstream windows can consume this version of gRPC or if we need to open a bug ourselves to "upgrade ASAP" once that fix is in.

[jsuereth]

Thanks for doing the update!

[lalitb]

@bogdandrutu Can you see if the Bazel tsan check failure here is related to this PR ? Thanks.

[lalitb]

@bogdandrutu . There are some failures for bazel on Windows and gcc4.8. Thanks.

`
ERROR: C:/users/runneradmin/_bazel_runneradmin/cefevaif/external/com_github_grpc_grpc/BUILD:3194:16: undeclared inclusion(s) in rule '@com_github_grpc_grpc//:google_api_upbdefs':

this rule is missing dependency declarations for the following files included by 'com_github_grpc_grpc/src/core/ext/upbdefs-generated/google/protobuf/descriptor.upbdefs.c':
'bazel-out/x64_windows-fastbuild/bin/external/com_google_protobuf/google/protobuf/descriptor.upbdefs.h'
`

[lalitb]

closing this, as we have a similar gRPC upgrade PR #666