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

Need for recursive watcher #56

Open
jbowtie opened this issue Aug 6, 2013 · 24 comments

Comments

Projects
None yet
10 participants
@jbowtie
Copy link
Contributor

commented Aug 6, 2013

I've had to implement this several times now, and I really think that a recursive option should be part of the core API because it really is needed in many scenarios.

My usual implementation is to use filepath.Walk on the root directory and call watcher.Watch on every directory; I think it's not obvious and I also have to remember to handle directories specially on a Create event so that new directories are also watched.

I'd be happy to create a pull request for this functionality if there's a chance that you'll accept it. If you don't want to add such an option, perhaps we could provide an example that shows how to do a recursive watch?

@pmonks

This comment has been minimized.

Copy link

commented Aug 15, 2013

+1
It appears that this kind of thing is possible natively for most OSes - see https://github.com/gorakhargosh/watchdog/ for example.

@nathany

This comment has been minimized.

Copy link
Contributor

commented Sep 3, 2013

@jbowtie I also had to create a recursive watcher with the same characteristics for Looper. I am currently working to extract the code into a reusable package, along with adding options for Throttling, filtering by Pattern and logging to help in debugging. I would be grateful for some code review once it's further along.

@jbowtie

This comment has been minimized.

Copy link
Contributor Author

commented Sep 3, 2013

@nathany I'd be happy to review it when ready.

Regarding API changes, I would imagine the only user-facing change needs to be a boolean option for recursion. With implementation we can either go the platform-agnostic approach or modify the per-platform API calls. Since @howeyc hasn't indicated a preferred approach we should probably just go ahead and prepare pull requests for review.

@nathany

This comment has been minimized.

Copy link
Contributor

commented Sep 4, 2013

I'm going to aim for a platform agnostic approach first, given that's the code I have on hand, while keeping in mind that it should be possible override with platform-specific optimizations.

I'm spiking ideas for the public API here but the code itself is a mess. Working on cleaning it up tonight.

@howeyc

This comment has been minimized.

Copy link
Owner

commented Sep 4, 2013

Thanks for working on this, I appreciate it.

Send pull requests whenever you think it's ready.
On Sep 3, 2013 6:23 PM, "Nathan Youngman" notifications@github.com wrote:

I'm going to aim for a platform agnostic approach first, given that's the
code I have on hand, while keeping in mind that it should be possible
override with platform-specific optimizations.

I'm spiking ideas for the public API herehttps://github.com/gophertown/fsnotify_ext/issues/1but the code itself is a mess. Working on cleaning it up tonight.


Reply to this email directly or view it on GitHubhttps://github.com//issues/56#issuecomment-23759929
.

@nathany

This comment has been minimized.

Copy link
Contributor

commented Sep 4, 2013

@howeyc Will do. Making use of the internal state in fsnotify will be better than wrapping it with another package and having to query things again (eg. IsDir()). I'm now considering fsnotify_ext a throw-away spike.

Here is the GoDoc for a proposed public API, including deprecations.

This was referenced Sep 8, 2013

@nathany

This comment has been minimized.

Copy link
Contributor

commented Sep 9, 2013

@pmonks I haven't had a chance to dig into Watchdog or Listen yet to find out which OSes support recursive watches. If anyone wants to do the research, that'd be great :-)

I imagine we could save a bunch of extra book keeping if all the supported OSes have native support. If not, I'll keep on working towards a cross-platform option that can be overridden after (with OS X being an exception, because as it turns out, FSEvents don't support non-recursive watchers!).

@htoothrot

This comment has been minimized.

Copy link

commented Sep 29, 2013

Windows can support this. ReadDirectoryChanges[1] has a "bWatchSubtree" argument. If this argument is true, you will receive events for all files/directories for the whole directory tree rooted at the given directory.

As watching a large/busy tree can lead to many events I thought this was worth bringing up: It may be helpful to have the buffer size configurable in some way. Right now it's a hard coded 4096 bytes. When many events fire quickly, events can be dropped when the buffer is full. There are caveats to simply increasing the size ( http://stackoverflow.com/a/3250454 explains some of the issues ) and it seems there's not one answer to fit all uses.

Thanks for your work on this.

[1] http://msdn.microsoft.com/en-us/library/windows/desktop/aa365465%28v=vs.85%29.aspx

@nathany

This comment has been minimized.

Copy link
Contributor

commented Sep 29, 2013

Thanks @htoothrot.

Does anyone know if inotify supports recursive watches? or kqueue?

@htoothrot

This comment has been minimized.

Copy link

commented Sep 29, 2013

http://lists.qt-project.org/pipermail/development/2012-July/005279.html

has a quick overview. It's from last year but some checking seems to indicate that nothing has changed...

@dubcanada

This comment has been minimized.

Copy link

commented Oct 6, 2013

Poco.DirectoryWatcher requires each directory to be watched separately. Most likely recursive watching is not crossplatform.

watchdog and listen both add each directory to monitor if recursive is turned on.

watchdog example...

for root, directories, files in path_walk(self._path, recursive):
    for directory_name in directories:
      try:
        directory_path = os.path.join(root, directory_name)
        stat_info = os.stat(directory_path)
        self._stat_snapshot[directory_path] = stat_info
        self._inode_to_path[stat_info.st_ino] = directory_path
        walker_callback(directory_path, stat_info)
      except OSError:
        continue

if recursive is turned on, it looks through all the directories and adds them to the snapshot, which is then all monitored (individually on the system level).

Moral of the story is this isn't possible 100% crossplatform. You would need to monitor each individually on Linux (Windows has recursive, inotify isn't recursive, and neither is kqueue).

@nathany

This comment has been minimized.

Copy link
Contributor

commented Oct 7, 2013

@dubcanada Thanks for the info.

I have some Go code for a recursive watcher here, which I've started to generalize for inclusion in fsnotify.

My intention is to have Watch() ask the adapter (FSEvents, inotify, etc.) to setup a recursive watch, and if it can't, enable a manual approach like this.

I still haven't figured out exactly how some of the internal state will work. Particularly, applying all the same flags and options for the parent folder to each subfolder. Lots of Mutexes & pointers in the fsnotify internals, which I'm not use to individually, much less in combination. Mostly I just need to spend a little more time on it, and allow someone to catch my mistakes in code review. :-)

@nathany

This comment has been minimized.

Copy link
Contributor

commented Oct 28, 2013

Just added a Wiki page to list any reference materials that would be helpful. Please help fill it in. Thanks!

@bronze1man

This comment has been minimized.

Copy link
Contributor

commented Nov 19, 2013

I had wrapped this library with a recursive watch directory interface...
https://github.com/bronze1man/kmg/tree/master/fsnotify
doc:
http://godoc.org/github.com/bronze1man/kmg/fsnotify

@nathany nathany referenced this issue Nov 30, 2013

Closed

Milestones #78

6 of 9 tasks complete
@ahtik

This comment has been minimized.

Copy link

commented Mar 4, 2014

Could someone confirm the status of recursive watchers?

As I understand recursive watchers were actively in development but then with the move to become os/fsnotify in Go 1.3 it got less priority.

From https://codereview.appspot.com/48310043/ it seems that by being part of Go 1.3 distribution recursive watching becomes a higher-level feature that should be implemented outside the distribution.

Is this observation correct, fsnotify is not going to have recursive watcher any time soon?
Should we wait for the 1.3 to get the stabilized fsnotify API and then write our own package on top of fsnotify for recursive watching?

At the moment I'm using fsnotify for single folder files successfully and my hope was to use fsnotify also for recursive watches at one point but unfortunately I don't have the technical capacity to implement the whole recursive watcher stack myself.

@nathany

This comment has been minimized.

Copy link
Contributor

commented Mar 4, 2014

@ahtik os/fsnotify was pushed back to Go 1.4. the recursive watcher is still up for discussion, but we are looking at fsevents on OS X which is recursive and Windows also has a subtree watch. I suspect there will be some form of recursive watching, but I'm not sure if it will just expose it when available on the host OS or actually provide a user-space recursive watch on on BSD, Linux and Solaris. Either way I aim to provide something, whether in fsnotify proper or a separate package that depends on it.

@ahtik

This comment has been minimized.

Copy link

commented Mar 4, 2014

@nathany That's comforting to hear, thank you for the update.

@TriskalJM

This comment has been minimized.

Copy link

commented Jul 24, 2015

Any updates on this?

@nathany

This comment has been minimized.

Copy link
Contributor

commented Jul 24, 2015

@TriskalJM No updates on this for fsnotify, but see https://github.com/rjeczalik/notify.

@TriskalJM

This comment has been minimized.

Copy link

commented Jul 24, 2015

@nathany Is fsnotify still going to be included in the golang spec moving forward, or is the project you linked to going to replace it?

@nathany

This comment has been minimized.

Copy link
Contributor

commented Jul 24, 2015

I don't actually know if something is going to be added to the standard library at all. At this point, the go tool could just use an internal package if it needs event notifications.

The related issue is golang/go#4068

@markvincze

This comment has been minimized.

Copy link

commented Oct 2, 2016

What's the situation with this feature?
Is it possible to watch a directory recursively with fsnotify? If not, is there a recommended alternative package?

@nathany

This comment has been minimized.

Copy link
Contributor

commented Oct 2, 2016

You can give https://github.com/rjeczalik/notify a try.

Fsnotify doesn't yet support recursive watching.

@markvincze

This comment has been minimized.

Copy link

commented Oct 2, 2016

@nathany, ah thanks, it seems to work nicely on Linux so far.
I'll give it a go on OSX and Windows too later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.