Subject: Re: bug#35418: [PATCH] Don't poll auto-revert files that use notification

From: Michael Albinus <[email-masked]>
Date: Sat, 04 May 2019 21:43:28 +0200

Mattias Engdegård <[email-masked]> writes:

Hi Mattias,

>> Maybe we could also add the other major reason why polling is necessary:
>> file notification is not supported for all types of remote file systems
>> (Tramp).
>
> Is the problem that notification over Tramp may fail to activate or
> suddenly fail functioning, in both cases without any indication of
> error to file-notify?

Yes we have both cases. But file-notify is told about.

The first case is that file notification cannot be activated. Several
Tramp methods do not support this, and they return nil for the
respective file-notify-add-watch call, as specified. auto-revert
understands this, and knows that the respective file must be handled by
polling.

The second case is, that file notification for a remote file ceases to
work. Since Tramp implements file notification as asynchronous
processes, this could happen if the respective process is killed on the
remote side, or if the connection between the local Emacs and the remote
host is broken temporarily. The respective process shall own a sentinel,
which sends a "stopped" event in this case. I've just checked the code;
this is not implemented. Will do.

> If so, we should take care of this in code instead. Assuming the
> above, I suggest that file-notify consider all notification from file
> name handlers to be unreliable, and provide an interface to
> auto-revert, which will then keep polling those files.

We have this interface already. For file-notify-add-watch it works
already as expected. For broken notifications, the interface is the
"stopped" event, which must (will be) implemented in Tramp.

Btw, I believe all this file notification vs polling behavior is not
covered yet in autorevert-tests.el. Would you like to add respective
test cases for local and remote files? Some days ago I've added tests
for remote files; you could use this mechanism as well.

Best regards, Michael.