-
Notifications
You must be signed in to change notification settings - Fork 624
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
wire: support loading type-checking with Go modules #10
Comments
The proper approach seems to be to use |
We should be instructing users to run stable things, and more adventurous folks can adapt to use Go modules. I just ran the test suite locally from a `go get` and it compiled and ran fine. Updates #78 Updates #208
This is blocked on golang/go#26509. |
No longer blocked on the go/packages issue, but now blocked on golang.org/cl/127258 which allows modifying the list of files before it hits the type checker. |
The primary motivation is to permit a move to using go/packages instead of go/loader. go/packages runs exclusively by shelling out to the go tool, which precludes use of the in-memory "magic" GOPATH being used up to this point. This has a secondary effect of removing a lot of code to support "magic" GOPATH from the test infrastructure. This is on the whole good, but neccessitated a change in the error scrubbing: since the filenames are no longer fixed, error scrubbing also must remove the leading $GOPATH/src lines. Another related change: since all callers of Generate needed to know the package path in order to write out wire_gen.go (necessitating a find-only import search) and Generate already has this information, Generate now returns this information to the caller. This should further reduce callers' coupling to Wire's load internals. It also eliminates code duplication. This should hopefully shake out any difference in path separators for running on Windows, but I have not tested that yet. Updates #78 Updates google#323
The primary motivation is to permit a move to using go/packages instead of go/loader. go/packages runs exclusively by shelling out to the go tool, which precludes use of the in-memory "magic" GOPATH being used up to this point. This has a secondary effect of removing a lot of code to support "magic" GOPATH from the test infrastructure. This is on the whole good, but necessitated a change in the error scrubbing: since the filenames are no longer fixed, error scrubbing also must remove the leading $GOPATH/src lines. Another related change: since all callers of Generate needed to know the package path in order to write out wire_gen.go (necessitating a find-only import search) and Generate already has this information, Generate now returns this information to the caller. This should further reduce callers' coupling to Wire's load internals. It also eliminates code duplication. This should hopefully shake out any difference in path separators for running on Windows, but I have not tested that yet. Updates #78 Updates #323
Unfortunately, this does come with a ~4x slowdown to Wire, as it is now pulling source for all transitively depended packages, but not trimming comments or function bodies. This is due to limitations with the ParseFile callback in go/packages. This comes with a single semantic change: when performing analysis, Wire will now evaluate everything with the wireinject build tag. I updated the build tags tests accordingly. I deleted the vendoring test, as vendoring becomes obsolete with modules. go/packages now parses comments by default, so now the generated code includes comments for non-injector declarations. Fixes #78
Unfortunately, this does come with a ~4x slowdown to Wire, as it is now pulling source for all transitively depended packages, but not trimming comments or function bodies. This is due to limitations with the ParseFile callback in go/packages. This comes with a single semantic change: when performing analysis, Wire will now evaluate everything with the wireinject build tag. I updated the build tags tests accordingly. Prior to this PR, only the packages directly named by the package patterns would be evaluated with the wireinject build tag. Dependencies would not have the wireinject build tag applied. There isn't a way to selectively apply build tags in go/packages, and there isn't a clear benefit to applying it selectively. Being consistent with other Go tooling provides greater benefit. I deleted the vendoring test, as non-top-level vendoring becomes obsolete with modules. go/packages now parses comments by default, so now the generated code includes comments for non-injector declarations. Fixes #78
We should be instructing users to run stable things, and more adventurous folks can adapt to use Go modules. I just ran the test suite locally from a `go get` and it compiled and ran fine. Updates google/go-cloud#78 Updates google/go-cloud#208
…loud#616) The primary motivation is to permit a move to using go/packages instead of go/loader. go/packages runs exclusively by shelling out to the go tool, which precludes use of the in-memory "magic" GOPATH being used up to this point. This has a secondary effect of removing a lot of code to support "magic" GOPATH from the test infrastructure. This is on the whole good, but necessitated a change in the error scrubbing: since the filenames are no longer fixed, error scrubbing also must remove the leading $GOPATH/src lines. Another related change: since all callers of Generate needed to know the package path in order to write out wire_gen.go (necessitating a find-only import search) and Generate already has this information, Generate now returns this information to the caller. This should further reduce callers' coupling to Wire's load internals. It also eliminates code duplication. This should hopefully shake out any difference in path separators for running on Windows, but I have not tested that yet. Updates google/go-cloud#78 Updates google/go-cloud#323
Unfortunately, this does come with a ~4x slowdown to Wire, as it is now pulling source for all transitively depended packages, but not trimming comments or function bodies. This is due to limitations with the ParseFile callback in go/packages. This comes with a single semantic change: when performing analysis, Wire will now evaluate everything with the wireinject build tag. I updated the build tags tests accordingly. Prior to this PR, only the packages directly named by the package patterns would be evaluated with the wireinject build tag. Dependencies would not have the wireinject build tag applied. There isn't a way to selectively apply build tags in go/packages, and there isn't a clear benefit to applying it selectively. Being consistent with other Go tooling provides greater benefit. I deleted the vendoring test, as non-top-level vendoring becomes obsolete with modules. go/packages now parses comments by default, so now the generated code includes comments for non-injector declarations. Fixes google/go-cloud#78
Wire uses
golang.org/x/tools/go/loader
to get type information, but this does not use Go modules. The current workaround is to runvgo mod -vendor
, but this occasionally requires adding more imports temporarily to pin all the dependencies (especially when usingawscloud.AWS
orgcpcloud.GCP
). There isn't a good way to solve this until Go module tooling hits mainstream, but I'm keeping this issue here to track.The text was updated successfully, but these errors were encountered: