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 <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
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 454574a..57a8c2c 100644
--- a/src/cmd/go/internal/fsys/fsys.go
+++ b/src/cmd/go/internal/fsys/fsys.go
@@ -295,6 +295,10 @@
// 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 @@
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 @@
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 b441e19..2ab2bb2 100644
--- a/src/cmd/go/internal/fsys/fsys_test.go
+++ b/src/cmd/go/internal/fsys/fsys_test.go
@@ -827,7 +827,7 @@
testenv.MustHaveSymlink(t)
initOverlay(t, `{
- "Replace": {"overlay_symlink": "symlink"}
+ "Replace": {"overlay_symlink/file": "symlink/file"}
}
-- dir/file --`)
@@ -841,11 +841,15 @@
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 @@
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 dce8e09..6019789 100644
--- a/src/cmd/go/internal/modindex/scan.go
+++ b/src/cmd/go/internal/modindex/scan.go
@@ -23,12 +23,12 @@
// 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 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 36e0532..627f91f 100644
--- a/src/cmd/go/internal/modload/search.go
+++ b/src/cmd/go/internal/modload/search.go
@@ -23,6 +23,7 @@
"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 @@
_, 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 @@
}
}
- 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 fad1acb..9f216d5 100644
--- a/src/cmd/go/internal/search/search.go
+++ b/src/cmd/go/internal/search/search.go
@@ -125,11 +125,16 @@
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 @@
}
}
+ // 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 a306498..71f38e2 100644
--- a/src/cmd/go/internal/workcmd/use.go
+++ b/src/cmd/go/internal/workcmd/use.go
@@ -131,7 +131,10 @@
}
// 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 989a8c2..40c9943 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 @@
# 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 0000000..8df1982
--- /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
