GNU bug report logs - #75626
31.0.50; Dired misses or double-processes files when auto-revert-mode is enabled

Previous Next

Full log

View this message in rfc822 format

From: Tassilo Horn <tsdh <at> gnu.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: michael_heerdegen <at> web.de, 75626 <at> debbugs.gnu.org, michael.albinus <at> gmx.de
Subject: bug#75626: 31.0.50; Dired misses or double-processes files when auto-revert-mode is enabled
Date: Tue, 21 Jan 2025 21:18:18 +0100

Tassilo Horn <tsdh <at> gnu.org> writes:

> I'll make the change to the new more general inhibit-auto-revert when
> time permits.

I have to declare emacs programming bankruptcy.  The current version
with dired--inhibit-auto-revert works fine.  Now I wanted to change that
into a more general inhibit-auto-revert.  Here's the patch (with -b to
make it easier to grasp):

--8<---------------cut here---------------start------------->8---
diff --git a/lisp/autorevert.el b/lisp/autorevert.el
index 1dcfe8e911f..0cd0623c59b 100644
--- a/lisp/autorevert.el
+++ b/lisp/autorevert.el
@@ -778,6 +778,11 @@ auto-revert-active-p
       auto-revert-tail-mode
       auto-revert--global-mode))
 
+(defvar-local inhibit-auto-revert nil
+  "A non-nil value prevents `auto-revert-mode' from reverting the buffer.
+Should be used in let-bindings to temporarily disable auto-reverts in a
+buffer.")
+
 (defun auto-revert-handler ()
   "Revert current buffer, if appropriate.
 This is an internal function used by Auto-Revert Mode."
@@ -787,6 +792,8 @@ auto-revert-handler
          ;; the values.
          (remote-file-name-inhibit-cache t)
          (revert
+          (and
+           (not inhibit-auto-revert)
            (if buffer-file-name
                (and (or auto-revert-remote-files
                         (not (file-remote-p buffer-file-name)))
@@ -805,7 +812,7 @@ auto-revert-handler
                       global-auto-revert-non-file-buffers)
                   (funcall (or buffer-stale-function
                                #'buffer-stale--default-function)
-                          t))))
+                           t)))))
          eob eoblist)
     (setq auto-revert-notify-modified-p nil
           auto-revert--last-time (current-time))
diff --git a/lisp/dired.el b/lisp/dired.el
index d2071d80bf3..7c9d7310efb 100644
--- a/lisp/dired.el
+++ b/lisp/dired.el
@@ -944,9 +944,6 @@ dired-mark-if
                           ""))))
     (and (> count 0) count)))
 
-(defvar-local dired--inhibit-auto-revert nil
-  "A non-nil value prevents `auto-revert-mode' from reverting the buffer.")
-
 (defmacro dired-map-over-marks (body arg &optional show-progress
 				     distinguish-one-marked)
   "Eval BODY with point on each marked line.  Return a list of BODY's results.
@@ -983,8 +980,8 @@ dired-map-over-marks
   ;;endless loop.
   ;;This warning should not apply any longer, sk  2-Sep-1991 14:10.
   `(prog1
-       (let ((dired--inhibit-auto-revert t)
-             (inhibit-read-only t)
+       (let ((inhibit-read-only t)
+             (inhibit-auto-revert t)
              case-fold-search found results)
 	 (if (and ,arg (not (eq ,arg 'marked)))
 	     (if (integerp ,arg)
@@ -1294,12 +1291,6 @@ dired-buffer-stale-p
 	 ;; Do not auto-revert when the dired buffer can be currently
 	 ;; written by the user as in `wdired-mode'.
 	 buffer-read-only
-         ;; When a dired operation using dired-map-over-marks is in
-         ;; progress, dired--inhibit-auto-revert is bound to some
-         ;; non-nil value and we must not auto-revert because that could
-         ;; change the order of files leading to skipping or
-         ;; double-processing (see bug#75626).
-         (not dired--inhibit-auto-revert)
 	 (dired-directory-changed-p dirname))))
 
 (defcustom dired-auto-revert-buffer nil
@@ -4089,13 +4080,12 @@ dired-internal-do-deletions
 	    (while l
 	      (goto-char (marker-position (cdr (car l))))
               (dired-move-to-filename)
-	      (let ((inhibit-read-only t))
+	      (let ((inhibit-read-only t)
+                    ;; Temporarily prevent auto-revert while deleting
+                    ;; entry in the dired buffer (bug#71264).
+                    (inhibit-auto-revert t))
 		(condition-case err
-		    (let ((fn (car (car l)))
-                          ;; Temporarily prevent auto-revert while
-                          ;; deleting entry in the dired buffer
-                          ;; (bug#71264).
-                          (auto-revert-mode nil))
+		    (let ((fn (car (car l))))
 		      (dired-delete-file fn dired-recursive-deletes trash)
 		      ;; if we get here, removing worked
 		      (setq succ (1+ succ))
--8<---------------cut here---------------end--------------->8---

That last hunk in dired-internal-do-deletions is due to a wrong fix for
the bug#71264.  When auto-revert-mode itself is bound to nil when
auto-revert kicks in, the buffer will be removed from
auto-revert-buffer-list causing auto-revert to be disabled forever in
that buffer.  (At least that's my reading of the code...)

Anyway, the above solution with the new inhibit-auto-revert does not
work.  auto-revert-buffers is called from a timer and eventually
auto-revert-handler is called for my dired buffer where the compression
of 1000 is still ongoing but inhibit-auto-revert is nil there and
revert-buffer is called causing the issue of this bug again.

I can't understand why it doesn't see the non-nil inhibit-auto-revert
binding in the expansion of dired-map-over-marks.  Where is the
difference to the current working solution with
dired--inhibit-auto-revert?  It's bound the same way and accessed from
auto-revert-handler via the funcall to the dired buffer's
buffer-stale-function, i.e., dired-buffer-stale-p.

Bye,
Tassilo

Previous Next

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