From: Bryan C. Mills Date: Fri, 13 Jan 2023 18:59:19 +0000 (-0500) Subject: cmd/go: traverse module-root symlinks in Walk calls X-Git-Tag: go1.21rc1~1698 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=f36c7c5983e039e36e187d7fcd7e7e5a1aa2d74f;p=gostls13.git cmd/go: traverse module-root symlinks in Walk calls fsys.Walk, like filepath.Walk, avoids traversing symlinks. Also like filepath.Walk, it follows a symlink at the root if the root path ends in a file separator (consistent with POSIX pathname resolution¹). If the user's working directory is within a repository stored in (and symlinked to) a different filesystem path, we want to follow the symlink instead of treating the module as completely empty. ¹https://pubs.opengroup.org/onlinepubs/9699919799.2013edition/basedefs/V1_chap04.html#tag_04_12 Fixes #50807. Updates #57754. Change-Id: Idaf6168dfffafe879e05b4ded5fda287fcd3eeec Reviewed-on: https://go-review.googlesource.com/c/go/+/463179 Auto-Submit: Bryan Mills TryBot-Result: Gopher Robot Reviewed-by: Russ Cox Run-TryBot: Bryan Mills --- diff --git a/src/cmd/go/internal/fsys/fsys.go b/src/cmd/go/internal/fsys/fsys.go index 454574a592..57a8c2c352 100644 --- a/src/cmd/go/internal/fsys/fsys.go +++ b/src/cmd/go/internal/fsys/fsys.go @@ -295,6 +295,10 @@ func parentIsOverlayFile(name string) (string, bool) { // return an error. var errNotDir = errors.New("not a directory") +func nonFileInOverlayError(overlayPath string) error { + return fmt.Errorf("replacement path %q is a directory, not a file", overlayPath) +} + // readDir reads a dir on disk, returning an error that is errNotDir if the dir is not a directory. // Unfortunately, the error returned by os.ReadDir if dir is not a directory // can vary depending on the OS (Linux, Mac, Windows return ENOTDIR; BSD returns EINVAL). @@ -354,18 +358,21 @@ func ReadDir(dir string) ([]fs.FileInfo, error) { case to.isDeleted(): delete(files, name) default: - // This is a regular file. - f, err := os.Lstat(to.actualFilePath) + // To keep the data model simple, if the overlay contains a symlink we + // always stat through it (using Stat, not Lstat). That way we don't need + // to worry about the interaction between Lstat and directories: if a + // symlink in the overlay points to a directory, we reject it like an + // ordinary directory. + fi, err := os.Stat(to.actualFilePath) if err != nil { files[name] = missingFile(name) continue - } else if f.IsDir() { - return nil, fmt.Errorf("for overlay of %q to %q: overlay Replace entries can't point to directories", - filepath.Join(dir, name), to.actualFilePath) + } else if fi.IsDir() { + return nil, &fs.PathError{Op: "Stat", Path: filepath.Join(dir, name), Err: nonFileInOverlayError(to.actualFilePath)} } // Add a fileinfo for the overlaid file, so that it has // the original file's name, but the overlaid file's metadata. - files[name] = fakeFile{name, f} + files[name] = fakeFile{name, fi} } } sortedFiles := diskfis[:0] @@ -541,14 +548,22 @@ func overlayStat(path string, osStat func(string) (fs.FileInfo, error), opName s switch { case node.isDeleted(): - return nil, &fs.PathError{Op: "lstat", Path: cpath, Err: fs.ErrNotExist} + return nil, &fs.PathError{Op: opName, Path: cpath, Err: fs.ErrNotExist} case node.isDir(): return fakeDir(filepath.Base(path)), nil default: - fi, err := osStat(node.actualFilePath) + // To keep the data model simple, if the overlay contains a symlink we + // always stat through it (using Stat, not Lstat). That way we don't need to + // worry about the interaction between Lstat and directories: if a symlink + // in the overlay points to a directory, we reject it like an ordinary + // directory. + fi, err := os.Stat(node.actualFilePath) if err != nil { return nil, err } + if fi.IsDir() { + return nil, &fs.PathError{Op: opName, Path: cpath, Err: nonFileInOverlayError(node.actualFilePath)} + } return fakeFile{name: filepath.Base(path), real: fi}, nil } } diff --git a/src/cmd/go/internal/fsys/fsys_test.go b/src/cmd/go/internal/fsys/fsys_test.go index b441e19afe..2ab2bb2fba 100644 --- a/src/cmd/go/internal/fsys/fsys_test.go +++ b/src/cmd/go/internal/fsys/fsys_test.go @@ -827,7 +827,7 @@ func TestWalkSymlink(t *testing.T) { testenv.MustHaveSymlink(t) initOverlay(t, `{ - "Replace": {"overlay_symlink": "symlink"} + "Replace": {"overlay_symlink/file": "symlink/file"} } -- dir/file --`) @@ -841,11 +841,15 @@ func TestWalkSymlink(t *testing.T) { dir string wantFiles []string }{ - {"control", "dir", []string{"dir", "dir" + string(filepath.Separator) + "file"}}, + {"control", "dir", []string{"dir", filepath.Join("dir", "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"}}, - {"overlay_to_symlink_to_dir", "overlay_symlink", []string{"overlay_symlink"}}, + {"overlay_to_symlink_to_dir", "overlay_symlink", []string{"overlay_symlink", filepath.Join("overlay_symlink", "file")}}, + + // However, adding filepath.Separator should cause the link to be resolved. + {"symlink_with_slash", "symlink" + string(filepath.Separator), []string{"symlink" + string(filepath.Separator), filepath.Join("symlink", "file")}}, + {"overlay_to_symlink_to_dir", "overlay_symlink" + string(filepath.Separator), []string{"overlay_symlink" + string(filepath.Separator), filepath.Join("overlay_symlink", "file")}}, } for _, tc := range testCases { @@ -853,6 +857,7 @@ func TestWalkSymlink(t *testing.T) { var got []string err := Walk(tc.dir, func(path string, info fs.FileInfo, err error) error { + t.Logf("walk %q", path) got = append(got, path) if err != nil { t.Errorf("walkfn: got non nil err argument: %v, want nil err argument", err) diff --git a/src/cmd/go/internal/modindex/scan.go b/src/cmd/go/internal/modindex/scan.go index dce8e09a23..60197898a0 100644 --- a/src/cmd/go/internal/modindex/scan.go +++ b/src/cmd/go/internal/modindex/scan.go @@ -23,12 +23,12 @@ import ( // moduleWalkErr returns filepath.SkipDir if the directory isn't relevant // when indexing a module or generating a filehash, ErrNotIndexed, // if the module shouldn't be indexed, and nil otherwise. -func moduleWalkErr(modroot string, path string, info fs.FileInfo, err error) error { +func moduleWalkErr(root string, path string, info fs.FileInfo, err error) error { if err != nil { return ErrNotIndexed } // stop at module boundaries - if info.IsDir() && path != modroot { + if info.IsDir() && path != root { if fi, err := fsys.Stat(filepath.Join(path, "go.mod")); err == nil && !fi.IsDir() { return filepath.SkipDir } @@ -52,18 +52,23 @@ func moduleWalkErr(modroot string, path string, info fs.FileInfo, err error) err func indexModule(modroot string) ([]byte, error) { fsys.Trace("indexModule", modroot) var packages []*rawPackage - err := fsys.Walk(modroot, func(path string, info fs.FileInfo, err error) error { - if err := moduleWalkErr(modroot, path, info, err); err != nil { + + // If the root itself is a symlink to a directory, + // we want to follow it (see https://go.dev/issue/50807). + // Add a trailing separator to force that to happen. + root := str.WithFilePathSeparator(modroot) + err := fsys.Walk(root, func(path string, info fs.FileInfo, err error) error { + if err := moduleWalkErr(root, path, info, err); err != nil { return err } if !info.IsDir() { return nil } - if !str.HasFilePathPrefix(path, modroot) { + if !strings.HasPrefix(path, root) { panic(fmt.Errorf("path %v in walk doesn't have modroot %v as prefix", path, modroot)) } - rel := str.TrimFilePathPrefix(path, modroot) + rel := path[len(root):] packages = append(packages, importRaw(modroot, rel)) return nil }) diff --git a/src/cmd/go/internal/modload/search.go b/src/cmd/go/internal/modload/search.go index 36e05321ae..627f91f09c 100644 --- a/src/cmd/go/internal/modload/search.go +++ b/src/cmd/go/internal/modload/search.go @@ -23,6 +23,7 @@ import ( "cmd/go/internal/modindex" "cmd/go/internal/par" "cmd/go/internal/search" + "cmd/go/internal/str" "cmd/go/internal/trace" "cmd/internal/pkgpattern" @@ -77,7 +78,10 @@ func matchPackages(ctx context.Context, m *search.Match, tags map[string]bool, f _, span := trace.StartSpan(ctx, "walkPkgs "+root) defer span.Done() - root = filepath.Clean(root) + // If the root itself is a symlink to a directory, + // we want to follow it (see https://go.dev/issue/50807). + // Add a trailing separator to force that to happen. + root = str.WithFilePathSeparator(filepath.Clean(root)) err := fsys.Walk(root, func(pkgDir string, fi fs.FileInfo, err error) error { if err != nil { m.AddError(err) @@ -100,9 +104,7 @@ func matchPackages(ctx context.Context, m *search.Match, tags map[string]bool, f } } - rel := strings.TrimPrefix(filepath.ToSlash(pkgDir[len(root):]), "/") - name := path.Join(importPathRoot, rel) - + name := path.Join(importPathRoot, filepath.ToSlash(pkgDir[len(root):])) if !treeCanMatch(name) { want = false } diff --git a/src/cmd/go/internal/search/search.go b/src/cmd/go/internal/search/search.go index fad1acb610..9f216d5756 100644 --- a/src/cmd/go/internal/search/search.go +++ b/src/cmd/go/internal/search/search.go @@ -125,11 +125,16 @@ func (m *Match) MatchPackages() { if (m.pattern == "std" || m.pattern == "cmd") && src != cfg.GOROOTsrc { continue } + + // If the root itself is a symlink to a directory, + // we want to follow it (see https://go.dev/issue/50807). + // Add a trailing separator to force that to happen. src = str.WithFilePathSeparator(filepath.Clean(src)) root := src if m.pattern == "cmd" { root += "cmd" + string(filepath.Separator) } + err := fsys.Walk(root, func(path string, fi fs.FileInfo, err error) error { if err != nil { return err // Likely a permission error, which could interfere with matching. @@ -269,6 +274,10 @@ func (m *Match) MatchDirs(modRoots []string) { } } + // If dir is actually a symlink to a directory, + // we want to follow it (see https://go.dev/issue/50807). + // Add a trailing separator to force that to happen. + dir = str.WithFilePathSeparator(dir) err := fsys.Walk(dir, func(path string, fi fs.FileInfo, err error) error { if err != nil { return err // Likely a permission error, which could interfere with matching. diff --git a/src/cmd/go/internal/workcmd/use.go b/src/cmd/go/internal/workcmd/use.go index a306498d58..71f38e2e30 100644 --- a/src/cmd/go/internal/workcmd/use.go +++ b/src/cmd/go/internal/workcmd/use.go @@ -131,7 +131,10 @@ func runUse(ctx context.Context, cmd *base.Command, args []string) { } // Add or remove entries for any subdirectories that still exist. - fsys.Walk(useDir, func(path string, info fs.FileInfo, err error) error { + // If the root itself is a symlink to a directory, + // we want to follow it (see https://go.dev/issue/50807). + // Add a trailing separator to force that to happen. + fsys.Walk(str.WithFilePathSeparator(useDir), func(path string, info fs.FileInfo, err error) error { if err != nil { return err } diff --git a/src/cmd/go/testdata/script/list_goroot_symlink.txt b/src/cmd/go/testdata/script/list_goroot_symlink.txt index 989a8c2dd5..40c9943065 100644 --- a/src/cmd/go/testdata/script/list_goroot_symlink.txt +++ b/src/cmd/go/testdata/script/list_goroot_symlink.txt @@ -55,7 +55,6 @@ stdout '^encoding/binary: '$WORK${/}lib${/}goroot${/}src${/}encoding${/}binary'$ # 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$' +stdout '^encoding/binary: '$WORK${/}lib${/}goroot${/}src${/}encoding${/}binary'$' +! stderr . diff --git a/src/cmd/go/testdata/script/list_symlink_dotdotdot.txt b/src/cmd/go/testdata/script/list_symlink_dotdotdot.txt new file mode 100644 index 0000000000..8df1982484 --- /dev/null +++ b/src/cmd/go/testdata/script/list_symlink_dotdotdot.txt @@ -0,0 +1,20 @@ +[!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