diff --git a/CHANGELOG.md b/CHANGELOG.md index 841055a9..361396fe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,14 +2,14 @@ ## notify 9.0.0 (unreleased) - CHANGE: raise MSRV to 1.82 **breaking** - -## notify (unreleased) - FIX: Fix the bug that `FsEventWatcher` crashes when dealing with empty path [#718] +- FEATURE: remove `Watcher::paths_mut` and introduce `update_paths` [#705] [#718]: https://github.com/notify-rs/notify/pull/718 +[#705]: https://github.com/notify-rs/notify/pull/705 -## notify 8.2.0 (2025-08-03) +## notify 8.2.0 - FEATURE: notify user if inotify's `max_user_watches` has been reached [#698] - FIX: `INotifyWatcher` ignore events with unknown watch descriptors (instead of `EventMask::Q_OVERFLOW`) [#700] diff --git a/notify/src/config.rs b/notify/src/config.rs index cc65c65f..9d20db8a 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 watched + 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 3d88bd7f..42bdfc07 100644 --- a/notify/src/error.rs +++ b/notify/src/error.rs @@ -1,7 +1,8 @@ //! Error types -use crate::Config; +use crate::{Config, PathOp}; 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,61 @@ impl From> for Error { } } -#[test] -fn display_formatted_errors() { - let expected = "Some error"; +/// The error provided by [`crate::Watcher::update_paths`] method +#[derive(Debug)] +pub struct UpdatePathsError { + /// The original error + pub source: Error, + + /// The remaining operations that haven't been applied + pub remaining: Vec, +} + +impl fmt::Display for UpdatePathsError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "unable to apply the batch operation: {}", self.source) + } +} - assert_eq!(expected, format!("{}", Error::generic(expected))); +impl StdError for UpdatePathsError { + fn source(&self) -> Option<&(dyn StdError + 'static)> { + Some(&self.source) + } +} - assert_eq!( - expected, - format!("{}", Error::io(io::Error::other(expected))) - ); +impl From for Error { + fn from(value: UpdatePathsError) -> Self { + value.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::io(io::Error::other(expected))) + ); + } + + #[test] + fn display_update_paths() { + let actual = UpdatePathsError { + source: Error::generic("Some error"), + remaining: Default::default(), + } + .to_string(); + + 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 2324c951..d6c0510e 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::*, PathOp}; +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_paths_inner( + &mut self, + ops: Vec, + ) -> crate::StdResult<(), crate::UpdatePathsError> { + self.stop(); + + 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 + let _ = self.run(); + + result + } + #[inline] fn is_running(&self) -> bool { self.runloop.is_some() @@ -588,14 +580,14 @@ 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_paths(&mut self, ops: Vec) -> crate::StdResult<(), crate::UpdatePathsError> { + self.update_paths_inner(ops) + } + fn configure(&mut self, config: Config) -> Result { let (tx, rx) = unbounded(); self.configure_raw_mode(config, tx); diff --git a/notify/src/lib.rs b/notify/src/lib.rs index 0eb73c7b..e66511a0 100644 --- a/notify/src/lib.rs +++ b/notify/src/lib.rs @@ -162,11 +162,12 @@ #![deny(missing_docs)] -pub use config::{Config, RecursiveMode}; -pub use error::{Error, ErrorKind, Result}; +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; +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"))] @@ -293,23 +294,6 @@ pub enum WatcherKind { NullWatcher, } -/// 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<()>; - - /// Remove a path from watching. See [`Watcher::unwatch`] for more. - fn remove(&mut self, path: &Path) -> Result<()>; - - /// Ensure added/removed paths are applied. - /// - /// 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<()>; -} - /// Type that can deliver file activity notifications /// /// `Watcher` is implemented per platform using the best implementation available on that platform. @@ -345,40 +329,40 @@ 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. + /// 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, Result}; - /// # use std::path::Path; - /// # fn main() -> Result<()> { - /// # let many_paths_to_add = vec![]; - /// let mut watcher = notify::recommended_watcher(|_event| { /* event handler */ })?; - /// let mut watcher_paths = watcher.paths_mut(); + /// # use notify::{Watcher, RecursiveMode, PathOp}; + /// # use std::path::{Path, PathBuf}; + /// # fn main() -> Result<(), Box> { + /// # 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 in many_paths_to_add { - /// watcher_paths.add(path, RecursiveMode::Recursive)?; + /// batch.push(PathOp::watch_recursive(path)); + /// } + /// + /// for path in many_paths_to_remove { + /// batch.push(PathOp::unwatch(path)); /// } - /// watcher_paths.commit()?; + /// + /// // real work is done there + /// watcher.update_paths(batch)?; /// # 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 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), + }) } /// Configure the watcher at runtime. @@ -442,6 +426,22 @@ where RecommendedWatcher::new(event_handler, Config::default()) } +pub(crate) fn update_paths(ops: Vec, mut apply: F) -> StdResult<(), UpdatePathsError> +where + F: FnMut(PathOp) -> Result<()>, +{ + let mut iter = ops.into_iter(); + while let Some(op) = iter.next() { + if let Err(source) = apply(op) { + return Err(UpdatePathsError { + source, + remaining: iter.collect(), + }); + } + } + Ok(()) +} + #[cfg(test)] mod tests { use std::{ @@ -542,7 +542,7 @@ mod tests { } #[test] - fn test_paths_mut() -> std::result::Result<(), Box> { + fn test_update_paths() -> std::result::Result<(), Box> { let dir = tempdir()?; let dir_a = dir.path().join("a"); @@ -555,12 +555,16 @@ mod tests { let mut watcher = RecommendedWatcher::new(tx, Config::default())?; // 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.commit()?; - } + 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 let a_file1 = dir_a.join("file1"); @@ -586,11 +590,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.remove(&dir_a)?; - watcher_paths.commit()?; - } + watcher.update_paths(vec![PathOp::unwatch(&dir_a)])?; // create file2 in both a and b let a_file2 = dir_a.join("file2"); @@ -612,4 +612,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_file = dir.path().join("not_existent_file"); + 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_file.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::write(existing_dir_1.join("1"), "")?; + fs::write(¬_existent_file, "")?; + let waiting_path = existing_dir_2.join("1"); + fs::write(&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_file, + "unexpected {:?} event", + not_existent_file + ); + if path == &waiting_path || path == &waiting_path.canonicalize()? { + return Ok(()); + } + } + + panic!("Did not receive the event of {waiting_path:?}"); + } }