search

bazelbuild / rules_go

Go rules for Bazel
Apache License 2.0
1.36k stars 637 forks source link

proto: add go_proto_apiv2 and go_grpc_apiv2 compilers #2522

Open johanbrandhorst opened 4 years ago

johanbrandhorst commented 4 years ago

What version of rules_go are you using?

v0.23.2

What version of gazelle are you using?

v0.21.1

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?

Wanted to generate protofiles with the new Go protobuf generator stack, protoc-gen-go v1.24.0 and protoc-gen-go-grpc.

What did you expect to see?

I expected to see some go_proto_compiler definitions for these compiler options

What did you see instead?

No such compiler options

Discussion

I put together some example implementations of these two generators for the grpc-gateway to use, and I think we should consider upstreaming them (after v1 of protoc-gen-go-grpc is complete).

Here they are:

protoc-gen-go v1.24.0:

go_proto_compiler(
    name = "go_proto_apiv2", # up for discussion, of course
    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",
    ],
)

protoc-gen-go-grpc v0.0.0 (pending first release):


go_proto_compiler(
    name = "go_grpc_apiv2", # naming TBD
    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",
    ],
)

Both of these can be combined with the following gazelle directives to be used today:

# gazelle:go_proto_compilers //:go_proto_apiv2
# gazelle:go_grpc_compilers //:go_proto_apiv2, //:go_grpc_apiv2

One of the concerns raised when discussing this earlier was that we can't depend on the grpc repo, since it has a large dependency tree. However, the protoc-gen-go-grpc tool is actually its own module, with only a single dependency (google.golang.org/protobuf naturally), so I think it should be OK to depend on as a go_repository. See https://github.com/grpc/grpc-go/tree/master/cmd/protoc-gen-go-grpc.

jayconrod commented 4 years ago

Tentatively tagging this for the next release. I've reached out to some of the folks developing these generators to understand how these should be supported through rules_go and Gazelle.

jayconrod commented 3 years ago

@ghasemloo Continuing discussion here from #2587:

As I mentioned earlier, I'm not sure whether google.golang.org/grpc/cmd/protoc-gen-go-grpc is stable. It's checked into the master branch, but it's in its own submodule which has no tagged releases. I'm not sure what that means for stability. I'll try and find out.

johanbrandhorst commented 3 years ago

Pretty sure it's not stable still. It's also blocking me tagging v2 of the grpc-gateway :man_shrugging:.

jayconrod commented 3 years ago

The main protoc-gen-go-grpc confirmed it's still under development, and while they're aiming for a release in a few months, they anticipate breaking change before then.

I'll drop this from the v0.25 milestone for now. We can re-prioritize when there's a stable release. Anything before then will be experimental with compatibility promises.

PaulSonOfLars commented 3 years ago

Friendly heads-up that protoc-gen-go-grpc has been released, making this issue relevant again :)

jayconrod commented 3 years ago

@PaulSonOfLars Thanks for the heads up! Adding to v0.25 milestone.

ghasemloo commented 3 years ago

Hi Jay.

Any progress or timeline for this? :)

jayconrod commented 3 years ago

@ghasemloo Not yet I'm afraid.

cc @bazelbuild/go-maintainers Anyone interested in working on this?

achew22 commented 3 years ago

@jayconrod do you have any idea what would be involved in this? Is there something that could be patterned off of?

jayconrod commented 3 years ago

The main thing is go_proto_compiler targets need to be defined, wrapping the go_binary targets for these generators and declaring packages they import from generated code.

The go_proto_compiler targets for APIv1 are declared in @io_bazel_rules_go//proto. The APIv2 targets could potentially go there, too. It would bump the minimum version required for those modules, but that's probably okay. I think those targets could be added to the build file without actually needing those repositories to be declared, which is the main thing I'm scared of (gRPC has a ton of dependencies).

If that doesn't work out, Gazelle could have some logic to automatically declare go_proto_compiler targets for those packages when invoked by go_repository. It does something like that for other targets in com_github_golang_protobuf, so it wouldn't be unprecedented. Kind of "magical" though.

I think that's all that would need to be done. People could generate go_proto_library targets that use the new generators with the # gazelle:go_proto_compilers and # gazelle:go_grpc_compilers directives.

ash2k commented 3 years ago

FWIW I'm trying to do the same in rules_proto_grpc: https://github.com/rules-proto-grpc/rules_proto_grpc/pull/98 It's not going well and any help is appreciated!

arnehormann commented 2 years ago

@johanbrandhorst according to the 0.33.0 release notes, this should be fixed by making the 2 proto library the default: https://github.com/bazelbuild/rules_go/releases/tag/v0.33.0

It patches gazelle: https://github.com/bazelbuild/rules_go/blob/master/third_party/com_github_golang_protobuf-gazelle.patch