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 <rsc@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>

diff --git a/src/cmd/go/internal/fsys/fsys.go b/src/cmd/go/internal/fsys/fsys.go
index 07bdc16aba2a2a2451c76f241d82f102ce1bd3f6..454574a592e5a9142e530e82fdb8484890fa9dee 100644 (file)
--- a/src/cmd/go/internal/fsys/fsys.go
+++ b/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

diff --git a/src/cmd/go/internal/fsys/fsys_test.go b/src/cmd/go/internal/fsys/fsys_test.go
index deb63f22e622be24a17b40de50d7c06f06db5de8..b441e19afefcaf3c29efd1e9d784d27bc3bb0637 100644 (file)
--- a/src/cmd/go/internal/fsys/fsys_test.go
+++ b/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 {

diff --git a/src/cmd/go/script_test.go b/src/cmd/go/script_test.go
index 3cbaeff8ade4c4e870599af2bc94de4cfa21f57a..4211fb6121c674af33e60c0c7406609a5206ec7a 100644 (file)
--- a/src/cmd/go/script_test.go
+++ b/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,

diff --git a/src/cmd/go/testdata/script/list_goroot_symlink.txt b/src/cmd/go/testdata/script/list_goroot_symlink.txt
new file mode 100644 (file)
index 0000000..8e50e4b
--- /dev/null
+++ b/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$'

diff --git a/src/cmd/go/testdata/script/list_symlink_dotdotdot.txt b/src/cmd/go/testdata/script/list_symlink_dotdotdot.txt
deleted file mode 100644 (file)
index 8df1982..0000000
--- a/src/cmd/go/testdata/script/list_symlink_dotdotdot.txt
+++ /dev/null
@@ -1,20 +0,0 @@
-[!symlink] skip
-
-symlink $WORK/gopath/src/sym -> $WORK/gopath/src/tree
-symlink $WORK/gopath/src/tree/squirrel -> $WORK/gopath/src/dir2 # this symlink should not be followed
-cd sym
-go list ./...
-cmp stdout $WORK/gopath/src/want_list.txt
--- tree/go.mod --
-module example.com/tree
-
-go 1.20
--- tree/tree.go --
-package tree
--- tree/branch/branch.go --
-package branch
--- dir2/squirrel.go --
-package squirrel
--- want_list.txt --
-example.com/tree
-example.com/tree/branch