From bda287922822afe41dd6da122b44da9c852723a2 Mon Sep 17 00:00:00 2001
From: Qi Xiao <xiaqqaix@gmail.com>
Date: Mon, 10 Apr 2023 23:18:02 +0100
Subject: [PATCH] pkg/glob: When enumerating files, keep going if Lstat fails.

When encountering files that can't be lstat'ed, simply ignore them instead of
terminating the entire globbing process. This is consistent with how directories
that can't be read are already silently ignored.

This fixes #1674.
---
 pkg/glob/glob.go        | 18 ++++++++++++------
 pkg/glob/glob_test.go   |  2 ++
 website/ref/language.md |  5 +++++
 3 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/pkg/glob/glob.go b/pkg/glob/glob.go
index 5b8f1c40..93968042 100644
--- a/pkg/glob/glob.go
+++ b/pkg/glob/glob.go
@@ -63,7 +63,8 @@ func isDrive(s string) bool {
 
 // glob finds all filenames matching the given Segments in the given dir, and
 // calls the callback on all of them. If the callback returns false, globbing is
-// interrupted, and glob returns false. Otherwise it returns true.
+// interrupted, and glob returns false. Otherwise it returns true. Files that
+// can't be lstat'ed and directories that can't be read are ignored silently.
 func glob(segs []Segment, dir string, cb func(PathInfo) bool) bool {
 	// Consume non-wildcard path elements simply by following the path. This may
 	// seem like an optimization, but is actually required for "." and ".." to
@@ -98,7 +99,7 @@ func glob(segs []Segment, dir string, cb func(PathInfo) bool) bool {
 
 	infos, err := readDir(dir)
 	if err != nil {
-		// TODO(xiaq): Silently drop the error.
+		// Ignore directories that can't be read.
 		return true
 	}
 
@@ -166,12 +167,17 @@ func glob(segs []Segment, dir string, cb func(PathInfo) bool) bool {
 	for _, info := range infos {
 		name := info.Name()
 		if matchElement(segs, name) {
-			dirname := dir + name
-			info, err := os.Lstat(dirname)
+			fullname := dir + name
+			info, err := os.Lstat(fullname)
 			if err != nil {
-				return true
+				// Either the file was removed between ReadDir and Lstat, or the
+				// OS has some special rule that prevents it from being lstat'ed
+				// (see b.elv.sh/1674 for a known case on macOS; SELinux and
+				// FreeBSD's MAC might be able to do the same). In either case,
+				// ignore the file.
+				continue
 			}
-			if !cb(PathInfo{dirname, info}) {
+			if !cb(PathInfo{fullname, info}) {
 				return false
 			}
 		}
diff --git a/pkg/glob/glob_test.go b/pkg/glob/glob_test.go
index d79c2a3e..3de0e79b 100644
--- a/pkg/glob/glob_test.go
+++ b/pkg/glob/glob_test.go
@@ -60,6 +60,8 @@ var globCases = []globCase{
 	{"xxxx/*", []string{}},
 	{"a/*/", []string{}},
 
+	// TODO: Add more tests for situations where Lstat fails.
+
 	// TODO Test cases against dotfiles.
 }
 
diff --git a/website/ref/language.md b/website/ref/language.md
index b3e23f9f..6824bf4f 100644
--- a/website/ref/language.md
+++ b/website/ref/language.md
@@ -1285,6 +1285,11 @@ Note the following caveats:
 -   Likewise, you always need to use `**` to match slashes, even if the matcher
     includes `/`. For example `*[set:abc/]` is the same as `*[set:abc]`.
 
+Files that the Elvish runtime doesn't have appropriate access to are omitted
+silently. For example, if the runtime doesn't have appropriate access to either
+the `d` directory or the `d/y.cc` file, the result of `*/y.cc` may omit
+`d/y.cc`.
+
 ## Order of evaluation
 
 An expression can use a combination of indexing, tilde expansion, wildcard and