Revert "internal/fsys: follow root symlink in fsys.Walk"

This reverts CL 448360 and adds a regression test for #57754.

Reason for revert: broke 'go list' in Debian's distribution of the Go toolchain

Fixes #57754.
Updates #50807.

Change-Id: I3e8b9126294c55f21572774b549fb28f29eded0f
Reviewed-on: https://go-review.googlesource.com/c/go/+/461959
Reviewed-by: Russ Cox <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Auto-Submit: Bryan Mills <[email protected]>
Run-TryBot: Bryan Mills <[email protected]>

Author: Bryan Mills

src/cmd/go/internal/fsys/fsys.go

@@ -476,23 +476,19 @@ func IsDirWithGoFiles(dir string) (bool, error) {
 
 // walk recursively descends path, calling walkFn. Copied, with some
 // modifications from path/filepath.walk.
-// Walk follows the root if it's a symlink, but reports the original paths,
-// so it calls walk with both the resolvedPath (which is the path with the root resolved)
-// and path (which is the path reported to the walkFn).
-func walk(path, resolvedPath string, info fs.FileInfo, walkFn filepath.WalkFunc) error {
+func walk(path string, info fs.FileInfo, walkFn filepath.WalkFunc) error {
 	if err := walkFn(path, info, nil); err != nil || !info.IsDir() {
 		return err
 	}
 
-	fis, err := ReadDir(resolvedPath)
+	fis, err := ReadDir(path)
 	if err != nil {
 		return walkFn(path, info, err)
 	}
 
 	for _, fi := range fis {
 		filename := filepath.Join(path, fi.Name())
-		resolvedFilename := filepath.Join(resolvedPath, fi.Name())
-		if err := walk(filename, resolvedFilename, fi, walkFn); err != nil {
+		if err := walk(filename, fi, walkFn); err != nil {
 			if !fi.IsDir() || err != filepath.SkipDir {
 				return err
 			}
@@ -509,23 +505,7 @@ func Walk(root string, walkFn filepath.WalkFunc) error {
 	if err != nil {
 		err = walkFn(root, nil, err)
 	} else {
-		resolved := root
-		if info.Mode()&os.ModeSymlink != 0 {
-			// Walk follows root if it's a symlink (but does not follow other symlinks).
-			if op, ok := OverlayPath(root); ok {
-				resolved = op
-			}
-			resolved, err = os.Readlink(resolved)
-			if err != nil {
-				return err
-			}
-			// Re-stat to get the info for the resolved file.
-			info, err = Lstat(resolved)
-			if err != nil {
-				return err
-			}
-		}
-		err = walk(root, resolved, info, walkFn)
+		err = walk(root, info, walkFn)
 	}
 	if err == filepath.SkipDir {
 		return nil

src/cmd/go/internal/fsys/fsys_test.go

@@ -844,8 +844,8 @@ func TestWalkSymlink(t *testing.T) {
 		{"control", "dir", []string{"dir", "dir" + string(filepath.Separator) + "file"}},
 		// ensure Walk doesn't walk into the directory pointed to by the symlink
 		// (because it's supposed to use Lstat instead of Stat).
-		{"symlink_to_dir", "symlink", []string{"symlink", "symlink" + string(filepath.Separator) + "file"}},
-		{"overlay_to_symlink_to_dir", "overlay_symlink", []string{"overlay_symlink", "overlay_symlink" + string(filepath.Separator) + "file"}},
+		{"symlink_to_dir", "symlink", []string{"symlink"}},
+		{"overlay_to_symlink_to_dir", "overlay_symlink", []string{"overlay_symlink"}},
 	}
 
 	for _, tc := range testCases {

src/cmd/go/script_test.go

@@ -211,6 +211,7 @@ func scriptEnv(srv *vcstest.Server, srvCertFile string) ([]string, error) {
 		"GOROOT_FINAL=" + testGOROOT_FINAL, // causes spurious rebuilds and breaks the "stale" built-in if not propagated
 		"GOTRACEBACK=system",
 		"TESTGO_GOROOT=" + testGOROOT,
+		"TESTGO_EXE=" + testGo,
 		"TESTGO_VCSTEST_HOST=" + httpURL.Host,
 		"TESTGO_VCSTEST_TLS_HOST=" + httpsURL.Host,
 		"TESTGO_VCSTEST_CERT=" + srvCertFile,

src/cmd/go/testdata/script/list_goroot_symlink.txt

@@ -0,0 +1,63 @@
+# Regression test for https://go.dev/issue/57754: 'go list' failed if ../src
+# relative to the location of the go executable was a symlink to the real src
+# directory. (cmd/go expects that ../src is GOROOT/src, but it appears that the
+# Debian build of the Go toolchain is attempting to split GOROOT into binary and
+# source artifacts in different parent directories.)
+
+[short] skip 'copies the cmd/go binary'
+[!symlink] skip 'tests symlink-specific behavior'
+
+# Ensure that the relative path to $WORK/lib/goroot/src from $PWD is a different
+# number of ".." hops than the relative path to it from $WORK/share/goroot/src.
+
+cd $WORK
+
+# Construct a fake GOROOT in $WORK/lib/goroot whose src directory is a symlink
+# to a subdirectory of $WORK/share. This mimics the directory structure reported
+# in https://go.dev/issue/57754.
+#
+# Symlink everything else to the original $GOROOT to avoid needless copying work.
+
+mkdir $WORK/lib/goroot
+mkdir $WORK/share/goroot
+symlink $WORK/share/goroot/src -> $GOROOT${/}src
+symlink $WORK/lib/goroot/src -> ../../share/goroot/src
+symlink $WORK/lib/goroot/pkg -> $GOROOT${/}pkg
+
+# Verify that our symlink shenanigans don't prevent cmd/go from finding its
+# GOROOT using os.Executable.
+#
+# To do so, we copy the actual cmd/go executable — which is implemented as the
+# cmd/go test binary instead of the original $GOROOT/bin/go, which may be
+# arbitrarily stale — into the bin subdirectory of the fake GOROOT, causing
+# os.Executable to report a path in that directory.
+
+mkdir $WORK/lib/goroot/bin
+cp $TESTGO_EXE $WORK/lib/goroot/bin/go$GOEXE
+
+env GOROOT=''  # Clear to force cmd/go to find GOROOT itself.
+exec $WORK/lib/goroot/bin/go env GOROOT
+stdout $WORK${/}lib${/}goroot
+
+# Now verify that 'go list' can find standard-library packages in the symlinked
+# source tree, with paths matching the one reported by 'go env GOROOT'.
+
+exec $WORK/lib/goroot/bin/go list -f '{{.ImportPath}}: {{.Dir}}' encoding/binary
+stdout '^encoding/binary: '$WORK${/}lib${/}goroot${/}src${/}encoding${/}binary'$'
+
+	# BUG(#50807): This doesn't work on Windows for some reason — perhaps
+	# a bug in the Windows Lstat implementation with trailing separators?
+[!GOOS:windows] exec $WORK/lib/goroot/bin/go list -f '{{.ImportPath}}: {{.Dir}}' std
+[!GOOS:windows] stdout '^encoding/binary: '$WORK${/}lib${/}goroot${/}src${/}encoding${/}binary'$'
+
+# Most path lookups in GOROOT are not sensitive to symlinks. However, patterns
+# involving '...' wildcards must use Walk to check the GOROOT tree, which makes
+# them more sensitive to symlinks (because Walk doesn't follow them).
+#
+# So we check such a pattern to confirm that it works and reports a path relative
+# to $GOROOT/src (and not the symlink target).
+
+	# BUG(#50807): This should report encoding/binary, not "matched no packages".
+exec $WORK/lib/goroot/bin/go list -f '{{.ImportPath}}: {{.Dir}}' .../binary
+! stdout .
+stderr '^go: warning: "\.\.\./binary" matched no packages$'