diff --git a/src/backend/FSEvents.zig b/src/backend/FSEvents.zig index d7316b6..deab2f3 100644 --- a/src/backend/FSEvents.zig +++ b/src/backend/FSEvents.zig @@ -229,21 +229,36 @@ fn callback( // an error back to the caller. Stopping the stream from inside the // callback would require a separate signal channel and is not worth // the complexity; the stream will keep delivering future events. - if (flags & kFSEventStreamEventFlagItemCreated != 0) { - ctx.handler.change(path, .created, ot) catch {}; - } + // FSEvents coalesces multiple operations into a single callback with + // multiple flags set. Processing all flags independently produces + // spurious duplicate events (e.g. ItemRemoved|ItemRenamed -> two + // deletes; ItemCreated|ItemRemoved -> spurious create before delete). + // + // Priority chain: Removed > Renamed > Modified > Created. + // Modified beats Created because FSEvents sets ItemCreated on writes to + // existing files when O_CREAT is used (e.g. `echo > file`), producing + // spurious create events. When both are set, the file already exists, + // so Modified is the accurate description. + // + // Exception: ItemRenamed|ItemModified on an existing path emits both + // created and modified, because a rename-in followed by a write can be + // coalesced by FSEvents into a single callback. if (flags & kFSEventStreamEventFlagItemRemoved != 0) { ctx.handler.change(path, .deleted, ot) catch {}; - } - if (flags & kFSEventStreamEventFlagItemRenamed != 0) { + } else if (flags & kFSEventStreamEventFlagItemRenamed != 0) { // FSEvents fires ItemRenamed for both sides of a rename unpaired. // Normalize to created/deleted based on whether the path still exists, // so move-in appears as created and move-out as deleted on all platforms. const exists = if (std.fs.accessAbsolute(path, .{})) |_| true else |_| false; ctx.handler.change(path, if (exists) .created else .deleted, ot) catch {}; - } - if (flags & kFSEventStreamEventFlagItemModified != 0) { + // If a write was coalesced with a move-in, also emit the modify. + if (exists and flags & kFSEventStreamEventFlagItemModified != 0) { + ctx.handler.change(path, .modified, ot) catch {}; + } + } else if (flags & kFSEventStreamEventFlagItemModified != 0) { ctx.handler.change(path, .modified, ot) catch {}; + } else if (flags & kFSEventStreamEventFlagItemCreated != 0) { + ctx.handler.change(path, .created, ot) catch {}; } } } diff --git a/src/backend/KQueue.zig b/src/backend/KQueue.zig index 542031c..7396bbf 100644 --- a/src/backend/KQueue.zig +++ b/src/backend/KQueue.zig @@ -171,8 +171,11 @@ fn thread_fn(self: *@This(), allocator: std.mem.Allocator) void { std.log.err("nightwatch: handler returned {s}, stopping watch thread", .{@errorName(e)}); return; }; + // Clean up snapshot so that a new dir at the same path is not + // skipped by scan_dir's snapshots.contains() check. + self.remove_watch(allocator, dir_path); } else if (ev.fflags & NOTE_RENAME != 0) { - self.handler.change(dir_path, EventType.renamed, .dir) catch |e| { + self.handler.change(dir_path, EventType.deleted, .dir) catch |e| { std.log.err("nightwatch: handler returned {s}, stopping watch thread", .{@errorName(e)}); return; }; @@ -285,6 +288,10 @@ fn scan_dir(self: *@This(), allocator: std.mem.Allocator, dir_path: []const u8) // a rename (old disappears, new appears) reports the source path before the dest. for (new_dirs.items) |full_path| { try self.handler.change(full_path, EventType.created, .dir); + // Start watching the moved-in dir so future changes inside it are detected + // and so its deletion fires NOTE_DELETE rather than being silently missed. + self.add_watch(allocator, full_path) catch |e| + std.log.err("nightwatch: add_watch on moved-in dir failed: {s}", .{@errorName(e)}); try self.emit_subtree_created(allocator, full_path); } for (to_delete.items) |name| { @@ -322,6 +329,9 @@ fn emit_subtree_created(self: *@This(), allocator: std.mem.Allocator, dir_path: if (ot == .file) { self.register_file_watch(allocator, full_path); } else { + // Watch nested subdirs so changes inside them are detected after move-in. + self.add_watch(allocator, full_path) catch |e| + std.log.err("nightwatch: add_watch on moved-in subdir failed: {s}", .{@errorName(e)}); try self.emit_subtree_created(allocator, full_path); } } diff --git a/src/backend/KQueueDir.zig b/src/backend/KQueueDir.zig index be082e7..4028ae2 100644 --- a/src/backend/KQueueDir.zig +++ b/src/backend/KQueueDir.zig @@ -136,7 +136,7 @@ fn thread_fn(self: *@This(), allocator: std.mem.Allocator) void { return; }; } else if (ev.fflags & NOTE_RENAME != 0) { - self.handler.change(watch_path, EventType.renamed, .file) catch |e| { + self.handler.change(watch_path, EventType.deleted, .file) catch |e| { std.log.err("nightwatch: handler returned {s}, stopping watch thread", .{@errorName(e)}); return; }; @@ -152,8 +152,11 @@ fn thread_fn(self: *@This(), allocator: std.mem.Allocator) void { std.log.err("nightwatch: handler returned {s}, stopping watch thread", .{@errorName(e)}); return; }; + // Clean up snapshot so that a new dir at the same path is not + // skipped by scan_dir's snapshots.contains() check. + self.remove_watch(allocator, watch_path); } else if (ev.fflags & NOTE_RENAME != 0) { - self.handler.change(watch_path, EventType.renamed, .dir) catch |e| { + self.handler.change(watch_path, EventType.deleted, .dir) catch |e| { std.log.err("nightwatch: handler returned {s}, stopping watch thread", .{@errorName(e)}); return; }; @@ -279,7 +282,11 @@ fn scan_dir(self: *@This(), allocator: std.mem.Allocator, dir_path: []const u8) // Order: new dirs, deletions (source before dest for renames), creations, modifications. for (new_dirs.items) |full_path| { try self.handler.change(full_path, EventType.created, .dir); - try self.emit_subtree_created(full_path); + // Start watching the moved-in dir so future changes inside it are detected + // and so its deletion fires NOTE_DELETE rather than being silently missed. + self.add_watch(allocator, full_path) catch |e| + std.log.err("nightwatch: add_watch on moved-in dir failed: {s}", .{@errorName(e)}); + try self.emit_subtree_created(allocator, full_path); } for (to_delete.items) |name| { var path_buf: [std.fs.max_path_bytes]u8 = undefined; @@ -302,7 +309,7 @@ fn scan_dir(self: *@This(), allocator: std.mem.Allocator, dir_path: []const u8) // Walk dir_path recursively, emitting 'created' events for all contents. // Called after a new dir appears in scan_dir so callers see individual // 'created' events for the pre-existing tree of a moved-in directory. -fn emit_subtree_created(self: *@This(), dir_path: []const u8) error{HandlerFailed}!void { +fn emit_subtree_created(self: *@This(), allocator: std.mem.Allocator, dir_path: []const u8) !void { var dir = std.fs.openDirAbsolute(dir_path, .{ .iterate = true }) catch return; defer dir.close(); var iter = dir.iterate(); @@ -315,7 +322,12 @@ fn emit_subtree_created(self: *@This(), dir_path: []const u8) error{HandlerFaile var path_buf: [std.fs.max_path_bytes]u8 = undefined; const full_path = std.fmt.bufPrint(&path_buf, "{s}/{s}", .{ dir_path, entry.name }) catch continue; try self.handler.change(full_path, EventType.created, ot); - if (ot == .dir) try self.emit_subtree_created(full_path); + if (ot == .dir) { + // Watch nested subdirs so changes inside them are detected after move-in. + self.add_watch(allocator, full_path) catch |e| + std.log.err("nightwatch: add_watch on moved-in subdir failed: {s}", .{@errorName(e)}); + try self.emit_subtree_created(allocator, full_path); + } } } diff --git a/src/nightwatch_test.zig b/src/nightwatch_test.zig index 6d4ed68..6ad7c11 100644 --- a/src/nightwatch_test.zig +++ b/src/nightwatch_test.zig @@ -234,6 +234,11 @@ fn testModifyFile(comptime Watcher: type, allocator: std.mem.Allocator) !void { var watcher = try Watcher.init(allocator, &th.handler); defer watcher.deinit(); try watcher.watch(tmp); + // Drain before writing: FSEvents may deliver a coalesced create+modify if the + // file was created just before the stream started. A drain here separates any + // stale creation event from the upcoming write, so the write arrives in its + // own callback with only ItemModified set. + try drainEvents(Watcher, &watcher); { const f = try std.fs.openFileAbsolute(file_path, .{ .mode = .write_only }); @@ -794,7 +799,9 @@ test "creating a file emits a 'created' event" { test "writing to a file emits a 'modified' event" { inline for (comptime std.enums.values(nw.Variant)) |variant| { - try testModifyFile(nw.Create(variant), std.testing.allocator); + testModifyFile(nw.Create(variant), std.testing.allocator) catch |e| { + if (e != error.SkipZigTest) return e; + }; } } @@ -860,7 +867,9 @@ test "rename: old-name event precedes new-name event" { test "rename-then-modify: rename event precedes the subsequent modify event" { inline for (comptime std.enums.values(nw.Variant)) |variant| { - try testRenameThenModify(nw.Create(variant), std.testing.allocator); + testRenameThenModify(nw.Create(variant), std.testing.allocator) catch |e| { + if (e != error.SkipZigTest) return e; + }; } } diff --git a/src/types.zig b/src/types.zig index cf5695b..507211f 100644 --- a/src/types.zig +++ b/src/types.zig @@ -33,7 +33,7 @@ pub const EventType = enum { /// destination side. /// /// - **kqueue / kqueuedir**: when a watched *directory* is itself - /// renamed, a `renamed` change event is emitted for the old directory + /// renamed, a `deleted` change event is emitted for the old directory /// path (the new path is not known). Renames of *files inside* a /// watched directory are detected indirectly via directory-level /// `NOTE_WRITE` events and appear as `deleted` + `created`.