diff --git a/notify/src/fsevent.rs b/notify/src/fsevent.rs index dff66104..b4771d2b 100644 --- a/notify/src/fsevent.rs +++ b/notify/src/fsevent.rs @@ -537,7 +537,7 @@ unsafe fn callback_impl( let mut handle_event = false; for (p, (r, filt)) in &(*info).recursive_info { - if path.starts_with(p) && filt.should_watch(&path) { + if path.starts_with(p) && filt.should_emit_event(&path) { if *r || &path == p { handle_event = true; break; diff --git a/notify/src/inotify.rs b/notify/src/inotify.rs index 1e43dcd8..bfd1bafb 100644 --- a/notify/src/inotify.rs +++ b/notify/src/inotify.rs @@ -228,6 +228,21 @@ impl EventLoop { None => self.paths.get(&event.wd).cloned(), }; + // Look up the filter for the watch descriptor that produced this event. + // For child events, the descriptor belongs to the containing directory, + // while `event.name` identifies the child path. + let watch_filter = self + .paths + .get(&event.wd) + .and_then(|watch_path| self.watches.get(watch_path)) + .map(|(_, _, _, _, watch_filter)| watch_filter); + let should_emit_path = match (&path, watch_filter) { + (Some(path), Some(watch_filter)) => { + watch_filter.should_emit_event(path) + } + _ => true, + }; + let mut evs = Vec::new(); if event.mask.contains(EventMask::MOVED_FROM) { @@ -371,8 +386,12 @@ impl EventLoop { ); } - for ev in evs { - self.event_handler.handle_event(Ok(ev)); + // Apply the emit filter only at dispatch time. The event translation above also + // updates recursive watch bookkeeping, so it must still run for suppressed paths + if should_emit_path { + for ev in evs { + self.event_handler.handle_event(Ok(ev)); + } } } @@ -408,29 +427,43 @@ impl EventLoop { mut watch_self: bool, watch_filter: WatchFilter, ) -> Result<()> { - if !watch_filter.should_watch(&path) { - return Ok(()); - } - // If the watch is not recursive, or if we determine (by stat'ing the path to get its // metadata) that the watched path is not a directory, add a single path watch. if !is_recursive || !metadata(&path).map_err(Error::io_watch)?.is_dir() { - return self.add_single_watch(path, false, true, WatchFilter::accept_all()); + return self.add_single_watch(path, false, watch_self, watch_filter); + } + + // Recursive directory walk. Short-circuit if the root is pruned. + if !watch_filter.should_watch_directory(&path) { + return Ok(()); } - for entry in WalkDir::new(path) + // We use the raw `WalkDir::IntoIter` so we can call `skip_current_dir()` for pruned entries. + // Non-directory entries are skipped because watches are only registered on directories + // (file-level events arrive through their parent's watch via `event.name`). + let mut iter = WalkDir::new(path) .follow_links(self.follow_links) - .into_iter() - .filter_map(filter_dir) - .filter(|e| watch_filter.should_watch(e.path())) - { - self.add_single_watch( - entry.path().to_path_buf(), - is_recursive, - watch_self, - watch_filter.clone(), - )?; - watch_self = false; + .into_iter(); + while let Some(entry_result) = iter.next() { + let entry = match entry_result { + Ok(e) => e, + Err(_) => continue, + }; + if !entry.file_type().is_dir() { + continue; + } + + if watch_filter.should_watch_directory(entry.path()) { + self.add_single_watch( + entry.path().to_path_buf(), + true, + watch_self, + watch_filter.clone(), + )?; + watch_self = false; + } else { + iter.skip_current_dir(); + } } Ok(()) @@ -546,18 +579,6 @@ impl EventLoop { } } -/// return `DirEntry` when it is a directory -fn filter_dir(e: walkdir::Result) -> Option { - if let Ok(e) = e { - if let Ok(metadata) = e.metadata() { - if metadata.is_dir() { - return Some(e); - } - } - } - None -} - impl INotifyWatcher { fn from_event_handler( event_handler: Box, diff --git a/notify/src/lib.rs b/notify/src/lib.rs index 205a9f66..9dacbc30 100644 --- a/notify/src/lib.rs +++ b/notify/src/lib.rs @@ -287,16 +287,40 @@ pub enum WatcherKind { NullWatcher, } -type FilterFn = dyn Fn(&Path) -> bool + Send + Sync; -/// Path filter to limit what gets watched. +type ShouldEmitEventFn = dyn Fn(&Path) -> bool + Send + Sync; +type ShouldWatchDirectoryFn = dyn Fn(&Path) -> bool + Send + Sync; + #[derive(Clone)] -pub struct WatchFilter(Option>); +struct Filters { + should_watch_directory: Arc, + should_emit_event: Arc, +} + +/// Path filter for [`Watcher::watch_filtered`]. +/// +/// Either accepts everything ([`WatchFilter::accept_all`]) or carries a +/// pair of predicates ([`WatchFilter::with_filter`]): +/// +/// * `should_watch_directory` — per-directory, consulted at recursive +/// watch-registration time. Returning `false` means that directory and +/// its descendants are not watched. Currently only consulted by the +/// inotify backend (Linux/Android); other backends have no recursive +/// registration step and ignore it. +/// +/// * `should_emit_event` — per-event predicate. Asked for every event path +/// on the inotify (Linux/Android), FSEvents (macOS), and +/// ReadDirectoryChangesW (Windows) backends. Returning `false` suppresses +/// the event. The kqueue and polling backends currently ignore this +/// predicate. +#[derive(Clone)] +pub struct WatchFilter(Option); impl std::fmt::Debug for WatchFilter { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - f.debug_tuple("WatchFilterFn") - .field(&self.0.as_ref().map_or("no filter", |_| "filter fn")) - .finish() + match &self.0 { + None => f.write_str("WatchFilter(accept_all)"), + Some(_) => f.write_str("WatchFilter(custom)"), + } } } @@ -306,15 +330,51 @@ impl WatchFilter { WatchFilter(None) } - /// A fitler to limit the paths that get watched. + /// Construct a filter from a directory-watch predicate and an event-emit predicate. + /// + /// * `should_watch_directory` — per-directory, consulted at recursive watch-registration time. + /// Only directories for which this returns `true` will be watched. + /// * `should_emit_event` — per-event predicate. + /// Only paths for which this returns `true` will emit events. /// - /// Only paths for which `filter` returns `true` will be watched. - pub fn with_filter(filter: Arc) -> WatchFilter { - WatchFilter(Some(filter)) + /// # Invariant: monotonicity over path containment + /// + /// If `should_watch_directory(child)` returns `true`, every ancestor of + /// `child` up to the watched root must also return `true`. + /// The registration walk visits ancestors before descendants. + /// A rejected ancestor unreachably prunes everything beneath it. + pub fn with_filter( + should_watch_directory: Arc, + should_emit_event: Arc, + ) -> WatchFilter { + WatchFilter(Some(Filters { + should_watch_directory, + should_emit_event, + })) } - fn should_watch(&self, path: &Path) -> bool { - self.0.as_ref().map_or(true, |f| f(path)) + /// Returns `true` when recursive registration should watch `path`. + #[cfg_attr(not(any(target_os = "linux", target_os = "android")), allow(dead_code))] + pub(crate) fn should_watch_directory(&self, path: &Path) -> bool { + self.0 + .as_ref() + .map_or(true, |f| (f.should_watch_directory)(path)) + } + + /// Returns `true` if an event for `path` should be emitted to the caller. + #[cfg_attr( + not(any( + target_os = "linux", + target_os = "android", + all(target_os = "macos", not(feature = "macos_kqueue")), + target_os = "windows" + )), + allow(dead_code) + )] + pub(crate) fn should_emit_event(&self, path: &Path) -> bool { + self.0 + .as_ref() + .map_or(true, |f| (f.should_emit_event)(path)) } } @@ -492,4 +552,57 @@ mod tests { Ok(()) } + + #[test] + fn accept_all_watches_directories_and_emits_events_everywhere() { + let filter = WatchFilter::accept_all(); + let path = Path::new("/some/path"); + assert!(filter.should_watch_directory(path)); + assert!(filter.should_emit_event(path)); + } + + #[test] + fn with_filter_plumbs_directory_watch_and_event_emit_predicates() { + let should_watch_directory: Arc = Arc::new(|p: &Path| { + matches!( + p.file_name().and_then(|n| n.to_str()), + Some("both" | "descend_only") + ) + }); + let should_emit_event: Arc = + Arc::new(|p: &Path| p.file_name().and_then(|n| n.to_str()) == Some("both")); + let filter = WatchFilter::with_filter(should_watch_directory, should_emit_event); + + assert!(filter.should_watch_directory(Path::new("/x/both"))); + assert!(filter.should_watch_directory(Path::new("/x/descend_only"))); + assert!(!filter.should_watch_directory(Path::new("/x/emit_only"))); + assert!(!filter.should_watch_directory(Path::new("/x/other"))); + + assert!(filter.should_emit_event(Path::new("/x/both"))); + assert!(!filter.should_emit_event(Path::new("/x/descend_only"))); + assert!(!filter.should_emit_event(Path::new("/x/emit_only"))); + assert!(!filter.should_emit_event(Path::new("/x/other"))); + } + + #[test] + fn with_filter_directory_watch_and_event_emit_are_independent() { + let filter = WatchFilter::with_filter( + Arc::new(|p: &Path| p.to_string_lossy().contains("keep")), + Arc::new(|p: &Path| p.file_name().and_then(|n| n.to_str()) == Some("HEAD")), + ); + // Emit-only: directory watch says no, but event emit says yes. + let head = Path::new("/repo/.git/HEAD"); + assert!(filter.should_emit_event(head)); + assert!(!filter.should_watch_directory(head)); + + // Directory-watch-only: directory watch says yes, but event emit says no. + let keep = Path::new("/repo/keep/subdir"); + assert!(!filter.should_emit_event(keep)); + assert!(filter.should_watch_directory(keep)); + + // Neither: both gates reject. + let other = Path::new("/other"); + assert!(!filter.should_emit_event(other)); + assert!(!filter.should_watch_directory(other)); + } } diff --git a/notify/src/windows.rs b/notify/src/windows.rs index a559b3db..92727275 100644 --- a/notify/src/windows.rs +++ b/notify/src/windows.rs @@ -373,7 +373,7 @@ unsafe extern "system" fn handle_event( Some(ref watch_path) => *watch_path != path, }; - skip = skip || !request.watch_filter.should_watch(&path); + skip = skip || !request.watch_filter.should_emit_event(&path); if !skip { log::trace!( diff --git a/notify/tests/watch_filter.rs b/notify/tests/watch_filter.rs new file mode 100644 index 00000000..18ae7ac4 --- /dev/null +++ b/notify/tests/watch_filter.rs @@ -0,0 +1,162 @@ +//! Integration tests for `WatchFilter`'s directory-watch and event-emit predicates. +//! +//! The directory-watch gate (registration-time pruning) is only consulted by +//! the inotify backend today, so these tests are Linux/Android only. + +#![cfg(any(target_os = "linux", target_os = "android"))] + +use std::{ + fs, + path::{Path, PathBuf}, + sync::Arc, + time::Duration, +}; + +use notify::{Config, Event, RecommendedWatcher, RecursiveMode, Result, WatchFilter, Watcher}; + +/// Drains the receiver for up to `timeout`, collecting every successful event. +fn collect_events(rx: &std::sync::mpsc::Receiver>, timeout: Duration) -> Vec { + let deadline = std::time::Instant::now() + timeout; + let mut events = Vec::new(); + while let Some(remaining) = deadline.checked_duration_since(std::time::Instant::now()) { + match rx.recv_timeout(remaining) { + Ok(Ok(event)) => events.push(event), + Ok(Err(_)) => {} + Err(_) => break, + } + } + events +} + +fn paths_under(events: &[Event], root: &Path) -> Vec { + let mut paths: Vec<_> = events + .iter() + .flat_map(|event| event.paths.iter()) + .filter(|path| path.starts_with(root)) + .cloned() + .collect(); + paths.sort(); + paths.dedup(); + paths +} + +/// Builds a fake `.git/` directory layout under `root` containing both +/// allowlisted (`HEAD`) and excluded (`objects/blob`) files. +fn build_fake_repo(root: &Path) -> std::io::Result<()> { + let git_dir = root.join(".git"); + fs::create_dir_all(git_dir.join("refs").join("heads"))?; + fs::create_dir_all(git_dir.join("objects").join("ab"))?; + fs::write(git_dir.join("HEAD"), b"ref: refs/heads/main\n")?; + fs::write( + git_dir.join("objects").join("ab").join("blob"), + b"initial\n", + )?; + fs::write(root.join("file.txt"), b"hello\n")?; + Ok(()) +} + +/// Marks `.git/objects/` as a directory that should not be watched so +/// walkdir prunes the subtree at registration time. Verifies: +/// +/// * Modifying `.git/HEAD` produces an event (we registered watches along +/// the path needed to reach it). +/// * Modifying `.git/objects/` does NOT produce an event (the +/// subtree was pruned before any watch was registered). +/// * Modifying a regular file in the repo root still produces an event. +#[test] +fn should_watch_directory_prunes_git_objects() -> std::result::Result<(), Box> +{ + let dir = tempfile::tempdir()?; + build_fake_repo(dir.path())?; + + let (tx, rx) = std::sync::mpsc::channel(); + let mut watcher = RecommendedWatcher::new(tx, Config::default())?; + + let filter = WatchFilter::with_filter( + Arc::new(|path: &Path| !path.to_string_lossy().contains("/.git/objects")), + Arc::new(|path: &Path| !path.to_string_lossy().contains("/.git/objects")), + ); + watcher.watch_filtered(dir.path(), RecursiveMode::Recursive, filter)?; + + // Give the watcher a moment to settle before generating events. + std::thread::sleep(Duration::from_millis(100)); + + let head = dir.path().join(".git").join("HEAD"); + fs::write(&head, b"ref: refs/heads/feature\n")?; + + let blob = dir + .path() + .join(".git") + .join("objects") + .join("ab") + .join("blob"); + fs::write(&blob, b"updated\n")?; + + let regular = dir.path().join("file.txt"); + fs::write(®ular, b"world\n")?; + + let events = collect_events(&rx, Duration::from_secs(2)); + let observed = paths_under(&events, dir.path()); + + assert!( + observed.iter().any(|p| p == &head), + "expected event for `.git/HEAD`, got: {observed:#?}" + ); + assert!( + observed.iter().any(|p| p == ®ular), + "expected event for `file.txt`, got: {observed:#?}" + ); + assert!( + !observed.iter().any(|p| p == &blob), + "should not have received an event for `.git/objects/`, got: {observed:#?}" + ); + + Ok(()) +} + +/// With the old single-predicate behaviour, a filter that excluded `.git/` +/// the directory but allowed `.git/HEAD` the file would prune the entire +/// `.git/` subtree at registration time and miss every event under it. +/// The directory-watch/event-emit split lets us express the case correctly: +/// watch through `.git/` (so we reach `HEAD`), but only emit events for +/// `HEAD` itself. +#[test] +fn should_emit_event_allows_subset_of_watched_paths( +) -> std::result::Result<(), Box> { + let dir = tempfile::tempdir()?; + build_fake_repo(dir.path())?; + + let (tx, rx) = std::sync::mpsc::channel(); + let mut watcher = RecommendedWatcher::new(tx, Config::default())?; + + let filter = WatchFilter::with_filter( + Arc::new(|_: &Path| true), + Arc::new(|path: &Path| { + path.file_name().map(|n| n == "HEAD").unwrap_or(false) + && path.to_string_lossy().contains("/.git/") + }), + ); + watcher.watch_filtered(dir.path(), RecursiveMode::Recursive, filter)?; + + std::thread::sleep(Duration::from_millis(100)); + + let head = dir.path().join(".git").join("HEAD"); + fs::write(&head, b"ref: refs/heads/feature\n")?; + + let regular = dir.path().join("file.txt"); + fs::write(®ular, b"world\n")?; + + let events = collect_events(&rx, Duration::from_secs(2)); + let observed = paths_under(&events, dir.path()); + + assert!( + observed.iter().any(|p| p == &head), + "expected event for `.git/HEAD` (watched through `.git/`), got: {observed:#?}" + ); + assert!( + !observed.iter().any(|p| p == ®ular), + "did not expect an event for `file.txt` (should_emit_event rejects it), got: {observed:#?}" + ); + + Ok(()) +}