feat: rework PathsMut to be more consistent

[riberk]

The feature to add / remove paths to watch in batch was introduced in #692 but 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

[dfaust]

if you drop the instance without commit it may leave the watcher in an inconsistent state (especially fsevents).

So if I'm getting this right: When the FsEventPathsMut is used to add/remove paths and then dropped without calling commit, it leaves the watcher in an inconsistent state because the paths have been changed, but the watcher is still watching the old paths. It looks like the paths are not used again (eg. to build a full path when an event is emitted). So this should not cause any errors. But it is confusing and should therefore be fixed.

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

I would like to keep the API lean. Therefore I would deprecate paths_mut.

@branchseer, @JohnTitor Do you have an opinion on this?

[riberk]

it leaves the watcher in an inconsistent state because the paths have been changed

The main problem is not having paths changed but is having watcher stopped if we don't call commit

struct FsEventPathsMut<'a>(&'a mut FsEventWatcher);
impl<'a> FsEventPathsMut<'a> {
    fn new(watcher: &'a mut FsEventWatcher) -> Self {
        watcher.stop(); // <----- If we doesn't call commit the watcher is stopped and the only way to start it is call watch or unwatch
        // watch and unwatch runs the watcher whether encounters with error or not

        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<Self>) -> Result<()> {
        // ignore return error: may be empty path list
        let _ = self.0.run();
        Ok(())
    }
}

[riberk]

I would like to keep the API lean

I agree. User can do it by themself if they want.

Ok, I'll delete it

[dfaust]

Ok, I'll delete it

By deprecating it, I meant marking it with the deprecated attribute:

#[deprecated(since = "9.0.0", note = "paths_mut has been replaced with update_watches")]

Btw. since we are using the term path in other places, I would rename the function to update_paths.

[dfaust]

Also, if the PathsMut compatibility API is the same as the current one, then we can release it as a feature update (v. 8.3.0).

I would wait a day and give the others a chance to add their opinions. Then I can cut a release tomorrow.

[riberk]

Also, if the PathsMut compatibility API is the same as the current one, then we can release it as a feature update (v. 8.3.0).

It's possible, i'll try to do that

[riberk]

Ok, in addition to the renaming and saving API compatibility I changed PathOp argument to us own struct - WatchPathConfig. It'll help us to keep compatibility when new watch-related features are added. It's like a Config but for a single watch

[riberk]

@branchseer my main idea wasn't to remake it just because I want to, but to make the code more ready to #632 . I want this feature, because it allows inotify-watcher to skip big directories that a user doesn't care about and it will cause drastically increasing performance in some cases (inotify spent ALL the time to scan directories, not to syscalls).

[riberk]

@dfaust

Hi! Sorry to bother you — would you mind taking another look at the PR when you get a chance? I'd really appreciate your thoughts.

[dfaust]

@riberk

Sorry, I don't have much mental capacity right now. I will try to look at it on the weekend. Please send me a reminder if I forget.