From 1c6f639aae50dbb1e0b87915849b4bb206e5bc05 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Mon, 25 Jul 2016 15:05:58 +0000 Subject: [PATCH] imports: don't ignore GOPATH if GOROOT is a prefix of GOPATH Fixes golang/go#16458 Change-Id: I1aaec5d115dd703dd702101f6bec37bf8b02a73d Reviewed-on: https://go-review.googlesource.com/25192 Reviewed-by: Andrew Gerrand --- imports/fix.go | 14 ++++++++++++- imports/fix_test.go | 50 ++++++++++++++++++++++++++++++++++++--------- 2 files changed, 53 insertions(+), 11 deletions(-) diff --git a/imports/fix.go b/imports/fix.go index 4d7dfebf..e99d4429 100644 --- a/imports/fix.go +++ b/imports/fix.go @@ -661,7 +661,7 @@ func findImportGoPath(pkgName string, symbols map[string]bool, filename string) // TODO(bradfitz): run each $GOPATH entry async. But nobody // really has more than one anyway, so low priority. scanGoRootOnce.Do(scanGoRoot) // async - if !strings.HasPrefix(filename, build.Default.GOROOT) { + if !fileInDir(filename, build.Default.GOROOT) { scanGoPathOnce.Do(scanGoPath) // blocking } <-scanGoRootDone @@ -898,3 +898,15 @@ func findImportStdlib(shortPkg string, symbols map[string]bool) (importPath stri } return importPath, false, importPath != "" } + +// fileInDir reports whether the provided file path looks like +// it's in dir. (without hitting the filesystem) +func fileInDir(file, dir string) bool { + rest := strings.TrimPrefix(file, dir) + if len(rest) == len(file) { + // dir is not a prefix of file. + return false + } + // Check for boundary: either nothing (file == dir), or a slash. + return len(rest) == 0 || rest[0] == '/' || rest[0] == '\\' +} diff --git a/imports/fix_test.go b/imports/fix_test.go index 0a05f584..887e1a07 100644 --- a/imports/fix_test.go +++ b/imports/fix_test.go @@ -1153,6 +1153,11 @@ func TestFindImportStdlib(t *testing.T) { } type testConfig struct { + // goroot and gopath optionally specifies the path on disk + // to use for the GOROOT and GOPATH. If empty, a temp directory + // is made if needed. + goroot, gopath string + // gorootFiles optionally specifies the complete contents of GOROOT to use, // If nil, the normal current $GOROOT is used. gorootFiles map[string]string // paths relative to $GOROOT/src to contents @@ -1190,22 +1195,23 @@ func mapToDir(destDir string, files map[string]string) error { } func (c testConfig) test(t *testing.T, fn func(*goimportTest)) { - var goroot string - var gopath string + goroot := c.goroot + gopath := c.gopath - if c.gorootFiles != nil { + if c.gorootFiles != nil && goroot == "" { goroot = mustTempDir(t, "goroot-") defer os.RemoveAll(goroot) - if err := mapToDir(goroot, c.gorootFiles); err != nil { - t.Fatal(err) - } } - if c.gopathFiles != nil { + if err := mapToDir(goroot, c.gorootFiles); err != nil { + t.Fatal(err) + } + + if c.gopathFiles != nil && gopath == "" { gopath = mustTempDir(t, "gopath-") defer os.RemoveAll(gopath) - if err := mapToDir(gopath, c.gopathFiles); err != nil { - t.Fatal(err) - } + } + if err := mapToDir(gopath, c.gopathFiles); err != nil { + t.Fatal(err) } withEmptyGoPath(func() { @@ -1398,6 +1404,30 @@ func TestSkipNodeModules(t *testing.T) { }) } +// golang.org/issue/16458 -- if GOROOT is a prefix of GOPATH, GOPATH is ignored. +func TestGoRootPrefixOfGoPath(t *testing.T) { + dir := mustTempDir(t, "importstest") + defer os.RemoveAll(dir) + testConfig{ + goroot: filepath.Join(dir, "go"), + gopath: filepath.Join(dir, "gopath"), + gopathFiles: map[string]string{ + "example.com/foo/pkg.go": "package foo\nconst X = 1", + }, + }.test(t, func(t *goimportTest) { + const in = "package x\n\nconst _ = foo.X\n" + const want = "package x\n\nimport \"example.com/foo\"\n\nconst _ = foo.X\n" + buf, err := Process(t.gopath+"/src/x/x.go", []byte(in), nil) + if err != nil { + t.Fatal(err) + } + if string(buf) != want { + t.Errorf("wrong output.\ngot:\n%q\nwant:\n%q\n", buf, want) + } + }) + +} + func strSet(ss []string) map[string]bool { m := make(map[string]bool) for _, s := range ss {