Consider unifying traditional and future-based APIs

[hannobraun]

Currently there are two parallel APIs: The original one, and the new future-based one. I think it would be ideal if we could unify those.

One problem I see is efficiency. The stream-based API requires one additional heap allocation for every event that has a name. The reason for this are lifetime issues that probably can't be resolved, at least not in safe Rust.

I think it's best for now to keep things as they are, while keeping a look at how futures develop. Feedback is very welcome!

[calmofthestorm]

One thought on this would be that the current stream API takes &mut on the Inotify and does not provide a way to add additional watches. I'm working on something where I want to recursively watch a directory, and thus need to add new watches on any newly created directories. One solution would be to allow mutably borrowing an inotify from the stream, or adding functions to add/remove watches directly.

Personally I would favor adding a method to clone an Inotify. It should be safe (in a Rust sense) and the underlying representation is already friendly to this. I understand this would permit certain undesirable uses (e.g., two streams on the same inotify would each get a subset of events).

I've tried a few ways of writing this involving multiple threads, etc, which have all been clumsy. Next I want to try using select along with read_events which I think should be simpler.

[hannobraun]

Thank you for your feedback, @calmofthestorm. I'm very open to improving the situation and make your use case possible, but I think allowing users to clone Inotify would be taking it too far. It would be very error-prone, for the reasons you mention. Adding more methods to EventStream would certainly be acceptable. Maybe there could be a separate API, Watches, with add and remove methods that can be accessed both through Inotify and EventStream (e.g. inotify.watches().add(...)). Maybe it could even be possible to clone a Watches and use it anywhere.

But does creating an EventStream even prevent you from using the original Inotify instance? Inotify::event_stream takes &mut self, but the resulting EventStream doesn't keep that reference. That means it should already be possible to use an Inotify and an arbitrary number of EventStreams in parallel, which seems wrong to me.

Maybe it would be better to replace Inotify::event_stream with an into_event_stream method that consumes the Inotify, and make sure EventStream has all the methods that users need.

[calmofthestorm]

Thanks, I missed that the mut reference is not saved by EventStream (but I agree with your concern that this is wrong).

I like into_event_stream (and ideally a way to go back as well if it's easy to implement), along with having .watches() on both Inotify and EventStream. I could even see an argument to make Watches cloneable and of separated ownership (i.e., contains its own reference to the Arc with the fd).

Then you have two separate concerns -- configuring the inotify, and consuming its events. The latter can be made unique to solve the two streams on the same fd problem.

[hannobraun]

Fully agreed!

I've opened #176 and #177 to track the improvements we talked about here. Any help in implementing them is very welcome. I don't have much time to work on this crate these days, but I'm standing by to review and merge any incoming pull requests.