Add recursive backend that wraps any other non-recursive backend.
Consider this a draft. It would be useful to hear whether there is interest in this approach before investing more time in it, and if so to get some help from others to debug platform specifics, especially for BSD, as I have little experience there. The public interface needs some consideration, currently I've just set this up to force the use of the recursive backend in all cases, as that was easiest for testing.
There is interest in supporting recursive watchers (e.g. #673, #114, #18). I have been interested in this for a PR on rclone also (rclone/rclone#8258), where I wrote a user-level recursive watcher around fsnotify that works on Linux and Mac, so in some sense this is looking at the feasibility of upstreaming that.
The idea of this approach is to provide a recursive backend that wraps any other non-recursive backend to provide support for recursive watchers, so that it is not necessary to add support for every backend separately. It does that using a pipeline approach, starting a goroutine that consumes from the existing event channel, does its work for recursion, and emits to a second event channel from which the user of fsnotify consumes.
For the Windows backend, where there is system-level support, it mostly passes through, although still acts as a wrapper to provide consistent behavior with create events. I realise the inotify backend has a user-level recursive implementation too; much of this is based on that actually, and might be seen as factoring that out to be usable by other backends too.
Recursion already works for Windows and inotify; adding to it kqueue isn't actually that hard and I did it already last year, but then I accidentally deleted all my work by freeing up space and deleting my FreeBSD VM 😭
I need to re-do that work and rewrite the kqueue backend (again), not just for the recursion, but also for other features like event filtering and tracking renames.
That leaves us with illumos/FEN. I think adding it to that isn't too much effort, but I haven't looked at it yet. I'm also kind of okay shipping without FEN support if that's really a hang-up, because it's so little used. But we'll see how it goes.
So with that, it kind of makes this approach redundant? It's also something you can do from the "outside": that is, I don't think you strictly need any code in here to make it work?
The idea is to factor out recursion in the design to ensure consistent behavior across all backends (that do not natively support it, so all but Windows I think?). It allows the inotify code to be simplified by removing the recursive bit, and there would be no need to finish recursive implementations for the other backends. Whereas, implementing recursion for each backend (that does not natively support it) separately may mean inconsistent behavior, more opportunity for bugs, and more code to maintain.
Of course, this is not my decision to make, just making the case for consideration.
The idea is to factor out recursion in the design to ensure consistent behavior across all backends (that do not natively support it, so all but Windows I think?). It allows the inotify code to be simplified by removing the recursive bit, and there would be no need to finish recursive implementations for the other backends
Yeah, that would be nice. I guess my concern is stuff like:
if inEvent.Mask&unix.IN_MOVE_SELF == unix.IN_MOVE_SELF {
if watch.recurse { // Do nothing
return Event{}, true
}
Which is not necessarily easy to reliably reproduce with a wrapper? There is some other stuff as well, such as renames within the watched directory: since the file descriptor/watch is retained, all we need to do is update the internal structure.
Other than that, adding recursion isn't that difficult as such. If you look at the ionotify backend then it's very little code and all of it is very straight-forward. It's just that other stuff such as bugs have always taken priority, and often it's quite time-consuming to debug/fix these in a cross-platform way, and that the general design of things is a bit messy in parts due to a long history (hence the need for a rewrite).