GNU bug report logs - #20972
25.0.50; eieio-persistent broken

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 20972 in the body.
You can then email your comments to 20972 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: Jan Tatarik <jan.tatarik <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 25.0.50; eieio-persistent broken
Date: Fri, 03 Jul 2015 13:34:22 +0200

The commit a0010db41ca83a8211162b649e679162dd4153a6 has broken eieio
persistence.

Besides my own stuff, it affects gnus registry - just lost my registry
file, because gnus couldn't read the registry file, so it created a new,
empty one...

I found three issues, the recipe can be reproduced with emacs -Q:

(require 'eieio)
(require 'eieio-base)

(defclass test-persist (eieio-persistent)
  ((slot :initarg :slot :type string))
  "Test class.")

;; In theory, calling it like this should override the :file slot
;; defined in eieio-persistent
(eieio-persistent-save (test-persist :slot "foo") "/tmp/test.eieio")

;; But when no :file has been specified, it breaks
;; Debugger entered--Lisp error: (unbound-slot test-persist "#<test-persist test-persist>" file oref)

;; with explicit :file it works
(eieio-persistent-save (test-persist :file "/tmp/test.eieio" :slot "foo"))

;; But this stores the file in :file, no override takes place, contrary to the docs
(eieio-persistent-save (test-persist :file "/tmp/test.eieio" :slot "foo")
                        "/tmp/another_file.eieio")


;; And finally, the most important issue - we cannot read the files back in
(eieio-persistent-read "/tmp/test.eieio")
;; Debugger entered--Lisp error: (wrong-type-argument arrayp test-persist)


The first two issues (if they are issues), must have been already
present for some time.

The eieio-persistent-read was caused by the latest changes to eieio
(tested with 8bab1490f14207eeeee4b2f4ad30b5d695db8245 and it still
worked).

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Jan Tatarik <jan.tatarik <at> gmail.com>
Cc: 20972 <at> debbugs.gnu.org
Subject: Re: bug#20972: 25.0.50; eieio-persistent broken
Date: Fri, 03 Jul 2015 11:28:16 -0400

> The commit a0010db41ca83a8211162b649e679162dd4153a6 has broken eieio
> persistence.

After a0010db41ca83a8211162b649e679162dd4153a6, eieio-base.el needs to
be recompiled (even though it hasn't changed).

If "grep eieio-class-definition **/*.elc" finds a match in
eieio-base.elc, then please "rm eieio-base.elc" and try again.

> The first two issues (if they are issues), must have been already
> present for some time.

Do you have a rough idea of when it worked?


        Stefan

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

From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
To: Jan Tatarik <jan.tatarik <at> gmail.com>
Cc: 20972 <at> debbugs.gnu.org
Subject: Re: bug#20972: 25.0.50; eieio-persistent broken
Date: Sat, 04 Jul 2015 15:16:36 -0400

> ;; In theory, calling it like this should override the :file slot
> ;; defined in eieio-persistent
> (eieio-persistent-save (test-persist :slot "foo") "/tmp/test.eieio")

Indeed, the code seems completely broken in this respect (it mostly
ignores the `file' argument, tho not completely).

I'm not 100% sure what the code *should* do, but can you try the patch
below for you use case and confirm that it does the right thing for you?


        Stefan


diff --git a/lisp/emacs-lisp/eieio-base.el b/lisp/emacs-lisp/eieio-base.el
index 2e28036..400bdb9 100644
--- a/lisp/emacs-lisp/eieio-base.el
+++ b/lisp/emacs-lisp/eieio-base.el
@@ -429,37 +429,28 @@ Optional argument COMMENT is a header line comment."
   "Save persistent object THIS to disk.
 Optional argument FILE overrides the file name specified in the object
 instance."
-  (save-excursion
-    (let ((b (set-buffer (get-buffer-create " *tmp object write*")))
-	  (default-directory (file-name-directory (oref this file)))
-	  (cfn (oref this file)))
-      (unwind-protect
-	  (save-excursion
-	    (erase-buffer)
-	    (let ((standard-output (current-buffer)))
-	      (oset this file
-		    (if file
-			(eieio-persistent-path-relative this file)
-		      (file-name-nondirectory cfn)))
-	      (object-write this (oref this file-header-line)))
-	    (let ((backup-inhibited (not (oref this do-backups)))
-		  (cs (car (find-coding-systems-region
-			    (point-min) (point-max)))))
-	      (unless (eq cs 'undecided)
-		(setq buffer-file-coding-system cs))
-	      ;; Old way - write file.  Leaves message behind.
-	      ;;(write-file cfn nil)
-
-	      ;; New way - Avoid the vast quantities of error checking
-	      ;; just so I can get at the special flags that disable
-	      ;; displaying random messages.
-	      (write-region (point-min) (point-max)
-			    cfn nil 1)
-	      ))
-	;; Restore :file, and kill the tmp buffer
-	(oset this file cfn)
-	(setq buffer-file-name nil)
-	(kill-buffer b)))))
+  (when file (setq file (expand-file-name file)))
+  (with-temp-buffer
+    (let* ((cfn (or file (oref this file)))
+           (default-directory (file-name-directory cfn)))
+      (cl-letf ((standard-output (current-buffer))
+                ((oref this file)       ;FIXME: Why change it?
+                 (if file
+                     ;; FIXME: Makes a name relative to (oref this file),
+                     ;; whereas I think it should be relative to cfn.
+                     (eieio-persistent-path-relative this file)
+                   (file-name-nondirectory cfn))))
+        (object-write this (oref this file-header-line)))
+      (let ((backup-inhibited (not (oref this do-backups)))
+            (coding-system-for-write 'utf-8-emacs))
+        ;; Old way - write file.  Leaves message behind.
+        ;;(write-file cfn nil)
+
+        ;; New way - Avoid the vast quantities of error checking
+        ;; just so I can get at the special flags that disable
+        ;; displaying random messages.
+        (write-region (point-min) (point-max) cfn nil 1)
+        ))))
 
 ;; Notes on the persistent object:
 ;; It should also set up some hooks to help it keep itself up to date.

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

From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
To: Jan Tatarik <jan.tatarik <at> gmail.com>
Cc: 20972-done <at> debbugs.gnu.org
Subject: Re: bug#20972: 25.0.50; eieio-persistent broken
Date: Mon, 06 Jul 2015 11:56:49 -0400

[ Please keep the "Cc:".  ]

>> I'm not 100% sure what the code *should* do, but can you try the patch
>> below for you use case and confirm that it does the right thing for you?
> Yes, this works fine, thanks.

Thanks for confirming.  Installed into "master".


        Stefan

Previous Next

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