From 9f58bf5f6b9ffd0c6e15d717fb741fd235339512 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Fri, 29 Jan 2016 12:18:32 -0500 Subject: [PATCH] cmd/go: avoid a few symlink-induced errors in internal and vendor checks This CL expands symlinks only when an error would be reported otherwise. Since the expansions are only on error paths, anything that worked yesterday should still work after this CL. This CL fixes a regression from Go 1.5 in "go run", or else we'd probably postpone it. Changing only the error paths is meant as a way to reduce the risk of making this change so late in the release cycle, but it may actually be the right strategy for symlinks in general. Fixes #14054. Change-Id: I42ed1276f67a0c395297a62bcec7d36c14c06404 Reviewed-on: https://go-review.googlesource.com/19102 Run-TryBot: Russ Cox TryBot-Result: Gobot Gobot Reviewed-by: Brad Fitzpatrick --- src/cmd/go/go_test.go | 54 ++++++++++++++++++++++++++++++++++++++++++- src/cmd/go/main.go | 9 ++++++++ src/cmd/go/pkg.go | 21 +++++++++++++++++ 3 files changed, 83 insertions(+), 1 deletion(-) diff --git a/src/cmd/go/go_test.go b/src/cmd/go/go_test.go index a901ca8666..0136ba4b1b 100644 --- a/src/cmd/go/go_test.go +++ b/src/cmd/go/go_test.go @@ -1621,7 +1621,7 @@ func TestGoTestDashOWritesBinary(t *testing.T) { } // Issue 4568. -func TestSymlinksDoNotConfuseGoList(t *testing.T) { +func TestSymlinksList(t *testing.T) { switch runtime.GOOS { case "plan9", "windows": t.Skipf("skipping symlink test on %s", runtime.GOOS) @@ -1640,6 +1640,58 @@ func TestSymlinksDoNotConfuseGoList(t *testing.T) { } } +// Issue 14054. +func TestSymlinksVendor(t *testing.T) { + switch runtime.GOOS { + case "plan9", "windows": + t.Skipf("skipping symlink test on %s", runtime.GOOS) + } + + tg := testgo(t) + defer tg.cleanup() + tg.setenv("GO15VENDOREXPERIMENT", "1") + tg.tempDir("gopath/src/dir1/vendor/v") + tg.tempFile("gopath/src/dir1/p.go", "package main\nimport _ `v`\nfunc main(){}") + tg.tempFile("gopath/src/dir1/vendor/v/v.go", "package v") + tg.must(os.Symlink(tg.path("gopath/src/dir1"), tg.path("symdir1"))) + tg.setenv("GOPATH", tg.path("gopath")) + tg.cd(tg.path("symdir1")) + tg.run("list", "-f", "{{.Root}}", ".") + if strings.TrimSpace(tg.getStdout()) != tg.path("gopath") { + t.Error("list confused by symlinks") + } + + // All of these should succeed, not die in vendor-handling code. + tg.run("run", "p.go") + tg.run("build") + tg.run("install") +} + +func TestSymlinksInternal(t *testing.T) { + switch runtime.GOOS { + case "plan9", "windows": + t.Skipf("skipping symlink test on %s", runtime.GOOS) + } + + tg := testgo(t) + defer tg.cleanup() + tg.tempDir("gopath/src/dir1/internal/v") + tg.tempFile("gopath/src/dir1/p.go", "package main\nimport _ `dir1/internal/v`\nfunc main(){}") + tg.tempFile("gopath/src/dir1/internal/v/v.go", "package v") + tg.must(os.Symlink(tg.path("gopath/src/dir1"), tg.path("symdir1"))) + tg.setenv("GOPATH", tg.path("gopath")) + tg.cd(tg.path("symdir1")) + tg.run("list", "-f", "{{.Root}}", ".") + if strings.TrimSpace(tg.getStdout()) != tg.path("gopath") { + t.Error("list confused by symlinks") + } + + // All of these should succeed, not die in internal-handling code. + tg.run("run", "p.go") + tg.run("build") + tg.run("install") +} + // Issue 4515. func TestInstallWithTags(t *testing.T) { tg := testgo(t) diff --git a/src/cmd/go/main.go b/src/cmd/go/main.go index c8697ffe98..d384594722 100644 --- a/src/cmd/go/main.go +++ b/src/cmd/go/main.go @@ -524,6 +524,15 @@ func hasFilePathPrefix(s, prefix string) bool { } } +// expandPath returns the symlink-expanded form of path. +func expandPath(p string) string { + x, err := filepath.EvalSymlinks(p) + if err == nil { + return x + } + return p +} + // treeCanMatchPattern(pattern)(name) reports whether // name or children of name can possibly match pattern. // Pattern is the same limited glob accepted by matchPattern. diff --git a/src/cmd/go/pkg.go b/src/cmd/go/pkg.go index 112f820d80..95a06ffedc 100644 --- a/src/cmd/go/pkg.go +++ b/src/cmd/go/pkg.go @@ -415,11 +415,18 @@ func vendoredImportPath(parent *Package, path string) (found string) { if parent == nil || parent.Root == "" || !go15VendorExperiment { return path } + dir := filepath.Clean(parent.Dir) root := filepath.Join(parent.Root, "src") + if !hasFilePathPrefix(dir, root) { + // Look for symlinks before reporting error. + dir = expandPath(dir) + root = expandPath(root) + } if !hasFilePathPrefix(dir, root) || len(dir) <= len(root) || dir[len(root)] != filepath.Separator { fatalf("invalid vendoredImportPath: dir=%q root=%q separator=%q", dir, root, string(filepath.Separator)) } + vpath := "vendor/" + path for i := len(dir); i >= len(root); i-- { if i < len(dir) && dir[i] != filepath.Separator { @@ -533,6 +540,13 @@ func disallowInternal(srcDir string, p *Package, stk *importStack) *Package { return p } + // Look for symlinks before reporting error. + srcDir = expandPath(srcDir) + parent = expandPath(parent) + if hasFilePathPrefix(filepath.Clean(srcDir), filepath.Clean(parent)) { + return p + } + // Internal is present, and srcDir is outside parent's tree. Not allowed. perr := *p perr.Error = &PackageError{ @@ -630,6 +644,13 @@ func disallowVendorVisibility(srcDir string, p *Package, stk *importStack) *Pack return p } + // Look for symlinks before reporting error. + srcDir = expandPath(srcDir) + parent = expandPath(parent) + if hasFilePathPrefix(filepath.Clean(srcDir), filepath.Clean(parent)) { + return p + } + // Vendor is present, and srcDir is outside parent's tree. Not allowed. perr := *p perr.Error = &PackageError{