grpc / grpc-web

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

protobuf generator relies on private APIs #1437

Closed benjaminp closed 1 week ago

benjaminp commented 1 week ago

In changes like protocolbuffers/protobuf@8b1be51f08f2945e709813ca09c260049b540f9f, protobuf has made private certain descriptor APIs. The grpc-web protobuf generator uses these APIs and thus fails to build with recent protobuf versions. The particular problematic APIs are FieldDescriptor::has_optional_keyword() and OneofDescriptor::is_synthetic().

third_party/grpc-web/javascript/net/grpc/web/generator/grpc_generator.cc: In function 'void grpc::web::{anonymous}::PrintProtoDtsMessage(google::protobuf::io::Printer*, const google::protobuf::Descriptor*, const google::protobuf::FileDescriptor*)':
third_party/grpc-web/javascript/net/grpc/web/generator/grpc_generator.cc:842:36: error: 'bool google::protobuf::FieldDescriptor::has_optional_keyword() const' is private within this context
  842 |     if (field->has_optional_keyword() ||
      |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~^~
In file included from bazel-out/k8-opt-exec-ST-d57f47055a04/bin/external/protobuf~/src/google/protobuf/compiler/_virtual_includes/code_generator/google/protobuf/compiler/code_generator.h:25,
                 from third_party/grpc-web/javascript/net/grpc/web/generator/grpc_generator.cc:19:
bazel-out/k8-opt-exec-ST-d57f47055a04/bin/external/protobuf~/src/google/protobuf/_virtual_includes/protobuf/google/protobuf/descriptor.h:1049:8: note: declared private here
 1049 |   bool has_optional_keyword() const;
      |        ^~~~~~~~~~~~~~~~~~~~
third_party/grpc-web/javascript/net/grpc/web/generator/grpc_generator.cc:847:86: error: 'bool google::protobuf::FieldDescriptor::has_optional_keyword() const' is private within this context
  847 |     if (field->type() == FieldDescriptor::TYPE_MESSAGE || field->has_optional_keyword() ||
      |                                                           ~~~~~~~~~~~~~~~~~~~~~~~~~~~^~
bazel-out/k8-opt-exec-ST-d57f47055a04/bin/external/protobuf~/src/google/protobuf/_virtual_includes/protobuf/google/protobuf/descriptor.h:1049:8: note: declared private here
 1049 |   bool has_optional_keyword() const;
      |        ^~~~~~~~~~~~~~~~~~~~
third_party/grpc-web/javascript/net/grpc/web/generator/grpc_generator.cc:870:29: error: 'bool google::protobuf::OneofDescriptor::is_synthetic() const' is private within this context
  870 |     if (!oneof->is_synthetic()) {
      |          ~~~~~~~~~~~~~~~~~~~^~
bazel-out/k8-opt-exec-ST-d57f47055a04/bin/external/protobuf~/src/google/protobuf/_virtual_includes/protobuf/google/protobuf/descriptor.h:2739:13: note: declared private here
 2739 | inline bool OneofDescriptor::is_synthetic() const {
      |             ^~~~~~~~~~~~~~~
third_party/grpc-web/javascript/net/grpc/web/generator/grpc_generator.cc:905:88: error: 'bool google::protobuf::FieldDescriptor::has_optional_keyword() const' is private within this context
  905 |     if ((field->type() != FieldDescriptor::TYPE_MESSAGE && !field->has_optional_keyword()) ||
      |                                                             ~~~~~~~~~~~~~~~~~~~~~~~~~~~^~
bazel-out/k8-opt-exec-ST-d57f47055a04/bin/external/protobuf~/src/google/protobuf/_virtual_includes/protobuf/google/protobuf/descriptor.h:1049:8: note: declared private here
 1049 |   bool has_optional_keyword() const;
      |        ^~~~~~~~~~~~~~~~~~~~
sampajano commented 1 week ago

@benjaminp Thank you for the report! Good to note!

I suppose this isn't an immediate issue so long as we depend on older versions of the protobuf compiler?

But would be good to resolve soon too so it allows us to upgrade again!

benjaminp commented 1 week ago

It comes up now for people depending on grpc-web in a monorepo and substituting their own modern versions of protobuf for the 2 year old one in grpc-web's WORKSPACE.

sampajano commented 1 week ago

@benjaminp Aha thanks for informing! That's good to note!

Just to confirm tho, does this error comes up only during compilation time or also during runtime?

If the latter, starting from which version of the proto compiler does it no longer work with grpc-web?

Thanks!

benjaminp commented 1 week ago

I can't speak to the runtime because the generator not compiling prevents me from testing that out. (It seems all the errors are in the dts generation code, though.)

sampajano commented 1 week ago

Ah got it! Thanks!

Looks to me this is a compile time error (for now), that if you use the binaries in our releases it should work for now. Please test that out if you don't mind.

It would certainly be good to upgrade the code soon so it compile with newer protobuf source code.

PRs are very welcome from all here!