Gazelle is generating helloworld_proto_proto instead of helloworld_proto

[nclinger]

When I run Gazelle from an empty build target for a proto file named helloworld.proto containing a grpc service, Gazelle generates:

proto_library( name = "helloworld_proto_proto", srcs = ["helloworld.proto"], visibility = ["//visibility:public"], )

go_grpc_library( name = "helloworld_proto_go_proto", importpath = "foo/builds/examples/protobuf", proto = ":helloworld_proto_proto", visibility = ["//visibility:public"], )

go_library( name = "go_default_library", importpath = "foo/builds/examples/protobuf", library = ":helloworld_proto_go_proto", )

Thoughts on why its appending a _proto twice?

[jayconrod]

Gazelle generates the name for proto_library by appending _proto to the Go package name. Sounds like Gazelle thinks the Go package name is helloworld_proto.

There are a few heuristics for determining the package name. Gazelle tries to predict what the proto compiler will do.

• If the Go package is set explicitly, Gazelle will use that. For example, if you have option go_package = "github.com/example/helloworld"; then the package name is helloworld.
• Gazelle will try the proto package next, replacing '.' with '_'. For example, if you have package hello.world;, then the Go package name is hello_world.
• Otherwise, the package name is derived from the file name. The .proto extension is trimmed, and non-alphanumeric characters are replaced with '_'. So hello-world.proto would be hello_world.

If these heuristics aren't working correctly, could you post a small proto file that shows this problem?

[nclinger]

Ah I see. Thanks for the explanation Jay!

[cgrushko]

@jayconrod - what is the significance of the name of a proto_library? Is it used as the package name of the generated code? (my impression is that no - I was able to change bla_bla_bla_bla_bla_proto to just proto in my BUILD files and everything built fine).

I'm sharing a .proto file between Go and Java, so there's some hand-written BUILD files, and because my proto packages have long names it becomes a mess.

I could manually specify option go_package but I'd rather not do what protoc already does, which is infer a package name.

[jayconrod]

@cgrushko There's no significance to the name of a proto_library rule as far as the Go rules are concerned. There are a few conventions mentioned above (mostly for Gazelle's benefit), but that's it.

The important thing is that the value in importpath of go_proto_library and any go_library that embeds it matches the string dependent libraries use to import it. Gazelle tries to infer a reasonable import path from option go_package, the package name, and failing that, the file name. However, the importpath in build files is the source of truth, and you can set it to whatever you want (use a # keep comment to prevent Gazelle from changing it).

[cgrushko]

If I understand correctly, it means that Gazelle can choose a different name for the proto_library rules it creates - foo_proto for foo.proto.

Unless the issue is that Gazelle puts multiple .proto files into the same library? I'd recommend against it based on Google's experience.

Alternatively, Gazelle should detect existing proto_library rules that srcs the .proto files, and not add another proto_library rule with a different name. I believe it already detects this case (it warned me, I think), so only left to use the existing one.

Does one (or both) approaches seem reasonable?

[jayconrod]

Gazelle will name the rule foo_proto where foo is what Gazelle thinks the Go package name should be. At the moment, Gazelle will only update one proto_library rule with the correct name.

Unless the issue is that Gazelle puts multiple .proto files into the same library? I'd recommend against it based on Google's experience.

Gazelle only supports one package per directory right now. It groups source files (including protos) by Go package name. If there are multiple .proto files that will generate .go files with the same package name, they will be grouped in the same rule. Supporting multiple packages is really something I want to improve, but there's a ton of stuff going on, and I haven't had time yet.

Unless the issue is that Gazelle puts multiple .proto files into the same library? I'd recommend against it based on Google's experience.

Could you expand on that (publicly or privately)? I know you have a lot more experience here with the Blaze proto rules. Why shouldn't protos in the same package be grouped in the same rule?

Alternatively, Gazelle should detect existing proto_library rules that srcs the .proto files, and not add another proto_library rule with a different name. I believe it already detects this case (it warned me, I think), so only left to use the existing one.

I intend to do this at some point (it will be necessary for multiple packages or multiple binaries / tests to ever work), but it will be hard to get it right.

[cgrushko]

Why shouldn't protos in the same package be grouped in the same rule?

There's something about protos that causes people to make huge piles of them in a single directory called "proto". The situation is bad as it is with people putting way too many messages and services into the same .proto file, so I think that extending this to directories would mean a single proto rule for the entire project.

For small projects that's no big deal, but as a project grows this becomes a drag on productivity as build times suffer, all tests trigger on each change, flaky tests deteriorate the benefits of your CI, etc.

---

I might be overreacting :) for now, I use the names that Gazelle generates. I got beauties such as

proto_library(
    name = "java_com_google_devtools_javatools_jade_pkgloader_messages_proto",
    srcs = ["messages.proto"],
)

which is a pain in the ass to use, but not the end of the world.

[nclinger]

I strongly agree that it would be nice if Gazelle supported having separate rules per proto. This is especially important if you have monolithic services (see Gaia or super hot) where there are a dozens or hundreds of proto files that compose a single service. With the current grouping of all proto files into a single target if you need to use one aspect of the service you are forced to depend on all the photos in the service if they are placed in a single directory. There are many issues with this as highlighted by Carmi but I think the worst are the increase in presubmit time due to increased build time and unecessary test targets being triggered, and binary bloat.

[nclinger]

Typed from a device I sit on, excuse the typos.

[jayconrod]

I'm pretty much in agreement. I've filed bazelbuild/bazel-gazelle#138 to implement this.