cmd/go: avoid spurious readdir during fsys.Walk

fsys.Walk is cloned from filepath.Walk, which has always handled
a walk of a directory by reading the full directory before calling the
callback on the directory itself. So if the callback returns fs.SkipDir,
those entries are thrown away, but the expense of reading them was
still incurred. (Worse, this is the expensive directory read that also
calls Stat on every entry.) On machines with slow file system I/O,
these reads are particularly annoying. For example, if I do

	go list m...

there is a call to filepath.Walk that is told about $GOROOT/src/archive
and responds by returning filepath.SkipDir because archive does not
start with m, but it only gets the chance to do that after the archive
directory has been read. (Same for all the other top-level directories.)
Even something like go list github.com/foo/bar/... reads every top-level
$GOPATH/src directory.

When we designed filepath.WalkDir, one of the changes we made was
to allow calling the callback twice for a directory: once before reading it,
and then possibly again if the read produces an error (uncommon).
This CL changes fsys.Walk to use that same model. None of the callbacks
need changing, but now the $GOROOT/src/archive and other top-level
directories won't be read when evaluating a pattern like 'm...'.

For #53577.
Fixes #53765.

Change-Id: Idfa3b9e2cc335417bfd9d66dd584cb16f92bd12e
Reviewed-on: https://go-review.googlesource.com/c/go/+/416179
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
This commit is contained in:
Russ Cox 2022-07-06 13:21:34 -04:00
parent d3d7998756
commit 913d05133c
4 changed files with 38 additions and 18 deletions

View file

@ -464,28 +464,20 @@ func IsDirWithGoFiles(dir string) (bool, error) {
// walk recursively descends path, calling walkFn. Copied, with some
// modifications from path/filepath.walk.
func walk(path string, info fs.FileInfo, walkFn filepath.WalkFunc) error {
if !info.IsDir() {
return walkFn(path, info, nil)
if err := walkFn(path, info, nil); err != nil || !info.IsDir() {
return err
}
fis, readErr := ReadDir(path)
walkErr := walkFn(path, info, readErr)
// If readErr != nil, walk can't walk into this directory.
// walkErr != nil means walkFn want walk to skip this directory or stop walking.
// Therefore, if one of readErr and walkErr isn't nil, walk will return.
if readErr != nil || walkErr != nil {
// The caller's behavior is controlled by the return value, which is decided
// by walkFn. walkFn may ignore readErr and return nil.
// If walkFn returns SkipDir, it will be handled by the caller.
// So walk should return whatever walkFn returns.
return walkErr
fis, err := ReadDir(path)
if err != nil {
return walkFn(path, info, err)
}
for _, fi := range fis {
filename := filepath.Join(path, fi.Name())
if walkErr = walk(filename, fi, walkFn); walkErr != nil {
if !fi.IsDir() || walkErr != filepath.SkipDir {
return walkErr
if err := walk(filename, fi, walkFn); err != nil {
if !fi.IsDir() || err != filepath.SkipDir {
return err
}
}
}

View file

@ -0,0 +1,6 @@
# Test that go list prefix... does not read directories not beginning with prefix.
env GODEBUG=gofsystrace=1
go list m...
stderr mime
stderr mime[\\/]multipart
! stderr archive

View file

@ -11,12 +11,11 @@ stdout '^example.com/noread$'
go list ./empty/...
stderr 'matched no packages'
[root] stop # Root typically ignores file permissions.
# Make the directory ./noread unreadable, and verify that 'go list' reports an
# explicit error for a pattern that should match it (rather than treating it as
# equivalent to an empty directory).
[root] stop # Root typically ignores file permissions.
[windows] skip # Does not have Unix-style directory permissions.
[plan9] skip # Might not have Unix-style directory permissions.

23
src/cmd/go/testdata/script/mod_perm.txt vendored Normal file
View file

@ -0,0 +1,23 @@
# go list should work in ordinary conditions.
go list ./...
! stdout _data
# skip in conditions where chmod 0 may not work.
# plan9 should be fine, but copied from list_perm.txt unchanged.
[root] skip
[windows] skip
[plan9] skip
# go list should work with unreadable _data directory.
chmod 0 _data
go list ./...
! stdout _data
-- go.mod --
module m
-- x.go --
package m
-- _data/x.go --
package p