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

protoc-gen-go: better support for controlling output filenames #515

Closed
neild opened this issue Feb 15, 2018 · 15 comments
Closed

protoc-gen-go: better support for controlling output filenames #515

neild opened this issue Feb 15, 2018 · 15 comments

Comments

@neild
Copy link
Contributor

neild commented Feb 15, 2018

Background

There are lots of options controlling the package name and/out filename of generated files. Most of them are some combination of not useful or difficult to use correctly. Some of this is a documentation problem, some of this is not.

Related issues, probably not exhaustive: #39, #63, #64, #167, #181, #239, #240

When processing a .proto file, we need to know several things:

The Go package name

Needed so we can write a package foo line in the generated code. Set by the go_package option if there is one, otherwise derived from the proto package name. This isn't usually a problem.

You can also pass --go_out=import_path=x to specify a package name. This is strictly worse than specifying the name in the source file. Note that despite the name, this doesn't set the import path--only the package name. (This is relevant when generating output files; see below.)

The Go import path

Needed so we can write import statements when importing this source file. Set by the go_package option if there is one, otherwise guessed by some process I'm not bothering to look up right now.

I'm going to make a possibly controversial assertion here: Every .proto file that is used to generate Go code should have a go_package option that specifies the import path. It's too late to change now, but we really should just refuse to compile anything that doesn't have a go_package option. It's better than trying to guess and much better than taking a flag on the command line.

You can pass --go_out=M=foo.proto=import/path to specify the import path associated with a particular source file. I cannot imagine ever wanting to do this instead of including a go_package option. Nobody should ever need to do this, except possibly for authors of build systems in some strange cases.

The output file

This is where things get interesting.

Right now, protoc-gen-go assumes that it's been called with --go_out=$GOPATH/src, and writes output files into the subdirectory corresponding to the generated package's import path. So if you're compiling foo.proto which contains a go_package option saying that the import path is github.com/neild/quux, the generated file goes in github.com/neild/quux/foo.pb.go.

You might think that if you passed --go_out=import_path=some/import/path:. foo.proto the output file would go in some/import/path/foo.pb.go, but no: it goes in foo.pb.go instead. That's probably a bug, but it's not really very important (I think) because there's no reason anyone should ever need to specify the import path on the command line--just put it in a go_package option.

This all works some of the time, but it breaks down if you don't want to put the generated file in a canonical location relative to $GOPATH. What if you're in a vendor directory, for example?

Proposal(s)

I'm in some directory I have foo.proto, the source file contains option go_package = "github.com/neild/mypackage";. I want to generate foo.pb.go in the current directory. Here are a few possible options.

Strip a prefix from the path of generated files

protoc --go_out=trim_import_prefix=github.com/neild/mypackage:. foo.proto

The trim_output_prefix flag will remove the provided prefix from any generated files. It will generate an error if any files don't start with that prefix.

Generate files in the current directory

protoc --go_out=flat:. foo.proto

Compared to the previous option, this has the advantage of not requiring the protoc line to contain any reference to the import path if the .proto files are in the same directory that the Go code should go into.

This will not work if you're compiling more than one package at a time. You can't do that today, but we could (and probably should) relax that restriction (#39, #240).

I don't really like this option; it's really easy in the simple case (source and output file in the same directory) but I don't think it generalizes as well.

RFC

Would either of these features be useful to you? Both? Neither? Something else entirely? Tell me what you think!

@neild
Copy link
Contributor Author

neild commented Feb 16, 2018

Another thought I just had:

protoc --go_out=foo.go foo.proto

If the output path ends in .go and there is exactly one input file, treat the output path as a filename rather than a directory. This would integrate really cleanly with Makefiles:

%.pb.go: %.proto
  protoc --go_out=$@ $<

@neild
Copy link
Contributor Author

neild commented Feb 16, 2018

Another related issue: #464

@neild
Copy link
Contributor Author

neild commented Feb 16, 2018

If the output path ends in .go and there is exactly one input file, treat the output path as a filename rather than a directory.

Upon further reflection, probably a bad idea: This doesn't really work with multiple source files in the same Go package, since we require that you compile all the inputs to a package in a single action.

It might be nice if you could compile packages one file at time, but that would require much more significant changes. (You'd need to figure out which file gets the package doc comment, figure out how to avoid conflicts in the file descriptor var, probably some other things. Possible, but not simple.)

@dsnet
Copy link
Member

dsnet commented Feb 20, 2018

\cc @awalterschulze @xfxyjwf

@awalterschulze
Copy link

I find import paths very hard to reason about and I have fewer problems with output paths than our users, so I would prefer to them on this issue:
@alecthomas @ashishnegi @theobat @mwitkow @colinmarc @roadrunner @jacovieASAPP
What do you think?

But here is one personal bad experience I had.
I want to generate all the well known types in the same folder:

https://github.com/gogo/protobuf/blob/master/protobuf/Makefile

  1. I have to run this Makefile from a different folder than were the generated files end up, which makes it a bit harder to follow the generation logic.
  2. I have to "sed" all the go_package lines.
  3. I have to make sure to generate all these files in a single protoc call, otherwise the generated go code ends up importing the package that it is generated in, which causes an import cycle. That took a very long time on the backburner to figure out.

I had a similar problem trying to create a genprotos clone for gogoprotobuf
https://github.com/gogo/googleapis/blob/master/Makefile
Again it works, but the Makefile is nothing that screams with the ease of use of go.

@neild
Copy link
Contributor Author

neild commented Feb 20, 2018

After playing around with this some more, I think that we should probably follow the approach used by the C++ compiler (and probably others): The output filename is generated by replacing the .proto suffix with .pb.go. foo.proto is foo.pb.go, path/to/bar.proto is path/to/bar.pb.go, etc.

I was worried about names like ../foo.proto, but protoc already handles these in a reasonable way:

$ protoc --cpp_out=out ../foo.proto
../foo.proto: File does not reside within any path specified using --proto_path (or -I).  You must specify a --proto_path which encompasses this file.  Note that the proto_path must be an exact prefix of the .proto file names -- protoc is too dumb to figure out when two paths (e.g. absolute and relative) are equivalent (it's harder than you think).
$ protoc --cpp_out=out -I.. ../foo.proto
$ ls out
foo.pb.cc  foo.pb.h

We can't just change the default behavior without breaking existing build systems that rely on it, so the main question I have is how to opt in to this new behavior. protoc --go_out=relative:.?

@dsnet
Copy link
Member

dsnet commented Feb 21, 2018

\cc @zombiezen @cybrcodr , as I vaguely recall that Google Cloud APIs had some issues with go_package in the past as well.

@zombiezen
Copy link
Contributor

I don't think the issues we had are pertinent here.

neild added a commit to neild/protobuf that referenced this issue Feb 26, 2018
When --go_out=paths=source_relative:., output filenames are always
derived from the input filenames. For example, the output file
for a/b/c.proto will always be a/b/c.pb.go, regardless of the
presence or absence of a go_package option in the input file.

Fixes golang#515.

Change-Id: I7483449e68f65757bc459233b1644636b98885b9
@cybrcodr
Copy link
Contributor

Just a side note. If you specify go_package with just the package name w/o path, an invocation of

protoc -I. --go_out=. acme.com/bar/foo.proto

will produce foo.pb.go under acme.com/bar/.

You get the same effect if you wanted to run protoc under the same directory as foo.proto with ...

protoc -I../../ --go_out=../../ foo.proto

@neild
Copy link
Contributor Author

neild commented Feb 27, 2018

This is true, but you really should always specify the full import path in go_package so that import statements for files which import the package are correct.

@cybrcodr
Copy link
Contributor

Agree. I mentioned that merely as a note to your proposed option of having the additional trim_import_prefix or trim_output_prefix options, which I'm not seeing much value for since one would typically want the generated source files to be under a directory structure that contains the go_package path.

@neild
Copy link
Contributor Author

neild commented Feb 27, 2018

Good point. I'm pretty convinced now that the right answer is to have some way to restore the behavior you get without an import path in the go_package option--just s/\.proto$/.pb.go/ to produce the output filename.

neild added a commit to neild/protobuf that referenced this issue Mar 2, 2018
When --go_out=paths=source_relative:., output filenames are always
derived from the input filenames. For example, the output file
for a/b/c.proto will always be a/b/c.pb.go, regardless of the
presence or absence of a go_package option in the input file.

Fixes golang#515.

Change-Id: I7483449e68f65757bc459233b1644636b98885b9
neild added a commit to neild/protobuf that referenced this issue Mar 3, 2018
When --go_out=paths=source_relative:., output filenames are always
derived from the input filenames. For example, the output file
for a/b/c.proto will always be a/b/c.pb.go, regardless of the
presence or absence of a go_package option in the input file.

Fixes golang#515.
neild added a commit that referenced this issue Mar 6, 2018
When --go_out=paths=source_relative:., output filenames are always
derived from the input filenames. For example, the output file
for a/b/c.proto will always be a/b/c.pb.go, regardless of the
presence or absence of a go_package option in the input file.

Fixes #515.
@neild
Copy link
Contributor Author

neild commented Mar 6, 2018

You can now pass the compiler option paths=source_relative to get output filenames based strictly on the source filenames. e.g., the following will produce path/to/foo.pb.go regardless of what the go_package option in path/to/foo.proto is.

protoc --go_out=paths=source_relative:. path/to/foo.proto

@neild neild closed this as completed Mar 6, 2018
izumin5210 added a commit to izumin5210/grpc-gateway that referenced this issue Jul 30, 2018
Make protoc-gen-grpc-gateway support paths option such like golang/protobuf#515.
izumin5210 added a commit to izumin5210/grpc-gateway that referenced this issue Aug 2, 2018
Make protoc-gen-grpc-gateway support paths option such like golang/protobuf#515.
johanbrandhorst pushed a commit to grpc-ecosystem/grpc-gateway that referenced this issue Aug 2, 2018
Make protoc-gen-grpc-gateway support paths option such like golang/protobuf#515.
dmacthedestroyer pushed a commit to dmacthedestroyer/grpc-gateway that referenced this issue Aug 2, 2018
* Add explicit dependency versions (grpc-ecosystem#696)

Version constraints are copied from existing Bazel rules. In the
future, these version upgrades must be performed atomically.

* Add OpenTracing support to docs (grpc-ecosystem#705)

* protoc-gen-swagger: support all well-known wrapper types

There were a few well-known wrapper types missing from
the wkSchemas map. In specific UInt32Value, UInt64Value
and BytesValue. This change handles them (and maps them to
the same swagger types as the non-wrapped values)

This also fixes the mapping of Int64Value. The Int64Value handling
maps the value to a swagger integer. The documentation for
Int64Value indicates that it should be mapped to a JSON
string (also the mapping for normal int64 in protoc-gen-swagger
maps it to a string, so this was inconsistent.)

* Add test case and proposed fix for path component with trailing colon (and string) (grpc-ecosystem#708)

* If a pattern has no verb, match with a verb arg appended to last component

Parsing out a "verb" from the input path is a convenience for
parsing. Whether the colon and string are _actually_ a verb depends on
the matching HTTP rule. This change gives a verb-less pattern a chance
to match a path by assuming the colon+string suffix are a part of the
final component.

Fixes grpc-ecosystem#224

Signed-off-by: James Hamlin <[email protected]>

* Fix up examples

* Make support paths option

Make protoc-gen-grpc-gateway support paths option such like golang/protobuf#515.
@pdmccormick
Copy link

If you're like me and came across this issue in the context of working with gRPC, you can do the following:

protoc --go_out=plugins=grpc,paths=source_relative:. path/to/foo.proto

This will produce ./foo.proto.pb.go. Especially useful in conjunction with Go modules.

bradbeam added a commit to bradbeam/talos that referenced this issue Dec 29, 2019
Came across an issue that mentioned `paths=source_relative` to address
the full go import path generation.

ref: golang/protobuf#515 (comment)
Signed-off-by: Brad Beam <[email protected]>
bradbeam added a commit to bradbeam/talos that referenced this issue Jan 15, 2020
Came across an issue that mentioned `paths=source_relative` to address
the full go import path generation.

ref: golang/protobuf#515 (comment)
Signed-off-by: Brad Beam <[email protected]>
bradbeam added a commit to bradbeam/talos that referenced this issue Jan 22, 2020
Came across an issue that mentioned `paths=source_relative` to address
the full go import path generation.

ref: golang/protobuf#515 (comment)
Signed-off-by: Brad Beam <[email protected]>
bradbeam added a commit to bradbeam/talos that referenced this issue Jan 22, 2020
Came across an issue that mentioned `paths=source_relative` to address
the full go import path generation.

ref: golang/protobuf#515 (comment)
Signed-off-by: Brad Beam <[email protected]>
bradbeam added a commit to bradbeam/talos that referenced this issue Jan 23, 2020
Came across an issue that mentioned `paths=source_relative` to address
the full go import path generation.

ref: golang/protobuf#515 (comment)
Signed-off-by: Brad Beam <[email protected]>
andrewrynhard pushed a commit to siderolabs/talos that referenced this issue Jan 23, 2020
Came across an issue that mentioned `paths=source_relative` to address
the full go import path generation.

ref: golang/protobuf#515 (comment)
Signed-off-by: Brad Beam <[email protected]>
@steadylearner
Copy link

steadylearner commented Mar 19, 2020

I made a minimal example to use go mod with gRPC.

If you want a quick solution you can start from it. The official documenation is outdated so it was difficult to find the solution.

adasari pushed a commit to adasari/grpc-gateway that referenced this issue Apr 9, 2020
Make protoc-gen-grpc-gateway support paths option such like golang/protobuf#515.
@golang golang locked as resolved and limited conversation to collaborators Jun 25, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants