Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Removing recursive watches #41

Closed
cespare opened this issue Sep 12, 2014 · 6 comments
Closed

Removing recursive watches #41

cespare opened this issue Sep 12, 2014 · 6 comments
Labels

Comments

@cespare
Copy link
Contributor

cespare commented Sep 12, 2014

I've implemented recursive watching for a few different projects. The way I've basically done this is:

  • Add a watch for the root dir
  • When there's a create event, walk the created dir and add new watches for all dirs
  • When there's a remove event, remove the watch for that dir

(I'm using fsnotify.v1.) I've realized that this isn't correct, because I don't delete the watches recursively. So I guess I need to keep track of the full tree of watches on my end as well? Maybe fsnotify could help out by exposing a 'delete all watches with this prefix' method or something.

Suggestions?

Additionally, #40 means that even if I could know all the dirs to remove watches for, I still couldn't delete them.

@cespare cespare changed the title Cannot remove watch for deleted dir Removing recursive watches Sep 12, 2014
@nathany nathany added this to the v3 Recursive Watcher milestone Sep 13, 2014
@nathany
Copy link
Contributor

nathany commented Sep 13, 2014

Right now I can only think of two possible ways to remove recursive watches for #18:

  1. keep track of all the folders we watch, this would be like asking it to remove a specific watch we added
  2. just walk the subfolders of a path and remove any watches that exist

I'm in favour of 2, as additional watches could be created, and the API at present takes a path rather than a handle to a previous watch.

However, I'd still like to see how Windows and FSEvents (OS X) handle recursive watches and removals before tackling this... which puts it a fair ways off.

@cespare
Copy link
Contributor Author

cespare commented Sep 13, 2014

I'm not quite clear about what we're discussing here.

I'm trying to implement recursive watching myself, today, independently of #18. fsnotify does not let me do this without keeping track of all watches (i.e. duplicating w.watches) myself.

just walk the subfolders of a path and remove any watches that exist

You can't do this once the directory is gone.

So I'm interested in a way of getting at w.watches directly, without relying on current FS state.

@nathany
Copy link
Contributor

nathany commented Sep 13, 2014

Quite some time ago I attempted to track all the watched folders independently, but ran into an issue much like #40 in kqueue. I thought the watches were just magically removed when the folder was deleted, but I may have been wrong. (nathany/looper@212051b)

It might be worth building a small test case to see if unwatching deleted folders is necessary. I started hacking on fslog as a place to do those sort of experiments (without all the other stuff fsnotify adds). But so far it only has kqueue.

It should be relatively easy to resolve #40, but I'd rather not expose w.watches as an API. It's supposed to be an internal detail.

@cespare
Copy link
Contributor Author

cespare commented Sep 13, 2014

It might be worth building a small test case to see if unwatching deleted folders is necessary.

It is not necessary from an inotify perspective; the watch is gone when the inode goes away.

A satisfactory solution to this might very well be "you don't have to delete watches for removed directories; they are removed automatically." But then we'd have to make sure that works:

  1. Ensure that the OS API (inotify, etc) watches are removed automatically on all platforms (may just work, like it does for inotify)
  2. Automatically delete removed paths from w.watches, which is not even possible to do manually (from a client) right now because of Cannot remove watch for deleted file #40.

@nathany
Copy link
Contributor

nathany commented Sep 13, 2014

So for 2, fsnotify should itself watch for remove event and do the internal cleanup of w.watches. That could be tricky later on if the user doesn't wish to subscribe to remove events (#7).

But for now, my focus is on cleaning up the existing code, primarily separating the lower-level stuff out. Also doing more research as with 1. Fixing #40 and doing the internal cleanup could fit into that.

@arp242
Copy link
Member

arp242 commented Aug 3, 2022

fsnotify does not let me do this without keeping track of all watches (i.e. duplicating w.watches) myself.

There is now the WatchList() method, so this at least is no longer needed.

Better support for e.g. Watcher.Remove("dir/...") is something I'll pick up as part of #18, so closing this as a duplicate of that one.

@arp242 arp242 closed this as completed Aug 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants