berkus / dir_monitor

Filesystem changes monitor using boost::asio © Boris Schaeling
Boost Software License 1.0
88 stars 40 forks source link

Recursive monitoring request #36

Open Vizor opened 8 years ago

Vizor commented 8 years ago

Please, it's possible to add support for recursive monitoring? On Windows there is argument bWatchSubtree (see here), which is set to FALSE for now.

sbelsky commented 8 years ago

short answer: yes :) but this requires a changes in the dir_monitor interface.

long answer: i have got a working copy with functional requested by you. and i want to merge it in the nearest future. but I can't do it right now because my changes have been made onto base Boris's implementation of dir_monitor. so i need some additional job for adoptation.

Vizor commented 8 years ago

Ok, thank you for answer :).

sbelsky commented 8 years ago

how much time you can endure without this feature?)))

Vizor commented 8 years ago

I have own fork with only setted this argument, so there's no hurry :).

akapelrud commented 8 years ago

It would be nice if this was consistent between the different implementations.

berkus commented 8 years ago

Agree with @akapelrud lets add some flag in the interface (or another method set like add_directory_recursive()) for the recursive case and make default non-recursive.

Opinions?

sbelsky commented 8 years ago

agree in common but need to clarify details in my realization.. on monday.

akapelrud commented 8 years ago

@sbelsky I implemented adding of non-recursively monitored directories for inotify on my own master branch before easter. Have a look at commit 51275f6. Feel free to take that code if you need it. (I pushed my local changes to my master branch, but since then you have started using a develop branch, so my fork is a bit of a mess atm.)

Making the add_directory() functions work in both recursive and non-recursive mode isn't that hard, the hassle lies in implementing this so that directories can be removed afterwards. For instance, a test case could be

  1. Add one directory foo/bar/foo2 non-recursively.
  2. Add the directory foo recursively.
  3. Remove the directory foo.
  4. First directory should still be watched by dir_monitor.
    • On OSX (fsevents) you can just add directories and check paths against the stored directory paths as the events happen.
    • For inotify you can be a bit more clever, as it's non-recursive by default.
    • On windows you can choose either strategies above, as the API supports both.
sbelsky commented 8 years ago

@akapelrud thank you for analyze and code sharing. I fully agree with your arguments but want to add my view. recursive directory monitoring is more complex task than even you describe because performance questions come into play. just imagine a folder with a big big subtree. i don't think real user want to watch over all leafs, just a subset. my proposal:

  1. yes, this must be consistent between the different implementations. each of us specializes in its own operating system and it should be used.
  2. library's interface should stay backward compatible
  3. we can introduce new overloaded add_directory method with second unsigned integer argument. the meaning of this argument is a recursion depth. 0 means no recursion. i did it in my company's fork but as the wrapper of dir_monitor and only for windows.
akapelrud commented 8 years ago
  1. Yes, and it currently isn't. That can give some surprises if portable code is needed.
  2. Yes, or simply alter the current add_directory function to have a second default-valued argument. I would prefer your suggestion though.
  3. I like the recursion depth argument, maybe simply a negative value can be interpreted as infinite recursion (the current default for inotify).
GamePad64 commented 8 years ago

Oh, I have tested dir_monitor only on the inotify backend. I thought it already has recursive directory scanning for all platforms (inotify backend is recursive).

BurningEnlightenment commented 7 years ago

From my point of view one can't keep the interface backwards compatible and fix the inconsistent behaviour at the same time, because by fixing the inconsistent behaviour the semantics of add_directory are changed on at least one platform. My suggestion is to introduce two new methods with well defined semantics like add_directory_recursive and add_directory_shallow. While we leave add_directory untouched and deprecate it (with c++14 the [[deprecated]] attribute could be used).