os, path/filepath: don't ignore Lstat errors in Readdir

os: don't ignore LStat errors in Readdir. If it's ENOENT,
on the second pass, just treat it as missing. If it's another
error, it's real.

path/filepath: use ReaddirNames instead of Readdir in Walk,
in order to obey the documented WalkFunc contract of returning
each walked item's LStat error, if any.

Fixes #6656
Fixes #6680

R=golang-dev, r
CC=golang-dev
https://golang.org/cl/43530043

3 files changed, 84 insertions, 17 deletions

diff --git a/src/pkg/path/filepath/export_test.go b/src/pkg/path/filepath/export_test.go
new file mode 100644
index 0000000000..0cf9e3bca1
--- /dev/null
+++ b/src/pkg/path/filepath/export_test.go
@@ -0,0 +1,7 @@
+// Copyright 2013 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package filepath
+
+var LstatP = &lstat
diff --git a/src/pkg/path/filepath/path.go b/src/pkg/path/filepath/path.go
index f8c7e4b2f4..65d29bf9f9 100644
--- a/src/pkg/path/filepath/path.go
+++ b/src/pkg/path/filepath/path.go
@@ -336,6 +336,8 @@ var SkipDir = errors.New("skip this directory")
 // the next file.
 type WalkFunc func(path string, info os.FileInfo, err error) error
 
+var lstat = os.Lstat // for testing
+
 // walk recursively descends path, calling w.
 func walk(path string, info os.FileInfo, walkFn WalkFunc) error {
 	err := walkFn(path, info, nil)
@@ -350,17 +352,25 @@ func walk(path string, info os.FileInfo, walkFn WalkFunc) error {
 		return nil
 	}
 
-	list, err := readDir(path)
+	names, err := readDirNames(path)
 	if err != nil {
 		return walkFn(path, info, err)
 	}
 
-	for _, fileInfo := range list {
-		err = walk(Join(path, fileInfo.Name()), fileInfo, walkFn)
+	for _, name := range names {
+		filename := Join(path, name)
+		fileInfo, err := lstat(filename)
 		if err != nil {
-			if !fileInfo.IsDir() || err != SkipDir {
+			if err := walkFn(filename, fileInfo, err); err != nil && err != SkipDir {
 				return err
 			}
+		} else {
+			err = walk(filename, fileInfo, walkFn)
+			if err != nil {
+				if !fileInfo.IsDir() || err != SkipDir {
+					return err
+				}
+			}
 		}
 	}
 	return nil
@@ -380,30 +390,22 @@ func Walk(root string, walkFn WalkFunc) error {
 	return walk(root, info, walkFn)
 }
 
-// readDir reads the directory named by dirname and returns
+// readDirNames reads the directory named by dirname and returns
 // a sorted list of directory entries.
-// Copied from io/ioutil to avoid the circular import.
-func readDir(dirname string) ([]os.FileInfo, error) {
+func readDirNames(dirname string) ([]string, error) {
 	f, err := os.Open(dirname)
 	if err != nil {
 		return nil, err
 	}
-	list, err := f.Readdir(-1)
+	names, err := f.Readdirnames(-1)
 	f.Close()
 	if err != nil {
 		return nil, err
 	}
-	sort.Sort(byName(list))
-	return list, nil
+	sort.Strings(names)
+	return names, nil
 }
 
-// byName implements sort.Interface.
-type byName []os.FileInfo
-
-func (f byName) Len() int           { return len(f) }
-func (f byName) Less(i, j int) bool { return f[i].Name() < f[j].Name() }
-func (f byName) Swap(i, j int)      { f[i], f[j] = f[j], f[i] }
-
 // Base returns the last element of path.
 // Trailing path separators are removed before extracting the last element.
 // If the path is empty, Base returns ".".
diff --git a/src/pkg/path/filepath/path_test.go b/src/pkg/path/filepath/path_test.go
index d32b70d6e2..1adc8cb072 100644
--- a/src/pkg/path/filepath/path_test.go
+++ b/src/pkg/path/filepath/path_test.go
@@ -5,6 +5,7 @@
 package filepath_test
 
 import (
+	"errors"
 	"io/ioutil"
 	"os"
 	"path/filepath"
@@ -458,6 +459,63 @@ func TestWalk(t *testing.T) {
 	}
 }
 
+func touch(t *testing.T, name string) {
+	f, err := os.Create(name)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := f.Close(); err != nil {
+		t.Fatal(err)
+	}
+}
+
+func TestWalkFileError(t *testing.T) {
+	td, err := ioutil.TempDir("", "walktest")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.RemoveAll(td)
+
+	touch(t, filepath.Join(td, "foo"))
+	touch(t, filepath.Join(td, "bar"))
+	dir := filepath.Join(td, "dir")
+	if err := os.MkdirAll(filepath.Join(td, "dir"), 0755); err != nil {
+		t.Fatal(err)
+	}
+	touch(t, filepath.Join(dir, "baz"))
+	touch(t, filepath.Join(dir, "stat-error"))
+	defer func() {
+		*filepath.LstatP = os.Lstat
+	}()
+	statErr := errors.New("some stat error")
+	*filepath.LstatP = func(path string) (os.FileInfo, error) {
+		if strings.HasSuffix(path, "stat-error") {
+			return nil, statErr
+		}
+		return os.Lstat(path)
+	}
+	got := map[string]error{}
+	err = filepath.Walk(td, func(path string, fi os.FileInfo, err error) error {
+		rel, _ := filepath.Rel(td, path)
+		got[filepath.ToSlash(rel)] = err
+		return nil
+	})
+	if err != nil {
+		t.Errorf("Walk error: %v", err)
+	}
+	want := map[string]error{
+		".":              nil,
+		"foo":            nil,
+		"bar":            nil,
+		"dir":            nil,
+		"dir/baz":        nil,
+		"dir/stat-error": statErr,
+	}
+	if !reflect.DeepEqual(got, want) {
+		t.Errorf("Walked %#v; want %#v", got, want)
+	}
+}
+
 var basetests = []PathTest{
 	{"", "."},
 	{".", "."},