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

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

Open
jmhodges opened this issue Jan 4, 2024 · 6 comments · May be fixed by #1711
Open

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

jmhodges opened this issue Jan 4, 2024 · 6 comments · May be fixed by #1711

Comments

@jmhodges
Copy link
Contributor

jmhodges commented Jan 4, 2024

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
Copy link
Member

fmeum commented Jan 4, 2024

@ryanpbrewster @mering Are you seeing this as well? Would you be willing to look into this as a follow-up to bazel-contrib/rules_go#3761?

@ryanpbrewster
Copy link

A few quick notes:

  • I think you can instruct gazelle to leave your hand-altered build targets alone by putting # keep after them. That might be a short-term workaround
  • I'll take a look at gazelle to see if there's an easy fix for this. I haven't actually upgraded my bazel workspace yet due to other headaches around gazelle interop

@ryanpbrewster
Copy link

ryanpbrewster commented Jan 4, 2024

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 mering linked a pull request Jan 4, 2024 that will close this issue
@mering
Copy link
Contributor

mering commented Jan 4, 2024

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
Copy link

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
Copy link
Member

fmeum commented Jan 9, 2024

We have removed the warning temporarily in bazel-contrib/rules_go@8beaf2e and will provide a way to apply default overrides in #1716.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants