GNU bug report logs - #20000
25.0.50; auto-revert tries to install notification handler on remote files

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 20000 in the body.
You can then email your comments to 20000 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Filipp Gunbin <fgunbin <at> fastmail.fm>
To: bug-gnu-emacs <at> gnu.org
Subject: 25.0.50;
 auto-revert tries to install notification handler on remote files
Date: Wed, 04 Mar 2015 15:10:46 +0300

Hi,

Most lines of the patch are whitespace changes, the only important thing
is that upper-level `when' is changed to `if'.

Ok to commit?

Filipp


diff --git a/lisp/autorevert.el b/lisp/autorevert.el
index 6489a3e..357916c 100644
--- a/lisp/autorevert.el
+++ b/lisp/autorevert.el
@@ -503,36 +503,37 @@
   "Enable file notification for current buffer's associated file."
   ;; We can assume that `buffer-file-name' and
   ;; `auto-revert-use-notify' are non-nil.
-  (when (or (string-match auto-revert-notify-exclude-dir-regexp
-			  (expand-file-name default-directory))
-	    (file-symlink-p (or buffer-file-name default-directory)))
-    ;; Fallback to file checks.
-    (set (make-local-variable 'auto-revert-use-notify) nil))
-
-  (when (not auto-revert-notify-watch-descriptor)
-    (setq auto-revert-notify-watch-descriptor
-	  (ignore-errors
-	    (if buffer-file-name
-		(file-notify-add-watch
-		 (expand-file-name buffer-file-name default-directory)
-		 '(change attribute-change)
-		 'auto-revert-notify-handler)
-	      (file-notify-add-watch
-	       (expand-file-name default-directory)
-	       '(change)
-	       'auto-revert-notify-handler))))
-    (if auto-revert-notify-watch-descriptor
-	(progn
-	  (puthash
-	   auto-revert-notify-watch-descriptor
-	   (cons (current-buffer)
-		 (gethash auto-revert-notify-watch-descriptor
-			  auto-revert-notify-watch-descriptor-hash-list))
-	   auto-revert-notify-watch-descriptor-hash-list)
-	  (add-hook (make-local-variable 'kill-buffer-hook)
-		    'auto-revert-notify-rm-watch))
+  (if (or (string-match auto-revert-notify-exclude-dir-regexp
+			(expand-file-name default-directory))
+	  (file-symlink-p (or buffer-file-name default-directory)))
+
       ;; Fallback to file checks.
-      (set (make-local-variable 'auto-revert-use-notify) nil))))
+      (set (make-local-variable 'auto-revert-use-notify) nil)
+
+    (when (not auto-revert-notify-watch-descriptor)
+      (setq auto-revert-notify-watch-descriptor
+	    (ignore-errors
+	      (if buffer-file-name
+		  (file-notify-add-watch
+		   (expand-file-name buffer-file-name default-directory)
+		   '(change attribute-change)
+		   'auto-revert-notify-handler)
+		(file-notify-add-watch
+		 (expand-file-name default-directory)
+		 '(change)
+		 'auto-revert-notify-handler))))
+      (if auto-revert-notify-watch-descriptor
+	  (progn
+	    (puthash
+	     auto-revert-notify-watch-descriptor
+	     (cons (current-buffer)
+		   (gethash auto-revert-notify-watch-descriptor
+			    auto-revert-notify-watch-descriptor-hash-list))
+	     auto-revert-notify-watch-descriptor-hash-list)
+	    (add-hook (make-local-variable 'kill-buffer-hook)
+		      'auto-revert-notify-rm-watch))
+	;; Fallback to file checks.
+	(set (make-local-variable 'auto-revert-use-notify) nil)))))
 
 ;; If we have file notifications, we want to update the auto-revert buffers
 ;; immediately when a notification occurs. Since file updates can happen very

Message #8 received at 20000 <at> debbugs.gnu.org (full text, mbox):

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Filipp Gunbin <fgunbin <at> fastmail.fm>
Cc: 20000 <at> debbugs.gnu.org
Subject: Re: bug#20000: 25.0.50;
 auto-revert tries to install notification handler on remote files
Date: Wed, 04 Mar 2015 15:04:05 +0100

Filipp Gunbin <fgunbin <at> fastmail.fm> writes:

> Hi,

Hi Filipp,

> Most lines of the patch are whitespace changes, the only important thing
> is that upper-level `when' is changed to `if'.
>
> Ok to commit?

I don't understand. Could you, please, explain what you intend to fix?

> Filipp

Best regards, Michael.

Message #11 received at 20000 <at> debbugs.gnu.org (full text, mbox):

From: Filipp Gunbin <fgunbin <at> fastmail.fm>
To: Michael Albinus <michael.albinus <at> gmx.de>
Cc: 20000 <at> debbugs.gnu.org
Subject: Re: bug#20000: 25.0.50;
 auto-revert tries to install notification handler on remote files
Date: Wed, 04 Mar 2015 17:17:54 +0300

On 04/03/2015 15:04 +0100, Michael Albinus wrote:

> I don't understand. Could you, please, explain what you intend to fix?

This is the existing code:

-  (when (or (string-match auto-revert-notify-exclude-dir-regexp
-			  (expand-file-name default-directory))
-	    (file-symlink-p (or buffer-file-name default-directory)))
-    ;; Fallback to file checks.
-    (set (make-local-variable 'auto-revert-use-notify) nil))
-
-  (when (not auto-revert-notify-watch-descriptor)
-    (setq auto-revert-notify-watch-descriptor
...

So even when string-match will return non-nil (as it will with remote
files e.g. starting with "/ssh:...") we go on and set
`auto-revert-notify-watch-descriptor' which is what
`auto-revert-notify-exclude-dir-regexp' is supposed to protect us from
(if I understand it correctly).

I ran across this in this way:

1) open remote file via tramp and ssh
2) turn on global-auto-revert-mode

Tramp then asked for the password for the remote host.

Now this behaviour is gone.

Filipp

Message #14 received at 20000 <at> debbugs.gnu.org (full text, mbox):

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Filipp Gunbin <fgunbin <at> fastmail.fm>
Cc: 20000 <at> debbugs.gnu.org
Subject: Re: bug#20000: 25.0.50;
 auto-revert tries to install notification handler on remote files
Date: Wed, 04 Mar 2015 15:32:44 +0100

Filipp Gunbin <fgunbin <at> fastmail.fm> writes:

Hi Filipp,

> I ran across this in this way:
>
> 1) open remote file via tramp and ssh
> 2) turn on global-auto-revert-mode
>
> Tramp then asked for the password for the remote host.

Have you set auto-revert-remote-files to nil? Otherwise, it would be the
expected behaviour.

> Filipp

Best regards, Michael.

Message #17 received at 20000 <at> debbugs.gnu.org (full text, mbox):

From: Filipp Gunbin <fgunbin <at> fastmail.fm>
To: Michael Albinus <michael.albinus <at> gmx.de>
Cc: 20000 <at> debbugs.gnu.org
Subject: Re: bug#20000: 25.0.50;
 auto-revert tries to install notification handler on remote files
Date: Wed, 04 Mar 2015 17:59:33 +0300

On 04/03/2015 15:32 +0100, Michael Albinus wrote:

> Filipp Gunbin <fgunbin <at> fastmail.fm> writes:
>
> Hi Filipp,
>
>> I ran across this in this way:
>>
>> 1) open remote file via tramp and ssh
>> 2) turn on global-auto-revert-mode
>>
>> Tramp then asked for the password for the remote host.
>
> Have you set auto-revert-remote-files to nil? Otherwise, it would be the
> expected behaviour.

Of course, it is nil.

In fact, `auto-revert-remote-files' is checked only in
`auto-revert-handler' (and that function is called from the notification
handler when notification is triggered).

Notification handler is called before `auto-revert-handler':

		(when (auto-revert-active-p)
		  ;; Enable file notification.
		  (when (and auto-revert-use-notify
			     (not auto-revert-notify-watch-descriptor))
		    (auto-revert-notify-add-watch))
		  (auto-revert-handler)))


As I see, there are two kinds of revert prevention for remote files:

1) For files matching `auto-revert-notify-exclude-dir-regexp' the
notification handler is not set all (but is set anyway and my patch
fixes it).

2) If we still get into `auto-revert-handler' in a remote file (like
when we called `global-auto-revert-mode' and it called
'auto-revert-handler') then the `auto-revert-remote-files' will prevent
the actual revert.

This fix applies only to notification handler installation, other
functionality seems to work right.

Filipp

Message #20 received at 20000 <at> debbugs.gnu.org (full text, mbox):

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Filipp Gunbin <fgunbin <at> fastmail.fm>
Cc: 20000 <at> debbugs.gnu.org
Subject: Re: bug#20000: 25.0.50;
 auto-revert tries to install notification handler on remote files
Date: Wed, 04 Mar 2015 16:46:45 +0100

Filipp Gunbin <fgunbin <at> fastmail.fm> writes:

> This fix applies only to notification handler installation, other
> functionality seems to work right.

Yes, indeed, you are right. From my pov it is OK to install the patch.

> Filipp

Thanks, Best regards, Michael.

Message #25 received at 20000-done <at> debbugs.gnu.org (full text, mbox):

From: Filipp Gunbin <fgunbin <at> fastmail.fm>
To: Michael Albinus <michael.albinus <at> gmx.de>
Cc: 20000-done <at> debbugs.gnu.org
Subject: Re: bug#20000: 25.0.50;
 auto-revert tries to install notification handler on remote files
Date: Wed, 04 Mar 2015 19:40:56 +0300

On 04/03/2015 16:46 +0100, Michael Albinus wrote:

> Filipp Gunbin <fgunbin <at> fastmail.fm> writes:
>
>> This fix applies only to notification handler installation, other
>> functionality seems to work right.
>
> Yes, indeed, you are right. From my pov it is OK to install the patch.

Thanks, installed.

Filipp

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.