From aaaac45b87c4b0e587b08c795e8a461a87e4d77e Mon Sep 17 00:00:00 2001 From: Pavel Kurdikov Date: Fri, 18 Jul 2025 07:36:33 +0300 Subject: [PATCH 1/7] feat: rework PathsMut to be more consistent The feature to add / remove paths to watch in batch was introduced in #692 There was a problem with the implementation: if you drop the instance without `commit` it may leave the watcher in an inconsistent state (especially fsevents). This commit changes behavior and api and introduces `update_watches` method of the `Watcher` trait, which do the same stuff but in more consistent way. The `paths_mut` method was kept, but returning struct is just a builder for the method `update_watches` that will be called by `PathsMut::commit` related #653 closes #704 --- notify/src/error.rs | 60 ++++++++++++++++--- notify/src/fsevent.rs | 61 ++++++++++--------- notify/src/inotify.rs | 4 ++ notify/src/kqueue.rs | 4 ++ notify/src/lib.rs | 132 +++++++++++++++++++++++++++++++----------- notify/src/null.rs | 4 ++ notify/src/poll.rs | 4 ++ notify/src/windows.rs | 4 ++ 8 files changed, 199 insertions(+), 74 deletions(-) diff --git a/notify/src/error.rs b/notify/src/error.rs index 3d88bd7f..1081e006 100644 --- a/notify/src/error.rs +++ b/notify/src/error.rs @@ -1,7 +1,8 @@ //! Error types -use crate::Config; +use crate::{Config, WatchOp}; use std::error::Error as StdError; +use std::fmt::Debug; use std::path::PathBuf; use std::result::Result as StdResult; use std::{self, fmt, io}; @@ -158,14 +159,55 @@ impl From> for Error { } } -#[test] -fn display_formatted_errors() { - let expected = "Some error"; +/// The error provided by [`crate::Watcher::update_watches`] method +#[derive(Debug)] +pub struct UpdateWatchesError { + /// The original error + pub source: Error, + + /// The remaining operations that haven't been applied + pub remaining: Vec, +} + +impl fmt::Display for UpdateWatchesError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "unable to apply the batch operation: {}", self.source) + } +} + +impl StdError for UpdateWatchesError { + fn source(&self) -> Option<&(dyn StdError + 'static)> { + Some(&self.source) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn display_formatted_errors() { + let expected = "Some error"; + + assert_eq!(expected, format!("{}", Error::generic(expected))); - assert_eq!(expected, format!("{}", Error::generic(expected))); + assert_eq!( + expected, + format!("{}", Error::io(io::Error::other(expected))) + ); + } + + #[test] + fn display_update_watches() { + let actual = UpdateWatchesError { + source: Error::generic("Some error"), + remaining: Default::default(), + } + .to_string(); - assert_eq!( - expected, - format!("{}", Error::io(io::Error::other(expected))) - ); + assert_eq!( + format!("unable to apply the batch operation: Some error"), + actual + ); + } } diff --git a/notify/src/fsevent.rs b/notify/src/fsevent.rs index 01a87b6f..3b5ecb6b 100644 --- a/notify/src/fsevent.rs +++ b/notify/src/fsevent.rs @@ -14,10 +14,8 @@ #![allow(non_upper_case_globals, dead_code)] -use crate::event::*; -use crate::{ - unbounded, Config, Error, EventHandler, PathsMut, RecursiveMode, Result, Sender, Watcher, -}; +use crate::{event::*, WatchOp}; +use crate::{unbounded, Config, Error, EventHandler, RecursiveMode, Result, Sender, Watcher}; use fsevent_sys as fs; use fsevent_sys::core_foundation as cf; use std::collections::HashMap; @@ -267,29 +265,6 @@ extern "C" { fn CFRunLoopIsWaiting(runloop: cf::CFRunLoopRef) -> cf::Boolean; } -struct FsEventPathsMut<'a>(&'a mut FsEventWatcher); -impl<'a> FsEventPathsMut<'a> { - fn new(watcher: &'a mut FsEventWatcher) -> Self { - watcher.stop(); - Self(watcher) - } -} -impl PathsMut for FsEventPathsMut<'_> { - fn add(&mut self, path: &Path, recursive_mode: RecursiveMode) -> Result<()> { - self.0.append_path(path, recursive_mode) - } - - fn remove(&mut self, path: &Path) -> Result<()> { - self.0.remove_path(path) - } - - fn commit(self: Box) -> Result<()> { - // ignore return error: may be empty path list - let _ = self.0.run(); - Ok(()) - } -} - impl FsEventWatcher { fn from_event_handler(event_handler: Arc>) -> Result { Ok(FsEventWatcher { @@ -321,6 +296,23 @@ impl FsEventWatcher { result } + fn update_watches_inner( + &mut self, + ops: Vec, + ) -> crate::StdResult<(), crate::UpdateWatchesError> { + self.stop(); + + let result = crate::update_watches(ops, |op| match op { + crate::WatchOp::Watch(path, recursive_mode) => self.append_path(&path, recursive_mode), + crate::WatchOp::Unwatch(path) => self.remove_path(&path), + }); + + // ignore return error: may be empty path list + let _ = self.run(); + + result + } + #[inline] fn is_running(&self) -> bool { self.runloop.is_some() @@ -586,14 +578,17 @@ impl Watcher for FsEventWatcher { self.watch_inner(path, recursive_mode) } - fn paths_mut<'me>(&'me mut self) -> Box { - Box::new(FsEventPathsMut::new(self)) - } - fn unwatch(&mut self, path: &Path) -> Result<()> { self.unwatch_inner(path) } + fn update_watches( + &mut self, + ops: Vec, + ) -> crate::StdResult<(), crate::UpdateWatchesError> { + self.update_watches_inner(ops) + } + fn configure(&mut self, config: Config) -> Result { let (tx, rx) = unbounded(); self.configure_raw_mode(config, tx); @@ -603,6 +598,10 @@ impl Watcher for FsEventWatcher { fn kind() -> crate::WatcherKind { crate::WatcherKind::Fsevent } + + fn paths_mut(&mut self) -> crate::PathsMut<'_> { + crate::PathsMut::new(self) + } } impl Drop for FsEventWatcher { diff --git a/notify/src/inotify.rs b/notify/src/inotify.rs index fd27a39a..a355a177 100644 --- a/notify/src/inotify.rs +++ b/notify/src/inotify.rs @@ -601,6 +601,10 @@ impl Watcher for INotifyWatcher { fn kind() -> crate::WatcherKind { crate::WatcherKind::Inotify } + + fn paths_mut(&mut self) -> crate::PathsMut<'_> { + crate::PathsMut::new(self) + } } impl Drop for INotifyWatcher { diff --git a/notify/src/kqueue.rs b/notify/src/kqueue.rs index 0f814b7a..0e3113b6 100644 --- a/notify/src/kqueue.rs +++ b/notify/src/kqueue.rs @@ -447,6 +447,10 @@ impl Watcher for KqueueWatcher { fn kind() -> crate::WatcherKind { crate::WatcherKind::Kqueue } + + fn paths_mut(&mut self) -> crate::PathsMut<'_> { + crate::PathsMut::new(self) + } } impl Drop for KqueueWatcher { diff --git a/notify/src/lib.rs b/notify/src/lib.rs index 0eb73c7b..8f557bb5 100644 --- a/notify/src/lib.rs +++ b/notify/src/lib.rs @@ -163,10 +163,11 @@ #![deny(missing_docs)] pub use config::{Config, RecursiveMode}; -pub use error::{Error, ErrorKind, Result}; +pub use error::{Error, ErrorKind, Result, UpdateWatchesError}; pub use notify_types::event::{self, Event, EventKind}; -use std::path::Path; +use std::path::{Path, PathBuf}; +pub(crate) type StdResult = std::result::Result; pub(crate) type Receiver = std::sync::mpsc::Receiver; pub(crate) type Sender = std::sync::mpsc::Sender; #[cfg(any(target_os = "linux", target_os = "android", target_os = "windows"))] @@ -295,19 +296,56 @@ pub enum WatcherKind { /// Providing methods for adding and removing paths to watch. /// -/// `Box` is created by [`Watcher::paths_mut`]. See its documentation for more. -pub trait PathsMut { - /// Add a new path to watch. See [`Watcher::watch`] for more. - fn add(&mut self, path: &Path, recursive_mode: RecursiveMode) -> Result<()>; +/// It is created by [`Watcher::paths_mut`]. +pub struct PathsMut<'a> { + ops: Vec, + watcher: &'a mut dyn Watcher, +} + +impl<'a> Extend for PathsMut<'a> { + fn extend>(&mut self, iter: T) { + self.ops.extend(iter); + } +} + +impl<'a> PathsMut<'a> { + /// Creates new [`PathsMut`] to operate with watched paths in a bulk manner + pub fn new(watcher: &'a mut W) -> Self { + Self { + ops: Default::default(), + watcher, + } + } + + /// Plan a path to be added to watching. See [`Watcher::unwatch`] for more information. + pub fn watch(&mut self, path: &Path, recursive_mode: RecursiveMode) { + self.ops + .push(WatchOp::Watch(path.to_owned(), recursive_mode)); + } - /// Remove a path from watching. See [`Watcher::unwatch`] for more. - fn remove(&mut self, path: &Path) -> Result<()>; + /// Plan a path to be removed from watching. See [`Watcher::unwatch`] for more information. + pub fn unwatch(&mut self, path: &Path) { + self.ops.push(WatchOp::Unwatch(path.to_owned())); + } - /// Ensure added/removed paths are applied. + /// Commit all the operations. /// - /// The behaviour of dropping a [`PathsMut`] without calling [`commit`] is unspecified. - /// The implementation is free to ignore the changes or not, and may leave the watcher in a started or stopped state. - fn commit(self: Box) -> Result<()>; + /// Dropping a [`PathsMut`] without calling [`Self::commit`] is noop. + pub fn commit(self) -> StdResult<(), UpdateWatchesError> { + self.watcher.update_watches(self.ops) + } +} + +/// An operation to apply to a watcher +/// +/// See [`Watcher::paths_mut`] for more information +#[derive(Debug, Clone, PartialEq, Eq, Hash)] +pub enum WatchOp { + /// Path should be watcher + Watch(PathBuf, RecursiveMode), + + /// Path should be unwatched + Unwatch(PathBuf), } /// Type that can deliver file activity notifications @@ -345,40 +383,47 @@ pub trait Watcher { /// fails. fn unwatch(&mut self, path: &Path) -> Result<()>; - /// Add/remove paths to watch. + /// Add/remove paths to watch in batch. /// - /// For some watcher implementations this method provides better performance than multiple calls to [`Watcher::watch`] and [`Watcher::unwatch`] if you want to add/remove many paths at once. + /// It is ergonomic way to call [`Watcher::update_watches`] /// /// # Examples /// /// ``` - /// # use notify::{Watcher, RecursiveMode, Result}; + /// # use notify::{Watcher, RecursiveMode}; /// # use std::path::Path; - /// # fn main() -> Result<()> { + /// # fn main() -> Result<(), Box> { /// # let many_paths_to_add = vec![]; + /// # let many_paths_to_remove = vec![]; /// let mut watcher = notify::recommended_watcher(|_event| { /* event handler */ })?; /// let mut watcher_paths = watcher.paths_mut(); + /// /// for path in many_paths_to_add { - /// watcher_paths.add(path, RecursiveMode::Recursive)?; + /// // just some memory stuff + /// watcher_paths.watch(path, RecursiveMode::Recursive); /// } + /// + /// for path in many_paths_to_remove { + /// // just some memory stuff + /// watcher_paths.unwatch(path); + /// } + /// + /// // real work is done there /// watcher_paths.commit()?; /// # Ok(()) /// # } /// ``` - fn paths_mut<'me>(&'me mut self) -> Box { - struct DefaultPathsMut<'a, T: ?Sized>(&'a mut T); - impl<'a, T: Watcher + ?Sized> PathsMut for DefaultPathsMut<'a, T> { - fn add(&mut self, path: &Path, recursive_mode: RecursiveMode) -> Result<()> { - self.0.watch(path, recursive_mode) - } - fn remove(&mut self, path: &Path) -> Result<()> { - self.0.unwatch(path) - } - fn commit(self: Box) -> Result<()> { - Ok(()) - } - } - Box::new(DefaultPathsMut(self)) + fn paths_mut(&mut self) -> crate::PathsMut<'_>; + + /// Add/remove paths to watch in batch. + /// + /// For some [`Watcher`] implementations this method provides better performance than multiple + /// calls to [`Watcher::watch`] and [`Watcher::unwatch`] if you want to add/remove many paths at once. + fn update_watches(&mut self, ops: Vec) -> StdResult<(), UpdateWatchesError> { + update_watches(ops, |op| match op { + WatchOp::Watch(path, recursive_mode) => self.watch(&path, recursive_mode), + WatchOp::Unwatch(path) => self.unwatch(&path), + }) } /// Configure the watcher at runtime. @@ -442,6 +487,25 @@ where RecommendedWatcher::new(event_handler, Config::default()) } +pub(crate) fn update_watches( + ops: Vec, + mut apply: F, +) -> StdResult<(), UpdateWatchesError> +where + F: FnMut(WatchOp) -> Result<()>, +{ + let mut iter = ops.into_iter(); + while let Some(op) = iter.next() { + if let Err(source) = apply(op) { + return Err(UpdateWatchesError { + source, + remaining: iter.collect(), + }); + } + } + Ok(()) +} + #[cfg(test)] mod tests { use std::{ @@ -557,8 +621,8 @@ mod tests { // start watching a and b { let mut watcher_paths = watcher.paths_mut(); - watcher_paths.add(&dir_a, RecursiveMode::Recursive)?; - watcher_paths.add(&dir_b, RecursiveMode::Recursive)?; + watcher_paths.watch(&dir_a, RecursiveMode::Recursive); + watcher_paths.watch(&dir_b, RecursiveMode::Recursive); watcher_paths.commit()?; } @@ -588,7 +652,7 @@ mod tests { // stop watching a { let mut watcher_paths = watcher.paths_mut(); - watcher_paths.remove(&dir_a)?; + watcher_paths.unwatch(&dir_a); watcher_paths.commit()?; } diff --git a/notify/src/null.rs b/notify/src/null.rs index bbcd80d9..965e6bb1 100644 --- a/notify/src/null.rs +++ b/notify/src/null.rs @@ -36,4 +36,8 @@ impl Watcher for NullWatcher { fn kind() -> crate::WatcherKind { crate::WatcherKind::NullWatcher } + + fn paths_mut(&mut self) -> crate::PathsMut<'_> { + crate::PathsMut::new(self) + } } diff --git a/notify/src/poll.rs b/notify/src/poll.rs index fecaf22e..e092aaa0 100644 --- a/notify/src/poll.rs +++ b/notify/src/poll.rs @@ -661,6 +661,10 @@ impl Watcher for PollWatcher { fn kind() -> crate::WatcherKind { crate::WatcherKind::PollWatcher } + + fn paths_mut(&mut self) -> crate::PathsMut<'_> { + crate::PathsMut::new(self) + } } impl Drop for PollWatcher { diff --git a/notify/src/windows.rs b/notify/src/windows.rs index 797052ed..c946fbe7 100644 --- a/notify/src/windows.rs +++ b/notify/src/windows.rs @@ -585,6 +585,10 @@ impl Watcher for ReadDirectoryChangesWatcher { fn kind() -> crate::WatcherKind { WatcherKind::ReadDirectoryChangesWatcher } + + fn paths_mut(&mut self) -> crate::PathsMut<'_> { + crate::PathsMut::new(self) + } } impl Drop for ReadDirectoryChangesWatcher { From 7530d192006adb2b471888421082b9c8858fd483 Mon Sep 17 00:00:00 2001 From: Pavel Kurdikov Date: Fri, 18 Jul 2025 16:04:55 +0300 Subject: [PATCH 2/7] fix: remove paths mut --- notify/src/fsevent.rs | 4 -- notify/src/inotify.rs | 4 -- notify/src/kqueue.rs | 4 -- notify/src/lib.rs | 87 ++++++++----------------------------------- notify/src/null.rs | 4 -- notify/src/poll.rs | 4 -- notify/src/windows.rs | 4 -- 7 files changed, 16 insertions(+), 95 deletions(-) diff --git a/notify/src/fsevent.rs b/notify/src/fsevent.rs index 3b5ecb6b..13f782ed 100644 --- a/notify/src/fsevent.rs +++ b/notify/src/fsevent.rs @@ -598,10 +598,6 @@ impl Watcher for FsEventWatcher { fn kind() -> crate::WatcherKind { crate::WatcherKind::Fsevent } - - fn paths_mut(&mut self) -> crate::PathsMut<'_> { - crate::PathsMut::new(self) - } } impl Drop for FsEventWatcher { diff --git a/notify/src/inotify.rs b/notify/src/inotify.rs index a355a177..fd27a39a 100644 --- a/notify/src/inotify.rs +++ b/notify/src/inotify.rs @@ -601,10 +601,6 @@ impl Watcher for INotifyWatcher { fn kind() -> crate::WatcherKind { crate::WatcherKind::Inotify } - - fn paths_mut(&mut self) -> crate::PathsMut<'_> { - crate::PathsMut::new(self) - } } impl Drop for INotifyWatcher { diff --git a/notify/src/kqueue.rs b/notify/src/kqueue.rs index 0e3113b6..0f814b7a 100644 --- a/notify/src/kqueue.rs +++ b/notify/src/kqueue.rs @@ -447,10 +447,6 @@ impl Watcher for KqueueWatcher { fn kind() -> crate::WatcherKind { crate::WatcherKind::Kqueue } - - fn paths_mut(&mut self) -> crate::PathsMut<'_> { - crate::PathsMut::new(self) - } } impl Drop for KqueueWatcher { diff --git a/notify/src/lib.rs b/notify/src/lib.rs index 8f557bb5..6ae43c6b 100644 --- a/notify/src/lib.rs +++ b/notify/src/lib.rs @@ -294,51 +294,9 @@ pub enum WatcherKind { NullWatcher, } -/// Providing methods for adding and removing paths to watch. -/// -/// It is created by [`Watcher::paths_mut`]. -pub struct PathsMut<'a> { - ops: Vec, - watcher: &'a mut dyn Watcher, -} - -impl<'a> Extend for PathsMut<'a> { - fn extend>(&mut self, iter: T) { - self.ops.extend(iter); - } -} - -impl<'a> PathsMut<'a> { - /// Creates new [`PathsMut`] to operate with watched paths in a bulk manner - pub fn new(watcher: &'a mut W) -> Self { - Self { - ops: Default::default(), - watcher, - } - } - - /// Plan a path to be added to watching. See [`Watcher::unwatch`] for more information. - pub fn watch(&mut self, path: &Path, recursive_mode: RecursiveMode) { - self.ops - .push(WatchOp::Watch(path.to_owned(), recursive_mode)); - } - - /// Plan a path to be removed from watching. See [`Watcher::unwatch`] for more information. - pub fn unwatch(&mut self, path: &Path) { - self.ops.push(WatchOp::Unwatch(path.to_owned())); - } - - /// Commit all the operations. - /// - /// Dropping a [`PathsMut`] without calling [`Self::commit`] is noop. - pub fn commit(self) -> StdResult<(), UpdateWatchesError> { - self.watcher.update_watches(self.ops) - } -} - /// An operation to apply to a watcher /// -/// See [`Watcher::paths_mut`] for more information +/// See [`Watcher::update_watches`] for more information #[derive(Debug, Clone, PartialEq, Eq, Hash)] pub enum WatchOp { /// Path should be watcher @@ -385,40 +343,33 @@ pub trait Watcher { /// Add/remove paths to watch in batch. /// - /// It is ergonomic way to call [`Watcher::update_watches`] + /// For some [`Watcher`] implementations this method provides better performance than multiple + /// calls to [`Watcher::watch`] and [`Watcher::unwatch`] if you want to add/remove many paths at once. /// /// # Examples /// /// ``` - /// # use notify::{Watcher, RecursiveMode}; + /// # use notify::{Watcher, RecursiveMode, WatchOp}; /// # use std::path::Path; /// # fn main() -> Result<(), Box> { /// # let many_paths_to_add = vec![]; /// # let many_paths_to_remove = vec![]; - /// let mut watcher = notify::recommended_watcher(|_event| { /* event handler */ })?; - /// let mut watcher_paths = watcher.paths_mut(); + /// let mut watcher = notify::NullWatcher; + /// let mut batch = Vec::new(); /// - /// for path in many_paths_to_add { - /// // just some memory stuff - /// watcher_paths.watch(path, RecursiveMode::Recursive); + /// for (path, recursive_mode) in many_paths_to_add { + /// batch.push(WatchOp::Watch(path, recursive_mode)); /// } /// /// for path in many_paths_to_remove { - /// // just some memory stuff - /// watcher_paths.unwatch(path); + /// batch.push(WatchOp::Unwatch(path)); /// } /// /// // real work is done there - /// watcher_paths.commit()?; + /// watcher.update_watches(batch)?; /// # Ok(()) /// # } /// ``` - fn paths_mut(&mut self) -> crate::PathsMut<'_>; - - /// Add/remove paths to watch in batch. - /// - /// For some [`Watcher`] implementations this method provides better performance than multiple - /// calls to [`Watcher::watch`] and [`Watcher::unwatch`] if you want to add/remove many paths at once. fn update_watches(&mut self, ops: Vec) -> StdResult<(), UpdateWatchesError> { update_watches(ops, |op| match op { WatchOp::Watch(path, recursive_mode) => self.watch(&path, recursive_mode), @@ -606,7 +557,7 @@ mod tests { } #[test] - fn test_paths_mut() -> std::result::Result<(), Box> { + fn test_update_watches() -> std::result::Result<(), Box> { let dir = tempdir()?; let dir_a = dir.path().join("a"); @@ -619,12 +570,10 @@ mod tests { let mut watcher = RecommendedWatcher::new(tx, Config::default())?; // start watching a and b - { - let mut watcher_paths = watcher.paths_mut(); - watcher_paths.watch(&dir_a, RecursiveMode::Recursive); - watcher_paths.watch(&dir_b, RecursiveMode::Recursive); - watcher_paths.commit()?; - } + watcher.update_watches(vec![ + WatchOp::Watch(dir_a.clone(), RecursiveMode::Recursive), + WatchOp::Watch(dir_b.clone(), RecursiveMode::Recursive), + ])?; // create file1 in both a and b let a_file1 = dir_a.join("file1"); @@ -650,11 +599,7 @@ mod tests { assert!(b_file1_encountered, "Did not receive event of {b_file1:?}"); // stop watching a - { - let mut watcher_paths = watcher.paths_mut(); - watcher_paths.unwatch(&dir_a); - watcher_paths.commit()?; - } + watcher.update_watches(vec![WatchOp::Unwatch(dir_a.clone())])?; // create file2 in both a and b let a_file2 = dir_a.join("file2"); diff --git a/notify/src/null.rs b/notify/src/null.rs index 965e6bb1..bbcd80d9 100644 --- a/notify/src/null.rs +++ b/notify/src/null.rs @@ -36,8 +36,4 @@ impl Watcher for NullWatcher { fn kind() -> crate::WatcherKind { crate::WatcherKind::NullWatcher } - - fn paths_mut(&mut self) -> crate::PathsMut<'_> { - crate::PathsMut::new(self) - } } diff --git a/notify/src/poll.rs b/notify/src/poll.rs index e092aaa0..fecaf22e 100644 --- a/notify/src/poll.rs +++ b/notify/src/poll.rs @@ -661,10 +661,6 @@ impl Watcher for PollWatcher { fn kind() -> crate::WatcherKind { crate::WatcherKind::PollWatcher } - - fn paths_mut(&mut self) -> crate::PathsMut<'_> { - crate::PathsMut::new(self) - } } impl Drop for PollWatcher { diff --git a/notify/src/windows.rs b/notify/src/windows.rs index c946fbe7..797052ed 100644 --- a/notify/src/windows.rs +++ b/notify/src/windows.rs @@ -585,10 +585,6 @@ impl Watcher for ReadDirectoryChangesWatcher { fn kind() -> crate::WatcherKind { WatcherKind::ReadDirectoryChangesWatcher } - - fn paths_mut(&mut self) -> crate::PathsMut<'_> { - crate::PathsMut::new(self) - } } impl Drop for ReadDirectoryChangesWatcher { From f3a13c24d031e83256cbe2907031044be5af31d3 Mon Sep 17 00:00:00 2001 From: Pavel Kurdikov Date: Fri, 18 Jul 2025 18:09:30 +0300 Subject: [PATCH 3/7] fix: deprecate paths_mut, introduce WatchPathConfig and some renames `WatchPathConfig` provides a way to configure single watch. Now it's just a wrapper around a `RecursiveMode` but it'll help us to add some functionality, like filter watched paths or specify a mask for events without breaking changes --- notify/src/config.rs | 61 +++++++++++++++++++++- notify/src/error.rs | 16 +++--- notify/src/fsevent.rs | 21 ++++---- notify/src/lib.rs | 117 +++++++++++++++++++++++++++++------------- 4 files changed, 159 insertions(+), 56 deletions(-) diff --git a/notify/src/config.rs b/notify/src/config.rs index cc65c65f..059129d3 100644 --- a/notify/src/config.rs +++ b/notify/src/config.rs @@ -1,6 +1,6 @@ //! Configuration types -use std::time::Duration; +use std::{path::PathBuf, time::Duration}; /// Indicates whether only the provided directory or its sub-directories as well should be watched #[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Debug, Hash)] @@ -123,3 +123,62 @@ impl Default for Config { } } } + +/// Single watch backend configuration +/// +/// This contains some settings that may relate to only one specific backend, +/// such as to correctly configure each backend regardless of what is selected during runtime. +#[derive(Debug)] +pub struct WatchPathConfig { + recursive_mode: RecursiveMode, +} + +impl WatchPathConfig { + /// Creates new instance with provided [`RecursiveMode`] + pub fn new(recursive_mode: RecursiveMode) -> Self { + Self { recursive_mode } + } + + /// Set [`RecursiveMode`] for the watch + pub fn with_recursive_mode(mut self, recursive_mode: RecursiveMode) -> Self { + self.recursive_mode = recursive_mode; + self + } + + /// Returns current setting + pub fn recursive_mode(&self) -> RecursiveMode { + self.recursive_mode + } +} + +/// An operation to apply to a watcher +/// +/// See [`Watcher::update_paths`] for more information +#[derive(Debug)] +pub enum PathOp { + /// Path should be watcher + Watch(PathBuf, WatchPathConfig), + + /// Path should be unwatched + Unwatch(PathBuf), +} + +impl PathOp { + /// Watch the path with [`RecursiveMode::Recursive`] + pub fn watch_recursive>(path: P) -> Self { + Self::Watch(path.into(), WatchPathConfig::new(RecursiveMode::Recursive)) + } + + /// Watch the path with [`RecursiveMode::NonRecursive`] + pub fn watch_non_recursive>(path: P) -> Self { + Self::Watch( + path.into(), + WatchPathConfig::new(RecursiveMode::NonRecursive), + ) + } + + /// Unwatch the path + pub fn unwatch>(path: P) -> Self { + Self::Unwatch(path.into()) + } +} diff --git a/notify/src/error.rs b/notify/src/error.rs index 1081e006..78831029 100644 --- a/notify/src/error.rs +++ b/notify/src/error.rs @@ -1,6 +1,6 @@ //! Error types -use crate::{Config, WatchOp}; +use crate::{Config, PathOp}; use std::error::Error as StdError; use std::fmt::Debug; use std::path::PathBuf; @@ -159,23 +159,23 @@ impl From> for Error { } } -/// The error provided by [`crate::Watcher::update_watches`] method +/// The error provided by [`crate::Watcher::update_paths`] method #[derive(Debug)] -pub struct UpdateWatchesError { +pub struct UpdatePathsError { /// The original error pub source: Error, /// The remaining operations that haven't been applied - pub remaining: Vec, + pub remaining: Vec, } -impl fmt::Display for UpdateWatchesError { +impl fmt::Display for UpdatePathsError { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!(f, "unable to apply the batch operation: {}", self.source) } } -impl StdError for UpdateWatchesError { +impl StdError for UpdatePathsError { fn source(&self) -> Option<&(dyn StdError + 'static)> { Some(&self.source) } @@ -198,8 +198,8 @@ mod tests { } #[test] - fn display_update_watches() { - let actual = UpdateWatchesError { + fn display_update_paths() { + let actual = UpdatePathsError { source: Error::generic("Some error"), remaining: Default::default(), } diff --git a/notify/src/fsevent.rs b/notify/src/fsevent.rs index 13f782ed..326614c2 100644 --- a/notify/src/fsevent.rs +++ b/notify/src/fsevent.rs @@ -14,7 +14,7 @@ #![allow(non_upper_case_globals, dead_code)] -use crate::{event::*, WatchOp}; +use crate::{event::*, PathOp}; use crate::{unbounded, Config, Error, EventHandler, RecursiveMode, Result, Sender, Watcher}; use fsevent_sys as fs; use fsevent_sys::core_foundation as cf; @@ -296,15 +296,15 @@ impl FsEventWatcher { result } - fn update_watches_inner( + fn update_paths_inner( &mut self, - ops: Vec, - ) -> crate::StdResult<(), crate::UpdateWatchesError> { + ops: Vec, + ) -> crate::StdResult<(), crate::UpdatePathsError> { self.stop(); - let result = crate::update_watches(ops, |op| match op { - crate::WatchOp::Watch(path, recursive_mode) => self.append_path(&path, recursive_mode), - crate::WatchOp::Unwatch(path) => self.remove_path(&path), + let result = crate::update_paths(ops, |op| match op { + crate::PathOp::Watch(path, config) => self.append_path(&path, config.recursive_mode()), + crate::PathOp::Unwatch(path) => self.remove_path(&path), }); // ignore return error: may be empty path list @@ -582,11 +582,8 @@ impl Watcher for FsEventWatcher { self.unwatch_inner(path) } - fn update_watches( - &mut self, - ops: Vec, - ) -> crate::StdResult<(), crate::UpdateWatchesError> { - self.update_watches_inner(ops) + fn update_paths(&mut self, ops: Vec) -> crate::StdResult<(), crate::UpdatePathsError> { + self.update_paths_inner(ops) } fn configure(&mut self, config: Config) -> Result { diff --git a/notify/src/lib.rs b/notify/src/lib.rs index 6ae43c6b..daeca44f 100644 --- a/notify/src/lib.rs +++ b/notify/src/lib.rs @@ -162,10 +162,10 @@ #![deny(missing_docs)] -pub use config::{Config, RecursiveMode}; -pub use error::{Error, ErrorKind, Result, UpdateWatchesError}; +pub use config::{Config, PathOp, RecursiveMode, WatchPathConfig}; +pub use error::{Error, ErrorKind, Result, UpdatePathsError}; pub use notify_types::event::{self, Event, EventKind}; -use std::path::{Path, PathBuf}; +use std::path::Path; pub(crate) type StdResult = std::result::Result; pub(crate) type Receiver = std::sync::mpsc::Receiver; @@ -294,16 +294,47 @@ pub enum WatcherKind { NullWatcher, } -/// An operation to apply to a watcher +/// Providing methods for adding and removing paths to watch. /// -/// See [`Watcher::update_watches`] for more information -#[derive(Debug, Clone, PartialEq, Eq, Hash)] -pub enum WatchOp { - /// Path should be watcher - Watch(PathBuf, RecursiveMode), - - /// Path should be unwatched - Unwatch(PathBuf), +/// `Box` is created by [`Watcher::paths_mut`]. See its documentation for more. +#[deprecated( + since = "9.0.0", + note = "paths_mut has been replaced with update_paths" +)] +pub trait PathsMut { + /// Add a new path to watch. See [`Watcher::watch`] for more. + fn add(&mut self, path: &Path, recursive_mode: RecursiveMode) -> Result<()>; + + /// Remove a path from watching. See [`Watcher::unwatch`] for more. + fn remove(&mut self, path: &Path) -> Result<()>; + + /// Ensure added/removed paths are applied. + fn commit(self: Box) -> Result<()>; +} + +struct PathsMutInternal<'a, T: ?Sized> { + ops: Vec, + watcher: &'a mut T, +} + +#[allow(deprecated)] +impl<'a, T: Watcher + ?Sized> PathsMut for PathsMutInternal<'a, T> { + fn add(&mut self, path: &Path, recursive_mode: RecursiveMode) -> Result<()> { + self.ops.push(PathOp::Watch( + path.to_path_buf(), + WatchPathConfig::new(recursive_mode), + )); + Ok(()) + } + + fn remove(&mut self, path: &Path) -> Result<()> { + self.ops.push(PathOp::Unwatch(path.to_path_buf())); + Ok(()) + } + + fn commit(self: Box) -> Result<()> { + self.watcher.update_paths(self.ops).map_err(|v| v.source) + } } /// Type that can deliver file activity notifications @@ -341,6 +372,19 @@ pub trait Watcher { /// fails. fn unwatch(&mut self, path: &Path) -> Result<()>; + /// deprecated + #[deprecated( + since = "9.0.0", + note = "paths_mut has been replaced with update_paths" + )] + #[allow(deprecated)] + fn paths_mut<'me>(&'me mut self) -> Box { + Box::new(PathsMutInternal { + watcher: self, + ops: Default::default(), + }) + } + /// Add/remove paths to watch in batch. /// /// For some [`Watcher`] implementations this method provides better performance than multiple @@ -349,31 +393,31 @@ pub trait Watcher { /// # Examples /// /// ``` - /// # use notify::{Watcher, RecursiveMode, WatchOp}; - /// # use std::path::Path; + /// # use notify::{Watcher, RecursiveMode, PathOp}; + /// # use std::path::{Path, PathBuf}; /// # fn main() -> Result<(), Box> { - /// # let many_paths_to_add = vec![]; - /// # let many_paths_to_remove = vec![]; + /// # let many_paths_to_add: Vec = vec![]; + /// # let many_paths_to_remove: Vec = vec![]; /// let mut watcher = notify::NullWatcher; /// let mut batch = Vec::new(); /// - /// for (path, recursive_mode) in many_paths_to_add { - /// batch.push(WatchOp::Watch(path, recursive_mode)); + /// for path in many_paths_to_add { + /// batch.push(PathOp::watch_recursive(path)); /// } /// /// for path in many_paths_to_remove { - /// batch.push(WatchOp::Unwatch(path)); + /// batch.push(PathOp::unwatch(path)); /// } /// /// // real work is done there - /// watcher.update_watches(batch)?; + /// watcher.update_paths(batch)?; /// # Ok(()) /// # } /// ``` - fn update_watches(&mut self, ops: Vec) -> StdResult<(), UpdateWatchesError> { - update_watches(ops, |op| match op { - WatchOp::Watch(path, recursive_mode) => self.watch(&path, recursive_mode), - WatchOp::Unwatch(path) => self.unwatch(&path), + fn update_paths(&mut self, ops: Vec) -> StdResult<(), UpdatePathsError> { + update_paths(ops, |op| match op { + PathOp::Watch(path, config) => self.watch(&path, config.recursive_mode()), + PathOp::Unwatch(path) => self.unwatch(&path), }) } @@ -438,17 +482,14 @@ where RecommendedWatcher::new(event_handler, Config::default()) } -pub(crate) fn update_watches( - ops: Vec, - mut apply: F, -) -> StdResult<(), UpdateWatchesError> +pub(crate) fn update_paths(ops: Vec, mut apply: F) -> StdResult<(), UpdatePathsError> where - F: FnMut(WatchOp) -> Result<()>, + F: FnMut(PathOp) -> Result<()>, { let mut iter = ops.into_iter(); while let Some(op) = iter.next() { if let Err(source) = apply(op) { - return Err(UpdateWatchesError { + return Err(UpdatePathsError { source, remaining: iter.collect(), }); @@ -557,7 +598,7 @@ mod tests { } #[test] - fn test_update_watches() -> std::result::Result<(), Box> { + fn test_update_paths() -> std::result::Result<(), Box> { let dir = tempdir()?; let dir_a = dir.path().join("a"); @@ -570,9 +611,15 @@ mod tests { let mut watcher = RecommendedWatcher::new(tx, Config::default())?; // start watching a and b - watcher.update_watches(vec![ - WatchOp::Watch(dir_a.clone(), RecursiveMode::Recursive), - WatchOp::Watch(dir_b.clone(), RecursiveMode::Recursive), + watcher.update_paths(vec![ + PathOp::Watch( + dir_a.clone(), + WatchPathConfig::new(RecursiveMode::Recursive), + ), + PathOp::Watch( + dir_b.clone(), + WatchPathConfig::new(RecursiveMode::Recursive), + ), ])?; // create file1 in both a and b @@ -599,7 +646,7 @@ mod tests { assert!(b_file1_encountered, "Did not receive event of {b_file1:?}"); // stop watching a - watcher.update_watches(vec![WatchOp::Unwatch(dir_a.clone())])?; + watcher.update_paths(vec![PathOp::unwatch(&dir_a)])?; // create file2 in both a and b let a_file2 = dir_a.join("file2"); From 79f2d0eebe12063da3a0dd3ec47ec4262aee8f19 Mon Sep 17 00:00:00 2001 From: Pavel Kurdikov Date: Sat, 19 Jul 2025 09:29:16 +0300 Subject: [PATCH 4/7] chore: update changelog for #705 --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4b42a422..d8089c68 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,9 +3,11 @@ ## notify 8.2.0 (unreleased) - FEATURE: notify a user if the `max_user_watches` has been reached implicitly (`INotifyWatcher`) [#698] - FIX: `INotifyWatcher` ignores events with unknown watch descriptors (instead of `EventMask::Q_OVERFLOW`) [#700] +- FEATURE: deprecate `Watcher::paths_mut` and introduce `update_paths` [#705] [#698]: https://github.com/notify-rs/notify/pull/698 [#700]: https://github.com/notify-rs/notify/pull/700 +[#705]: https://github.com/notify-rs/notify/pull/705 ## notify 8.1.0 (2025-07-03) - FEATURE: added support for the [`flume`](https://docs.rs/flume) crate From cc1bc730229247aea0fa0d51cf94f93bde822e68 Mon Sep 17 00:00:00 2001 From: Pavel Kurdikov Date: Mon, 21 Jul 2025 15:43:59 +0300 Subject: [PATCH 5/7] feat: UpdatePathsError into Error conversion --- notify/src/error.rs | 6 ++++++ notify/src/lib.rs | 48 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+) diff --git a/notify/src/error.rs b/notify/src/error.rs index 78831029..42bdfc07 100644 --- a/notify/src/error.rs +++ b/notify/src/error.rs @@ -181,6 +181,12 @@ impl StdError for UpdatePathsError { } } +impl From for Error { + fn from(value: UpdatePathsError) -> Self { + value.source + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/notify/src/lib.rs b/notify/src/lib.rs index daeca44f..d3dedef9 100644 --- a/notify/src/lib.rs +++ b/notify/src/lib.rs @@ -668,4 +668,52 @@ mod tests { } panic!("Did not receive the event of {b_file2:?}"); } + + #[test] + fn update_paths_in_a_loop_with_errors() -> StdResult<(), Box> { + let dir = tempdir()?; + let existing_dir_1 = dir.path().join("existing_dir_1"); + let not_existent_dir = dir.path().join("noт_existent_dir"); + let existing_dir_2 = dir.path().join("existing_dir_2"); + + fs::create_dir(&existing_dir_1)?; + fs::create_dir(&existing_dir_2)?; + + let mut paths_to_add = vec![ + PathOp::watch_recursive(existing_dir_1.clone()), + PathOp::watch_recursive(not_existent_dir.clone()), + PathOp::watch_recursive(existing_dir_2.clone()), + ]; + + let (tx, rx) = std::sync::mpsc::channel(); + let mut watcher = RecommendedWatcher::new(tx, Config::default())?; + + while !paths_to_add.is_empty() { + if let Err(e) = watcher.update_paths(std::mem::take(&mut paths_to_add)) { + paths_to_add = e.remaining; + } + } + + fs::create_dir(existing_dir_1.join("1"))?; + fs::create_dir(¬_existent_dir)?; + let waiting_path = existing_dir_2.join("1"); + fs::create_dir(&waiting_path)?; + + for event in iter_with_timeout(&rx) { + let path = event + .paths + .first() + .unwrap_or_else(|| panic!("event must have a path: {event:?}")); + assert!( + path != ¬_existent_dir, + "unexpeced {:?} event", + not_existent_dir + ); + if path == &waiting_path { + return Ok(()); + } + } + + panic!("Did not receive the event of {waiting_path:?}"); + } } From 1a127c0b3b35402c7f31eea68180337e0a72718b Mon Sep 17 00:00:00 2001 From: Pavel Kurdikov Date: Mon, 21 Jul 2025 16:52:50 +0300 Subject: [PATCH 6/7] fix: typos --- notify/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/notify/src/lib.rs b/notify/src/lib.rs index d3dedef9..33dca741 100644 --- a/notify/src/lib.rs +++ b/notify/src/lib.rs @@ -706,7 +706,7 @@ mod tests { .unwrap_or_else(|| panic!("event must have a path: {event:?}")); assert!( path != ¬_existent_dir, - "unexpeced {:?} event", + "unexpected {:?} event", not_existent_dir ); if path == &waiting_path { From 5997e676d68b97acbcfc02d749fd78e99c68f57d Mon Sep 17 00:00:00 2001 From: Pavel Kurdikov Date: Mon, 21 Jul 2025 20:41:02 +0300 Subject: [PATCH 7/7] fix(test): macos fsevents may return non-canonicalized paths --- notify/src/lib.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/notify/src/lib.rs b/notify/src/lib.rs index 33dca741..a2cc1a05 100644 --- a/notify/src/lib.rs +++ b/notify/src/lib.rs @@ -673,7 +673,7 @@ mod tests { fn update_paths_in_a_loop_with_errors() -> StdResult<(), Box> { let dir = tempdir()?; let existing_dir_1 = dir.path().join("existing_dir_1"); - let not_existent_dir = dir.path().join("noт_existent_dir"); + let not_existent_file = dir.path().join("noт_existent_dir"); let existing_dir_2 = dir.path().join("existing_dir_2"); fs::create_dir(&existing_dir_1)?; @@ -681,7 +681,7 @@ mod tests { let mut paths_to_add = vec![ PathOp::watch_recursive(existing_dir_1.clone()), - PathOp::watch_recursive(not_existent_dir.clone()), + PathOp::watch_recursive(not_existent_file.clone()), PathOp::watch_recursive(existing_dir_2.clone()), ]; @@ -694,10 +694,10 @@ mod tests { } } - fs::create_dir(existing_dir_1.join("1"))?; - fs::create_dir(¬_existent_dir)?; + fs::write(existing_dir_1.join("1"), "")?; + fs::write(¬_existent_file, "")?; let waiting_path = existing_dir_2.join("1"); - fs::create_dir(&waiting_path)?; + fs::write(&waiting_path, "")?; for event in iter_with_timeout(&rx) { let path = event @@ -705,11 +705,11 @@ mod tests { .first() .unwrap_or_else(|| panic!("event must have a path: {event:?}")); assert!( - path != ¬_existent_dir, + path != ¬_existent_file, "unexpected {:?} event", - not_existent_dir + not_existent_file ); - if path == &waiting_path { + if path == &waiting_path || path == &waiting_path.canonicalize()? { return Ok(()); } }