grep: one file it cannot read is not the end of the search

`look.grep` aborted the whole walk and returned `OpenFailed` the moment any
file refused to open. One root-owned 0600 file in the tree — or one deleted
between the walk and the read, which is routine in a build tree — turned a
search of ten thousand files into zero results and a word on the message row
that explains nothing. A grep is a question about the files you can read, and
the ones you cannot are not an answer to it: they are skipped now, along with
a read that fails partway.

Opened NONBLOCK for the reason readFile has it, so a FIFO in the tree cannot
stop the search until somebody writes to it.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_016Q4RATpafkwahrovHQLKRf

1 files changed, 55 insertions, 4 deletions

diff --git a/src/look.zig b/src/look.zig
index 8c7014f5..236b6950 100644
--- a/src/look.zig
+++ b/src/look.zig
@@ -801,20 +801,32 @@ pub fn grep(arena: std.mem.Allocator, gpa: std.mem.Allocator, dir: []const u8, b
         if (hits >= find_max_hits or written == out.len) break;
         var pathbuf: [4096]u8 = undefined;
         const path_z = std.fmt.bufPrintSentinel(&pathbuf, "{s}", .{path}, 0) catch return error.PathTooLong;
-        const fd = libc.open(path_z, .{ .ACCMODE = .RDONLY, .CLOEXEC = true });
-        if (fd < 0) return error.OpenFailed;
+        // A FILE THIS WALK CANNOT READ IS A FILE THIS WALK SKIPS. It used to
+        // abort the whole grep and report `OpenFailed`, so ONE root-owned 0600
+        // file — or one deleted between the walk and the read, which is routine
+        // in a build tree — turned a search of ten thousand files into zero
+        // results and a word that explains nothing. A grep is a question about
+        // the files you can read; the ones you cannot are not an answer to it.
+        // NONBLOCK for the reason `readFile` has it: a FIFO in the tree would
+        // otherwise stop the search until somebody wrote to it.
+        const fd = libc.open(path_z, .{ .ACCMODE = .RDONLY, .CLOEXEC = true, .NONBLOCK = true });
+        if (fd < 0) continue;
         var len: usize = 0;
+        var readable = true;
         while (len < buf.len) {
             const n = libc.read(fd, buf[len..].ptr, buf.len - len);
             if (n < 0) {
                 if (libc.errno(n) == .INTR) continue;
-                _ = libc.close(fd);
-                return error.ReadFailed;
+                // Skipped, not fatal, for the same reason: whatever this is, it
+                // is not text this search can answer with.
+                readable = false;
+                break;
             }
             if (n == 0) break;
             len += @intCast(n);
         }
         _ = libc.close(fd);
+        if (!readable) continue;
         const text = buf[0..len];
         if (std.mem.indexOfScalar(u8, text[0..@min(len, 1024)], 0) != null) continue; // binary
         // per PATH, not per root: one root can straddle the asking pane's
@@ -831,6 +843,45 @@ pub fn grep(arena: std.mem.Allocator, gpa: std.mem.Allocator, dir: []const u8, b
     return written;
 }
 
+test "grep skips a file it cannot read instead of abandoning the search" {
+    if (!platform_has_fs) return;
+    const gpa = std.testing.allocator;
+    var tmp = std.testing.tmpDir(.{});
+    defer tmp.cleanup();
+    var base_buf: [std.fs.max_path_bytes]u8 = undefined;
+    const dir = base_buf[0..try tmp.dir.realPath(std.testing.io, &base_buf)];
+
+    // Two files, and the unreadable one sorts FIRST — the walk reads in sorted
+    // order, so `a-` is the one that used to abort the search before `b-` was
+    // ever opened.
+    try tmp.dir.writeFile(std.testing.io, .{ .sub_path = "a-locked.txt", .data = "needle here\n" });
+    try tmp.dir.writeFile(std.testing.io, .{ .sub_path = "b-open.txt", .data = "needle here\n" });
+    var locked_buf: [std.fs.max_path_bytes]u8 = undefined;
+    const locked = try std.fmt.bufPrintSentinel(&locked_buf, "{s}/a-locked.txt", .{dir}, 0);
+    if (libc.chmod(locked, 0) != 0) return;
+    // Running as root reads it anyway, and then this test is testing nothing:
+    // say so by not pretending to have run.
+    const probe = libc.open(locked, .{ .ACCMODE = .RDONLY });
+    if (probe >= 0) {
+        _ = libc.close(probe);
+        _ = libc.chmod(locked, 0o644);
+        return error.SkipZigTest;
+    }
+
+    var arena: std.heap.ArenaAllocator = .init(gpa);
+    defer arena.deinit();
+    const out = try gpa.alloc(u8, 64 * 1024);
+    defer gpa.free(out);
+    const n = try grep(arena.allocator(), gpa, dir, dir, "needle", out);
+    _ = libc.chmod(locked, 0o644); // so `tmp.cleanup` can remove it
+
+    // The readable file's hit came back. Before this, the whole call returned
+    // `error.OpenFailed` and the +Grep buffer was empty.
+    try std.testing.expect(std.mem.indexOf(u8, out[0..n], "b-open.txt") != null);
+    try std.testing.expect(std.mem.indexOf(u8, out[0..n], "a-locked.txt") == null);
+}
+
+
 /// true if `path` exists and is a directory (open(O_DIRECTORY), no stat needed)
 fn isDir(path: [*:0]const u8) bool {
     const fd = libc.open(path, .{ .ACCMODE = .RDONLY, .DIRECTORY = true, .CLOEXEC = true });