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

Support Go modules #27

Open
stevvooe opened this issue Apr 13, 2019 · 5 comments · Fixed by #29
Open

Support Go modules #27

stevvooe opened this issue Apr 13, 2019 · 5 comments · Fixed by #29

Comments

@stevvooe
Copy link
Member

It might be time to start considering support for Go modules. Suggestions and support are welcome.

@stevvooe
Copy link
Member Author

Ok, the chief problem with supporting go modules is that, even with vendoring enabled, non-Go files and non-Go package dependencies are not included in the vendored code (we can see this from @rsc's disappointing response in golang/go#26366 (comment)). The solution here will be to add another "include" implementation that searches through checked out, unmodified modules to find protobuf files and their on-disk locations to the include path.

It looks like we can do this using the following command:

go mod download -json

The above will output json records in this format:

{
	"Path": "k8s.io/utils",
	"Version": "v0.0.0-20190607212802-c55fbcfc754a",
	"Info": "/Users/sday/go/pkg/mod/cache/download/k8s.io/utils/@v/v0.0.0-20190607212802-c55fbcfc754a.info",
	"GoMod": "/Users/sday/go/pkg/mod/cache/download/k8s.io/utils/@v/v0.0.0-20190607212802-c55fbcfc754a.mod",
	"Zip": "/Users/sday/go/pkg/mod/cache/download/k8s.io/utils/@v/v0.0.0-20190607212802-c55fbcfc754a.zip",
	"Dir": "/Users/sday/go/pkg/mod/k8s.io/utils@v0.0.0-20190607212802-c55fbcfc754a",
	"Sum": "h1:2jUDc9gJja832Ftp+QbDV0tVhQHMISFn01els+2ZAcw=",
	"GoModSum": "h1:sZAwmy6armz5eXlNoLmJcl4F1QuKu7sr+mFQ0byX7Ew="
}

This solves the problem of where these files will be on disk but it doesn't solve the issue of mapping proto imports to Go package names, which is the key benefit from protobuild.

It seems like we'd also have to make temporary set of links with the go module paths that link out to the expanded version names to setup the correct import paths. For the example above (which doesn't have protobufs, so its contrived) would create a directory k8s.io/utils with a link back to /Users/sday/go/pkg/mod/k8s.io/utils@v0.0.0-20190607212802-c55fbcfc754a. We'd then manufacture this include path with links during compilation time and pass that to the protobuf compiler.

This doesn't seem like a hack and may actually be a decent improvement if we can make it work out of tree.

@stevvooe
Copy link
Member Author

Ok, looking into this a bit more now to make it work:

  1. go mod vendor now includes non-Go files, so we don't need to do this crazy plan.
  2. The "vendored" section of the config file works fine with no code modifications in protobuild.
  3. Make still want to call go mod vendor with protobuild to make it work for all cases.

Super glad I don't have to implement the craziness above!

@rsc
Copy link

rsc commented Jan 30, 2020

go mod vendor has always included non-Go files.
What it doesn't include (see my "disappointing response") is subdirectories.

@stevvooe
Copy link
Member Author

@rsc Sorry that I said it was disappointing (please don't take it personally ;) ), but we need a way to be able to bring in non-Go directories. There are a lot of ways to organize protobufs, so being opinionated on the matter makes it challenging to implement a solution without creating a full package manager for protobufs (which, maybe, we need).

Do you know a way I could create this vendor directory then clean it up as part of the build without polluting the user's source directory? I may still have to build a temporary directory to manage the output of protoc, which just outputs to $GOPATH/src today.

My approach right now is to have a tmpdir with the following:

tmpdir/
  output/
    <module name as path> # outputs this way due to protoc prefix
  include/ # assembled include path of protobufs collected from modules

What do you suggest?

@stevvooe
Copy link
Member Author

#29 only made it compatible with go get using go modules. We still need a solution that works with using only modules.

@stevvooe stevvooe reopened this Apr 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants