[WIP] FSEvent backend for 5.0 #144
Conversation
cmyr
force-pushed
the
fsevent-next
branch
2 times, most recently
from
893e474 to
e80fa74
Compare
| vec
There was a problem hiding this comment.
I've only given a cursory look to the FSEvent documentation, but it is my understanding that it cannot do that, i.e. FSEvent only watches directories (and Apple recommends using kqueue for single files instead).
There was a problem hiding this comment.
With the FileEvents flag available from 10.7, if I create an FSEventStream with a single path, to a file, I will receive only events for that specific file, not events in the parent directory.
My impression is that a lot of Apple's documentation on the matter was written for the initial release, and hasn't been updated.
There was a problem hiding this comment.
Yea, I was pleasantly surprised, because all the docs suggested this wasn't the case. :)
|
@passcod FSEvents will hold back on sending modify events while the modifying process has an open file descriptor. This means that the current tests checking for modification events will fail consistently. More than that: closing the fd does not guarantee that events are flushed; even when you ask the stream not to buffer events, some buffering occurs. To receive multiple events for multiple writes, I have to both close the fd and wait 500ms between them. More generally, on the filesystem-dev mailing list there's a very clear statement from a developer relations person that FSEvent is not a log of file system activity:
So this particular case is not something FSEvent will give us. It's not totally clear to me how I should handle this. Maybe we want a new capability like "EmitImmediately"? Curious to hear your thoughts. |
|
Actually, that's only a problem for the compliance tests, right? It doesn't make a different for usage in Notify or beyond. It's just that this backend will not behave how the compliance tests expect the backends to work. Instead of adding a capability just to cover this particularity of this backend, we can make an exception to the rule "all backends must pass compliance tests" and write those same tests for FSEvent. In essence, this backend will still "pass compliance tests", it's just that it won't use the centralised backend macro for it. So: copy the relevant tests from backend/src/compliance.rs, remove the capability checking, and rewrite them to test this backend with its particularities. When we add more compliance tests we'll need to also add them here, so that's a little more work, but it should work. |
|
Okay that works for me; If you don't think that having the definition of compliance be variable between different backends will be a problem then I am happy to defer to you! I'll have some time tomorrow to work on this, so should at least get the tests done then. |
|
Yeah, I'll have to write this up somewhere, but basically: the tests themselves as written are not super rigorous and they're not normative. "Compliance" is better defined with words, i.e. a spec (which isn't quite written yet), and the documentation for each Capability (which could be more precise, I'll work on that). That doesn't change. The |
|
@cmyr Hey, have you gotten any further on this? I have my Macbook back now, so I could work on this a bit, but I'd rather not duplicate efforts if you're on it :) |
|
@passcod Sorry I've been travelling a little bit so this fell off my list, but I'll have time early next week at the latest (and probably a few hours later today) |
|
No problem! |
This is very much a wip, although it compiles and has a first stab at the flag parsing logic. It doesn't actually run, though.
This patch is a very rough initial pass at handling some of the weird behaviour around FSEvent. In particular, we now track previous events, and use the existence of a previous event to determine how to interpret a new event.
|
Okay, I've updated this with a very very basic implementation. We keep track of previously received events; when we receive a new event, we check if there has been a recent event at the same path; if there has we send events for any flags that are in the new event but not the previous; and we also send the This doesn't feel great to me, but it's a reasonable starting place? Pardon the delay, I've been having trouble getting my head back into this. |
|
Good work! I like this usage of Regarding delay: don't worry :) This isn't blocking anything at the moment — I still have a bunch of work to do on the core interfaces and dayjob has been picking up so I haven't had as much time to work on notify recently. I'm very grateful of any time you do put in this! Though: if you know — at any point — you're not going to have time for a while (weeks, not days), could you chime in here? That way I can know that I could pick it if I have some time. |
|
My feeling right now is that refining this implementation should probably happen in concert with other work, when you'll be developing a better sense of how you're going to be unifying the various backends, and what would be most helpful for you to that end. So maybe give this a read when you have time, see if it makes approximate sense, and then it can be something to iterate from. If it looks like it will make sense, I'm happy to add some more documentation. |
|
Okay, sounds good. I'll do that. |
|
I'll merge this in soonish to work on it in-tree more closely, and bring it up to the newest interface. |
|
I looked some more at FSEvent and had a think and changed the plan... rather dramatically. The low down is this: FSEvent is so radically different that it I think it breaks user expectations of what a filesystem notifier should do. This is also supported by the prose in the Apple documentation. Their recommendation for the kind of semantics we're looking for is kqueue. The plan becomes this:
That makes the default macOS experience more in line with expectations, at the cost of efficiency (i.e. might see higher CPU usage) when watching large trees. People with requirements where FSEvent fits will be able to opt-in. I'm closing this PR, but be sure that this work is not wasted by any means! There's a merge-fsevent branch in the repo and that will form the basis for the backend, later on. |
|
I agree that kqueue is likely the correct API for us on macOS. Looking forward to the new work! |
Very much a rough sketch/first-pass right now (I have no idea if this runs/works, as I'm writing this), but wanted to see if I could get CI running. Will tighten this up in coming days.
progress towards #139