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

Go Modules #226

Closed
flicaflow opened this issue Jul 25, 2018 · 12 comments
Closed

Go Modules #226

flicaflow opened this issue Jul 25, 2018 · 12 comments
Labels
enhancement New feature or request v0.8 For release in v0.8

Comments

@flicaflow
Copy link

This morning I tried to convert our project to use go modules instead of dep.
I found that gqlgen makes this currently impossible, hence I take this opportunity to document my findings here.

The first minor annoyance is the tags in gqlgen are not recognized as version tags for go modules. This can be easily fixed by adding new version tags or renaming the existing ons ("0.3.0" -> "v0.3.0").

Using the gqlgen code generator outside the GOPATH in a module is currently not working. This is because gqlgen uses go/build to retrieve package information. go/build is currently not go modules aware.
There is an issue golang/go#26504 and a CL https://go-review.googlesource.com/c/go/+/125296 trying to fix this. It is also mentioned that ideally users should use https://godoc.org/golang.org/x/tools/go/packages instead, which however is not finalized and depends on go1.11.

So where does this leave us. I would recommend to switch the version tags to the go module format rather sooner than later. If something depends on the current format, tagging with both is an option.

For the second problem I think we should just wait for the go/build fix and see if it works for us. Switching to go/packages seems to restricting in my point of view.

@mtibben
Copy link
Member

mtibben commented Aug 10, 2018

Yep in order to future-proof this project for Go modules, we'll want to start using a leading v for version tags.

The leading v is required
- https://research.swtch.com/vgo-module

Go modules must be semantically versioned in the form v(major).(minor).(patch)
- https://github.com/golang/go/wiki/Modules

@flicaflow
Copy link
Author

flicaflow commented Aug 14, 2018

Update:
I got one step further by using go/packages instead of go/build. This is now working with go1.11rc1.
I have a fork here https://github.com/flicaflow/gqlgen/tree/go-modules

However the next blocker is the use of the x/tools/go/loader package which is not go-modules aware in the moment (see for a similar issue https://github.com/google/go-cloud/issues/78). The only issue I could find discussing the loader package golang/go#14120 is just mentioning module support.

@vvakame
Copy link
Collaborator

vvakame commented Sep 6, 2018

I tried to implement this issue but it is too painful...
gqlgen should process about 1. absolute path, 2. from $GOPATH, 3. from vendor/, 4. from go modules .
golang.org/x/tools/go/packages can't handles vendor/ with default gopackagesdriver, and go/build can't handle go modules.

@adamgoose
Copy link

My temporary solution to this is to clone my repository into the correct $GOPATH directory, and then tell my IDE to use GO111MODULES=on. This has allowed gqlgen to detect the correct import path even when using go modules.

@adamgoose
Copy link

A symlink actually works better, so I can have my code outside of the $GOPATH and simply cd into $GOPATH/... to run gqlgen.

Furthermore, if you are using VS Code for your terminal, see microsoft/vscode-go#1532 (comment).

@vektah
Copy link
Collaborator

vektah commented Oct 17, 2018

Just a quick update, It looks like the big warnings have been removed from golang.org/x/tools/go/packages, but its still a lot of work

As you can see from this list, uptake on tools that are doing static analysis has been slow.

@marcotuna
Copy link

I face the same issue today, I am using Go 1.11 with Go Modules, I had to do a symlink and do some more hackish things in order to use this. Is this library going to support Go Modules? Thanks.

@jhayotte
Copy link

Any update about this enhancement that starts to be mandatory?

@mathewbyrne
Copy link
Contributor

@jhayotte you can see from a few days ago we're planning for Go modules support prior to a 1.0 release. As has been mentioned a few times now, this requires some significant internal refactors that we will be undertaking soon. Any updates will be made here.

@atombender
Copy link

atombender commented Jan 22, 2019

go/packages actually works fine with Go 1.10 (contrary to the original issue comment which says 1.11 is required), and also works with pre-modules code.

I've converted two projects — go-sumtype and Mockery — and while it wasn't fun, the differences are mainly in the loading interface. Previously, you had to have a lot of loading code where you typically had to load each file individually, but with the new library you can just do packages.Load(&cfg, "file=" + fileName) and it will return all the related files for you, including syntax and type information. So in a way it's actually simpler. It might seem daunting at first, but the new library uses many of the same data types as the old one (e.g. go/types).

@genert
Copy link

genert commented Jan 31, 2019

Great work so far with next branch.

How can I contribute to 1.0 Release?

@mathewbyrne
Copy link
Contributor

@genert I'd say our current biggest class of assistance required is bug fixes, however the next branch has a lot of big refactors on it (and at least 1 more coming) that will probably affect fixes. So for now, I'd recommend wait until an 0.8 release in the coming few weeks, then if you're keen to contribute have a look at reported bugs.

@vektah vektah added the v0.8 For release in v0.8 label Feb 7, 2019
@vektah vektah closed this as completed Feb 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request v0.8 For release in v0.8
Projects
None yet
Development

No branches or pull requests

10 participants