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>

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)

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.

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{