Skip to content

Commit

Permalink
go/buildutil, cmd/guru: resolve symlinks in filenames through build.C…
Browse files Browse the repository at this point in the history
…ontext

Resolve symlinks in source directories (GOPATH, GOROOT, etc.) and source
files in order to find correct package. All I/O performed through
build.Context. Also add minor fix to guru unit tests in order to pass on
Windows.

Change-Id: Ie6134b9cd74eb7386e1d93603eb37c8e44b083b8
Reviewed-on: https://go-review.googlesource.com/33924
Reviewed-by: Alan Donovan <adonovan@google.com>
  • Loading branch information
stamblerre authored and adonovan committed Dec 6, 2016
1 parent 4073e78 commit 99be5a0
Show file tree
Hide file tree
Showing 3 changed files with 93 additions and 27 deletions.
21 changes: 11 additions & 10 deletions cmd/guru/unit_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"go/build"
"io/ioutil"
"os"
"path/filepath"
"runtime"
"strings"
"testing"
Expand Down Expand Up @@ -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
Expand All @@ -57,17 +58,17 @@ 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")},
}...)
}

for _, test := range successTests {
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")
}
}
Expand All @@ -82,22 +83,22 @@ 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"))},
}...)
}

for _, test := range failTests {
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)
}
}
Expand Down
45 changes: 42 additions & 3 deletions go/buildutil/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
Expand All @@ -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.
Expand Down
54 changes: 40 additions & 14 deletions go/buildutil/util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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)
}
}

Expand Down

0 comments on commit 99be5a0

Please sign in to comment.