Improve support for go_package option in .proto files #126
Conversation
Previously, specifying the option go_package caused the generator to write a file to the fully qualified path This would create the full directory tree this doesn't really work as it adds, to your project root dir, the full github.com/blah/... structure This change does two things adds support for the `paths=source_relative` like the google protoc plugin does which ensures we write to whatever directory the .proto file lives in secondly, it makes sure to use whatever go_package says when importing the module. It was generating imports with relative dirs from the project root With these two changes, you can use twirp with go 1.11's modules now
|
This sounds great, I totally agree that it is needed. I’m on vacation but should be able to take a closer look on Monday or Tuesday. Thanks very much. |
protoc-gen-twirp/generator.go
Outdated
| @@ -47,6 +47,9 @@ type twirp struct { | |||
| importPrefix string // String to prefix to imported package file names. | |||
| importMap map[string]string // Mapping from .proto file name to import path. | |||
|
|
|||
| // Package output: | |||
| paths string // instruction on where to write output files | |||
There was a problem hiding this comment.
Let's make this sourceRelativePaths bool instead of a string with a particular value.
There was a problem hiding this comment.
Done, makes sense. Not sure what I was thinking on that initial commit.
Initial commit was threading the string value through the system Now, we just interperet the option up front and use as a bool in later parts
|
On a total side note, if you wanted to use twirp with a go module based project (that doesn't live in GOPATH), adding:
to go.mod makes your project use the fork. I DO NOT suggest folks actually build against my fork as I'm not intending to maintain the fork past this pull request. I'm just posting it merely to unstick anyone who's currently stuck, or in case folks are trying to figure out in general how to override deps. |
|
Just checking in, was the bool change what you were expecting? |
|
@dbudworth this looks great, I was just away from github for a few days. Thanks! |
Previously, specifying the option go_package caused the generator
to write a file to the fully qualified path
This would create the full directory tree
this doesn't really work as it adds, to your project root dir, the full github.com/blah/...
structure
This change does two things
adds support for the
paths=source_relativelike the google protoc plugin doeswhich ensures we write to whatever directory the .proto file lives in
secondly, it makes sure to use whatever go_package says when importing the module.
It was generating imports with relative dirs from the project root
With these two changes, you can use twirp with go 1.11's modules now
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.