bug#21435: 25.0.50; file-notify has problems after renames

[Michael Albinus]

Eli Zaretskii <eliz@gnu.org> writes:

Hi Eli,

>> (file-notify (100286608 removed "xxx"))
>> (file-notify (100286608 added "xxx"))
>> (file-notify (100286560 removed "xxx"))
>>
>> The first two events are raised by the file name handler for d:/usr/eli/data.
>> Maybe "xxx" did exist already in that directory?
>
> No, it didn't.  I don't know why w32 sends this strange notification,
> but the fact is it does.  (That doesn't happen when moving a
> directory, btw, only when moving files.)

Hmm, maybe a bug in the MS Windows library?.

However, likely we have no practical problem. The first `removed' event
reports the removal of a non-existing file. Nothing a handler shall be
concerned about (except the test case Tassilo writes for this ...)

>> I would say, that w32notify does not send the renamed-from and
>> renamed-to events, as expected. Maybe they are sent only in case of
>> renaming a file in the same directory?
>
> Yes, that's exactly what happens.  Which IMO is entirely reasonable,
> since each watch watches only a single directory.  That inotify has
> some kind of "global" perspective on such rename events is a bonus,
> but we cannot expect that.

Of course. inotify does this by adding an additional cookie to the
`moved-from' and `moved-to' events. A pair of such events with the same
cookie belongs together. See the example I have shown:

1 -> (file-notify-handle-event (file-notify (1 (moved-from) "xxx" 49278) 
file-notify-callback))
1 -> (file-notify-handle-event (file-notify (2 (moved-to) "xxx" 49278) 
file-notify-callback))

> This, of course, breaks the basic assumption of the design intended to
> provide this feature:
>
>> Two days ago (commit dbdc459a48091f5953faf14bcaaa7e6d37fbf024), I've
>> changed filenotify.el to fire 2 events `renamed' in case the directories
>> of the source and target are different. This was triggered by a user
>> report, that he wants to have auto-revert-mode for two different
>> directories under dired control. So the event is sent for the two
>> different handlers activated by the respective *-add-watch calls.
>
> The design expects 2 'move'/'renamed' events, but that's not
> guaranteed, and doesn't happen on w32.

There's no guarantee. We must say that there will be `renamed' events
only when possible. And if not, one must expect two events `deleted' and
`created', handled by the respective handler.

> If we want to conflate 'removed' followed by 'added' into a rename
> across directories, we will need changes in filenotify.el, and will
> risk false positives, because it could really be a deletion followed
> by a creation of a file by the same name.

We will conflate them only when possible. inotify helps us by setting a
proper cookie. If w32notify cannot provide this, then we will deliver
`deleted' and `created'.

> However, if all we want is to make sure the destination directory gets
> a notification (so it could auto-revert), then this already happens on
> MS-Windows (see the 'created' event above), and therefore nothing
> should be done on Windows to support the user request above.

It's not only the destination directory, it's also the source directory
which matters. Remember the initial use case, two dired buffers under
`auto-revert-mode' control. The moved file must appear in the
destination dired buffer, and it must disappear in the source dired buffer.

> Therefore, I submit that a better solution would be to make inotify
> emulate what w32notify does, i.e. produce a synthetic 'added' event in
> the destination directory when we get a 'moved-to' event that
> specifies a destination directory different from the source.

Not necessary I believe. Due to inotify cookie mechanism, it will work
robustly. And don't forget gfilenotify, which sends a `rename' event
already on low-level.

> Finally, two more comments about this:

>  . I wish such changes were discussed, and the various alternatives
>    examined, before the code is changed

You are right, as always. I didn't expect that there would be such
diasagreement.

OTOH, I have encouraged Tassilo to improve file-notify-tests.el just in
order to have a common base of understanding. The tested (and suceeded)
behaviour will be what low-level libraries and filenotify.el have agreed.

>  . I'm not sure this kind of non-trivial logic is something that
>    belongs to filenotify.el; it could well have a better place in
>    auto-revert.el instead, as that is the level where the logic is
>    needed and understood, or even in the Dired-specific function that
>    auto-reverts a directory

If we only deliver `removed' and `created' events, none of those
libraries would have a chance to pair them to a rename action. Essential
information, like inotify cookies, will be lost.

And yes, this information will be needed. Recently, I saw a discussion
on sx, whether Emacs' `auto.revert-mode' could also support file
renaming. That is, when a buffer is associated by a file, and that file
is renamed outside Emacs, Emacs shall rename `buffer-name' and
`buffer-file-name', and then revert. Nice idea ...

Best regards, Michael.