mirror of
https://github.com/golang/go
synced 2024-11-05 18:36:08 +00:00
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:
parent
d3d7998756
commit
913d05133c
4 changed files with 38 additions and 18 deletions
|
@ -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
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
6
src/cmd/go/testdata/script/fsys_walk.txt
vendored
Normal file
6
src/cmd/go/testdata/script/fsys_walk.txt
vendored
Normal 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
|
|
@ -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
23
src/cmd/go/testdata/script/mod_perm.txt
vendored
Normal 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
|
Loading…
Reference in a new issue