New proto mode: one proto_library per proto

[jayconrod]

Based on discussion in bazel-contrib/rules_go#1118

Gazelle should be able to produce a proto_library rule (and corresponding go_proto_library rule) per .proto file. Currently, the default is to produce one of these one of these for all the protos in a directory in the default package. This doesn't make sense for projects that store many proto files in a common directory.

It's likely that this mode will be opt-in (at least at first) either via a command line flag (-proto=perfile) or directive # gazelle:proto perfile.

[ianthehat]

I think it can/should be detected based on the declared package in the file, just like the go_library rules will be.

…

On Fri, Feb 23, 2018, 11:31 Jay Conrod ***@***.***> wrote: Based on discussion in bazel-contrib/rules_go#1118 <bazel-contrib/rules_go#1118> Gazelle should be able to produce a proto_library rule (and corresponding go_proto_library rule) per .proto file. Currently, the default is to produce one of these one of these for all the protos in a directory in the default package. This doesn't make sense for projects that store many proto files in a common directory. It's likely that this mode will be opt-in (at least at first) either via a command line flag (-proto=perfile) or directive # gazelle:proto perfile. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#138>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA0a0DbTiKqSgsdaC0t5O6WuB_wHOR2Xks5tXufUgaJpZM4SRIpE> .

[jayconrod]

cc @cgrushko @nclinger

I had the impression the problem was lots of proto files in the same package (not just the same directory). Is that accurate?

If proto files are grouped in separate packages, then Gazelle should just support multiple packages per directory (#7); no new proto mode needed.

[nate-meta]

Yes, though currently it doesn't work in either case correct?

[jayconrod]

Correct.

[cgrushko]

I think package hygiene in protos is so-so, and wouldn't be surprised if a big chunk of them don't specify a package at all.

[jmillikin-stripe]

This is fixed now, right? The current release of Gazelle is able to generate multiple targets for .proto sources in the same package and directory, based on the presence of file options like go_package.

[jayconrod]

@jmillikin-stripe That's right, but to avoid breaking users in the default configuration, this is not on by default. You can use directives (or equivalent command line options) to turn it on:

# gazelle:proto package
# gazelle:proto_group go_package

The first directive switches Gazelle into package mode. By default, that groups .proto files by package declarations. The second directive groups by option go_package declarations instead.

This isn't quite the same as this feature request though. This is asking for separate proto_library and go_proto_library targets for each .proto file. I guess we would use embedding to link the go_proto_library targets together when they have the same option go_package / importpath.

[linzhp]

One proto_library per proto file is recommended by Bazel (even if they belong to the same go_package), and the scenario is better tested. For example, this bug doesn't occur when there is one proto_library per proto file, because it is covered in tests.

[jayconrod]

We're going to work on this soon. github.com/googleapis/googleapis will have one proto_library per proto file. However, we need go_proto_library to be able to build a package from multiple proto_library targets first. That's bazel-contrib/rules_go#1890.

[jmillikin-stripe]

Wouldn't each proto_library in googleapis correspond to one Go package, and thus one go_proto_library target? Looking at google/rpc/ as an example, the .proto files have distinct go_package options.

[jayconrod]

@vam-google can comment on the specifics better than I can.

Currently we use go_googleapis-gazelle.patch and a few other patches to declare Go build rules for that repository. There are a number of proto_library rules with multiple srcs. The sources are currently grouped by go_package (or what Gazelle thinks the go_package would be, if it's not set explicitly. It's not the common case, but it's not unusual either.

[vam-google]

@jmillikin-stripe, @jayconrod The google/rpc is a special snowflake, when all the three .proto files it has specify different go_package. For most of the places inside googleapis the situation is different. If we check google/api, it contains many different .proto files with prety chaotic file<->go_package relations. For example the following files all belong to same serviceconfig go package:

auth.proto
backend.proto
billing.proto
context.proto
control.proto
documentation.proto
endpoint.proto
log.proto
logging.proto
monitoring.proto
quota.proto
service.proto
source_info.proto
system_parameter.proto
usage.proto

[bufdev]

+1 to this feature - I was beginning to work on it on a branch here, but I think it's more involved than it appears at first glance - this isn't impossible to overcome, but some refactoring would be needed (however I might be missing something).

Language#Kinds() only allows one KindInfo per rule name. If Gazelle were to generate a proto_library rule per file, and also a top-level proto_library rule per package that had each per-file rule as a dep, you'd end up with srcs empty on the top-level proto_library rule, which the current setup would consider empty.

Example:

proto_library(
    name = "protos",
    deps = [
        ":job_api_proto",
        ":hello_proto",
    ],
    visibility = ["//visibility:public"],
)

proto_library(
    name = "job_api_proto",
    srcs = ["job_api.proto"],
    deps = [
        "//uber/proto/foo/baz/v1:feedback_proto",
        "//uber/proto/foo/baz/v1:itinerary_proto",
        "//uber/proto/schema/gis/v1:gis_proto",
        ":hello_proto",
    ],
    visibility = ["//:__subpackages__"],
)

proto_library(
    name = "hello_proto",
    srcs = ["hello.proto"],
    deps = ["@com_google_protobuf//:timestamp_proto"],
    visibility = ["//:__subpackages__"],
)

protos is the top-level package proto_library rule (equivalent to the proto_library rule that gazelle generates now). However, it has no srcs, as job_api_proto and hello_proto are the per-file rules that each have one source file. Note that having per-file rules allows your per-file dependencies to be more granular as well obviously.