proposal: protoc-gen-go should be able to compile multiple packages per run

[tamird]

Passing multiple packages' worth of *.proto files currently fails when using (protoc-gen-go)[https://github.com/golang/protobuf/blob/master/protoc-gen-go/generator/generator.go#L612:L614].

The same functionality is supported by other generators (at least the C++ generator shipped with protoc). protoc-gen-go should support this as well.

[dsymonds]

The standard way for Go packages to be distinguished is to be in separate directories. The package declaration in a Go file is not the way that package boundaries are defined; the go tool demands the package names to match, but it otherwise doesn't use it to determine what constitutes a package. The other languages that ship with protoc don't care about package boundaries in the same way as Go (the mapping to C++ is simply a namespace), and so Go requires you to pass a package-worth of .proto files to protoc at a time. That is sometimes trivial (if your .proto files are already grouped into the relevant directories), but will sometimes require a bit of manual work.

[tamird]

The standard way for Go packages to be distinguished is to be in separate directories. The package declaration in a Go file is not the way that package boundaries are defined; the go tool demands the package names to match, but it otherwise doesn't use it to determine what constitutes a package. The other languages that ship with protoc don't care about package boundaries in the same way as Go (the mapping to C++ is simply a namespace),

With you so far.

and so Go requires you to pass a package-worth of .proto files to protoc at a time.

This is where you lost me. That Go demands that a package is made up of files in the same directory is not the same as compiling a single package at a time. Rather, it seems that there's a requirement that each collection of files that belongs to a single package must be located in the same directory.

For these files which each belong to a different package:

        ├── extension_base
        │   └── extension_base.proto
        ├── extension_extra
        │   └── extension_extra.proto
        ├── extension_user
        │   └── extension_user.proto

It should be legal to invoke protoc --go_out=. **/*.proto and have the generated Go files mirror the layout of the proto files (both in packages and on disk).

[dsymonds]

Your last command seems to have been cut off.

I'm not saying that .proto files need to be grouped into directories, one package per dir. I'm saying that a single invocation of protoc should be given a single package's worth of .proto files when generating Go code. That might involve multiple invocations or some trivial shell scripting, but that's easy. I am not keen on adding more complexity to protoc-gen-go to support that use case.

[tamird]

I understand what you are saying, but you are just describing the status quo. Why is this not a valid use case for protoc-gen-go to support? The exact same use case is supported by the Java code generator.

[robpike]

See my comment in the pull request.

[kingkf]

so, how to solve the question? now every time I generate code for golang, I need do lots of manual work to fix generated code.

[zellyn]

We use https://github.com/square/goprotowrap - a wrapper that collects things into packages. Pull requests welcome :-)

The sad part is that it would be pretty straightforward to make Go protos work like other languages :-(

[lnshi]

@tamird So, has this been fixed or not? I cannot get it work, I have some structures like this:

module-proto/
  |
  |--lib/
  |    |--bool_res.protp
  |    |--empty.proto
  |    |--mobile_phone.proto
  |    |
  |--microservices/
  |    |--auth/
  |    |    |--v1/
  |    |    |   |--auth.proto
  |    |    |--apis.proto
  |    |--sync
  |    |    |--v1/
  |    |    |   |--sync.proto
  |    |    |--apis.proto

And under the path module-proto/, tried to run protoc -I=. --go_out=plugins=grpc:. **/*.proto, keep getting error:

protoc-gen-go: error:inconsistent package names: microservice lib

Where am I doing wrong?

[tamird]

It has not been fixed.

[gardell]

What's the status of this bug? Why is this and #67 both closed?

I have a number of protos which all depend on one proto declaring a few custom options declared in its own package. If I compile all the protos separately, I run into #67 and running them together I run into this issue. It seems like packaging protos in go is broken then?

[brobits]

Google doesn't want to fix this:

I am not keen on adding more complexity to protoc-gen-go to support that use case.

the most common golang protobuf usability issue is too complex apparently. thanks to Square for providing a practical solution to a real issue. shame on golang protobuf maintainers.

[dsnet]

I'm going through the issue tracker and figuring what needs to be done. Re-opening this so this doesn't get forgotten. This does not necessarily imply it will be implemented as every feature needs to be carefully thought through.

[dsnet]

Pinging this thread for subscribers to see #515.

[tigrus]

I wanted to play with golang and implement one of grpc services in it. Got this error, saw this thread.. This is the fastest turn around that language can provide.

[neild]

New year's update:

1. The v2 proto implementation (currently on track to release later this year) will completely fix this issue. It includes a rewritten protoc-gen-go which has no restrictions on what .proto source files can be simultaneously compiled. (You can find this version of protoc-gen-go on the api-v1 and api-v2 branches of this repository. Note that these branches are unreleased and subject to change without notice.)
2. While the currently released protoc-gen-go doesn't let you do protoc --go_out=. **/*.proto, you can get the (probable) desired effect with for x in **/*.proto; do protoc --go_out=paths=source_relative:. $x; done. If you have cases where this doesn't produce the desired results, I'd very much like to hear about them.
3. While not specifically about this issue, as a general rule I strongly recommend always passing the paths=source_relative option to the compiler (e.g., protoc --go_out=paths=source_relative:. foo.proto) and setting option go_package = "full/import/path/of/package"; in every .proto source file. Following these rules resolve most problems with generation that I've seen.

[zellyn]

I haven't tried it, but if it does what it sounds like, I doubt using source-relative paths would work for us. We explicitly set the go_package in places to work around the fact that (only) in Go, protobuf circular dependencies break at the package level, while in every other protobuf language, they break at the file level.

[dsnet]

@zellyn

protobuf circular dependencies break at the package level

Can you clarify what this means? In protobuf, cyclic dependencies are only possible within a proto file. Since each proto file is generated to exactly one Go source file, that should make it impossible to have cyclicly dependent Go packages result from that.

[zellyn]

If a/red.proto references b/red.proto and b/green.proto references a/green.proto, that's okay for protos, C++, Java, etc. In Go, package a and package b are not allowed to reference each other that way. This is a frequent cause of problems at Square, and was one of the original reasons we forked the go protoc plugin so severely back in the day: there used to be no way to change the package into which Go code gets generated.

You would not have this problem at Google, because you use blaze for building everything, and it can create multiple distinct Go packages in the same directory, based on your BUILD rules.

[dsnet]

If I understand you correctly, you're describing a file tree that looks like:

a
├── red.proto   # imports b/red.proto
└── green.proto
b
├── red.proto
└── green.proto # imports a/green.proto 

However, proto-file layout does not necessary correspond with go package layout since the go_package option dictates the final Go package, and may have no relationship with the proto file layout (nor the proto package itself). I recommend that they stay related, though.

If:

• a/red.proto and a/green.proto are generated into the same Go package, and
• b/red.proto and b/green.proto are generated into the same Go package
  then I agree this results that this is problematic and that source_relative as a default doesn't help.

With the go_package option (which I suggest every proto file specify), this could be avoided by having the following Go package organization:

• Each .proto file generates into its own Go package

or

• a/red.proto and b/green.proto belong to one Go package
• a/green.proto and b/red.proto belong to a different Go package

then this works. However, both Go package layouts would not work with source_relative, since the resulting .pb.go files need to be generating into a directory that is different from the source .proto files.

@zellyn, I'm curious. At a high-level, what modifications to protoc-gen-go did you do?

[zellyn]

Yes, you're describing the problem exactly, and it appears source_relative works as I understood it from the name. We use go_package precisely to avoid this problem.

As for modifications… we forked both protoc-gen-go fairly early on, which led to people assuming a freedom to make changes that has made unforking difficult. I'm almost the whole way there. Ignoring ill-thought changes, there are two big/unavoidable ones:

• placement of packages. This might have been before the go_package option existed, or maybe after it existed but before it could affect the output location of generated code. We modified our protoc-gen-go to place files based on the protobuf package. I only recently managed to get rid of this (I've been on a Java-only team for a couple of years in the interim) by writing a small Go tool that writes explicit go_package directives into all the .proto files before generating. Effectively, the sed solution :-)
• The other big thing is redaction of sensitive fields. Once I finish my current stream of unforking work, we'll be on master of github.com/golang/protobuf, with only one additional small patch. I have a proposal to upstream sensitive fields, and everyone seems open, but I've been told it's likely to be too intrusive to land. I intend (at some point) so write clean patches against the main runtimes (protoc, C++, Java, Go) implementing redaction, to prove it's not intrusive, but that hasn't been a high priority so far.

[zellyn]

One last random thought: that second bullet point (my current arc of unforking work) has been slow because I've been fixing every single place people use reflect.DeepEqual in tests. When they do it directly on protos, it's a simple fix, but the maps of maps of structs that have proto fields are less fun. Adding a proto-aware version of reflect.DeepEqual somewhere (in the proto package?) would be fantastic. If it did the right thing with time.Time too, even better.

Edit: it looks like github.com/google/go-cmp/cmp or a wrapper over that that adds a protobuf comparer might be what I'm looking for.

[rhass99]

New year's update:

1. The v2 proto implementation (currently on track to release later this year) will completely fix this issue. It includes a rewritten protoc-gen-go which has no restrictions on what .proto source files can be simultaneously compiled. (You can find this version of protoc-gen-go on the api-v1 and api-v2 branches of this repository. Note that these branches are unreleased and subject to change without notice.)
2. While the currently released protoc-gen-go doesn't let you do protoc --go_out=. **/*.proto, you can get the (probable) desired effect with for x in **/*.proto; do protoc --go_out=paths=source_relative:. $x; done. If you have cases where this doesn't produce the desired results, I'd very much like to hear about them.
3. While not specifically about this issue, as a general rule I strongly recommend always passing the paths=source_relative option to the compiler (e.g., protoc --go_out=paths=source_relative:. foo.proto) and setting option go_package = "full/import/path/of/package"; in every .proto source file. Following these rules resolve most problems with generation that I've seen.

How do I add the plugins=grpc to protoc --go_out=paths=source_relative:. foo.proto

[neild]

How do I add the plugins=grpc to protoc --go_out=paths=source_relative:. foo.proto

Everything before the : is a comma-separated list of options, so:

--go_out=plugins=grpc,paths=source_relative:.

[rhass99]

How do I add the plugins=grpc to protoc --go_out=paths=source_relative:. foo.proto

Everything before the : is a comma-separated list of options, so:

--go_out=plugins=grpc,paths=source_relative:.

Thanks alot, works like a charm

[dmarkhas]

Any updates on this?

[dsnet]

This should be fixed in the v1.20.0 release.