Using the new Go gRPC generator results in compilation errors

[johanbrandhorst]

What version of rules_go are you using?

This was tested against

git_repository(
    name = "io_bazel_rules_go",
    commit = "740ada94dfda52f2a079f718858e8b2b8ee0fdc6",
    remote = "https://github.com/bazelbuild/rules_go",
    shallow_since = "1589922540 -0400",
)

Since I suspected 740ada9 would've fixed this like it did other APIv2 incompatibilities, but it has not.

What version of gazelle are you using?

v0.21.0

What version of Bazel are you using?

v3.2.0

Does this issue reproduce with the latest releases of all the above?

Yes

What operating system and processor architecture are you using?

Linux amd64

What did you do?

Generate gRPC stubs using google.golang.org/grpc/cmd/protoc-gen-go-grpc, and protobuf stubs using google.golang.org/protobuf/cmd/protoc-gen-go.

What did you expect to see?

No compilation errors

What did you see instead?

Compilation errors for all gRPC services:

/root/.cache/bazel/_bazel_root/139d5bd62b3cea2b9ba60fa30fdc515d/sandbox/linux-sandbox/481/execroot/grpc_ecosystem_grpc_gateway/runtime/internal/examplepb/non_standard_names_grpc.pb.go:111:5: _NonStandardService_serviceDesc redeclared in this block
        previous declaration at /root/.cache/bazel/_bazel_root/139d5bd62b3cea2b9ba60fa30fdc515d/sandbox/linux-sandbox/481/execroot/grpc_ecosystem_grpc_gateway/bazel-out/k8-fastbuild/bin/runtime/internal/examplepb/examplepb_go_proto_/github.com/grpc-ecosystem/grpc-gateway/v2/runtime/internal/examplepb/non_standard_names.pb.go:931:5
/root/.cache/bazel/_bazel_root/139d5bd62b3cea2b9ba60fa30fdc515d/sandbox/linux-sandbox/481/execroot/grpc_ecosystem_grpc_gateway/runtime/internal/examplepb/non_standard_names_grpc.pb.go:34:36: too many errors

It compiles fine with go build ./....

[johanbrandhorst]

See grpc-ecosystem/grpc-gateway#1419 for an example of this

[jayconrod]

grpc-ecosystem/grpc-gateway#1419 looks like a pretty big change. I won't be able to look at it in depth this week.

Just taking a quick look though, the change to examples/internal/proto/examplepb/BUILD.bazel is suspicious. It's a go_library that includes a bunch of .pb.go files and embeds a go_proto_library. From the error message, I suspect that the same file is getting compiled twice, since go_proto_library will generate a bunch of .pb.go files. Normally, when using go_proto_library, .pb.go files should not be included in srcs lists. They should only be checked in for the benefit of go build.

[johanbrandhorst]

The BUILD file is generated by gazelle from the sources in the directory. Do you think I need to add some exclusion rules?

[johanbrandhorst]

The BUILD file is generated by gazelle from the sources in the directory. Do you think I need to add some exclusion rules?

I tried adding some exclusion rules to remove the suspicious sources and I get the same error. I'll work on a minimal example instead, sorry I didn't mean for you to actually look at the diff. I think this error will manifest itself anytime you try to use the new APIv2 generators together.

[johanbrandhorst]

Ah, I just saw this line:

    compilers = ["@io_bazel_rules_go//proto:go_grpc"],

I could see the error being because go_proto_library generates files with the old generator, including the gRPC parts of the generated file and causing duplicate types to be generated into the final output. Let me try with

    compilers = ["@io_bazel_rules_go//proto:go_proto"],

Sidenote: do we need to add a compiler here for the new APIv2 compiler stack?

[jayconrod]

@io_bazel_rules_go//proto:go_grpc is just @io_bazel_rules_go//proto:go_proto with plugins=grpc and extra implicit dependencies. They both refer to github.com/golang/protobuf/protoc-gen-go.

Earlier, I tried changing both to google.golang.org/protobuf/cmd/protoc-gen-go, but that one doesn't support gRPC.

It looks like google.golang.org/grpc/cmd/protoc-gen-go-grpc was just added, and it's not part of any release, so I'm just now learning about it. If you want to use it sooner, the right approach would probably be to define a go_proto_compiler target for it, then use that in go_proto_library compilers attributes.

[johanbrandhorst]

Update: it is possible to workaround this issue by adding

# gazelle:exclude xxx_grpc.pb.go

And

    compilers = ["@io_bazel_rules_go//proto:go_proto"],  #keep

The #keep comment is necessary because gazelle is overwriting it to be go_grpc, I'll raise an issue for gazelle separately. I think the exclusion requirement might also be a gazelle bug, maybe we shouldn't generate proto_libraries for files generated by google.golang.org/grpc/cmd/protoc-gen-go-grpc? How are we ignoring files generated by google.golang.org/protobuf/cmd/protoc-gen-go right now?

[jayconrod]

About excluding .pb.go files with Gazelle: depending on the proto mode, it will exclude .pb.go files with the same stem as .proto files in the same. You may need something stronger though:

# gazelle:exclude *.pb.go
# gazelle:exclude *.pb.gw.go

[johanbrandhorst]

We already have exclusions for the pb.gw.go files, I would not argue that those should be ignored automatically, however, I think there's a case for ignoring *_grpc.pb.go, which are what the files produced by the new generator are called.

[johanbrandhorst]

Sorry to sidetrack this, I will raise a gazelle issue instead.

[jayconrod]

Also, you can change the compilers attribute Gazelle will use for generated go_proto_library rules with the # gazelle:go_grpc_compilers and gazelle:go_proto_compilers directives.