Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Support protobuf APIv2 (google.golang.org/protobuf) #2395

Closed
4 tasks
jayconrod opened this issue Mar 3, 2020 · 4 comments
Closed
4 tasks

Support protobuf APIv2 (google.golang.org/protobuf) #2395

jayconrod opened this issue Mar 3, 2020 · 4 comments
Labels
enhancement proto
Milestone
v0.23

Comments

@jayconrod
Copy link
Contributor

As described at https://blog.golang.org/a-new-go-api-for-protocol-buffers

There is a new module for protocol buffers, google.golang.org/protobuf. The old version is github.com/golang/protobuf, which will continue to be maintained.

At least the following needs to be done:

  • Add a dependency on org_golang_google_protobuf with any necessary patches to go_rules_dependencies.
  • Change the default go_proto_compiler targets to point to the new compiler. My understanding is that code generated with the new compiler will work with the old API.
  • Update implicit dependencies for the go_proto_compiler targets.
  • Test that old generated code is compatible with the new API and new generated code is compatible with the old API.
@shahms
Copy link
Contributor

shahms commented Mar 13, 2020

It would also be nice to support v1.4.0 of the old API, which is implemented using the new API for transition purposes. It's often impractical to switch an entire codebase in one go.

  • github.com/golang/[email protected] is a version of APIv1 implemented in terms of APIv2. The API is the same, but the underlying implementation is backed by the new one. This version contains functions to convert between the APIv1 and APIv2 proto.Message interfaces to ease the transition between the two.

@shahms shahms mentioned this issue Mar 13, 2020
Closed
@jayconrod jayconrod added this to the v0.23 milestone Apr 22, 2020
jayconrod pushed a commit to jayconrod/rules_go that referenced this issue Apr 22, 2020
proto: upgrade go_proto_library to support APIv2
e944e1c 
* Added org_golang_google_protobuf at v1.21.0 to
  go_rules_dependencies. This contains the new protobuf runtime.
  It doesn't seem like any *_gen rules are needed, so just using the
  gazelle patch for now.
* Upgraded com_github_golang_protobuf to v1.4.0. The packages here are
  wrappers org_golang_google_protobuf. Newly generated code should
  work with the old interface. The *_gen targets are still in place,
  but they're simpler.
* Added new dependencies for packages generated with
  //proto:go_proto, //proto:go_grpc, and //proto:go_proto_bootstrap,
  since the proto compiler imports more runtime packages.
* We're still using the compiler from com_github_golang_protobuf. It's
  a wrapper for the new one. The new one doesn't support gRPC, and the
  gRPC proto compiler doesn't exist yet. See
  https://github.com/protocolbuffers/protobuf-go/releases#v1.20-grpc-support.

Updates bazel-contrib#2395
@jayconrod jayconrod mentioned this issue Apr 22, 2020
Merged
jayconrod pushed a commit that referenced this issue Apr 23, 2020
proto: upgrade go_proto_library to support APIv2 (#2455)
2634df7 
* Added org_golang_google_protobuf at v1.21.0 to
  go_rules_dependencies. This contains the new protobuf runtime.
  It doesn't seem like any *_gen rules are needed, so just using the
  gazelle patch for now.
* Upgraded com_github_golang_protobuf to v1.4.0. The packages here are
  wrappers org_golang_google_protobuf. Newly generated code should
  work with the old interface. The *_gen targets are still in place,
  but they're simpler.
* Added new dependencies for packages generated with
  //proto:go_proto, //proto:go_grpc, and //proto:go_proto_bootstrap,
  since the proto compiler imports more runtime packages.
* We're still using the compiler from com_github_golang_protobuf. It's
  a wrapper for the new one. The new one doesn't support gRPC, and the
  gRPC proto compiler doesn't exist yet. See
  https://github.com/protocolbuffers/protobuf-go/releases#v1.20-grpc-support.

Updates #2395
@jayconrod
Copy link
Contributor Author

Fixed in #2455.

I was not able to update //proto:go_proto to the new compiler in google.golang.org/protobuf, but the new version of the old compiler will generate code compatible with both APIs. The new compiler does not support gRPC, and there is no replacement gRPC compiler yet.

@ribrdb
Copy link

ribrdb commented May 6, 2020

I'm testing this out and seeing a bunch of warnings like this:

2020/05/05 17:52:22 WARNING: proto: message google.protobuf.OneofDescriptorProto is already registered
        previously from: "github.com/golang/protobuf/protoc-gen-go/descriptor"
        currently from:  "google.golang.org/protobuf/types/descriptorpb"
A future release will panic on registration conflicts. See:
https://developers.google.com/protocol-buffers/docs/reference/go/faq#namespace-conflict

Does that mean I've got the wrong version of some dependency?

@jayconrod
Copy link
Contributor Author

@ribrdb Please file a new issue with more information.

@drigz drigz mentioned this issue May 11, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement proto
Projects
None yet
Milestone
v0.23
Development

No branches or pull requests
3 participants
@ribrdb @jayconrod @shahms