Support go modules

[kasuboski]

Steps you follow to reproduce the error:

1. Using go1.11 outside of go path
2. Run go get -u github.com/grpc-ecosystem/grpc-gateway/protoc-gen-grpc-gateway

Returns

go: finding github.com/grpc-ecosystem/grpc-gateway/protoc-gen-grpc-gateway latest

# github.com/grpc-ecosystem/grpc-gateway/protoc-gen-grpc-gateway/descriptor

../../go/pkg/mod/github.com/grpc-ecosystem/grpc-gateway@v1.5.0/protoc-gen-grpc-gateway/descriptor/services.go:146:49: opts.ResponseBody undefined (type *annotations.HttpRule has no field or method ResponseBody)
-->

What did you expect to happen instead:
Successfully added to mod file

It seems like it's able to find the latest version 1.5.0, but then isn't up to date? The error that's output seems to be what was fixed with #731

[achew22]

Hey @kasuboski, thanks so much for your interest! We've been talking about doing this migration but no one has the time to do it for a while. Sounds like you're blazing a new path forward in the golang world, would you have any interest in generating anew, or migrating our existing gopkg file? That might be the quickest way to get to this working if that's your first priority. Let me know.

[kasuboski]

I looked into it today. It seems like the go modules don't like having the top level dependencies like is captured with the Gopkg.lock in the root. Let me know if I misunderstood how it's currently working.

I'm not going to continue looking into it for now, but might come back to it later.

[johanbrandhorst]

I don't know where you read that, go modules have their own lock file esque go.mod and go.sum at the root. There's even a way to convert from dep files to go modules files.

[kasuboski]

Yeah I didn't read it...I ran the conversion on the repo and got the module file well enough, but then had issues with the build, namely it couldn't resolve a package for the module.

Maybe it needs multiple modules for the different packages. I had tried that as well, but had issues with it still trying to download the source from github instead of using the higher level module.

My experience with it has only been with much simpler projects so I'm definitely interested to see how this works.

[johanbrandhorst]

I don't think we should need multiple modules, we'll definitely try to have a single root module and only resort to multiple if we absolutely need to. What error did you get?

[kasuboski]

The error I get is

can't load package: package github.com/grpc-ecosystem/grpc-gateway: unknown import path "github.com/grpc-ecosystem/grpc-gateway": cannot find module providing package github.com/grpc-ecosystem/grpc-gateway

go.mod at the root

module github.com/grpc-ecosystem/grpc-gateway

require (
	github.com/ghodss/yaml v1.0.0
	github.com/go-resty/resty v0.0.0-20180405024425-f8815663de1e
	github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b
	github.com/golang/protobuf v1.1.0
	github.com/rogpeppe/fastuuid v0.0.0-20150106093220-6724a57986af
	golang.org/x/net v0.0.0-20180502164142-640f4622ab69
	golang.org/x/text v0.3.0
	google.golang.org/genproto v0.0.0-20180808183934-383e8b2c3b9e
	google.golang.org/grpc v1.11.3
	gopkg.in/yaml.v2 v2.0.0-20170812160011-eb3733d160e7
)

[ogimenezb]

I have generated the same mod file and can build both.

Where are you having the problem?

[achew22]

@ogimenezb If you generated a go mod file for grpc-gateway, would you be willing to upload it as a PR?

[kasuboski]

Yeah it worked when I tried it now...must have had some old files somewhere.

[ogimenezb]

Yep, the only thing is the makefile will be broken becase it relays on GOPATH. Only if go 1.11 and not working from GOPATH.

Have tried a couple of things, but still no luck.

[timoohr]

Some additional observations while trying to make the repo go gettable again:

When calling go get on a migrated version of gprc-gateway the original issue (#731) pops back up again. For some reason go seems adamant to resolve to an older version of google.golang.org/genproto. Was able to fix this by pinning it with a replace in the mod file:

replace google.golang.org/genproto => google.golang.org/genproto v0.0.0-20181101192439-c830210a61df

There also seems to be an issue with resty:

go: github.com/go-resty/resty@v1.10.1: parsing go.mod: unexpected module path "gopkg.in/resty.v1" 

I suspect this might be an issue with their go.mod file, since they specified the module path as gopkg.in/resty.v1 instead of gopkg.in/resty. Was able to circumvent this by removing the dependency, but this will obviously break the examples.

[johanbrandhorst]

The resty dependency issue has been discussed before, are you sure it's an issue with their repo and not just us relying on an old version of go-swagger? If there definitely is an issue with resty, obviously please raise it with them.

[johanbrandhorst]

See #254 for more discussion on using go-swagger.

[timoohr]

Yes, I think you may be right.

I think the issue is that go mod requires modules to have the major version in their module/import path, except for v0 and v1, which must not appear in the path, except for modules hosted on gopkg.in, where this is allowed (and in a slightly different format, as well). (source)

So it would seem like resty needs to be imported from gopkg.in for go mod to work, since they specified a v1 version in their module path (or may be it's just because their module path differs from the import path in general).

Not sure about go-swagger, but in terms of go getting protoc-gen-grpc-gateway and protoc-gen-swagger replacing the resty github imports in the examples with gopkg.in imports seems to suffice.

[hexfusion]

@johanbrandhorst am I correct that you need help replacing swagger-codegen in order to move to go modules? I am motivated to get this done please advise.

[johanbrandhorst]

I don't think replacing swagger codegen is a hard requirement for the modules work. Please attempt a switch and let me know if you run into any problems, as has been stated we're keen the get this done.

[johanbrandhorst]

Something we do want to consider at the same time though is looking into creating the Bazel dependency versions from the go.sum file; @drigz @achew22 ought to be able to assist in that effort.

[achew22]

@johanbrandhorst, there is no extra magic that is necessary to achieve that outcome. The integration that we already did with Gazelle should handle that.

[johanbrandhorst]

Might be relevant: https://www.youtube.com/watch?v=ms5l0zxC-uM

[hexfusion]

Great link, and I must say the go mod has created a nightmare. The video is very well done but for some projects jumping a major version is simply not feasible for a tooling change. Saying that this project is blocking my ability to use go mod effectively. Basically the codegen creates the example files with an improper path.

import "gopkg.in/resty.v1"
https://github.com/go-resty/resty#usage

vs

grpc-gateway/examples/clients/abe/api_client.go

Line 21 in ee3ef70

"github.com/go-resty/resty"

which I believe is the cause of this

go: github.com/go-resty/resty@v1.11.0: parsing go.mod: unexpected module path "gopkg.in/resty.v1"
go: error loading module requirements

Would you accept as a solution of post processing on these example files with sed or ? so they use the correct import path? I just don't see it realistic to remove codegen for this reason alone as it is not trivial. If there is another way around this please let me know.

[johanbrandhorst]

Resty is known to cause problems, I think we'd rather replace that generator than mess with some post processing step. Do you not think this would be solved by implementing modules in this repo? I don't understand the module system myself yet but we want to move this repo to use modules ASAP for other reasons as well.

[hexfusion]

Do you not think this would be solved by implementing modules in this repo?

No it seems the issues with resty via codegen is the core of the problem I am facing.

[johanbrandhorst]

Couldn't it be remedied by this repository explicitly choosing the resty version to use?

[hexfusion]

Honestly I am not sure.

[the-destro]

Also one question I have is how the include is supposed to work when we compile the protobuf files via protoc?

[johanbrandhorst]

I don't think we can declare proto file or protoc dependencies with go modules.

[the-destro]

I don't think we can declare proto file or protoc dependencies with go modules.

hmm, makes things tricky especially given the instructions from the README:

protoc -I/usr/local/include -I.
-I$GOPATH/src
-I$GOPATH/src/github.com/grpc-ecosystem/grpc-gateway/third_party/googleapis
--plugin=protoc-gen-grpc=grpc_ruby_plugin
--grpc-ruby_out=.
path/to/your/service.proto

If there is no GOPATH how do we run that, or how should we run that

[johanbrandhorst]

Modules don't mean you can't have a GOPATH, but I don't think it's necessary here anyway. We can assume a vendor directory for these instructions for example.

[izumin5210]

If there is no GOPATH how do we run that, or how should we run that

Modules don't mean you can't have a GOPATH, but I don't think it's necessary here anyway. We can assume a vendor directory for these instructions for example.

How about go list -m -f "{{.Dir}}'？

:) % ls $(go list -m -f "{{.Dir}}" github.com/grpc-ecosystem/grpc-gateway)
BUILD                   CONTRIBUTING.md         Gopkg.toml              LICENSE.txt             README.md               bin                     docs                    internal                protoc-gen-swagger      third_party
CHANGELOG.md            Gopkg.lock              ISSUE_TEMPLATE.md       Makefile                WORKSPACE               codegenerator           examples                protoc-gen-grpc-gateway runtime                 utilities

[the-destro]

Aye, I think this is mainly a documentation issue. I like @izumin5210 's answer and can submit a PR to update the docs. Is this something that we want to encourage users to do though? Hence the second half of my question, the should part :)

[johanbrandhorst]

This issue is concerned with changing our dependency management to use dep, which would presumably include documentation updates, but is not exclusive to documentation updates. We'd very much welcome a PR that would do both, though.