GNU bug report logs - #75188
persist.el should recursively copy hash tables

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 75188 in the body.
You can then email your comments to 75188 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: Joseph Turner <joseph <at> breatheoutbreathe.in>
To: bug-gnu-emacs <at> gnu.org
Subject: persist.el should recursively copy hash tables
Date: Mon, 30 Dec 2024 01:05:30 -0800

[Message part 1 (text/plain, inline)]

Hello!

This patch demonstrates a bug in persist.el, which may be the cause of
https://github.com/alphapapa/activities.el/issues/109 and a similar bug
in hyperdrive.el which causes the `persist' variable file to be deleted.
We have not yet found a way to reproduce the bug in either activities.el
or hyperdrive.el, so this `persist' bug may be unrelated.

[0001-Test-that-persist-copy-recursively-copies-hash-table.patch (text/x-diff, inline)]

From c28644c957d9fabf883990ff145cec19ed12ff10 Mon Sep 17 00:00:00 2001
From: Joseph Turner <joseph <at> breatheoutbreathe.in>
Date: Mon, 30 Dec 2024 00:40:33 -0800
Subject: [PATCH] Test that persist-copy recursively copies hash tables

---
 test/persist-tests.el | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/test/persist-tests.el b/test/persist-tests.el
index 6bf2ed3708..adf37a5bf0 100644
--- a/test/persist-tests.el
+++ b/test/persist-tests.el
@@ -151,3 +151,14 @@ (ert-deftest test-persist-reset ()
      (should-not (eq persist--test-reset-variable initial-value))
      (should-not (eq initial-value
                      (persist-default 'persist--test-reset-variable))))))
+
+(ert-deftest test-persist-copy-hash-table ()
+  "`persist-copy' should copy hash keys and values."
+  (let* ((hash (make-hash-table))
+         (rec (record 'a))
+         (_ (puthash 'foo rec hash))
+         (copy-hash (persist-copy hash)))
+    (setf (aref (gethash 'foo copy-hash) 0) 'b)
+    (should-not (persist-equal hash copy-hash))
+    (should-not (eq (gethash 'foo hash)
+                    (gethash 'foo copy-hash)))))
-- 
2.46.0

[Message part 3 (text/plain, inline)]

IIUC, the solution requires rewriting `persist-copy-tree' (which is
copied from Emacs 30's `copy-tree') so that it handles hash tables.  If
you all agree, I can do this in `persist.el`.

However, would it make sense for `copy-tree' to handle hash tables?

Forgive me if this has been discussed and settled before.  I did find
this thread about comparing hash tables with `equal':

https://yhetil.org/emacs-devel/871qvz4kdw.fsf <at> localhost/

Thank you!

Joseph

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

From: Joseph Turner <joseph <at> breatheoutbreathe.in>
To: 75188 <at> debbugs.gnu.org
Cc: Ihor Radchenko <yantar92 <at> posteo.net>,
 Daniel Mendler <mail <at> daniel-mendler.de>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>, Adam Porter <adam <at> alphapapa.net>,
 Eli Zaretskii <eliz <at> gnu.org>, phillip.lord <at> russet.org.uk
Subject: Re: bug#75188: persist.el should recursively copy hash tables
Date: Tue, 31 Dec 2024 14:12:00 -0800

> Hello!
>
> This patch demonstrates a bug in persist.el

[...]

This issue also relates to the bug#63513, in which the suggestion was
made to replace persist-copy-tree with (compat-call copy-tree) now that
Compat 30 is available:

https://yhetil.org/emacs-bugs/80479897-500e-fe60-6586-0a44ccb5993b <at> daniel-mendler.de/

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Joseph Turner <joseph <at> breatheoutbreathe.in>
Cc: yantar92 <at> posteo.net, mail <at> daniel-mendler.de, monnier <at> iro.umontreal.ca,
 75188 <at> debbugs.gnu.org, adam <at> alphapapa.net, phillip.lord <at> russet.org.uk
Subject: Re: bug#75188: persist.el should recursively copy hash tables
Date: Sat, 18 Jan 2025 11:11:48 +0200

> From: Joseph Turner <joseph <at> breatheoutbreathe.in>
> Cc:  Daniel Mendler <mail <at> daniel-mendler.de>,
> 	Ihor Radchenko <yantar92 <at> posteo.net>,
> 	Stefan Monnier <monnier <at> iro.umontreal.ca>,
> 	Adam Porter <adam <at> alphapapa.net>, Eli Zaretskii <eliz <at> gnu.org>,
> 	phillip.lord <at> russet.org.uk
> Date: Tue, 31 Dec 2024 14:12:00 -0800
> 
> > Hello!
> >
> > This patch demonstrates a bug in persist.el
> 
> [...]
> 
> This issue also relates to the bug#63513, in which the suggestion was
> made to replace persist-copy-tree with (compat-call copy-tree) now that
> Compat 30 is available:
> 
> https://yhetil.org/emacs-bugs/80479897-500e-fe60-6586-0a44ccb5993b <at> daniel-mendler.de/

Thanks, would you like to submit a patch along those lines?

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

From: Joseph Turner <joseph <at> breatheoutbreathe.in>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: yantar92 <at> posteo.net, mail <at> daniel-mendler.de, monnier <at> iro.umontreal.ca,
 75188 <at> debbugs.gnu.org, adam <at> alphapapa.net, phillip.lord <at> russet.org.uk
Subject: Re: bug#75188: persist.el should recursively copy hash tables
Date: Mon, 27 Jan 2025 18:08:57 -0800

[Message part 1 (text/plain, inline)]

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Joseph Turner <joseph <at> breatheoutbreathe.in>
>> Cc:  Daniel Mendler <mail <at> daniel-mendler.de>,
>> 	Ihor Radchenko <yantar92 <at> posteo.net>,
>> 	Stefan Monnier <monnier <at> iro.umontreal.ca>,
>> 	Adam Porter <adam <at> alphapapa.net>, Eli Zaretskii <eliz <at> gnu.org>,
>> 	phillip.lord <at> russet.org.uk
>> Date: Tue, 31 Dec 2024 14:12:00 -0800
>>
>> > Hello!
>> >
>> > This patch demonstrates a bug in persist.el
>>
>> [...]
>>
>> This issue also relates to the bug#63513, in which the suggestion was
>> made to replace persist-copy-tree with (compat-call copy-tree) now that
>> Compat 30 is available:
>>
>> https://yhetil.org/emacs-bugs/80479897-500e-fe60-6586-0a44ccb5993b <at> daniel-mendler.de/
>
> Thanks, would you like to submit a patch along those lines?

Please see patch:

[0001-Depend-on-compat.el-for-new-copy-tree-behavior.patch (text/x-diff, inline)]

From 8c3d276a977010d4b903d9de7344b6fad8de8eab Mon Sep 17 00:00:00 2001
From: Joseph Turner <joseph <at> breatheoutbreathe.in>
Date: Mon, 27 Jan 2025 18:05:10 -0800
Subject: [PATCH] Depend on compat.el for new copy-tree behavior

---
 persist.el | 34 ++++------------------------------
 1 file changed, 4 insertions(+), 30 deletions(-)

diff --git a/persist.el b/persist.el
index 49b3e9b51f..3260df0964 100644
--- a/persist.el
+++ b/persist.el
@@ -5,7 +5,7 @@
 ;; Author: Phillip Lord <phillip.lord <at> russet.org.uk>
 ;; Maintainer: Joseph Turner <persist-el <at> breatheoutbreathe.in>
 ;; Package-Type: multi
-;; Package-Requires: ((emacs "26.1"))
+;; Package-Requires: ((emacs "26.1") (compat "30.0.2.0"))
 ;; Version: 0.6.1
 
 ;; The contents of this file are subject to the GPL License, Version 3.0.
@@ -43,6 +43,8 @@
 
 ;;; Code:
 
+(require 'compat)
+
 (defvar persist--directory-location
   (locate-user-emacs-file "persist")
   "The location of persist directory.")
@@ -211,39 +213,11 @@ (defun persist-equal (a b)
              t))
     (equal a b)))
 
-(defun persist-copy-tree (tree &optional vectors-and-records)
-  "Make a copy of TREE.
-If TREE is a cons cell, this recursively copies both its car and its cdr.
-Contrast to `copy-sequence', which copies only along the cdrs.
-With the second argument VECTORS-AND-RECORDS non-nil, this
-traverses and copies vectors and records as well as conses."
-  (declare (side-effect-free error-free))
-  (if (consp tree)
-      (let (result)
-	(while (consp tree)
-	  (let ((newcar (car tree)))
-	    (if (or (consp (car tree))
-                    (and vectors-and-records
-                         (or (vectorp (car tree)) (recordp (car tree)))))
-		(setq newcar (persist-copy-tree (car tree) vectors-and-records)))
-	    (push newcar result))
-	  (setq tree (cdr tree)))
-	(nconc (nreverse result)
-               (if (and vectors-and-records (or (vectorp tree) (recordp tree)))
-                   (persist-copy-tree tree vectors-and-records)
-                 tree)))
-    (if (and vectors-and-records (or (vectorp tree) (recordp tree)))
-	(let ((i (length (setq tree (copy-sequence tree)))))
-	  (while (>= (setq i (1- i)) 0)
-	    (aset tree i (persist-copy-tree (aref tree i) vectors-and-records)))
-	  tree)
-      tree)))
-
 (defun persist-copy (obj)
   "Return copy of OBJ."
   (if (hash-table-p obj)
       (copy-hash-table obj)
-    (persist-copy-tree obj t)))
+    (compat-call copy-tree obj t)))
 
 (provide 'persist)
 ;;; persist.el ends here
-- 
2.46.0

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Joseph Turner <joseph <at> breatheoutbreathe.in>
Cc: yantar92 <at> posteo.net, mail <at> daniel-mendler.de, monnier <at> iro.umontreal.ca,
 75188 <at> debbugs.gnu.org, adam <at> alphapapa.net, phillip.lord <at> russet.org.uk
Subject: Re: bug#75188: persist.el should recursively copy hash tables
Date: Sat, 01 Feb 2025 12:30:04 +0200

> From: Joseph Turner <joseph <at> breatheoutbreathe.in>
> Cc: 75188 <at> debbugs.gnu.org,  mail <at> daniel-mendler.de,  yantar92 <at> posteo.net,
>   monnier <at> iro.umontreal.ca,  adam <at> alphapapa.net,
>   phillip.lord <at> russet.org.uk
> Date: Mon, 27 Jan 2025 18:08:57 -0800
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
> >> From: Joseph Turner <joseph <at> breatheoutbreathe.in>
> >> Cc:  Daniel Mendler <mail <at> daniel-mendler.de>,
> >> 	Ihor Radchenko <yantar92 <at> posteo.net>,
> >> 	Stefan Monnier <monnier <at> iro.umontreal.ca>,
> >> 	Adam Porter <adam <at> alphapapa.net>, Eli Zaretskii <eliz <at> gnu.org>,
> >> 	phillip.lord <at> russet.org.uk
> >> Date: Tue, 31 Dec 2024 14:12:00 -0800
> >>
> >> > Hello!
> >> >
> >> > This patch demonstrates a bug in persist.el
> >>
> >> [...]
> >>
> >> This issue also relates to the bug#63513, in which the suggestion was
> >> made to replace persist-copy-tree with (compat-call copy-tree) now that
> >> Compat 30 is available:
> >>
> >> https://yhetil.org/emacs-bugs/80479897-500e-fe60-6586-0a44ccb5993b <at> daniel-mendler.de/
> >
> > Thanks, would you like to submit a patch along those lines?
> 
> Please see patch:

LGTM.  Phillip, would you please install?

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

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: yantar92 <at> posteo.net, mail <at> daniel-mendler.de,
 Joseph Turner <joseph <at> breatheoutbreathe.in>, monnier <at> iro.umontreal.ca,
 adam <at> alphapapa.net, 75188-done <at> debbugs.gnu.org, phillip.lord <at> russet.org.uk
Subject: Re: bug#75188: persist.el should recursively copy hash tables
Date: Thu, 13 Feb 2025 01:54:11 -0800

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Joseph Turner <joseph <at> breatheoutbreathe.in>
>> Cc: 75188 <at> debbugs.gnu.org,  mail <at> daniel-mendler.de,  yantar92 <at> posteo.net,
>>   monnier <at> iro.umontreal.ca,  adam <at> alphapapa.net,
>>   phillip.lord <at> russet.org.uk
>> Date: Mon, 27 Jan 2025 18:08:57 -0800
>>
>> Eli Zaretskii <eliz <at> gnu.org> writes:
>>
>> >> From: Joseph Turner <joseph <at> breatheoutbreathe.in>
>> >> Cc:  Daniel Mendler <mail <at> daniel-mendler.de>,
>> >> 	Ihor Radchenko <yantar92 <at> posteo.net>,
>> >> 	Stefan Monnier <monnier <at> iro.umontreal.ca>,
>> >> 	Adam Porter <adam <at> alphapapa.net>, Eli Zaretskii <eliz <at> gnu.org>,
>> >> 	phillip.lord <at> russet.org.uk
>> >> Date: Tue, 31 Dec 2024 14:12:00 -0800
>> >>
>> >> > Hello!
>> >> >
>> >> > This patch demonstrates a bug in persist.el
>> >>
>> >> [...]
>> >>
>> >> This issue also relates to the bug#63513, in which the suggestion was
>> >> made to replace persist-copy-tree with (compat-call copy-tree) now that
>> >> Compat 30 is available:
>> >>
>> >> https://yhetil.org/emacs-bugs/80479897-500e-fe60-6586-0a44ccb5993b <at> daniel-mendler.de/
>> >
>> > Thanks, would you like to submit a patch along those lines?
>>
>> Please see patch:
>
> LGTM.  Phillip, would you please install?

I'm not Phillip, but I installed the patch now.  Closing.

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

From: Joseph Turner <joseph <at> breatheoutbreathe.in>
To: Stefan Kangas <stefankangas <at> gmail.com>, Eli Zaretskii <eliz <at> gnu.org>
Cc: yantar92 <at> posteo.net, mail <at> daniel-mendler.de, monnier <at> iro.umontreal.ca,
 adam <at> alphapapa.net, 75188-done <at> debbugs.gnu.org, phillip.lord <at> russet.org.uk
Subject: Re: bug#75188: persist.el should recursively copy hash tables
Date: Thu, 13 Feb 2025 07:40:53 -0800

On February 13, 2025 1:54:11 AM PST, Stefan Kangas <stefankangas <at> gmail.com> wrote:

>I'm not Phillip, but I installed the patch now.  Closing.

Thank you!

Previous Next

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