Proposal: Overloading option go_package to indicate Go import path

[dsymonds]

Right now, a .proto file can have a line like

option go_package = "foo";

which means that the generated .pb.go file should have a package foo statement.

I am proposing to overload this option with two new possible syntaxes:

option go_package = "github.com/example/foo";

This means that generated code should be dropped into the github.com/example/foo directory (relative to whatever is passed to protoc's --go_out flag), overriding the default behaviour of matching the path to the .proto file. It also means the .pb.go file should have a package foo statement.

option go_package = "github.com/example/foo;bar";

This means the same as the previous syntax, but additionally means that the .pb.go file should have a package bar statement, for the rare cases where people want the package name to differ from the final path component of the import path.

This would hopefully mean that far fewer M parameters need to be passed through the --go_out flag. It would subsume #137 and maybe #63.

Thoughts? Opinions?

@robpike @zellyn @mwitkow @awalterschulze @peter-edge @tamird

[bufdev]

+1 support this

[dsymonds]

One extra thing I forgot to say. This would permit passing --go_out=$GOPATH/src to protoc and have a lot of things Just Work.

[bufdev]

Ya this would solve a lot of problems I have had on a mental todo list,
this is a really good idea

[albertog]

Make sense.

[awalterschulze]

I am cautiously optimistic :)
I can't think of anything that this change will break and I think its a definite improvement.

[zellyn]

I am +1000 on being able to specify output location using go_package.

Personally, I would just use periods: since they're currently replaced with underscores, it'll break only people who were using periods and trusting they'd get replaced with underscores, and those people can just use underscores now.

In fact, I'd go further, and use package as the default value for go_package the way java does. I'm guessing there's about a 0.0001% chance you like that idea though. :-)

[zellyn]

One other note: while making this change, it might be the right time to ask whether import_prefix should be blindly applied to both proto imports and regular imports: it seems very likely you'd want to put all your protos in a "protos" subdirectory. (At Square, they go in squareup/protos.) It seems very unlikely you'd also want to import, say, grpc or the proto helper from there.

[tamird]

This is not universally true, please don't do this.
github.com/cockroachdb/cockroach has protos in many different packages and
they are almost never in a "protos" directory.

On Wed, Mar 2, 2016 at 9:20 AM, Zellyn Hunter [email protected]
wrote:

One other note: while making this change, it might be the right time to
ask whether import_prefix should be blindly applied to both proto imports
and regular imports: it seems very likely you'd want to put all your protos
in a "protos" subdirectory. (At Square, they go in squareup/protos.) It
seems very unlikely you'd also want to import, say, grpc or the proto
helper from there.

—
Reply to this email directly or view it on GitHub
#139 (comment).

[zellyn]

@tamird I'm guessing you just never use import_prefix right?

[tamird]

https://github.com/cockroachdb/cockroach/blob/master/build/protobuf.mk#L61

On Wed, Mar 2, 2016 at 9:33 AM, Zellyn Hunter [email protected]
wrote:

@tamird https://github.com/tamird I'm guessing you just never use
import_prefix right?

—
Reply to this email directly or view it on GitHub
#139 (comment).

[zellyn]

@tamird Oh interesting. What do you do about this? https://github.com/golang/protobuf/blob/552c7b9542/protoc-gen-go/generator/generator.go#L1279

And this?
https://github.com/golang/protobuf/blob/552c7b9542/protoc-gen-go/internal/grpc/grpc.go#L119

[zellyn]

I think I'm taking this proposal off-topic, though. I don't want to detract from my +1000 on being able to place output files using go_package - this is an absolute requirement for anyone trying to use a large body of existing protos with Go.

[tamird]

@zellyn lots of sed.

[zellyn]

@tamird o_O

Then again, the protoc wrapper I'm writing has a sed-like stage :-(

[zellyn]

@tamird I feel like the fact you use sed is actually evidence that the plugin doesn't do what it should…

[dsymonds]

I'm not changing how import_prefix operates, at least not with this proposal.

@zellyn: The proto package is already the default for the generated package name. The go_package option overrides it. See (*FileDescriptor).goPackageName (generator.go, around line 272).

I was anticipating identifying the new form of go_package by the presence of a /. Does anyone see an issue with that?

[zellyn]

I understand (all too well :-)) how it works now. And neglected to notice previously that using . won't work because of github.com as a directory name.

LGTM

[dsymonds]

Let me know how it goes.

If everything looks fine, I plan to chase up the google/protobuf/*.proto owners and add relevant go_package lines to point at github.com/golang/protobuf/ptypes/*.

[zellyn]

Fantastic! I'll let you know how this turns out as soon as I can: I first have to add go_package to everything, and also get us into vendor folder instead of godep path rewriting... should be soon though :-)

[carl-mastrangelo]

@dsymonds : Could you give an example of what the canonical protos would look like (any.proto, descriptor.proto, duration.proto, etc.) with this?

[dsymonds]

descriptor.proto isn't such a proto.

But I hope to change duration.proto to have a line saying

option go_package = "github.com/golang/protobuf/ptypes/duration";

[awalterschulze]

Well there might be a thing. This would mean I would have to fork the .proto files. Maybe that is ok, since maybe I am going to that anyway.

[carl-mastrangelo]

@dsymonds What is the package for descriptor.proto? Is there a standard way to do reflection?

[dsymonds]

I don't understand your question.

[zellyn]

@dsymonds do we need a separate wkt_import_package then, in case you want the .pb.go files for well-known types to live somewhere else?

[thinxer]

@dsymonds We still don't have go_package lines in google/protobuf/*.proto yet. Is this happening? We need those lines for well-known types to work in Go.

I hope those lines can appear in protobuf 3.0.0 or beta-3.

[drewwells]

Wow this needs to be linked on the README. Went hunting for quite a while to figure out the fix for proto subdirectories. Our original solution was to import the entire GOPATH which was not ideal.

[davidhenrygao]

I think it's better to give the usage of 'go_package' option in the README page.
Or link this issue to the README page.