bug#22557: 25.0.90; Saving a buffer under auto-revert turns off file notifications

[Michael Albinus <michael.albinus@gmx.de>]

Eli Zaretskii <eliz@gnu.org> writes:

Hi Eli,

>   emacs -Q
>   C-x C-f SOME-FILE RET
>   M-x auto-revert-mode RET
>   M-: auto-revert-use-notify RET => t
>
> Modify the buffer and save it with "C-x C-s".  Then:
>
>   M-: auto-revert-use-notify RET => nil
>
> This is on MS-Windows; the same recipe on GNU/Linux with inotify
> doesn't have this problem.

For me the recipe fails in both cases. This is due to
`backup-by-copying' being nil; in case of t there's no problem.

But it shall work independent of this setting. This calls for a new test
case in test/automated/file-notify-tests.el. Will add it.

> Looking under the hood reveals that filenotify.el sends a 'stopped'
> notification when Emacs renames SOME-FILE to SOME-FILE~, to back it
> up.  In response, auto-revert-mode turns off file notifications and
> falls back to polling.
>
> It looks like the reason is this condition in filenotify.el:
>
>         ;; Check for stopped.
>         (setq
>          stopped
>          (or
>           stopped
>           (and
>            (memq action '(deleted renamed))
>            (= (length (cdr registered)) 1)
>            (or
>             (string-equal
>              (file-name-nondirectory file)
> 	     (file-name-nondirectory (car registered)))
>             (string-equal
>              (file-name-nondirectory file)
>              (car (cadr registered)))))))
>
> The second call to string-equal is the culprit: the file names match,
> so 'stopped' gets a non-nil value.

`stopped' shall be raised only in case of `deleted'. For `renamed', it
seems to be wrong. Will check.

> I don't understand the logic of this: at least with w32notify, when a
> file is in auto-revert-mode, we watch the entire parent directory of
> that file, so there's no need to send the 'stopped' event when the
> file is renamed or deleted, only when the parent directory is deleted.

In `auto-revert-notify-add-watch', only the file named by
`buffer-file-name' is watched. autorevert.el does not check, what the
underlying library of filenotify.el is able to do, watching single files
or whole directories. And as said, a renaming of a file should not turn
file notifications off. I will try to fix this.

> This second call to string-equal was added as part of kqueue
> back-port, but it affects all the backends, and there's no comment or
> anything in the log message that explains the rationale for this part
> of the change, only a laconic "Add kqueue support."

The changes were introduced in the scratch/kqueue branch. Later merged
to the master branch, later backported to the emacs-25 branch. Likely,
commit messages were lost this way.

I really hate it that we have no ChangeLog anymore, maintained by the
developers.

> What is the reason for this test, and should it perhaps be limited to
> kqueue?

Nope. If a file is supervised, and this is deleted on purpose, it
shouldn't come back under supervision if a file with the same name
appears later on.

There is the case of renaming files during backup, which could be seen
internally as delete+create file notification actions. This must be
handled, I agree.

Best regards, Michael.