Repository navigation
Conversation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
WalkthroughChangesThe watcher now reports metadata and move-from operations across inotify, kqueue, and Windows backends. Inotify queue overflow triggers batched synthetic write updates for watched paths. Trace tests add helpers and coverage for metadata and rename events. Watcher event handling
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Disclosing an overlap I should have found before opening this: #36248 already contains one hunk from this PR. It adds The rest of this PR is not covered by #36248, which only touches the inotify side:
If it would be easier to review, I can strip the I also closed #36417 (my watcher-shutdown PR) as a duplicate of #36253 for the same reason — that one was fully superseded. |
… overflow `src/watcher` mapped several `Op` bits it never asked the OS for, so those bits were unreachable and some filesystem changes produced no event at all. - `Op::METADATA` was dead on *every* platform. kqueue read `NOTE_ATTRIB` in `watch_event_from_kevent`, but `add_file_descriptor_to_kqueue_without_checks` only requested `NOTE_WRITE|NOTE_RENAME|NOTE_DELETE`; inotify never had `IN_ATTRIB` in either mask. `touch` and `chmod` therefore produced no watcher event on Linux at all, while ReadDirectoryChangesW reported them as a write. Now requested for **file** watches on both backends. Deliberately kept off directory watches: on inotify, `IN_ATTRIB` on a directory reports metadata changes of every entry inside it *by name*, and consumers treat a named directory event as "re-resolve this entry", so a bare `touch` would reload the module. Threading `WatchItemKind` into the kqueue registration is what makes that split expressible. - A rename inside a watched directory only reported its arrival half (`IN_MOVED_TO`). `IN_MOVED_FROM` is now requested and mapped to a new `Op::MOVE_FROM`, so the vacated name is invalidated too. - On Windows, `FILE_ACTION_ADDED` and `FILE_ACTION_RENAMED_NEW_NAME` produced a `WatchEvent` with no op bits at all, making file creation invisible to every consumer matching on `WRITE|DELETE|RENAME`. They now map to `CREATE` and `MOVE_TO`. `RENAMED_OLD_NAME` keeps `RENAME` alongside the new `MOVE_FROM` so existing consumers are unaffected. Dropped events were also ignored: `IN_Q_OVERFLOW` arrives with `wd == -1`, matches no watchlist entry, and fell through the lookup in `watch_loop_cycle`, so changes the kernel discarded were never noticed. `resync_after_overflow` now replays a synthetic `WRITE` per watched path. `WRITE` rather than `DELETE` is deliberate: it makes consumers re-check without driving the eviction path and its fixed 8096-entry `evict_list`. Two tests assert the new events through `BUN_WATCHER_TRACE`; both fail against a build without this change (no event at all for `utimes`, and only `move_to` for a rename). Platform skips are capability-based: ReadDirectoryChangesW cannot distinguish metadata from a content write, and kqueue has no per-entry departure event to map to `move_from`. Verified on Linux (test/cli/watch, test/cli/hot: 24 pass, 0 fail) and on macOS arm64, where `utimes on a watched file records a metadata event` passes — the end-to-end confirmation that `Op::METADATA` is reachable on kqueue. The kqueue semantics were checked against macOS 26.5.2 with a standalone probe: NOTE_ATTRIB fires for utimes/chmod on a file vnode, and a directory vnode reports it only for its own metadata, never its entries. Windows is compile-checked only.
7b49013 to
139cd5f
Compare
|
Rebased onto current Two conflicts, both from
Worth flagging that the The overlap with #36248 I noted above still stands — it has not merged, so nothing to drop yet. |
This branch has not been deployed
1 participant
What does this PR do?
src/watchermapped severalOpbits it never asked the OS for, so those bits were unreachable and some filesystem changes produced no event at all.Op::METADATAwas dead on every platform. kqueue readNOTE_ATTRIBinwatch_event_from_kevent, butadd_file_descriptor_to_kqueue_without_checksonly ever requestedNOTE_WRITE|NOTE_RENAME|NOTE_DELETE; inotify never hadIN_ATTRIBin either mask. Sotouchandchmodproduced no watcher event at all on Linux, while ReadDirectoryChangesW reported them as a write. Now requested for file watches on both backends.Deliberately kept off directory watches: on inotify,
IN_ATTRIBon a directory reports metadata changes of every entry inside it by name, and consumers treat a named directory event as "re-resolve this entry" — so a baretouchwould reload the module. ThreadingWatchItemKindinto the kqueue registration is what makes that split expressible.A rename inside a watched directory only reported its arrival half (
IN_MOVED_TO).IN_MOVED_FROMis now requested and mapped to a newOp::MOVE_FROM, so the vacated name is invalidated too.On Windows,
FILE_ACTION_ADDEDandFILE_ACTION_RENAMED_NEW_NAMEproduced aWatchEventwith no op bits at all, making file creation invisible to every consumer matching onWRITE|DELETE|RENAME. They now map toCREATEandMOVE_TO.RENAMED_OLD_NAMEkeepsRENAMEalongside the newMOVE_FROMso existing consumers are unaffected.Dropped events were also ignored:
IN_Q_OVERFLOWarrives withwd == -1, matches no watchlist entry, and fell through the lookup inwatch_loop_cycle, so changes the kernel discarded were never noticed.resync_after_overflowreplays a syntheticWRITEper watched path.WRITErather thanDELETEis deliberate: it makes consumers re-check without driving the eviction path and its fixed 8096-entryevict_list.The fixing lines are the two mask expressions in
watch_path/watch_dir, thefflagscomputation inadd_file_descriptor_to_kqueue_without_checks, thecreate_watch_eventmatch, and theis_queue_overflow()branch inwatch_loop_cycle. Everything else is theWatchItemKindplumbing those need.How did you verify your code works?
Two new tests assert the events through
BUN_WATCHER_TRACE, and both fail against a build without this change —utimesproduces no event at all, and a rename emits onlymove_to:utimes on a watched file records a metadata eventrenaming inside a watched directory records move_fromPlatform skips are capability-based, not workarounds: ReadDirectoryChangesW cannot distinguish metadata from a content write, and kqueue has no per-entry departure event to map to
move_from.Linux (x86_64):
test/cli/watch,test/cli/hot— 24 pass, 0 fail.macOS (arm64, 26.5.2):
utimes on a watched file records a metadata eventpasses, which is the end-to-end confirmation thatOp::METADATAis now reachable on kqueue.The kqueue semantics were checked against macOS 26.5.2 with a standalone probe rather than assumed —
NOTE_ATTRIBfires forutimes/chmodon a file vnode, and a directory vnode reports it only for its own metadata, never its entries. That probe also falsified an earlier revision of this change which requestedNOTE_EXTEND: it never arrives withoutNOTE_WRITE, so it was removed.Windows is compile-checked only (
cargo check --target x86_64-pc-windows-msvc); I have no Windows host, so CI is the gate for those hunks.Two pre-existing macOS failures are worth noting since they show up in any local macOS run of these suites, and reproduce identically on
mainbuilt from the same commit — neither is from this change:test/cli/hot/hot.test.tshangs aftershould work with sourcemap generation(indefinitely, because that file setslongTimeout = isDebug ? Infinity : 30_000), andfs.watch(dir) on macOS does not leak the resolved FSEvents pathtimes out.