Change Linux to report modify when close_write is detected.

[glennw]

The MODIFY event is fired any time a write syscall is fired.

This means that it is often received while a file is being saved, and/or sent
multiple times.

This change makes it only report the close of a file opened for writing,
which means only one event is received, and the writes have all completed
before the event is sent.

[glennw]

I don't know inotify very well, so apologies if this isn't a good fix!

It fixes the issues I was having when trying to detect reloads of large files saved in Sublime Text, and it seems reasonable based on reading /~https://github.com/guard/guard/wiki/Analysis-of-inotify-events-for-different-editors.

But if this will break something or there is a better fix, please let me know!

[passcod]

This should be compared against other backends (except polling), to see what they do. If they have the behaviour of this PR, great :) Otherwise I don't think this mitigation belongs in this (low-level) library.

[glennw]

I don't have access to Windows/Mac to test this on, sorry.

I don't think it's just a mitigation though - without this change, how can you detect when a file modification / save is complete with this library?

[glennw]

Here's an example of what I mentioned above. The events I receive on Linux when saving a large file without this patch are:

watcher Event { path: Some("hsts_preload.json"), op: Ok(WRITE) } 0
watcher Event { path: Some("hsts_preload.json"), op: Ok(WRITE) } 0
watcher Event { path: Some("hsts_preload.json"), op: Ok(WRITE) } 262144
watcher Event { path: Some("hsts_preload.json"), op: Ok(WRITE) } 262144
watcher Event { path: Some("hsts_preload.json"), op: Ok(WRITE) } 380928
watcher Event { path: Some("hsts_preload.json"), op: Ok(WRITE) } 417792
watcher Event { path: Some("hsts_preload.json"), op: Ok(WRITE) } 421536
watcher Event { path: Some("hsts_preload.json"), op: Ok(WRITE) } 421536
watcher Event { path: Some("hsts_preload.json"), op: Ok(WRITE) } 421536
watcher Event { path: Some("hsts_preload.json"), op: Ok(WRITE) } 421536

(The number at the end of each log line is the current file size in my event handler).

So without this patch, I'm not sure how you can reliably check when a file modification is complete with this library on Linux, as there's no event sent at the end of the file save?

[passcod]

I understand, but the situation is this:

• Either other libraries do send a single event at the end of file
  modification, in which case this is a very good bugfix;
• Or they send multiple events at each write, in which case this patch
  causes irreversible loss of information in a completely unexpected manner
  based on what other APIs do, and thus causes a bug.

In the second case, I would then consider whether to create a new Notify
(not inotify) event that is sent only at the end of file modification, for
convenience, then look at implementing that across all backends.

In the first case, I would do the same for "raw" write events, although
with less priority — someone wanting to have raw write events could just be
directed to use inotify directly, for example.

If I'd noticed this behaviour at the start of Notify, chances are I'd have
made the same choice as you, and backends would then have needed to comply
with it, but I haven't, so now I am more bound to respecting the de-facto
("compatible") behaviour than to blindly change things, even if I agree
with the intention (and I do).

Also I'm not sure if merging this would be a breaking change (3.0.0) or a
"minor" bugfix (2.5.whatever we're at), but that's less important. Gut says
bugfix.

On Mon, 18 Jan 2016 17:33 Glenn Watson notifications@github.com wrote:

Here's an example of what I mentioned above. The events I receive on Linux
when saving a large file without this patch are:

watcher Event { path: Some("hsts_preload.json"), op: Ok(WRITE) } 0
watcher Event { path: Some("hsts_preload.json"), op: Ok(WRITE) } 0
watcher Event { path: Some("hsts_preload.json"), op: Ok(WRITE) } 262144
watcher Event { path: Some("hsts_preload.json"), op: Ok(WRITE) } 262144
watcher Event { path: Some("hsts_preload.json"), op: Ok(WRITE) } 380928
watcher Event { path: Some("hsts_preload.json"), op: Ok(WRITE) } 417792
watcher Event { path: Some("hsts_preload.json"), op: Ok(WRITE) } 421536
watcher Event { path: Some("hsts_preload.json"), op: Ok(WRITE) } 421536
watcher Event { path: Some("hsts_preload.json"), op: Ok(WRITE) } 421536
watcher Event { path: Some("hsts_preload.json"), op: Ok(WRITE) } 421536

So without this patch, I'm not sure how you can reliably check when a file
modification is complete with this library on Linux, as there's no event
sent at the end of the file save?

—
Reply to this email directly or view it on GitHub
#48 (comment).

[glennw]

@passcod Yep, what you've written makes perfect sense. I don't have any Windows machines - I may be able to test on a Mac in a few days. If anyone else has access to those platforms and could run those checks that would be great. Otherwise I'll report what I find on Mac in a few days. Thanks.

[passcod]

No worries. Other watchers may yet chime in :) and if I have time I'll look for documentation of the relevant APIs for this information.

[purew]

Hello, I was just reading up on this library for use in a coming project of mine. Just wanted to chime in my thoughts:

Is it really a bug that MODIFY is sent for each write-syscall? To me, that sounds like the code is working like it should. Chances are MODIFY would be used early on to detect a write to a (large) file, while CLOSE_WRITE is used for the situation described in this PR.

Granted, I've only dealt with inotify before and not cross-plattform solutions so perhaps that is the reason for this problem?

[passcod]

This should probably be implemented as a debounce on MODIFY, instead of reporting falsely. That's how e.g. Chokidar does it (as an improvement over the native Node.js fs.watch).