bazel-contrib / bazel-gazelle

Gazelle is a Bazel build file generator for Bazel projects. It natively supports Go and protobuf, and it may be extended to support new languages and custom rule sets.
Apache License 2.0
1.19k stars 379 forks source link

gazelle generates and recreates grpc targets that rules_go complains about #1709

Open jmhodges opened 9 months ago

jmhodges commented 9 months ago

What version of gazelle are you using?

0.35

What version of rules_go are you using?

0.44.2

What version of Bazel are you using?

6.4.0

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

Yes for rules_go and gazelle, but I'm not able to test on bazel 7.0.0

What operating system and processor architecture are you using?

macOS 14.2.1

What did you do?

Gazelle 0.35 and rules_go (both 0.44.1 and 0.44.2) disagree on how to create grpc protobuf targets. If you use gazelle to generate targets for a proto3 proto file that contains service to create a GRPC service, it'll generate a go_proto_library target that sets compilers = ["@io_bazel_rules_go//proto:go_grpc"],.

However, rules_go will warn about that:

WARNING: /path/to/proto/BUILD.bazel:12:17: in go_proto_library rule //proto/proto:mytarget_go_proto: target '//proto/mytarget_go_proto' depends on deprecated target '@io_bazel_rules_go//proto:go_grpc': Migrate to //proto:go_grpc_v2 compiler (which you'll get automatically if you use the go_grpc_library() rule).

Which implies that you're supposed to use go_grpc_library as the macro (from load("@io_bazel_rules_go//proto:def.bzl", "go_grpc_library")).

The unfortunate thing here is that if you fix this by hand, gazelle will remove your go_grpc_library call and the load statement and replace it with the go_proto_library target with the compilers set on it.

rules_go seems to have decided on this new pattern a few weeks ago (with a tweak to the deprecation message a bit after that).

What did you expect to see?

Gazelle creating grpc targets without warnings

What did you see instead?

Gazelle creating grpc targets with warnings

fmeum commented 9 months ago

@ryanpbrewster @mering Are you seeing this as well? Would you be willing to look into this as a follow-up to https://github.com/bazelbuild/rules_go/pull/3761?

ryanpbrewster commented 9 months ago

A few quick notes:

ryanpbrewster commented 9 months ago

Looking at the documentation here it seems like you can also add a

# gazelle:go_grpc_compilers

directive at the top of a BUILD file to tell gazelle how you want grpc services compiled (it supposedly applies recursively, so if you do this at the top level then it ought to take effect in your entire workspace)

EDIT: just checked in a sample bazel repo and

# gazelle:go_grpc_compilers @io_bazel_rules_go//proto:go_proto,@io_bazel_rules_go//proto:go_grpc_v2

seems to work for me (at least it gets gazelle to stop fighting against me)

mering commented 9 months ago

I never really used Gazelle directly myself but gave it a shot at #1711. I managed to fix the generator but didn't quite manage to fix the fixer.

leungster commented 9 months ago

I'm seeing this warning as well but for external repos. I just switched over to using bzlmod and the gazelle extension doesn't have the ability to set gazelle directives for all godeps. I have to manually add module_override statements for each external repo that has protos defined.

Prior to bzlmod, I ran gazelle to generate go_repository rules with directives that i wanted for all external repos. bazelrun //:gazelle -- update-repos -from_file=go.mod -to_macro=godeps.bzl%go_dependencies -prune -build_directives "gazelle:go_generate_proto false"

fmeum commented 9 months ago

We have removed the warning temporarily in https://github.com/bazelbuild/rules_go/commit/8beaf2e7f24c5dbf1b21ef8d349e8814ebb46415 and will provide a way to apply default overrides in https://github.com/bazelbuild/bazel-gazelle/pull/1716.