grpc / grpc-web

gRPC for Web Clients
https://grpc.io
Apache License 2.0
8.45k stars 760 forks source link

Do not force third_party/protobuf to some old version. #1446

Closed benjaminp closed 1 week ago

sampajano commented 1 week ago

Wow thanks!!!

benjaminp commented 1 week ago

This seems reasonable but I don't think it can fix the protoc_plugin Dockerfile. That tries to build protobuf with autotools, which has long since been removed protocolbuffers/protobuf@ed5c57a574aa24bc49432baced51aa6d01fbf38a.

sampajano commented 1 week ago

Oh i see!!... So we might need https://github.com/grpc/grpc-web/pull/1447 afterall.. 😅

Do you know how to fix protoc_plugin by any chance?

It would be very appreciated if you could help too.. (I'm on a vacation soon i won't have time to fix this now..)

sampajano commented 1 week ago

@benjaminp The APIs you were using was probably added before https://github.com/protocolbuffers/protobuf/pull/10132, right?

So maybe it would work if we re-pin it back to a version before https://github.com/protocolbuffers/protobuf/pull/10132?

sampajano commented 1 week ago

It seems that the main reason why we need protoc_plugin now is it's used for releases on linux: https://github.com/grpc/grpc-web/blob/master/.github/workflows/make-plugin-linux.yml

So if we can migrate that to Bazel as well then maybe we can kill it off entirely :)

sampajano commented 1 week ago

@benjaminp Hi! Actually i need to revert this since we have other dependencies on that version as well.. (I should probably let CI run before merging it :))

https://github.com/grpc/grpc-web/blob/e6909cd2841061aa409e7f9365ac68b865fd8f83/packages/grpc-web/test/generated_code_test.js#L476