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 <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>

diff --git a/src/cmd/go/go_test.go b/src/cmd/go/go_test.go
index a901ca8666cfcf5a6ea7ade308309588a1a949a1..0136ba4b1b1c3bdd523e019e4cd4a77074380a11 100644 (file)
--- 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 c8697ffe98ca3c636606992e27672703d51e1599..d384594722511028a25ea87d21279a35c72e7468 100644 (file)
--- 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 112f820d802fcfd11d0ba7288382ad98095317a2..95a06ffedcd8b7deced2782677f0fb031bd08b87 100644 (file)
--- 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{