protobuild icon indicating copy to clipboard operation
protobuild copied to clipboard

Support Go modules

Open stevvooe opened this issue 5 years ago • 5 comments

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

stevvooe avatar Apr 13 '19 00:04 stevvooe

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 https://github.com/golang/go/issues/26366#issuecomment-405683150). 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/[email protected]",
	"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/[email protected]. 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 avatar Jul 31 '19 19:07 stevvooe

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!

stevvooe avatar Jan 30 '20 03:01 stevvooe

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

rsc avatar Jan 30 '20 14:01 rsc

@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 avatar Jan 31 '20 20:01 stevvooe

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

stevvooe avatar Apr 14 '20 17:04 stevvooe