From 6c803ab014286e086881c410cba381b02317423c Mon Sep 17 00:00:00 2001 From: Alan Donovan Date: Mon, 28 Nov 2016 16:48:17 -0500 Subject: [PATCH] go/loader: resolve imports in "created" packages w.r.t. parent dir not cwd Fixes golang/go#16580 Change-Id: Id891f40659257eac9dbb0d70dae17f725e911f9b Reviewed-on: https://go-review.googlesource.com/33589 Reviewed-by: Robert Griesemer --- go/loader/loader.go | 17 ++++++++---- go/loader/loader_test.go | 56 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 5 deletions(-) diff --git a/go/loader/loader.go b/go/loader/loader.go index f0171fc9..0d4c0d14 100644 --- a/go/loader/loader.go +++ b/go/loader/loader.go @@ -17,6 +17,7 @@ import ( "go/token" "go/types" "os" + "path/filepath" "sort" "strings" "sync" @@ -132,6 +133,8 @@ type Config struct { // Files are processed first, but typically only one of Files and // Filenames is provided. The path needn't be globally unique. // +// For vendoring purposes, the package's directory is the one that +// contains the first file. type PkgSpec struct { Path string // package path ("" => use package declaration) Files []*ast.File // ASTs of already-parsed files @@ -586,9 +589,8 @@ func (conf *Config) Load() (*Program, error) { imp.addFiles(info, files, false) } - createPkg := func(path string, files []*ast.File, errs []error) { - // TODO(adonovan): fix: use dirname of files, not cwd. - info := imp.newPackageInfo(path, conf.Cwd) + createPkg := func(path, dir string, files []*ast.File, errs []error) { + info := imp.newPackageInfo(path, dir) for _, err := range errs { info.appendError(err) } @@ -613,14 +615,19 @@ func (conf *Config) Load() (*Program, error) { path = "(unnamed)" } } - createPkg(path, files, errs) + + dir := "." + if len(files) > 0 && files[0].Pos().IsValid() { + dir = filepath.Dir(conf.fset().File(files[0].Pos()).Name()) + } + createPkg(path, dir, files, errs) } // Create external test packages. sort.Sort(byImportPath(xtestPkgs)) for _, bp := range xtestPkgs { files, errs := imp.conf.parsePackageFiles(bp, 'x') - createPkg(bp.ImportPath+"_test", files, errs) + createPkg(bp.ImportPath+"_test", bp.Dir, files, errs) } // -- finishing up (sequential) ---------------------------------------- diff --git a/go/loader/loader_test.go b/go/loader/loader_test.go index 1f05e183..540aa4cf 100644 --- a/go/loader/loader_test.go +++ b/go/loader/loader_test.go @@ -13,6 +13,8 @@ package loader_test import ( "fmt" "go/build" + "go/constant" + "go/types" "path/filepath" "reflect" "sort" @@ -477,6 +479,60 @@ func TestVendorCwd(t *testing.T) { } } +func TestVendorCwdIssue16580(t *testing.T) { + // Regression test for Go issue 16580. + // Import decls in "created" packages were vendor-resolved + // w.r.t. cwd, not the parent directory of the package's files. + ctxt := fakeContext(map[string]string{ + "a": ``, // mkdir a + "a/vendor": ``, // mkdir a/vendor + "a/vendor/b": `package b; const X = true`, + "b": `package b; const X = false`, + }) + for _, test := range []struct { + filename, cwd string + want bool // expected value of b.X; depends on filename, not on cwd + }{ + {filename: "c.go", cwd: "/go/src", want: false}, + {filename: "c.go", cwd: "/go/src/a", want: false}, + {filename: "c.go", cwd: "/go/src/a/b", want: false}, + {filename: "c.go", cwd: "/go/src/a/vendor/b", want: false}, + + {filename: "/go/src/a/c.go", cwd: "/go/src", want: true}, + {filename: "/go/src/a/c.go", cwd: "/go/src/a", want: true}, + {filename: "/go/src/a/c.go", cwd: "/go/src/a/b", want: true}, + {filename: "/go/src/a/c.go", cwd: "/go/src/a/vendor/b", want: true}, + + {filename: "/go/src/c/c.go", cwd: "/go/src", want: false}, + {filename: "/go/src/c/c.go", cwd: "/go/src/a", want: false}, + {filename: "/go/src/c/c.go", cwd: "/go/src/a/b", want: false}, + {filename: "/go/src/c/c.go", cwd: "/go/src/a/vendor/b", want: false}, + } { + conf := loader.Config{ + Cwd: test.cwd, + Build: ctxt, + } + f, err := conf.ParseFile(test.filename, `package dummy; import "b"; const X = b.X`) + if err != nil { + t.Fatal(f) + } + conf.CreateFromFiles("dummy", f) + + prog, err := conf.Load() + if err != nil { + t.Errorf("%+v: Load failed: %v", test, err) + continue + } + + x := constant.BoolVal(prog.Created[0].Pkg.Scope().Lookup("X").(*types.Const).Val()) + if x != test.want { + t.Errorf("%+v: b.X = %t", test, x) + } + } + + // TODO(adonovan): also test imports within XTestGoFiles. +} + // TODO(adonovan): more Load tests: // // failures: