bug#36341: 27.0.50; Reading from the Gnus dribble file leaves data inconsistent

[Eric Abrahamsen <eric@ericabrahamsen.net>]

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

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
>
>> I don't know why that happens, given that the entry in the hashtable and
>> the alist are identical lisp objects, or at least they are everywhere
>> else. But at least the solution is in sight!
>
> Great.  :-)

I think it has to be done the ugly way -- setting the hashtable and
alist separately.

The problem (I write this more as part of digesting the issue than
anything else) is that the alist is full of elements that look like
'("group" 4 ((1 2 3) (4 5 6))), whereas the corresponding value in the
hashtable looks like '(25 ("group" 4 ((1 2 3) (4 5 6)))).

The alist element is `eq' to the `cadr' of the hashtable value: they're
the same list object.

If you reach inside that list and change values (ie set 4 to 7), they
remain the same list, and the change is reflected in both hashtable and
alist.

If you replace the entire ("group"...) list, in the hashtable, it is no
longer the same object as that in the alist, and you get divergence.
This seems sensible in hindsight, but took me quite a while to figure
out.

I think `gnus-group-set-info' is the only place that happens, so it
isn't too terrible to just explicitly set both hashtable and alist in
that function. I've attached the commit that does that.

My plan for avoiding this class of errors in the future is to change the
representation of Gnus groups from lists to EIEIO objects. Then
`gnus-newsrc-alist' would merely be a disk serialization format, and the
hashtable would be the source of authority. That would also make the
"dummy.group" unnecessary. But let's see if I get there, and if the
changes are accepted...

Eric


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Make-sure-gnus-group-set-info-sets-both-the-hashtabl.patch --]
[-- Type: text/x-patch, Size: 1377 bytes --]

From c0b8589b1686d8488b4765783536217b7bb77b6c Mon Sep 17 00:00:00 2001
From: Eric Abrahamsen <eric@ericabrahamsen.net>
Date: Sun, 7 Jul 2019 16:25:14 -0700
Subject: [PATCH] Make sure gnus-group-set-info sets both the hashtable and
 alist

* lisp/gnus/gnus-group.el (gnus-group-set-info): Apparently this
  method of updating the group info will only apply to the
  gnus-newsrc-hashtb, not gnus-newsrc-alist. Do the alist explicitly.
---
 lisp/gnus/gnus-group.el | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/lisp/gnus/gnus-group.el b/lisp/gnus/gnus-group.el
index ea695aaa97..2668e4fb7c 100644
--- a/lisp/gnus/gnus-group.el
+++ b/lisp/gnus/gnus-group.el
@@ -4506,7 +4506,14 @@ gnus-group-set-info
 	    (when (and (not (eq (car entry) t))
 		       (gnus-active (gnus-info-group info)))
 	      (setcar entry (length
-			     (gnus-list-of-unread-articles (car info))))))
+			     (gnus-list-of-unread-articles (car info)))))
+	    ;; The above `setcar' will only affect the hashtable, not
+	    ;; the alist: update the alist separately.
+	    (push info (cdr (setq gnus-newsrc-alist
+				   (remove (assoc-string
+					    (gnus-info-group info)
+					    gnus-newsrc-alist)
+					   gnus-newsrc-alist)))))
 	(error "No such group: %s" (gnus-info-group info))))))
 
 ;; Ad-hoc function for inserting data from a different newsrc.eld
-- 
2.22.0