bug#33005: 27.0.50; Data loss with Gnus registry

[Eric Abrahamsen <eric@ericabrahamsen.net>]

[-- Attachment #1: Type: text/plain, Size: 733 bytes --]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
>
>> I'd be awfully surprised if it only worked for me -- I'd think we
>> would have seen bug reports by now.
>
> My impressions is: some people may have tried using the registry, but
> most of them gave up, and nearly nobody is using it.
>
>> I still blame your usage of debbugs-gnu :)
>
> I don't.  AFAICT the issue happens with (featurep 'debbugs-gnu) ==> nil
> all the time.

No, I didn't really mean that...

Would you try the attached patch? It leaves gnus-registry-db at nil to
start with, and also removes the root of the double-loading (which was a
problem only because it can potentially be very slow).

Thanks,
Eric

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Continued-fixes-to-gnus-registry-loading.patch --]
[-- Type: text/x-patch, Size: 4845 bytes --]

From ea39e2a7a1969e669c32ef4f883cda357ad144a8 Mon Sep 17 00:00:00 2001
From: Eric Abrahamsen <eric@ericabrahamsen.net>
Date: Tue, 3 Dec 2019 09:12:44 +0000
Subject: [PATCH] Continued fixes to gnus registry loading

* lisp/gnus/gnus-registry.el (gnus-registry-db): Do not initialize
this variable to an empty database, that should only be done for new
databases.
(gnus-registry-load): Remove "force" argument, don't check if the
database is already loaded, as we're only going to load it once.
(gnus-registry-initialize): Either load the db directly, or set up a
hook to do it later.
(gnus-registry-install-hooks): Don't load on
gnus-read-newsrc-el-hook.
---
 lisp/gnus/gnus-registry.el | 70 ++++++++++++++++----------------------
 1 file changed, 30 insertions(+), 40 deletions(-)

diff --git a/lisp/gnus/gnus-registry.el b/lisp/gnus/gnus-registry.el
index e6fb382c2f..ff4c640c82 100644
--- a/lisp/gnus/gnus-registry.el
+++ b/lisp/gnus/gnus-registry.el
@@ -340,7 +340,7 @@ gnus-registry-make-db
                   :precious nil
                   :tracked nil)))
 
-(defvar gnus-registry-db (gnus-registry-make-db)
+(defvar gnus-registry-db nil
   "The article registry by Message ID.  See `registry-db'.")
 
 ;; top-level registry data management
@@ -352,40 +352,33 @@ gnus-registry-remake-db
     (gnus-message 4 "Remaking the Gnus registry")
     (setq gnus-registry-db (gnus-registry-make-db))))
 
-(defun gnus-registry-load (&optional force)
-  "Load the registry from the cache file.
-If the registry is already loaded, don't reload unless FORCE is
-non-nil."
+(defun gnus-registry-load ()
+  "Load the registry from the cache file."
   (interactive)
-  (when (or force
-	    ;; The registry is loaded by both
-	    ;; `gnus-registry-initialize' and the read-newsrc hook.
-	    ;; Don't load twice.
-	    (null (eieio-object-p gnus-registry-db)))
-    (let ((file gnus-registry-cache-file))
-      (condition-case nil
-          (gnus-registry-read file)
-	(file-error
-	 ;; Fix previous mis-naming of the registry file.
-	 (let ((old-file-name
-		(concat (file-name-sans-extension
-			 gnus-registry-cache-file)
-			".eioio")))
-	   (if (and (file-exists-p old-file-name)
-		    (yes-or-no-p
-		     (format "Rename registry file from %s to %s? "
-			     old-file-name file)))
-	       (progn
-		 (gnus-registry-read old-file-name)
-		 (setf (oref gnus-registry-db file) file)
-		 (gnus-message 1 "Registry filename changed to %s" file))
-	     (gnus-registry-remake-db t))))
-	(error
-	 (gnus-message
-          1
-          "The Gnus registry could not be loaded from %s, creating a new one"
-          file)
-	 (gnus-registry-remake-db t))))))
+  (let ((file gnus-registry-cache-file))
+    (condition-case nil
+        (gnus-registry-read file)
+      (file-error
+       ;; Fix previous mis-naming of the registry file.
+       (let ((old-file-name
+	      (concat (file-name-sans-extension
+		       gnus-registry-cache-file)
+		      ".eioio")))
+	 (if (and (file-exists-p old-file-name)
+		  (yes-or-no-p
+		   (format "Rename registry file from %s to %s? "
+			   old-file-name file)))
+	     (progn
+	       (gnus-registry-read old-file-name)
+	       (setf (oref gnus-registry-db file) file)
+	       (gnus-message 1 "Registry filename changed to %s" file))
+	   (gnus-registry-remake-db t))))
+      (error
+       (gnus-message
+        1
+        "The Gnus registry could not be loaded from %s, creating a new one"
+        file)
+       (gnus-registry-remake-db t)))))
 
 (defun gnus-registry-read (file)
   "Do the actual reading of the registry persistence file."
@@ -1178,13 +1171,12 @@ gnus-registry-initialize
   (gnus-message 5 "Initializing the registry")
   (gnus-registry-install-hooks)
   (gnus-registry-install-shortcuts)
-  (gnus-registry-load))
+  (if (gnus-alive-p)
+      (gnus-registry-load)
+    (add-hook 'gnus-read-newsrc-el-hook 'gnus-registry-load)))
 
-;; FIXME: Why autoload this function?
-;;;###autoload
 (defun gnus-registry-install-hooks ()
   "Install the registry hooks."
-  (interactive)
   (setq gnus-registry-enabled t)
   (add-hook 'gnus-summary-article-move-hook 'gnus-registry-action)
   (add-hook 'gnus-summary-article-delete-hook 'gnus-registry-action)
@@ -1192,13 +1184,11 @@ gnus-registry-install-hooks
   (add-hook 'nnmail-spool-hook 'gnus-registry-spool-action)
 
   (add-hook 'gnus-save-newsrc-hook 'gnus-registry-save)
-  (add-hook 'gnus-read-newsrc-el-hook 'gnus-registry-load)
 
   (add-hook 'gnus-summary-prepare-hook 'gnus-registry-register-message-ids))
 
 (defun gnus-registry-unload-hook ()
   "Uninstall the registry hooks."
-  (interactive)
   (remove-hook 'gnus-summary-article-move-hook 'gnus-registry-action)
   (remove-hook 'gnus-summary-article-delete-hook 'gnus-registry-action)
   (remove-hook 'gnus-summary-article-expire-hook 'gnus-registry-action)
-- 
2.24.0