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 <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Reviewed-by: Russ Cox <[email protected]>
Run-TryBot: Bryan Mills <[email protected]>

Author: Bryan C. Mills

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
 	}
 }

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,18 +841,23 @@ 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 {
 		t.Run(tc.name, func(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)

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
 	})

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
 			}

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.

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
 			}

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 .

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