Skip to content

Commit

Permalink
wire/internal/wire: use on-disk GOPATH in generate tests (google/go-c…
Browse files Browse the repository at this point in the history
…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
  • Loading branch information
zombiezen committed Nov 13, 2018
1 parent ab113bf commit 64470a2
Show file tree
Hide file tree
Showing 24 changed files with 301 additions and 367 deletions.
22 changes: 6 additions & 16 deletions cmd/wire/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,12 @@
package main

import (
"context"
"errors"
"fmt"
"go/build"
"go/token"
"go/types"
"io/ioutil"
"os"
"path/filepath"
"reflect"
"sort"
"strconv"
Expand Down Expand Up @@ -74,25 +72,17 @@ func generate(pkg string) error {
if err != nil {
return err
}
pkgInfo, err := build.Default.Import(pkg, wd, build.FindOnly)
if err != nil {
return err
}
out, errs := wire.Generate(&build.Default, wd, pkg)
out, errs := wire.Generate(context.Background(), wd, os.Environ(), pkg)
if len(errs) > 0 {
logErrors(errs)
return errors.New("generate failed")
}
if len(out) == 0 {
if len(out.Content) == 0 {
// No Wire directives, don't write anything.
fmt.Fprintln(os.Stderr, "wire: no injector found for", pkg)
return nil
}
p := filepath.Join(pkgInfo.Dir, "wire_gen.go")
if err := ioutil.WriteFile(p, out, 0666); err != nil {
return err
}
return nil
return out.Commit()
}

// show runs the show subcommand.
Expand All @@ -106,7 +96,7 @@ func show(pkgs ...string) error {
if err != nil {
return err
}
info, errs := wire.Load(&build.Default, wd, pkgs)
info, errs := wire.Load(context.Background(), wd, os.Environ(), pkgs)
if info != nil {
keys := make([]wire.ProviderSetID, 0, len(info.Sets))
for k := range info.Sets {
Expand Down Expand Up @@ -185,7 +175,7 @@ func check(pkgs ...string) error {
if err != nil {
return err
}
_, errs := wire.Load(&build.Default, wd, pkgs)
_, errs := wire.Load(context.Background(), wd, os.Environ(), pkgs)
if len(errs) > 0 {
logErrors(errs)
return errors.New("error loading packages")
Expand Down
68 changes: 58 additions & 10 deletions internal/wire/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package wire

import (
"context"
"errors"
"fmt"
"go/ast"
Expand Down Expand Up @@ -190,11 +191,19 @@ type Value struct {
info *types.Info
}

// Load finds all the provider sets in the given packages, as well as
// the provider sets' transitive dependencies. It may return both errors
// and Info.
func Load(bctx *build.Context, wd string, pkgs []string) (*Info, []error) {
prog, errs := load(bctx, wd, pkgs)
// Load finds all the provider sets in the packages that match the given
// patterns, as well as the provider sets' transitive dependencies. It
// may return both errors and Info. The patterns are defined by the
// underlying build system. For the go tool, this is described at
// https://golang.org/cmd/go/#hdr-Package_lists_and_patterns
//
// wd is the working directory and env is the set of environment
// variables to use when loading the packages specified by patterns. If
// env is nil or empty, it is interpreted as an empty set of variables.
// In case of duplicate environment variables, the last one in the list
// takes precedence.
func Load(ctx context.Context, wd string, env []string, patterns []string) (*Info, []error) {
prog, errs := load(ctx, wd, env, patterns)
if len(errs) > 0 {
return nil, errs
}
Expand Down Expand Up @@ -275,12 +284,22 @@ func Load(bctx *build.Context, wd string, pkgs []string) (*Info, []error) {
return info, ec.errors
}

// load typechecks the packages, including function body type checking
// for the packages directly named.
func load(bctx *build.Context, wd string, pkgs []string) (*loader.Program, []error) {
// load typechecks the packages that match the given patterns, including
// function body type checking for the packages that directly match. The
// patterns are defined by the underlying build system. For the go tool,
// this is described at
// https://golang.org/cmd/go/#hdr-Package_lists_and_patterns
//
// wd is the working directory and env is the set of environment
// variables to use when loading the packages specified by patterns. If
// env is nil or empty, it is interpreted as an empty set of variables.
// In case of duplicate environment variables, the last one in the list
// takes precedence.
func load(ctx context.Context, wd string, env []string, patterns []string) (*loader.Program, []error) {
bctx := buildContextFromEnv(env)
var foundPkgs []*build.Package
ec := new(errorCollector)
for _, name := range pkgs {
for _, name := range patterns {
p, err := bctx.Import(name, wd, build.FindOnly)
if err != nil {
ec.add(err)
Expand Down Expand Up @@ -320,7 +339,7 @@ func load(bctx *build.Context, wd string, pkgs []string) (*loader.Program, []err
return pkg, err
},
}
for _, name := range pkgs {
for _, name := range patterns {
conf.Import(name)
}

Expand All @@ -334,6 +353,35 @@ func load(bctx *build.Context, wd string, pkgs []string) (*loader.Program, []err
return prog, nil
}

func buildContextFromEnv(env []string) *build.Context {
// TODO(#78): Remove this function in favor of using go/packages,
// which does not need a *build.Context.

getenv := func(name string) string {
for i := len(env) - 1; i >= 0; i-- {
if strings.HasPrefix(env[i], name+"=") {
return env[i][len(name)+1:]
}
}
return ""
}
bctx := new(build.Context)
*bctx = build.Default
if v := getenv("GOARCH"); v != "" {
bctx.GOARCH = v
}
if v := getenv("GOOS"); v != "" {
bctx.GOOS = v
}
if v := getenv("GOROOT"); v != "" {
bctx.GOROOT = v
}
if v := getenv("GOPATH"); v != "" {
bctx.GOPATH = v
}
return bctx
}

func importPathInPkgList(pkgs []*build.Package, path string) bool {
for _, p := range pkgs {
if path == p.ImportPath {
Expand Down
2 changes: 1 addition & 1 deletion internal/wire/testdata/BuildTagsRelativePkg/pkg
Original file line number Diff line number Diff line change
@@ -1 +1 @@
./example.com/foo
./foo
2 changes: 1 addition & 1 deletion internal/wire/testdata/Cycle/want/wire_errs.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/wire_gopath/src/example.com/foo/wire.go:x:y: cycle for example.com/foo.Bar:
example.com/foo/wire.go:x:y: cycle for example.com/foo.Bar:
example.com/foo.Bar (example.com/foo.provideBar) ->
example.com/foo.Foo (example.com/foo.provideFoo) ->
example.com/foo.Baz (example.com/foo.provideBaz) ->
Expand Down
2 changes: 1 addition & 1 deletion internal/wire/testdata/EmptyVar/want/wire_errs.txt
Original file line number Diff line number Diff line change
@@ -1 +1 @@
/wire_gopath/src/example.com/foo/wire.go:x:y: var example.com/foo.myFakeSet struct{} is not a provider or a provider set
example.com/foo/wire.go:x:y: var example.com/foo.myFakeSet struct{} is not a provider or a provider set
Original file line number Diff line number Diff line change
@@ -1 +1 @@
/wire_gopath/src/example.com/foo/wire.go:x:y: inject injectBar: input of example.com/foo.Foo conflicts with provider provideFoo at /wire_gopath/src/example.com/foo/foo.go:x:y
example.com/foo/wire.go:x:y: inject injectBar: input of example.com/foo.Foo conflicts with provider provideFoo at example.com/foo/foo.go:x:y
Original file line number Diff line number Diff line change
@@ -1 +1 @@
/wire_gopath/src/example.com/foo/wire.go:x:y: inject injectFoo: provider for example.com/foo.Foo returns cleanup but injection does not return cleanup function
example.com/foo/wire.go:x:y: inject injectFoo: provider for example.com/foo.Foo returns cleanup but injection does not return cleanup function
Original file line number Diff line number Diff line change
@@ -1 +1 @@
/wire_gopath/src/example.com/foo/wire.go:x:y: inject injectFoo: provider for example.com/foo.Foo returns error but injection not allowed to fail
example.com/foo/wire.go:x:y: inject injectFoo: provider for example.com/foo.Foo returns error but injection not allowed to fail
Original file line number Diff line number Diff line change
@@ -1 +1 @@
/wire_gopath/src/example.com/foo/wire.go:x:y: string does not implement example.com/foo.Fooer
example.com/foo/wire.go:x:y: string does not implement example.com/foo.Fooer
Original file line number Diff line number Diff line change
@@ -1 +1 @@
/wire_gopath/src/example.com/foo/wire.go:x:y: first argument to Bind must be a pointer to an interface type; found string
example.com/foo/wire.go:x:y: first argument to Bind must be a pointer to an interface type; found string
Original file line number Diff line number Diff line change
@@ -1 +1 @@
/wire_gopath/src/example.com/foo/wire.go:x:y: too few arguments in call to wire.Bind
example.com/foo/wire.go:x:y: too few arguments in call to wire.Bind
Original file line number Diff line number Diff line change
@@ -1 +1 @@
/wire_gopath/src/example.com/foo/wire.go:x:y: string does not implement io.Reader
example.com/foo/wire.go:x:y: string does not implement io.Reader
Original file line number Diff line number Diff line change
@@ -1 +1 @@
/wire_gopath/src/example.com/foo/wire.go:x:y: first argument to InterfaceValue must be a pointer to an interface type; found string
example.com/foo/wire.go:x:y: first argument to InterfaceValue must be a pointer to an interface type; found string
Original file line number Diff line number Diff line change
@@ -1 +1 @@
/wire_gopath/src/example.com/foo/wire.go:x:y: too few arguments in call to wire.InterfaceValue
example.com/foo/wire.go:x:y: too few arguments in call to wire.InterfaceValue
48 changes: 24 additions & 24 deletions internal/wire/testdata/MultipleBindings/want/wire_errs.txt
Original file line number Diff line number Diff line change
@@ -1,41 +1,41 @@
/wire_gopath/src/example.com/foo/wire.go:x:y: wire.Build has multiple bindings for example.com/foo.Foo
example.com/foo/wire.go:x:y: wire.Build has multiple bindings for example.com/foo.Foo
current:
<- provider "provideFooAgain" (/wire_gopath/src/example.com/foo/foo.go:x:y)
<- provider "provideFooAgain" (example.com/foo/foo.go:x:y)
previous:
<- provider "provideFoo" (/wire_gopath/src/example.com/foo/foo.go:x:y)
<- provider "provideFoo" (example.com/foo/foo.go:x:y)

/wire_gopath/src/example.com/foo/wire.go:x:y: wire.Build has multiple bindings for example.com/foo.Foo
example.com/foo/wire.go:x:y: wire.Build has multiple bindings for example.com/foo.Foo
current:
<- provider "provideFoo" (/wire_gopath/src/example.com/foo/foo.go:x:y)
<- provider "provideFoo" (example.com/foo/foo.go:x:y)
previous:
<- provider "provideFoo" (/wire_gopath/src/example.com/foo/foo.go:x:y)
<- provider set "Set" (/wire_gopath/src/example.com/foo/foo.go:x:y)
<- provider "provideFoo" (example.com/foo/foo.go:x:y)
<- provider set "Set" (example.com/foo/foo.go:x:y)

/wire_gopath/src/example.com/foo/wire.go:x:y: wire.Build has multiple bindings for example.com/foo.Foo
example.com/foo/wire.go:x:y: wire.Build has multiple bindings for example.com/foo.Foo
current:
<- provider "provideFoo" (/wire_gopath/src/example.com/foo/foo.go:x:y)
<- provider "provideFoo" (example.com/foo/foo.go:x:y)
previous:
<- provider "provideFoo" (/wire_gopath/src/example.com/foo/foo.go:x:y)
<- provider set "Set" (/wire_gopath/src/example.com/foo/foo.go:x:y)
<- provider set "SuperSet" (/wire_gopath/src/example.com/foo/foo.go:x:y)
<- provider "provideFoo" (example.com/foo/foo.go:x:y)
<- provider set "Set" (example.com/foo/foo.go:x:y)
<- provider set "SuperSet" (example.com/foo/foo.go:x:y)

/wire_gopath/src/example.com/foo/foo.go:x:y: SetWithDuplicateBindings has multiple bindings for example.com/foo.Foo
example.com/foo/foo.go:x:y: SetWithDuplicateBindings has multiple bindings for example.com/foo.Foo
current:
<- provider "provideFoo" (/wire_gopath/src/example.com/foo/foo.go:x:y)
<- provider set "Set" (/wire_gopath/src/example.com/foo/foo.go:x:y)
<- provider set "SuperSet" (/wire_gopath/src/example.com/foo/foo.go:x:y)
<- provider "provideFoo" (example.com/foo/foo.go:x:y)
<- provider set "Set" (example.com/foo/foo.go:x:y)
<- provider set "SuperSet" (example.com/foo/foo.go:x:y)
previous:
<- provider "provideFoo" (/wire_gopath/src/example.com/foo/foo.go:x:y)
<- provider set "Set" (/wire_gopath/src/example.com/foo/foo.go:x:y)
<- provider "provideFoo" (example.com/foo/foo.go:x:y)
<- provider set "Set" (example.com/foo/foo.go:x:y)

/wire_gopath/src/example.com/foo/wire.go:x:y: wire.Build has multiple bindings for example.com/foo.Foo
example.com/foo/wire.go:x:y: wire.Build has multiple bindings for example.com/foo.Foo
current:
<- wire.Value (/wire_gopath/src/example.com/foo/wire.go:x:y)
<- wire.Value (example.com/foo/wire.go:x:y)
previous:
<- provider "provideFoo" (/wire_gopath/src/example.com/foo/foo.go:x:y)
<- provider "provideFoo" (example.com/foo/foo.go:x:y)

/wire_gopath/src/example.com/foo/wire.go:x:y: wire.Build has multiple bindings for example.com/foo.Bar
example.com/foo/wire.go:x:y: wire.Build has multiple bindings for example.com/foo.Bar
current:
<- wire.Bind (/wire_gopath/src/example.com/foo/wire.go:x:y)
<- wire.Bind (example.com/foo/wire.go:x:y)
previous:
<- provider "provideBar" (/wire_gopath/src/example.com/foo/foo.go:x:y)
<- provider "provideBar" (example.com/foo/foo.go:x:y)
18 changes: 9 additions & 9 deletions internal/wire/testdata/MultipleMissingInputs/want/wire_errs.txt
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
/wire_gopath/src/example.com/foo/wire.go:x:y: inject injectMissingOutputType: no provider found for example.com/foo.Foo, output of injector
example.com/foo/wire.go:x:y: inject injectMissingOutputType: no provider found for example.com/foo.Foo, output of injector

/wire_gopath/src/example.com/foo/wire.go:x:y: inject injectMultipleMissingTypes: no provider found for example.com/foo.Foo
needed by example.com/foo.Baz in provider "provideBaz" (/wire_gopath/src/example.com/foo/foo.go:x:y)
example.com/foo/wire.go:x:y: inject injectMultipleMissingTypes: no provider found for example.com/foo.Foo
needed by example.com/foo.Baz in provider "provideBaz" (example.com/foo/foo.go:x:y)

/wire_gopath/src/example.com/foo/wire.go:x:y: inject injectMultipleMissingTypes: no provider found for example.com/foo.Bar
needed by example.com/foo.Baz in provider "provideBaz" (/wire_gopath/src/example.com/foo/foo.go:x:y)
example.com/foo/wire.go:x:y: inject injectMultipleMissingTypes: no provider found for example.com/foo.Bar
needed by example.com/foo.Baz in provider "provideBaz" (example.com/foo/foo.go:x:y)

/wire_gopath/src/example.com/foo/wire.go:x:y: inject injectMissingRecursiveType: no provider found for example.com/foo.Foo
needed by example.com/foo.Zip in provider "provideZip" (/wire_gopath/src/example.com/foo/foo.go:x:y)
needed by example.com/foo.Zap in provider "provideZap" (/wire_gopath/src/example.com/foo/foo.go:x:y)
needed by example.com/foo.Zop in provider "provideZop" (/wire_gopath/src/example.com/foo/foo.go:x:y)
example.com/foo/wire.go:x:y: inject injectMissingRecursiveType: no provider found for example.com/foo.Foo
needed by example.com/foo.Zip in provider "provideZip" (example.com/foo/foo.go:x:y)
needed by example.com/foo.Zap in provider "provideZap" (example.com/foo/foo.go:x:y)
needed by example.com/foo.Zop in provider "provideZop" (example.com/foo/foo.go:x:y)
Original file line number Diff line number Diff line change
@@ -1 +1 @@
/wire_gopath/src/example.com/foo/wire.go:x:y: inject injectFooer: no provider found for example.com/foo.Fooer, output of injector
example.com/foo/wire.go:x:y: inject injectFooer: no provider found for example.com/foo.Fooer, output of injector
2 changes: 1 addition & 1 deletion internal/wire/testdata/UnexportedStruct/want/wire_errs.txt
Original file line number Diff line number Diff line change
@@ -1 +1 @@
/wire_gopath/src/example.com/foo/wire.go:x:y: foo not exported by package bar
example.com/foo/wire.go:x:y: foo not exported by package bar
2 changes: 1 addition & 1 deletion internal/wire/testdata/UnexportedValue/want/wire_errs.txt
Original file line number Diff line number Diff line change
@@ -1 +1 @@
/wire_gopath/src/example.com/foo/wire.go:x:y: inject injectedMessage: value string can't be used: uses unexported identifier privateMsg
example.com/foo/wire.go:x:y: inject injectedMessage: value string can't be used: uses unexported identifier privateMsg
8 changes: 4 additions & 4 deletions internal/wire/testdata/UnusedProviders/want/wire_errs.txt
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/wire_gopath/src/example.com/foo/wire.go:x:y: inject injectBar: unused provider set "unusedSet"
example.com/foo/wire.go:x:y: inject injectBar: unused provider set "unusedSet"

/wire_gopath/src/example.com/foo/wire.go:x:y: inject injectBar: unused provider "provideUnused"
example.com/foo/wire.go:x:y: inject injectBar: unused provider "provideUnused"

/wire_gopath/src/example.com/foo/wire.go:x:y: inject injectBar: unused value of type string
example.com/foo/wire.go:x:y: inject injectBar: unused value of type string

/wire_gopath/src/example.com/foo/wire.go:x:y: inject injectBar: unused interface binding to type example.com/foo.Fooer
example.com/foo/wire.go:x:y: inject injectBar: unused interface binding to type example.com/foo.Fooer
Original file line number Diff line number Diff line change
@@ -1 +1 @@
/wire_gopath/src/example.com/foo/wire.go:x:y: inject injectBar: value int can't be used: f is not declared in package scope
example.com/foo/wire.go:x:y: inject injectBar: value int can't be used: f is not declared in package scope
Original file line number Diff line number Diff line change
@@ -1 +1 @@
/wire_gopath/src/example.com/foo/wire.go:x:y: argument to Value may not be an interface value (found io.Reader); use InterfaceValue instead
example.com/foo/wire.go:x:y: argument to Value may not be an interface value (found io.Reader); use InterfaceValue instead
Loading

0 comments on commit 64470a2

Please sign in to comment.