file_watcher: filter buck-out events before relativizing in the notify watcher#1351
Open
daandemeyer wants to merge 2 commits into
Open
file_watcher: filter buck-out events before relativizing in the notify watcher#1351daandemeyer wants to merge 2 commits into
daandemeyer wants to merge 2 commits into
Conversation
meta-cla
Bot
added
the
CLA Signed
Contributor
|
This pull request has been imported. If you are a Meta employee, you can view this in D110569871. (Because this pull request was imported automatically, there will not be any future comments.) |
keszybz
reviewed
Jul 3, 2026
daandemeyer
force-pushed
the
backslash-fix
branch
from
bb654b5 to
8c1c316
Compare
Contributor
Author
|
I'm aware that this doesn't mean I can store paths with backslashes in buck-out, but it does give me a chance to fix them up before buck notices them. |
daandemeyer
force-pushed
the
backslash-fix
branch
from
8c1c316 to
d67c591
Compare
The notify watcher relativized every event path before doing anything else. Relativizing rejects path components buck's path types cannot represent — a literal backslash (which build outputs can transiently contain, systemd's escaped unit names being one real-world source) or non-UTF-8 bytes — and the resulting error poisons the watcher state: every subsequent command fails with "Error relativizing: ... is not relative to project root" until the daemon is killed. Events arriving while the watcher is poisoned are also dropped without setting missed_events, so builds after the failing one silently use stale state. Rework the watcher so that no file name, whatever bytes it contains, can break it: - Check the buck-out prefix on the raw path before any conversion, so the dominant event class is discarded cheaply and regardless of whether the name is representable. - Relativize leniently (new ProjectRoot::relativize_relaxed, built on a new AbsNormPath::strip_prefix_untyped and the unchecked half of ForwardRelativePathNormalizer::normalize_path) and match the configured ignores against the unvalidated string, so ignored directories can contain names buck's path types reject. The strict relativize is now the relaxed version plus validation. - For paths that survive the ignores but still cannot be represented, record a change of the nearest representable parent directory instead of erroring, mirroring what the watchman and edenfs watchers already do: buck cannot read such files anyway, so invalidating the parent's listing is enough. Signed-off-by: Daan De Meyer <daan@amutable.com>
daandemeyer
force-pushed
the
backslash-fix
branch
from
d67c591 to
166a97a
Compare
Switch the notify watcher to a notify fork (patched in via the daandemeyer/notify GitHub fork) that adds a watch_filtered() API taking a filter which decides what gets watched: recursive scans do not descend into rejected directories, directories created later are checked against the filter before being auto-watched, and events beneath rejected directories are suppressed. The notify watcher passes a filter rejecting buck-out, ignored directories, and paths buck cannot represent. On Linux (inotify) these never get watch descriptors, so they generate no events at all: this eliminates the dominant source of useless wakeups during builds (buck-out writes), stops large buck-out trees from exhausting fs.inotify.max_user_watches, and speeds up daemon startup. On macOS (FSEvents) and Windows, which cannot watch directories selectively, the events are suppressed inside the notify backend before reaching our callback. The filter prunes any directory whose own path matches an ignore pattern, so a file-shaped glob (e.g. *.tmp) matching a directory name prunes that whole subtree.
daandemeyer
force-pushed
the
backslash-fix
branch
from
166a97a to
ca72204
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two changes to the notify file watcher.
The first fixes a bug where the watcher relativized every event path before doing anything else. Relativizing rejects components buck's path types can't represent, like a literal backslash (systemd's escaped unit names are one source of these in build outputs) or non-UTF-8 bytes, and the error poisons the watcher: every command after that fails with "Error relativizing: ... is not relative to project root" until the daemon is killed. It now checks the buck-out prefix on the raw path first, relativizes leniently for the ignore check, and for anything still unrepresentable invalidates the nearest representable parent directory instead of erroring, like the watchman and edenfs watchers already do.
The second stops the watcher watching buck-out and ignored directories at all. It watched the whole project root recursively and filtered those events out only after delivery. This switches to a notify fork with a watch_filtered() API that skips them at registration time, so on Linux they never get inotify watches and produce no events. That drops the main source of wakeups during builds, avoids running into fs.inotify.max_user_watches on large buck-out trees, and speeds up daemon startup. FSEvents and Windows can't watch selectively, so there the events are dropped inside notify before they reach us.
The second commit needs watch_filtered(), which isn't in a released notify yet, so it's pulled in with a patch.crates-io git dependency for now. The first commit doesn't depend on the fork.