diff options
author | Bryan Mills <bcmills@google.com> | 2023-01-13 20:20:31 +0000 |
---|---|---|
committer | Gopher Robot <gobot@golang.org> | 2023-01-13 21:45:47 +0000 |
commit | ebb572d82f97d19d0016a49956eb1fddc658eb76 (patch) | |
tree | 8a58806ab7fb2b09ba2bcbeda016e078863eb4b7 | |
parent | 16cec4e7a0ed4b1e5cf67a07a1bf24bdf2f6b04c (diff) | |
download | go-ebb572d82f97d19d0016a49956eb1fddc658eb76.tar.gz go-ebb572d82f97d19d0016a49956eb1fddc658eb76.zip |
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>
-rw-r--r-- | src/cmd/go/internal/fsys/fsys.go | 28 | ||||
-rw-r--r-- | src/cmd/go/internal/fsys/fsys_test.go | 4 | ||||
-rw-r--r-- | src/cmd/go/script_test.go | 1 | ||||
-rw-r--r-- | src/cmd/go/testdata/script/list_goroot_symlink.txt | 63 | ||||
-rw-r--r-- | src/cmd/go/testdata/script/list_symlink_dotdotdot.txt | 20 |
5 files changed, 70 insertions, 46 deletions
diff --git a/src/cmd/go/internal/fsys/fsys.go b/src/cmd/go/internal/fsys/fsys.go index 07bdc16aba..454574a592 100644 --- 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 deb63f22e6..b441e19afe 100644 --- 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 3cbaeff8ad..4211fb6121 100644 --- 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 index 0000000000..8e50e4beab --- /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 index 8df1982484..0000000000 --- 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 |