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.

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

Switch to new Go protobuf generators #1419

Merged
merged 1 commit into from
May 29, 2020

Conversation

johanbrandhorst
Copy link
Collaborator

@johanbrandhorst johanbrandhorst commented May 28, 2020

Switch from github.com/golang/protobuf/protoc-gen-go
to google.golang.org/protobuf/cmd/protoc-gen-go
and google.golang.org/grpc/cmd/protoc-gen-go-grpc.

Add APIv2 protoc-gen-go and protoc-gen-go-grpc bazel rules

@johanbrandhorst johanbrandhorst force-pushed the upgrade-protoc-gen-go-protoc-gen-go-grpc branch from f7b3404 to 3878da8 Compare May 28, 2020 12:30
@johanbrandhorst johanbrandhorst marked this pull request as draft May 28, 2020 12:30
@johanbrandhorst johanbrandhorst mentioned this pull request May 28, 2020
13 tasks
@johanbrandhorst johanbrandhorst force-pushed the upgrade-protoc-gen-go-protoc-gen-go-grpc branch 4 times, most recently from 34bc653 to 07e4bd3 Compare May 28, 2020 15:24
@codecov-commenter
Copy link

codecov-commenter commented May 28, 2020

Codecov Report

Merging #1419 into v2 will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##               v2    #1419   +/-   ##
=======================================
  Coverage   58.97%   58.97%           
=======================================
  Files          32       32           
  Lines        3895     3895           
=======================================
  Hits         2297     2297           
  Misses       1344     1344           
  Partials      254      254           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f0cca7d...88c0bef. Read the comment docs.

@johanbrandhorst johanbrandhorst force-pushed the upgrade-protoc-gen-go-protoc-gen-go-grpc branch 4 times, most recently from 6470f21 to 0fe3efa Compare May 28, 2020 16:44
@johanbrandhorst johanbrandhorst marked this pull request as ready for review May 28, 2020 16:47
@johanbrandhorst
Copy link
Collaborator Author

This is actually working as is, but the bazel side of things is still using the old generator, which is no good, so I'm going to try definine some new go_proto_compilers.

@johanbrandhorst johanbrandhorst force-pushed the upgrade-protoc-gen-go-protoc-gen-go-grpc branch from 0fe3efa to 6b1cc13 Compare May 28, 2020 21:15
@@ -8,14 +8,24 @@ package(default_visibility = ["//visibility:public"])
# TODO(yannic): Add examples/tests that use import_prefix/strip_import_prefix.

# gazelle:exclude a_bit_of_everything.pb.gw.go
# gazelle:exclude a_bit_of_everything_grpc.pb.go
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is still a bit of a hack, see bazel-contrib/bazel-gazelle#798

@johanbrandhorst johanbrandhorst force-pushed the upgrade-protoc-gen-go-protoc-gen-go-grpc branch from 6b1cc13 to c3ee7c7 Compare May 28, 2020 21:36
Switch from github.com/golang/protobuf/protoc-gen-go
to google.golang.org/protobuf/cmd/protoc-gen-go
and google.golang.org/grpc/cmd/protoc-gen-go-grpc.

Add APIv2 protoc-gen-go and protoc-gen-go-grpc bazel rules
@johanbrandhorst johanbrandhorst force-pushed the upgrade-protoc-gen-go-protoc-gen-go-grpc branch from c3ee7c7 to 88c0bef Compare May 28, 2020 21:50
Comment on lines +29 to +82

go_proto_compiler(
name = "go_apiv2",
options = [
"paths=source_relative",
],
plugin = "@org_golang_google_protobuf//cmd/protoc-gen-go",
suffix = ".pb.go",
visibility = ["//visibility:public"],
deps = [
"@com_github_golang_protobuf//proto:go_default_library",
"@io_bazel_rules_go//proto/wkt:any_go_proto",
"@io_bazel_rules_go//proto/wkt:api_go_proto",
"@io_bazel_rules_go//proto/wkt:compiler_plugin_go_proto",
"@io_bazel_rules_go//proto/wkt:descriptor_go_proto",
"@io_bazel_rules_go//proto/wkt:duration_go_proto",
"@io_bazel_rules_go//proto/wkt:empty_go_proto",
"@io_bazel_rules_go//proto/wkt:field_mask_go_proto",
"@io_bazel_rules_go//proto/wkt:source_context_go_proto",
"@io_bazel_rules_go//proto/wkt:struct_go_proto",
"@io_bazel_rules_go//proto/wkt:timestamp_go_proto",
"@io_bazel_rules_go//proto/wkt:type_go_proto",
"@io_bazel_rules_go//proto/wkt:wrappers_go_proto",
"@org_golang_google_protobuf//reflect/protoreflect:go_default_library",
"@org_golang_google_protobuf//runtime/protoimpl:go_default_library",
],
)

go_proto_compiler(
name = "go_grpc",
options = [
"paths=source_relative",
],
plugin = "@org_golang_google_grpc_cmd_protoc_gen_go_grpc//:protoc-gen-go-grpc",
suffix = "_grpc.pb.go",
visibility = ["//visibility:public"],
deps = [
"@io_bazel_rules_go//proto/wkt:any_go_proto",
"@io_bazel_rules_go//proto/wkt:api_go_proto",
"@io_bazel_rules_go//proto/wkt:compiler_plugin_go_proto",
"@io_bazel_rules_go//proto/wkt:descriptor_go_proto",
"@io_bazel_rules_go//proto/wkt:duration_go_proto",
"@io_bazel_rules_go//proto/wkt:empty_go_proto",
"@io_bazel_rules_go//proto/wkt:field_mask_go_proto",
"@io_bazel_rules_go//proto/wkt:source_context_go_proto",
"@io_bazel_rules_go//proto/wkt:struct_go_proto",
"@io_bazel_rules_go//proto/wkt:timestamp_go_proto",
"@io_bazel_rules_go//proto/wkt:type_go_proto",
"@io_bazel_rules_go//proto/wkt:wrappers_go_proto",
"@org_golang_google_grpc//:go_default_library",
"@org_golang_google_grpc//codes:go_default_library",
"@org_golang_google_grpc//status:go_default_library",
],
)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've raised bazel-contrib/rules_go#2522 to have these merged upstream

@@ -28,6 +28,8 @@ import (
var uuidgen = fastuuid.MustNewGenerator()

type _ABitOfEverythingServer struct {
examples.UnimplementedABitOfEverythingServiceServer
examples.UnimplementedStreamServiceServer
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These will eventually be required by protoc-gen-go-grpc, so I'm adding them now

@johanbrandhorst johanbrandhorst merged commit 81eab5c into v2 May 29, 2020
@johanbrandhorst johanbrandhorst deleted the upgrade-protoc-gen-go-protoc-gen-go-grpc branch May 29, 2020 11:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants