diff --git a/cmd/guru/unit_test.go b/cmd/guru/unit_test.go index f9cb662058..699e6a1b10 100644 --- a/cmd/guru/unit_test.go +++ b/cmd/guru/unit_test.go @@ -9,6 +9,7 @@ import ( "go/build" "io/ioutil" "os" + "path/filepath" "runtime" "strings" "testing" @@ -46,7 +47,7 @@ func TestIssue17515(t *testing.T) { } successTests := []SuccessTest{ - {home + "/go", home + "/go/src/test/test.go", home + "/go/src"}, + {home + "/go", home + "/go/src/test/test.go", filepath.FromSlash(home + "/go/src")}, } // Add symlink cases if not on Windows, Plan 9 @@ -57,9 +58,9 @@ func TestIssue17515(t *testing.T) { } successTests = append(successTests, []SuccessTest{ - {home + "/go", home + "/src/test/test.go", home + "/go/src"}, - {home, home + "/go/src/test/test.go", home + "/src"}, - {home, home + "/src/test/test.go", home + "/src"}, + {home + "/go", home + "/src/test/test.go", filepath.FromSlash(home + "/go/src")}, + {home, home + "/go/src/test/test.go", filepath.FromSlash(home + "/src")}, + {home, home + "/src/test/test.go", filepath.FromSlash(home + "/src")}, }...) } @@ -67,7 +68,7 @@ func TestIssue17515(t *testing.T) { buildContext.GOPATH = test.gopath srcdir, importPath, err := guessImportPath(test.filename, &buildContext) if srcdir != test.wantSrcdir || importPath != "test" || err != nil { - t.Errorf("guessImportPath(%v, %v) = %v, %v, %v; want %v, %v, %v", + t.Errorf("guessImportPath(%q, %q) = %q, %q, %q; want %q, %q, %q", test.filename, test.gopath, srcdir, importPath, err, test.wantSrcdir, "test", "nil") } } @@ -82,14 +83,14 @@ func TestIssue17515(t *testing.T) { } failTests := []FailTest{ - {home + "/go", home + "/go/src/fake/test.go", errFormat(home + "/go/src/fake")}, + {home + "/go", home + "/go/src/fake/test.go", errFormat(filepath.FromSlash(home + "/go/src/fake"))}, } if runtime.GOOS != "windows" && runtime.GOOS != "plan9" { failTests = append(failTests, []FailTest{ - {home + "/go", home + "/src/fake/test.go", errFormat(home + "/src/fake")}, - {home, home + "/src/fake/test.go", errFormat(home + "/src/fake")}, - {home, home + "/go/src/fake/test.go", errFormat(home + "/go/src/fake")}, + {home + "/go", home + "/src/fake/test.go", errFormat(filepath.FromSlash(home + "/src/fake"))}, + {home, home + "/src/fake/test.go", errFormat(filepath.FromSlash(home + "/src/fake"))}, + {home, home + "/go/src/fake/test.go", errFormat(filepath.FromSlash(home + "/go/src/fake"))}, }...) } @@ -97,7 +98,7 @@ func TestIssue17515(t *testing.T) { buildContext.GOPATH = test.gopath srcdir, importPath, err := guessImportPath(test.filename, &buildContext) if !strings.HasPrefix(fmt.Sprint(err), test.wantErr) { - t.Errorf("guessImportPath(%v, %v) = %v, %v, %v; want %v, %v, %v", + t.Errorf("guessImportPath(%q, %q) = %q, %q, %q; want %q, %q, %q", test.filename, test.gopath, srcdir, importPath, err, "", "", test.wantErr) } } diff --git a/go/buildutil/util.go b/go/buildutil/util.go index 6dc0cfb868..40c0f193cc 100644 --- a/go/buildutil/util.go +++ b/go/buildutil/util.go @@ -71,8 +71,7 @@ func ContainingPackage(ctxt *build.Context, dir, filename string) (*build.Packag // We assume that no source root (GOPATH[i] or GOROOT) contains any other. for _, srcdir := range ctxt.SrcDirs() { srcdirSlash := filepath.ToSlash(srcdir) + "/" - if dirHasPrefix(dirSlash, srcdirSlash) { - importPath := dirSlash[len(srcdirSlash) : len(dirSlash)-len("/")] + if importPath, ok := HasSubdir(ctxt, srcdirSlash, dirSlash); ok { return ctxt.Import(importPath, dir, build.FindOnly) } } @@ -92,7 +91,47 @@ func dirHasPrefix(dir, prefix string) bool { // (go/build.Context defines these as methods, but does not export them.) -// TODO(adonovan): HasSubdir? +// hasSubdir calls ctxt.HasSubdir (if not nil) or else uses +// the local file system to answer the question. +func HasSubdir(ctxt *build.Context, root, dir string) (rel string, ok bool) { + if f := ctxt.HasSubdir; f != nil { + return f(root, dir) + } + + // Try using paths we received. + if rel, ok = hasSubdir(root, dir); ok { + return + } + + // Try expanding symlinks and comparing + // expanded against unexpanded and + // expanded against expanded. + rootSym, _ := filepath.EvalSymlinks(root) + dirSym, _ := filepath.EvalSymlinks(dir) + + if rel, ok = hasSubdir(rootSym, dir); ok { + return + } + if rel, ok = hasSubdir(root, dirSym); ok { + return + } + return hasSubdir(rootSym, dirSym) +} + +func hasSubdir(root, dir string) (rel string, ok bool) { + const sep = string(filepath.Separator) + root = filepath.Clean(root) + if !strings.HasSuffix(root, sep) { + root += sep + } + + dir = filepath.Clean(dir) + if !strings.HasPrefix(dir, root) { + return "", false + } + + return filepath.ToSlash(dir[len(root):]), true +} // FileExists returns true if the specified file exists, // using the build context's file system interface. diff --git a/go/buildutil/util_test.go b/go/buildutil/util_test.go index dd55533ce6..72db3172a4 100644 --- a/go/buildutil/util_test.go +++ b/go/buildutil/util_test.go @@ -2,14 +2,11 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. -// Incomplete source tree on Android. - -// +build !android - package buildutil_test import ( "go/build" + "io/ioutil" "os" "path/filepath" "runtime" @@ -23,22 +20,51 @@ func TestContainingPackage(t *testing.T) { goroot := runtime.GOROOT() gopath := filepath.SplitList(os.Getenv("GOPATH"))[0] - tests := [][2]string{ - {goroot + "/src/fmt/print.go", "fmt"}, - {goroot + "/src/encoding/json/foo.go", "encoding/json"}, - {goroot + "/src/encoding/missing/foo.go", "(not found)"}, - {gopath + "/src/golang.org/x/tools/go/buildutil/util_test.go", + type Test struct { + gopath, filename, wantPkg string + } + + tests := []Test{ + {gopath, goroot + "/src/fmt/print.go", "fmt"}, + {gopath, goroot + "/src/encoding/json/foo.go", "encoding/json"}, + {gopath, goroot + "/src/encoding/missing/foo.go", "(not found)"}, + {gopath, gopath + "/src/golang.org/x/tools/go/buildutil/util_test.go", "golang.org/x/tools/go/buildutil"}, } + + if runtime.GOOS != "windows" && runtime.GOOS != "plan9" { + // Make a symlink to gopath for test + tmp, err := ioutil.TempDir(os.TempDir(), "go") + if err != nil { + t.Errorf("Unable to create a temporary directory in %s", os.TempDir()) + } + + defer os.RemoveAll(tmp) + + // symlink between $GOPATH/src and /tmp/go/src + // in order to test all possible symlink cases + if err := os.Symlink(gopath+"/src", tmp+"/src"); err != nil { + t.Fatal(err) + } + tests = append(tests, []Test{ + {gopath, tmp + "/src/golang.org/x/tools/go/buildutil/util_test.go", "golang.org/x/tools/go/buildutil"}, + {tmp, gopath + "/src/golang.org/x/tools/go/buildutil/util_test.go", "golang.org/x/tools/go/buildutil"}, + {tmp, tmp + "/src/golang.org/x/tools/go/buildutil/util_test.go", "golang.org/x/tools/go/buildutil"}, + }...) + } + for _, test := range tests { - file, want := test[0], test[1] - bp, err := buildutil.ContainingPackage(&build.Default, ".", file) - got := bp.ImportPath + var got string + var buildContext = build.Default + buildContext.GOPATH = test.gopath + bp, err := buildutil.ContainingPackage(&buildContext, ".", test.filename) if err != nil { got = "(not found)" + } else { + got = bp.ImportPath } - if got != want { - t.Errorf("ContainingPackage(%q) = %s, want %s", file, got, want) + if got != test.wantPkg { + t.Errorf("ContainingPackage(%q) = %s, want %s", test.filename, got, test.wantPkg) } }