bug#29220: 26.0.90; eieio-persistent-read fail to restore saved object.

bug#29220: 26.0.90; eieio-persistent-read fail to restore saved object.

[Pierre Téchoueyres]

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



Emacs 26 is unable to restore an previously saved eieio object.
Same receipt work fine in emacs 25.3

emacs-26 --batch -l eieio-fail.el produces:
Wrong number of arguments: #<subr stringp>, 3

where emacs 25.3
Loading /usr/share/emacs/site-lisp/site-start.d/autoconf-init.el (source)...
Loading /usr/share/emacs/site-lisp/site-start.d/cmake-init.el (source)...
Loading /usr/share/emacs/site-lisp/site-start.d/desktop-entry-mode-init.el (source)...
Loading /usr/share/emacs/site-lisp/site-start.d/git-init.el (source)...
Loading /usr/share/emacs/site-lisp/site-start.d/rpmdev-init.el (source)...
Loading /usr/share/emacs/site-lisp/site-start.d/systemtap-init.el (source)...
[eieio-class-tag--eieio-fail "tmp-25.3.1" nil nil #s(hash-table size 65 test eql rehash-size 1.5 rehash-threshold 0.8 data


[-- Attachment #2: receipe for eieio-persistent-read reading bug --]
[-- Type: text/plain, Size: 475 bytes --]

;;; -*- lexical-binding: t -*-
(require 'eieio)
(require 'eieio-base)

(defclass eieio-fail (eieio-persistent eieio-named)
  ((version :initarg :version :initform nil)
   (version-constant :allocation :class)
   (entries :initarg :entries :initform (make-hash-table))))

(let ((ef (make-instance 'eieio-fail :file "tmp")))
  (eieio-persistent-save ef (concat "tmp-" emacs-version)))

(let ((ef (eieio-persistent-read (concat "tmp-" emacs-version) 'eieio-fail t)))
  (pp ef))

[-- Attachment #3: Type: text/plain, Size: 3098 bytes --]



In GNU Emacs 26.0.90 (build 1, x86_64-fc26-linux-gnu, GTK+ Version 3.22.21)
 of 2017-11-08 built on killashandra.ballybran.fr
Repository revision: a215be999454b8f0118141d4e4f6cf58298f79e0
Windowing system distributor 'Fedora Project', version 11.0.11903000
Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.

Configured using:
 'configure --prefix=/usr/local --with-xwidgets --with-modules
 --host=x86_64-fc26-linux-gnu --with-mailutils --with-gconf
 host_alias=x86_64-fc26-linux-gnu'

Configured features:
XPM JPEG TIFF GIF PNG RSVG IMAGEMAGICK SOUND GPM DBUS GCONF GSETTINGS
NOTIFY ACL LIBSELINUX GNUTLS LIBXML2 FREETYPE M17N_FLT LIBOTF XFT ZLIB
TOOLKIT_SCROLL_BARS GTK3 X11 MODULES XWIDGETS LIBSYSTEMD LCMS2

Important settings:
  value of $LC_CTYPE: fr_FR.UTF-8
  value of $LANG: fr_FR.UTF-8
  value of $XMODIFIERS: @im=none
  locale-coding-system: utf-8-unix

Major mode: Emacs-Lisp

Minor modes in effect:
  diff-auto-refine-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message rmc puny seq byte-opt gv
bytecomp byte-compile cconv cl-loaddefs cl-lib dired dired-loaddefs
format-spec rfc822 mml mml-sec password-cache epa derived epg epg-config
gnus-util rmail rmail-loaddefs mm-decode mm-bodies mm-encode mail-parse
rfc2231 mailabbrev gmm-utils mailheader sendmail rfc2047 rfc2045
ietf-drums mm-util mail-prsvr mail-utils vc-git diff-mode easymenu
easy-mmode elec-pair time-date mule-util tooltip eldoc electric uniquify
ediff-hook vc-hooks lisp-float-type mwheel term/x-win x-win
term/common-win x-dnd tool-bar dnd fontset image regexp-opt fringe
tabulated-list replace newcomment text-mode elisp-mode lisp-mode
prog-mode register page menu-bar rfn-eshadow isearch timer select
scroll-bar mouse jit-lock font-lock syntax facemenu font-core
term/tty-colors frame cl-generic cham georgian utf-8-lang misc-lang
vietnamese tibetan thai tai-viet lao korean japanese eucjp-ms cp51932
hebrew greek romanian slovak czech european ethiopic indian cyrillic
chinese composite charscript charprop case-table epa-hook jka-cmpr-hook
help simple abbrev obarray minibuffer cl-preloaded nadvice loaddefs
button faces cus-face macroexp files text-properties overlay sha1 md5
base64 format env code-pages mule custom widget hashtable-print-readable
backquote dbusbind inotify lcms2 dynamic-setting system-font-setting
font-render-setting xwidget-internal move-toolbar gtk x-toolkit x
multi-tty make-network-process emacs)

Memory information:
((conses 16 99040 11809)
 (symbols 48 21157 1)
 (miscs 40 53 130)
 (strings 32 29949 1299)
 (string-bytes 1 792306)
 (vectors 16 14803)
 (vector-slots 8 509702 10218)
 (floats 8 53 175)
 (intervals 56 265 20)
 (buffers 992 13))

bug#29220: 26.0.90; eieio-persistent-read fail to restore saved object.

[Eric Abrahamsen]

pierre.techoueyres@free.fr (Pierre Téchoueyres) writes:

> Emacs 26 is unable to restore an previously saved eieio object.
> Same receipt work fine in emacs 25.3
>
> emacs-26 --batch -l eieio-fail.el produces:
> Wrong number of arguments: #<subr stringp>, 3

Yes, I made a bum commit to master, then fixed it, then cherry-picked
the bum commit over to emacs-26 without fixing it. The fix is now
cherry-picked as well, and I'm taking a short breather from patching
Emacs.

Sorry about this,
Eric

bug#29220: 26.0.90; eieio-persistent-read fail to restore saved object.

[Pierre Téchoueyres]

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

Hello Eric,
Firs, thank you for you fast answer. You fix resolved my first problem
with eieio-persistent-read, but I've hit another error when I try to use
complex objects save/restore.  In fact I try to find why pcache package
insnt working anymore with emacs 26.

So here is the same sample completed. Sorry.


[-- Attachment #2: eieio-persistent-read test. --]
[-- Type: text/plain, Size: 907 bytes --]

;;; -*- lexical-binding: t -*-
(require 'eieio)
(require 'eieio-base)

(defclass eieio-fail (eieio-persistent eieio-named)
  ((version :initarg :version :initform nil)
   (version-constant :allocation :class)
   (entries :initarg :entries :initform (make-hash-table))))

(let* ((ef (make-instance 'eieio-fail :file "tmp"))
       (entries (slot-value ef 'entries))
       (ef2 (make-instance 'eieio-fail :file "tmp2"))
       (entries2 (slot-value ef2 'entries)))
  (puthash 'foo 42 entries2)
  (oset ef2 :entries entries2)
  (puthash 'ef2 ef2 entries)
  (oset ef2 :entries entries2)
  (oset ef :entries entries)  
  (eieio-persistent-save ef (concat "tmp-" emacs-version)))

(let* ((ef (eieio-persistent-read (concat "tmp-" emacs-version) 'eieio-fail t))
       (entries (slot-value ef 'entries))
       (entries2 (slot-value (gethash 'ef2 entries) 'entries)))
  ;; (pp ef)
  (pp (gethash 'foo entries2)))

[-- Attachment #3: Type: text/plain, Size: 328 bytes --]



Eric Abrahamsen <eric@ericabrahamsen.net> writes:

> ...
>
> Yes, I made a bum commit to master, then fixed it, then cherry-picked
> the bum commit over to emacs-26 without fixing it. The fix is now
> cherry-picked as well, and I'm taking a short breather from patching
> Emacs.
>
> Sorry about this,
No worries
> Eric
Pierre

bug#29220: 26.0.90; eieio-persistent-read fail to restore saved object.

[Eric Abrahamsen]

pierre.techoueyres@free.fr (Pierre Téchoueyres) writes:

> Hello Eric,
> Firs, thank you for you fast answer. You fix resolved my first problem
> with eieio-persistent-read, but I've hit another error when I try to use
> complex objects save/restore.  In fact I try to find why pcache package
> insnt working anymore with emacs 26.

Sorry this is so frustrating! And thanks for the very concise recipe.

> So here is the same sample completed. Sorry.
>
> ;;; -*- lexical-binding: t -*-
> (require 'eieio)
> (require 'eieio-base)
>
> (defclass eieio-fail (eieio-persistent eieio-named)
>   ((version :initarg :version :initform nil)
>    (version-constant :allocation :class)
>    (entries :initarg :entries :initform (make-hash-table))))

This problem isn't related to my changes: it looks like the source of
the issue is the way the hash table is written, and the fact that one of
its entries holds an EIEIO object.

As far as I can tell, when the hash table is written with `prin1', the
EIEIO object inside is also getting written with `prin1' instead of
`object-write'. The `prin1' representation isn't readable, so the
persistent read process chokes on it.

The prin1 process for the hash table would have to detect that there's
an object in there, and write it with `object-write'.

I assume this used to work? There have been several changes to the
printing process in Emacs 26, but I don't have a good grasp of the
details -- hopefully Stefan or someone will chime in.

Eric

bug#29220: 26.0.90; eieio-persistent-read fail to restore saved object.

[Pierre Téchoueyres]

Eric Abrahamsen <eric@ericabrahamsen.net> writes:

> pierre.techoueyres@free.fr (Pierre Téchoueyres) writes:
>
>> Hello Eric,
>> Firs, thank you for you fast answer. You fix resolved my first problem
>> with eieio-persistent-read, but I've hit another error when I try to use
>> complex objects save/restore.  In fact I try to find why pcache package
>> insnt working anymore with emacs 26.
>
> Sorry this is so frustrating! And thanks for the very concise recipe.
No worries.
>
>> So here is the same sample completed. Sorry.
>>
>> ;;; -*- lexical-binding: t -*-
>> (require 'eieio)
>> (require 'eieio-base)
>>
>> (defclass eieio-fail (eieio-persistent eieio-named)
>>   ((version :initarg :version :initform nil)
>>    (version-constant :allocation :class)
>>    (entries :initarg :entries :initform (make-hash-table))))
>
> This problem isn't related to my changes: it looks like the source of
> the issue is the way the hash table is written, and the fact that one of
> its entries holds an EIEIO object.
I've never said that and thank for looking to it.

>
> As far as I can tell, when the hash table is written with `prin1', the
> EIEIO object inside is also getting written with `prin1' instead of
> `object-write'. The `prin1' representation isn't readable, so the
> persistent read process chokes on it.
>
> The prin1 process for the hash table would have to detect that there's
> an object in there, and write it with `object-write'.
>
> I assume this used to work?
Yes this it the way the pcache (https://github.com/sigma/pcache) package
work. Actually it's requires by the unicode-fonts
(https://github.com/rolandwalker/unicode-fonts) package.

> There have been several changes to the
> printing process in Emacs 26, but I don't have a good grasp of the
> details -- hopefully Stefan or someone will chime in.

Hope my receipt could help.

> Eric

Pierre

bug#29220: 26.0.90; eieio-persistent-read fail to restore saved object.

[Noam Postavsky]

Eric Abrahamsen <eric@ericabrahamsen.net> writes:

> pierre.techoueyres@free.fr (Pierre Téchoueyres) writes:
>
>> So here is the same sample completed. Sorry.
>>
>> ;;; -*- lexical-binding: t -*-
>> (require 'eieio)
>> (require 'eieio-base)
>>
>> (defclass eieio-fail (eieio-persistent eieio-named)
>>   ((version :initarg :version :initform nil)
>>    (version-constant :allocation :class)
>>    (entries :initarg :entries :initform (make-hash-table))))
>
> This problem isn't related to my changes: it looks like the source of
> the issue is the way the hash table is written, and the fact that one of
> its entries holds an EIEIO object.
>
> As far as I can tell, when the hash table is written with `prin1', the
> EIEIO object inside is also getting written with `prin1' instead of
> `object-write'. The `prin1' representation isn't readable, so the
> persistent read process chokes on it.
>
> The prin1 process for the hash table would have to detect that there's
> an object in there, and write it with `object-write'.
>
> I assume this used to work? There have been several changes to the
> printing process in Emacs 26, but I don't have a good grasp of the
> details -- hopefully Stefan or someone will chime in.

Stefan isn't subscribed to the bug list, so you have to Cc him (which
I've now done).  I note that adding (setq print-circle t) makes the
given recipe work.

bug#29220: 26.0.90; eieio-persistent-read fail to restore saved object.

[Pierre Téchoueyres]

Noam Postavsky <npostavs@users.sourceforge.net> writes:

> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
>
>> pierre.techoueyres@free.fr (Pierre Téchoueyres) writes:
>>
>>> So here is the same sample completed. Sorry.
>>>
>>> ;;; -*- lexical-binding: t -*-
>>> (require 'eieio)
>>> (require 'eieio-base)
>>>
>>> (defclass eieio-fail (eieio-persistent eieio-named)
>>>   ((version :initarg :version :initform nil)
>>>    (version-constant :allocation :class)
>>>    (entries :initarg :entries :initform (make-hash-table))))
>>
>> This problem isn't related to my changes: it looks like the source of
>> the issue is the way the hash table is written, and the fact that one of
>> its entries holds an EIEIO object.
>>
>> As far as I can tell, when the hash table is written with `prin1', the
>> EIEIO object inside is also getting written with `prin1' instead of
>> `object-write'. The `prin1' representation isn't readable, so the
>> persistent read process chokes on it.
>>
>> The prin1 process for the hash table would have to detect that there's
>> an object in there, and write it with `object-write'.
>>
>> I assume this used to work? There have been several changes to the
>> printing process in Emacs 26, but I don't have a good grasp of the
>> details -- hopefully Stefan or someone will chime in.
>
> Stefan isn't subscribed to the bug list, so you have to Cc him (which
> I've now done).  I note that adding (setq print-circle t) makes the
> given recipe work.

Unfortunately my receip is an incomplete subset of what pcache do with
the objects and hash tables. And even with `print-circle' set to t,
reading an pcache object still fail with emacs 26.

I've put an branch (pte/emacs-26) on github
(https://github.com/PierreTechoueyres/pcache.git) with some tests that
you can launch with

emacs-26 -batch -L . -l test/pcache-test.el -f ert-run-tests-batch-and-exit

Ask if I could provide more info.
Pierre

bug#29220: 26.0.90; eieio-persistent-read fail to restore saved object.

[Noam Postavsky]

pierre.techoueyres@free.fr (Pierre Téchoueyres) writes:

> Unfortunately my receip is an incomplete subset of what pcache do with
> the objects and hash tables. And even with `print-circle' set to t,
> reading an pcache object still fail with emacs 26.
>
> I've put an branch (pte/emacs-26) on github
> (https://github.com/PierreTechoueyres/pcache.git) with some tests that
> you can launch with
>
> emacs-26 -batch -L . -l test/pcache-test.el -f ert-run-tests-batch-and-exit

Hmm, not sure.  Every time I look deeper at something based on this
object stuff it turns out to be way overcomplicated.

When I set a breakpoint in eieio-persistent-read, the object being seems
correct.  If I evaluate (pcache-get ret 'foo) in edebug, I get 44, as
expected.  But somehow the object which gets returned from
(pcache-repository :object-name #1="pcache-test/tmp2") doesn't have
anything in its hash table.

PS The edebug declaration in pcache-with-repository is wrong, it should
be:

  (declare (indent 2) (debug (symbolp (&rest form) body)))

Or possibly

  (declare (indent 2) (debug (symbolp (&rest sexp) body)))

I can't tell if arglist is meant to be evaluated or not.  All the uses
pass a string which is self-quoting.

bug#29220: 26.0.90; eieio-persistent-read fail to restore saved object.

[Stefan Monnier]

> I can't tell if arglist is meant to be evaluated or not.

Both ;-(

It's used as:

    (apply pcache-repository ',arglist)

where it's not evaluated and as:

    (pcache-destroy-repository ,(car arglist))

where the first element is evaluated.


        Stefan

bug#29220: 26.0.90; eieio-persistent-read fail to restore saved object.

[Pierre Téchoueyres]

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

>> I can't tell if arglist is meant to be evaluated or not.
>
> Both ;-(
>

Maybe I've done some mistakes when I tried to remove deprecated methods
and to use lexical binding.

Can I do something to help ? (But I think it's out of  my capabilities
...)

> It's used as:
>
>     (apply pcache-repository ',arglist)
>
> where it's not evaluated and as:
>
>     (pcache-destroy-repository ,(car arglist))
>
> where the first element is evaluated.
>
>
>         Stefan

bug#29220: 26.0.90; eieio-persistent-read fail to restore saved object.

[Noam Postavsky]

Noam Postavsky <npostavs@users.sourceforge.net> writes:

> When I set a breakpoint in eieio-persistent-read, the object being seems
> correct.  If I evaluate (pcache-get ret 'foo) in edebug, I get 44, as
> expected.  But somehow the object which gets returned from
> (pcache-repository :object-name #1="pcache-test/tmp2") doesn't have
> anything in its hash table.

Okay, I found where the problem is, it's failing pcache-validate-repo
(what's the idea behind throwing an error which is caught and ignored
anyway?):

    (cl-defmethod make-instance ((cache (subclass pcache-repository)) &rest args)
      ...
                 (condition-case nil
                     (let* ((obj (eieio-persistent-read path 'pcache-repository)))
                       (and (or (pcache-validate-repo obj)
                                (error "wrong version")) ...
                   (error nil)))

Specifically, this part:

    (defun pcache-validate-repo (cache)
       ...
       (cl-every
        (lambda (entry)
          (and (object-of-class-p entry (slot-value cache 'entry-cls))
               ...
        (pcache-hash-table-values (slot-value cache 'entries)))))

I can't really tell what this function is supposed to be checking.

bug#29220: 26.0.90; eieio-persistent-read fail to restore saved object.

[Eric Abrahamsen]

Noam Postavsky <npostavs@users.sourceforge.net> writes:

> Noam Postavsky <npostavs@users.sourceforge.net> writes:
>
>> When I set a breakpoint in eieio-persistent-read, the object being seems
>> correct.  If I evaluate (pcache-get ret 'foo) in edebug, I get 44, as
>> expected.  But somehow the object which gets returned from
>> (pcache-repository :object-name #1="pcache-test/tmp2") doesn't have
>> anything in its hash table.
>
> Okay, I found where the problem is, it's failing pcache-validate-repo
> (what's the idea behind throwing an error which is caught and ignored
> anyway?):
>
>     (cl-defmethod make-instance ((cache (subclass pcache-repository)) &rest args)
>       ...
>                  (condition-case nil
>                      (let* ((obj (eieio-persistent-read path 'pcache-repository)))
>                        (and (or (pcache-validate-repo obj)
>                                 (error "wrong version")) ...
>                    (error nil)))

(Are you guys looking at a different version of the pcache repo? The one
on Github uses `constructor', not `make-instance'.)

> Specifically, this part:
>
>     (defun pcache-validate-repo (cache)
>        ...
>        (cl-every
>         (lambda (entry)
>           (and (object-of-class-p entry (slot-value cache 'entry-cls))
>                ...
>         (pcache-hash-table-values (slot-value cache 'entries)))))
>
> I can't really tell what this function is supposed to be checking.

The call to `eieio-persistent-read' does basic slot value validation,
but this function is additionally checking that all of the entries in
the hash table are of the proper class, that specified in the
`entry-cls' slot.

But I still think the problem isn't in reading, it's in writing: the
entries in the hash table are being written incorrectly, and so this
validation step is exploding.

I sure don't know why setting `print-circle' to t would have solved it,
though...

Eric

bug#29220: 26.0.90; eieio-persistent-read fail to restore saved object.

[Noam Postavsky]

Eric Abrahamsen <eric@ericabrahamsen.net> writes:

> (Are you guys looking at a different version of the pcache repo? The one
> on Github uses `constructor', not `make-instance'.)

The version mentioned in #23:

    I've put an branch (pte/emacs-26) on github
    (https://github.com/PierreTechoueyres/pcache.git) with some tests that
    you can launch with

> The call to `eieio-persistent-read' does basic slot value validation,
> but this function is additionally checking that all of the entries in
> the hash table are of the proper class, that specified in the
> `entry-cls' slot.
>
> But I still think the problem isn't in reading, it's in writing: the
> entries in the hash table are being written incorrectly, and so this
> validation step is exploding.

Yes, the way structs and classes are encoded in emacs-26 changed, so the
way they're written changed also.  It seems the round-tripping loses the
class/type maybe?  I can still see the '44' entry in the hash table, so
the hash table itself is still in there...

> I sure don't know why setting `print-circle' to t would have solved it,
> though...

Setting print-circle only lets the read work without error, the read
object still fails the pcache validation.

bug#29220: 26.0.90; eieio-persistent-read fail to restore saved object.

[Eric Abrahamsen]

Noam Postavsky <npostavs@users.sourceforge.net> writes:

> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
>
>> (Are you guys looking at a different version of the pcache repo? The one
>> on Github uses `constructor', not `make-instance'.)
>
> The version mentioned in #23:
>
>     I've put an branch (pte/emacs-26) on github
>     (https://github.com/PierreTechoueyres/pcache.git) with some tests that
>     you can launch with

Oops, I missed that.

[...]

>> But I still think the problem isn't in reading, it's in writing: the
>> entries in the hash table are being written incorrectly, and so this
>> validation step is exploding.
>
> Yes, the way structs and classes are encoded in emacs-26 changed, so the
> way they're written changed also.  It seems the round-tripping loses the
> class/type maybe?  I can still see the '44' entry in the hash table, so
> the hash table itself is still in there...

Here's what I think is going on:

(defclass person ()
  ((name :type string :initarg :name)))

(defclass classy (eieio-persistent)
  ((teacher
    :type person
    :initarg :teacher)
   (students
    :initarg :students :initform (make-hash-table))))

(setq jane (make-instance 'person :name "Jane"))
(setq bob (make-instance 'person :name "Bob"))

(setq class (make-instance 'classy
			   :teacher jane))
(puthash "Bob" bob (slot-value class 'students))

(object-write class) =>
(classy "classy-52080cc"
  :teacher
  (person "person-52080c4"
    :name "Jane")
  :students #s(hash-table size 65 test eql rehash-size 1.5
  rehash-threshold 0.8125 data ("Bob" #s(#s(eieio--class person nil nil
  [#s(cl-slot-descriptor name unbound string nil)] #s(hash-table size 65
  test eq rehash-size 1.5 rehash-threshold 0.8125 data (name 0)) nil
  ((:name . name)) [] [] #s(#2 unbound) (:custom-groups nil)) "Bob"))))

Jane is `read'-able, Bob isn't. `object-write' will recurse and use
`object-write' on any member objects, but the hashtable gets sent to
`prin1', which I assume uses `prin1' on all its values instead:

(object-write bob) =>
(person "person-482ca1c" :name "Bob")

(prin1 bob) =>
#s(#s(eieio--class person nil nil [#s(cl-slot-descriptor
name unbound string nil)] #s(hash-table size 65 test eq rehash-size 1.5
rehash-threshold 0.8125 data (name 0 ...)) nil ((:name . name)) [] []
#s(#1 unbound) (:custom-groups nil)) "Bob")

And I see I have yet again screwed up the changes to `object-write' in
master; off to fix that now.

bug#29220: 26.0.90; eieio-persistent-read fail to restore saved object.

[Noam Postavsky]

Eric Abrahamsen <eric@ericabrahamsen.net> writes:

> (defclass person ()
>   ((name :type string :initarg :name)))
>
> (defclass classy (eieio-persistent)
>   ((teacher
>     :type person
>     :initarg :teacher)
>    (students
>     :initarg :students :initform (make-hash-table))))
>
> (setq jane (make-instance 'person :name "Jane"))
> (setq bob (make-instance 'person :name "Bob"))
>
> (setq class (make-instance 'classy
> 			   :teacher jane))
> (puthash "Bob" bob (slot-value class 'students))

> Jane is `read'-able, Bob isn't. `object-write' will recurse and use
> `object-write' on any member objects, but the hashtable gets sent to
> `prin1', which I assume uses `prin1' on all its values instead:

Yes, but this is the case in emacs-25 as well.  The significant
difference seems to be in the class object:

25.3:
(eieio--object-class-tag bob) => eieio-class-tag--person
26:
(eieio--object-class-tag bob) => #2=#s(eieio--class person nil nil [#s(cl-slot-descriptor name unbound string nil)] #s(hash-table size 65 test eq rehash-size 1.5 rehash-threshold 0.8125 data (name 0)) nil ((:name . name)) #1=[] #1# #s(#2# unbound) (:custom-groups nil))

In Emacs 25 we have a simple symbol which will round-trip as an `eq'
object, in Emacs 26 we have a structure which will not (and furthermore
has some circularity, requiring print-circle to print readably).

bug#29220: 26.0.90; eieio-persistent-read fail to restore saved object.

[Eric Abrahamsen]

Noam Postavsky <npostavs@users.sourceforge.net> writes:

> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
>
>> (defclass person ()
>>   ((name :type string :initarg :name)))
>>
>> (defclass classy (eieio-persistent)
>>   ((teacher
>>     :type person
>>     :initarg :teacher)
>>    (students
>>     :initarg :students :initform (make-hash-table))))
>>
>> (setq jane (make-instance 'person :name "Jane"))
>> (setq bob (make-instance 'person :name "Bob"))
>>
>> (setq class (make-instance 'classy
>> 			   :teacher jane))
>> (puthash "Bob" bob (slot-value class 'students))
>
>> Jane is `read'-able, Bob isn't. `object-write' will recurse and use
>> `object-write' on any member objects, but the hashtable gets sent to
>> `prin1', which I assume uses `prin1' on all its values instead:
>
> Yes, but this is the case in emacs-25 as well.  The significant
> difference seems to be in the class object:
>
> 25.3:
> (eieio--object-class-tag bob) => eieio-class-tag--person
> 26:
> (eieio--object-class-tag bob) => #2=#s(eieio--class person nil nil
> [#s(cl-slot-descriptor name unbound string nil)] #s(hash-table size 65
> test eq rehash-size 1.5 rehash-threshold 0.8125 data (name 0)) nil
> ((:name . name)) #1=[] #1# #s(#2# unbound) (:custom-groups nil))
>
> In Emacs 25 we have a simple symbol which will round-trip as an `eq'
> object, in Emacs 26 we have a structure which will not (and furthermore
> has some circularity, requiring print-circle to print readably).

Yup, I sure noticed that change when it happened -- it made the already
verbose object representations well-nigh unreadable. Looks like it was
8e6f204f44b. I don't know what the right solution is.

bug#29220: 26.0.90; eieio-persistent-read fail to restore saved object.

[Pierre Téchoueyres]

Noam Postavsky <npostavs@users.sourceforge.net> writes:

> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
>
>> (defclass person ()
>>   ((name :type string :initarg :name)))
>>
>> (defclass classy (eieio-persistent)
>>   ((teacher
>>     :type person
>>     :initarg :teacher)
>>    (students
>>     :initarg :students :initform (make-hash-table))))
>>
>> (setq jane (make-instance 'person :name "Jane"))
>> (setq bob (make-instance 'person :name "Bob"))
>>
>> (setq class (make-instance 'classy
>> 			   :teacher jane))
>> (puthash "Bob" bob (slot-value class 'students))
>
>> Jane is `read'-able, Bob isn't. `object-write' will recurse and use
>> `object-write' on any member objects, but the hashtable gets sent to
>> `prin1', which I assume uses `prin1' on all its values instead:
>
> Yes, but this is the case in emacs-25 as well.  The significant
> difference seems to be in the class object:
>
> 25.3:
> (eieio--object-class-tag bob) => eieio-class-tag--person
> 26:
> (eieio--object-class-tag bob) => #2=#s(eieio--class person nil nil
> [#s(cl-slot-descriptor name unbound string nil)] #s(hash-table size 65
> test eq rehash-size 1.5 rehash-threshold 0.8125 data (name 0)) nil
> ((:name . name)) #1=[] #1# #s(#2# unbound) (:custom-groups nil))
>
> In Emacs 25 we have a simple symbol which will round-trip as an `eq'
> object, in Emacs 26 we have a structure which will not (and furthermore
> has some circularity, requiring print-circle to print readably).
>

I've tried to dive into print.c but I don't understand how you return
from print_object, when you're on an hash-table to obtain the vector
with an eieio-class-tag--<class> (as in 25.3) ?

Can anyone tell me how this work in 25.3 ?

bug#29220: 26.0.90; eieio-persistent-read fail to restore saved object.

[Noam Postavsky]

pierre.techoueyres@free.fr (Pierre Téchoueyres) writes:

>> 25.3:
>> (eieio--object-class-tag bob) => eieio-class-tag--person
>> 26:
>> (eieio--object-class-tag bob) => #2=#s(eieio--class person nil nil
>> [#s(cl-slot-descriptor name unbound string nil)] #s(hash-table size 65
>> test eq rehash-size 1.5 rehash-threshold 0.8125 data (name 0)) nil
>> ((:name . name)) #1=[] #1# #s(#2# unbound) (:custom-groups nil))
>>
>> In Emacs 25 we have a simple symbol which will round-trip as an `eq'
>> object, in Emacs 26 we have a structure which will not (and furthermore
>> has some circularity, requiring print-circle to print readably).
>>
>
> I've tried to dive into print.c but I don't understand how you return
> from print_object, when you're on an hash-table to obtain the vector
> with an eieio-class-tag--<class> (as in 25.3) ?
>
> Can anyone tell me how this work in 25.3 ?

Nothing has changed in print.c in this respect.  In 25.3, objects are
vectors, and class tags are symbols.  In 26, objects are records and
class tags are eieio--class records.  As a result, they print
differently.

bug#29220: 26.0.90; eieio-persistent-read fail to restore saved object.

[Pierre Téchoueyres]

Noam Postavsky <npostavs@users.sourceforge.net> writes:
>> I've tried to dive into print.c but I don't understand how you return
>> from print_object, when you're on an hash-table to obtain the vector
>> with an eieio-class-tag--<class> (as in 25.3) ?
>>
>> Can anyone tell me how this work in 25.3 ?
>
> Nothing has changed in print.c in this respect.  In 25.3, objects are
> vectors, and class tags are symbols.  In 26, objects are records and
> class tags are eieio--class records.  As a result, they print
> differently.

I trust you, but that's not the answer I expected, sorry.
I tried to follow the trace from the call of `eieio-persistent-save'
until I could find where it prints the content of the hash table.

What bothers me is that, in emacs 25, you had have  an short vector with
a symbol and now you've an big object dump instead.

Emacs 25 :
[eieio-class-tag--person "Bob"]

Emacs 26 :
#s(#2=#s(eieio--class person nil nil [#s(cl-slot-descriptor name #3=#:unbound string nil)] #s(hash-table size 65 test eq rehash-size 1.5 rehash-threshold 0.8125 data (name 0)) nil ((:name . name)) #1=[] #1# #s(#2# #3#) (:custom-groups nil)) "Bob")

As a side note: as class tags are now eieio--class objects you can't read
an object saved with emacs 25 in emacs 26. Maybe this incompatibility
should be documented into NEWS ?

P.S.
I hope that I'm not too harsh. It's not my intention.

bug#29220: 26.0.90; eieio-persistent-read fail to restore saved object.

[Noam Postavsky]

On Tue, Nov 28, 2017 at 4:39 PM, Pierre Téchoueyres
<pierre.techoueyres@free.fr> wrote:
> Noam Postavsky <npostavs@users.sourceforge.net> writes:
>>> I've tried to dive into print.c but I don't understand how you return
>>> from print_object, when you're on an hash-table to obtain the vector
>>> with an eieio-class-tag--<class> (as in 25.3) ?
>>>
>>> Can anyone tell me how this work in 25.3 ?
>>
>> Nothing has changed in print.c in this respect.  In 25.3, objects are
>> vectors, and class tags are symbols.  In 26, objects are records and
>> class tags are eieio--class records.  As a result, they print
>> differently.
>
> I trust you, but that's not the answer I expected, sorry.

When you say "that's not the answer I expected", do you mean that my
answer didn't sufficiently explain to you why things worked correctly
in Emacs 25.3 and not in Emacs 26? Or do you mean that you would
expect Emacs 26 to work differently (i.e., in a way that doesn't break
things)?

bug#29220: 26.0.90; eieio-persistent-read fail to restore saved object.

[Eric Abrahamsen]

pierre.techoueyres@free.fr (Pierre Téchoueyres) writes:

> Noam Postavsky <npostavs@users.sourceforge.net> writes:
>>> I've tried to dive into print.c but I don't understand how you return
>>> from print_object, when you're on an hash-table to obtain the vector
>>> with an eieio-class-tag--<class> (as in 25.3) ?
>>>
>>> Can anyone tell me how this work in 25.3 ?
>>
>> Nothing has changed in print.c in this respect.  In 25.3, objects are
>> vectors, and class tags are symbols.  In 26, objects are records and
>> class tags are eieio--class records.  As a result, they print
>> differently.
>
> I trust you, but that's not the answer I expected, sorry.
> I tried to follow the trace from the call of `eieio-persistent-save'
> until I could find where it prints the content of the hash table.
>
> What bothers me is that, in emacs 25, you had have  an short vector with
> a symbol and now you've an big object dump instead.
>
> Emacs 25 :
> [eieio-class-tag--person "Bob"]
>
> Emacs 26 :
> #s(#2=#s(eieio--class person nil nil [#s(cl-slot-descriptor name
> #3=#:unbound string nil)] #s(hash-table size 65 test eq rehash-size
> 1.5 rehash-threshold 0.8125 data (name 0)) nil ((:name . name)) #1=[]
> #1# #s(#2# #3#) (:custom-groups nil)) "Bob")

That's exactly what Noam was saying! The implementation of objects
changed from 25 to 26, and thus their printed representation also
changed.

> As a side note: as class tags are now eieio--class objects you can't read
> an object saved with emacs 25 in emacs 26. Maybe this incompatibility
> should be documented into NEWS ?

The real problem isn't that objects written with 25 can't be read by 26
(I don't think that kind of compatibility is guaranteed), but that
objects written by 26 can't be read at all -- at least not when they're
written internally, as members of other objects.

Personally I'm not sure how to fix the problem, which is why we keep
cc'ing Stefan on these messages (ahem).

Eric

bug#29220: 26.0.90; eieio-persistent-read fail to restore saved object.

[Pierre Téchoueyres]

Noam Postavsky <npostavs@users.sourceforge.net> writes:

> On Tue, Nov 28, 2017 at 4:39 PM, Pierre Téchoueyres
> <pierre.techoueyres@free.fr> wrote:
>> Noam Postavsky <npostavs@users.sourceforge.net> writes:
>>>> I've tried to dive into print.c but I don't understand how you return
>>>> from print_object, when you're on an hash-table to obtain the vector
>>>> with an eieio-class-tag--<class> (as in 25.3) ?
>>>>
>>>> Can anyone tell me how this work in 25.3 ?
>>>
>>> Nothing has changed in print.c in this respect.  In 25.3, objects are
>>> vectors, and class tags are symbols.  In 26, objects are records and
>>> class tags are eieio--class records.  As a result, they print
>>> differently.
>>
>> I trust you, but that's not the answer I expected, sorry.
>
> When you say "that's not the answer I expected", do you mean that my
> answer didn't sufficiently explain to you why things worked correctly
> in Emacs 25.3 and not in Emacs 26? Or do you mean that you would
> expect Emacs 26 to work differently (i.e., in a way that doesn't break
> things)?

Both and none of them :-)

I try to understand how, from print_object, objects in the hash table
are print.
I expected something like

prin1 -> print -> print_object -> something in lisp or C

I'm trying to make pcache work correctly with emacs 26.
Now I understand that eieio--class and eieio--class-tag have changed,
but I don't know if I can or how to correct the printing of objects in
order to read them back with eieio-persistent-read.

bug#29220: 26.0.90; eieio-persistent-read fail to restore saved object.

[Noam Postavsky]

pierre.techoueyres@free.fr (Pierre Téchoueyres) writes:

> I try to understand how, from print_object, objects in the hash table
> are print.
> I expected something like
>
> prin1 -> print -> print_object -> something in lisp or C

The sequence is

prin1 -> print -> print_object -> print_vectorlike

print_vectorlike calls print_object on all of the keys and values of the
hashtable (see 'case PVEC_HASH_TABLE').

> I'm trying to make pcache work correctly with emacs 26.
> Now I understand that eieio--class and eieio--class-tag have changed,
> but I don't know if I can or how to correct the printing of objects in
> order to read them back with eieio-persistent-read.

I see 3 possibilities:

1. Make eieio persistent save stuff use cl-print to print the whole
thing readably (this requires major additions to cl-print, currently it
doesn't preserve readability except by just punting to prin1).

2. Change the class tag to something that will be easily readable, and
round-trip as eq (i.e., some kind of symbol, like in Emacs 25 and
earlier).

3. Change child-of-class-p to use `equal'ness instead of `eq'ness for
classes.  I think would require an `equal' that can handle circular
objects.

Looks like #2 would be the easiest to implement.

bug#29220: 26.0.90; eieio-persistent-read fail to restore saved object.

[Stefan Monnier]

> That's exactly what Noam was saying! The implementation of objects
> changed from 25 to 26, and thus their printed representation also
> changed.

Indeed.  But the subject mentions "...eieio-persistent-read..." and
eieio-persistent was designed specifically to try and avoid dependence
on how Emacs represents EIEIO objects.  The problem here is that
eieio-persistent doesn't support hash-tables, so the EIEIO objects in
the hash-tables are printed with `princ` (i.e. in all their internal
glory) rather than going through eieio-persistent.

So I think the real fix here is to extend eieio-persistent with support
for hash-tables.

> The real problem isn't that objects written with 25 can't be read by 26
> (I don't think that kind of compatibility is guaranteed), but that
> objects written by 26 can't be read at all -- at least not when they're
> written internally, as members of other objects.

There are many different ways one might want to "print" objects,
depending on the intention:
- print for human consumption (i.e. eliding details to keep it concise).
  That's what cl-print aims for, for example (tho IIRC it doesn't
  support hash-tables either, so far).
- print in a way that can be read back in the future (possibly far
  future, where some details of the internal representation have
  changed).  This is what eieio-persistent aims for.
- print in a way that can be read back, preserving as many of the
  details as possible.  This is what `princ` aims for.  In the case of
  EIEIO objects, I'm not completely satisfied with the result in
  Emacs-26, but I think I'd rather live with it for now to get time to
  better understand the design space&constraints before we improve it.


-- Stefan

bug#29220: 26.0.90; eieio-persistent-read fail to restore saved object.

[Stefan Monnier]

> 2. Change the class tag to something that will be easily readable, and
> round-trip as eq (i.e., some kind of symbol, like in Emacs 25 and
> earlier).

Note that this could be done on the print-side without changing the actual
object representation.


        Stefan

bug#29220: 26.0.90; eieio-persistent-read fail to restore saved object.

[Eric Abrahamsen]

On 11/29/17 10:15 AM, Stefan Monnier wrote:
>> That's exactly what Noam was saying! The implementation of objects
>> changed from 25 to 26, and thus their printed representation also
>> changed.
>
> Indeed.  But the subject mentions "...eieio-persistent-read..." and
> eieio-persistent was designed specifically to try and avoid dependence
> on how Emacs represents EIEIO objects.  The problem here is that
> eieio-persistent doesn't support hash-tables, so the EIEIO objects in
> the hash-tables are printed with `princ` (i.e. in all their internal
> glory) rather than going through eieio-persistent.
>
> So I think the real fix here is to extend eieio-persistent with support
> for hash-tables.

Just hash tables? How about vectors? There seem to be many places where
objects could be hiding...

bug#29220: 26.0.90; eieio-persistent-read fail to restore saved object.

[Stefan Monnier]

>> So I think the real fix here is to extend eieio-persistent with support
>> for hash-tables.
> Just hash tables?

For the problem at hand, IIUC, hash-tables is what's needed, yes.

> How about vectors?

Sure, add that too.


        Stefan

bug#29220: 26.0.90; eieio-persistent-read fail to restore saved object.

[Eric Abrahamsen]

On 12/01/17 12:55 PM, Stefan Monnier wrote:
>>> So I think the real fix here is to extend eieio-persistent with support
>>> for hash-tables.
>> Just hash tables?
>
> For the problem at hand, IIUC, hash-tables is what's needed, yes.
>
>> How about vectors?
>
> Sure, add that too.

Before I do that, I just opened #29541 with a proposal for tweaking the
restore process -- maybe that could be done first?

bug#29220: 26.0.90; eieio-persistent-read fail to restore saved object.

[Pierre Téchoueyres]

Eric Abrahamsen <eric@ericabrahamsen.net> writes:

> On 12/01/17 12:55 PM, Stefan Monnier wrote:
>>>> So I think the real fix here is to extend eieio-persistent with support
>>>> for hash-tables.
>>> Just hash tables?
>>
>> For the problem at hand, IIUC, hash-tables is what's needed, yes.
>>
>>> How about vectors?
>>
>> Sure, add that too.
>
> Before I do that, I just opened #29541 with a proposal for tweaking the
> restore process -- maybe that could be done first?
>

I wishes it would, but this is a blocker bug for emacs 26, isn't it ?

bug#29220: 26.0.90; eieio-persistent-read fail to restore saved object.

[Eric Abrahamsen]

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


On 12/03/17 19:35 PM, Pierre Téchoueyres wrote:
> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
>
>> On 12/01/17 12:55 PM, Stefan Monnier wrote:
>>>>> So I think the real fix here is to extend eieio-persistent with support
>>>>> for hash-tables.
>>>> Just hash tables?
>>>
>>> For the problem at hand, IIUC, hash-tables is what's needed, yes.
>>>
>>>> How about vectors?
>>>
>>> Sure, add that too.
>>
>> Before I do that, I just opened #29541 with a proposal for tweaking the
>> restore process -- maybe that could be done first?
>>
>
> I wishes it would, but this is a blocker bug for emacs 26, isn't it ?

Could be! If so, let's fix this part first.

Here's a diff that seems to work. The read->with-output-to-string stuff
seems awful to me, and I'm hoping someone has something more elegant on
hand.

Eric


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: persistent-stuff.diff --]
[-- Type: text/x-patch, Size: 2364 bytes --]

diff --git a/lisp/emacs-lisp/eieio-base.el b/lisp/emacs-lisp/eieio-base.el
index 58dcd09d7e..840f7223c4 100644
--- a/lisp/emacs-lisp/eieio-base.el
+++ b/lisp/emacs-lisp/eieio-base.el
@@ -354,15 +354,34 @@ eieio-persistent-validate/fix-slot-value
 		   proposed-value))
 		 (t
 		  proposed-value))))
+	((stringp proposed-value)
+	 ;; Remove string properties.
+	 (substring-no-properties proposed-value))
+
+        ;; For hash-tables and vectors, the top-level `read' will not
+        ;; "look inside" member values, so we need to do that
+        ;; explicitly.
+        ((hash-table-p proposed-value)
+         (maphash
+          (lambda (key value)
+            (when (class-p (car-safe value))
+              (setf (gethash key proposed-value)
+                    (eieio-persistent-convert-list-to-object
+                     value))))
+          proposed-value)
+         proposed-value)
+
+        ((vectorp proposed-value)
+         (dotimes (i (length proposed-value))
+           (when (class-p (car-safe (aref proposed-value i)))
+             (aset proposed-value i
+                   (eieio-persistent-convert-list-to-object
+                    (aref proposed-value i)))))
+         proposed-value)
 
-	 ((stringp proposed-value)
-	  ;; Else, check for strings, remove properties.
-	  (substring-no-properties proposed-value))
-
-	 (t
-	  ;; Else, just return whatever the constant was.
-	  proposed-value))
-  )
+	(t
+	 ;; Else, just return whatever the constant was.
+	 proposed-value)))
 
 (defun eieio-persistent-slot-type-is-class-p (type)
   "Return the class referred to in TYPE.
diff --git a/lisp/emacs-lisp/eieio.el b/lisp/emacs-lisp/eieio.el
index d0d2ff5145..11637c8072 100644
--- a/lisp/emacs-lisp/eieio.el
+++ b/lisp/emacs-lisp/eieio.el
@@ -926,6 +926,23 @@ eieio-override-prin1
 	 (object-write thing))
 	((consp thing)
 	 (eieio-list-prin1 thing))
+	((hash-table-p thing)
+	 (maphash
+	  (lambda (key val)
+	    (setf (gethash key thing)
+		  (read
+		   (with-output-to-string
+		     (temp-eieio-override-prin1 val)))))
+	  thing)
+	 (prin1 thing))
+	((vectorp thing)
+	 (dotimes (i (length thing))
+	   (aset thing i
+		 (read
+		  (with-output-to-string
+		    (temp-eieio-override-prin1
+		     (aref thing i))))))
+	 (prin1 thing))
 	((eieio--class-p thing)
 	 (princ (eieio--class-print-name thing)))
 	(t (prin1 thing))))

bug#29220: 26.0.90; eieio-persistent-read fail to restore saved object.

[Stefan Monnier]

> --- a/lisp/emacs-lisp/eieio-base.el
> +++ b/lisp/emacs-lisp/eieio-base.el
> @@ -354,15 +354,34 @@ eieio-persistent-validate/fix-slot-value
>  		   proposed-value))
>  		 (t
>  		  proposed-value))))
> +	((stringp proposed-value)
> +	 ;; Remove string properties.
> +	 (substring-no-properties proposed-value))
> +
> +        ;; For hash-tables and vectors, the top-level `read' will not
> +        ;; "look inside" member values, so we need to do that
> +        ;; explicitly.
> +        ((hash-table-p proposed-value)
> +         (maphash
> +          (lambda (key value)
> +            (when (class-p (car-safe value))
> +              (setf (gethash key proposed-value)
> +                    (eieio-persistent-convert-list-to-object
> +                     value))))
> +          proposed-value)
> +         proposed-value)
> +
> +        ((vectorp proposed-value)
> +         (dotimes (i (length proposed-value))
> +           (when (class-p (car-safe (aref proposed-value i)))
> +             (aset proposed-value i
> +                   (eieio-persistent-convert-list-to-object
> +                    (aref proposed-value i)))))
> +         proposed-value)
 
> -	 ((stringp proposed-value)
> -	  ;; Else, check for strings, remove properties.
> -	  (substring-no-properties proposed-value))
> -
> -	 (t
> -	  ;; Else, just return whatever the constant was.
> -	  proposed-value))
> -  )
> +	(t
> +	 ;; Else, just return whatever the constant was.
> +	 proposed-value)))

Not sure why the `stringp` part was reindented, and an empty line was
lost (which de-synchronized the diff), but other than that, it looks fine.

> +	((hash-table-p thing)
> +	 (maphash
> +	  (lambda (key val)
> +	    (setf (gethash key thing)
> +		  (read
> +		   (with-output-to-string
> +		     (temp-eieio-override-prin1 val)))))
> +	  thing)
> +	 (prin1 thing))
> +	((vectorp thing)
> +	 (dotimes (i (length thing))
> +	   (aset thing i
> +		 (read
> +		  (with-output-to-string
> +		    (temp-eieio-override-prin1
> +		     (aref thing i))))))
> +	 (prin1 thing))

This looks wrong, OTOH:
- temp-eieio-override-prin1 does not exist in my copy of Emacs `master`.
- you modify the object you received, so while the print part will
  presumably work OK, the object left after that is unusable.

Maybe a better option is to print something of the form

   (vector <exp1> <exp2> ...)

i.e. not something that looks like a vector.  Or otherwise, print the
"standard" syntax of a vector/hash-table, but do it by hand rather than
rely on prin1 to do it for us.


        Stefan

bug#29220: 26.0.90; eieio-persistent-read fail to restore saved object.

[Eric Abrahamsen]

On 12/04/17 21:08 PM, Stefan Monnier wrote:
>> --- a/lisp/emacs-lisp/eieio-base.el
>> +++ b/lisp/emacs-lisp/eieio-base.el
>> @@ -354,15 +354,34 @@ eieio-persistent-validate/fix-slot-value
>>  		   proposed-value))
>>  		 (t
>>  		  proposed-value))))
>> +	((stringp proposed-value)
>> +	 ;; Remove string properties.
>> +	 (substring-no-properties proposed-value))
>> +
>> +        ;; For hash-tables and vectors, the top-level `read' will not
>> +        ;; "look inside" member values, so we need to do that
>> +        ;; explicitly.
>> +        ((hash-table-p proposed-value)
>> +         (maphash
>> +          (lambda (key value)
>> +            (when (class-p (car-safe value))
>> +              (setf (gethash key proposed-value)
>> +                    (eieio-persistent-convert-list-to-object
>> +                     value))))
>> +          proposed-value)
>> +         proposed-value)
>> +
>> +        ((vectorp proposed-value)
>> +         (dotimes (i (length proposed-value))
>> +           (when (class-p (car-safe (aref proposed-value i)))
>> +             (aset proposed-value i
>> +                   (eieio-persistent-convert-list-to-object
>> +                    (aref proposed-value i)))))
>> +         proposed-value)
>  
>> -	 ((stringp proposed-value)
>> -	  ;; Else, check for strings, remove properties.
>> -	  (substring-no-properties proposed-value))
>> -
>> -	 (t
>> -	  ;; Else, just return whatever the constant was.
>> -	  proposed-value))
>> -  )
>> +	(t
>> +	 ;; Else, just return whatever the constant was.
>> +	 proposed-value)))
>
> Not sure why the `stringp` part was reindented, and an empty line was
> lost (which de-synchronized the diff), but other than that, it looks fine.

Dunno about the reindentation, either, I guess I can discard those
changes.

>> +	((hash-table-p thing)
>> +	 (maphash
>> +	  (lambda (key val)
>> +	    (setf (gethash key thing)
>> +		  (read
>> +		   (with-output-to-string
>> +		     (temp-eieio-override-prin1 val)))))
>> +	  thing)
>> +	 (prin1 thing))
>> +	((vectorp thing)
>> +	 (dotimes (i (length thing))
>> +	   (aset thing i
>> +		 (read
>> +		  (with-output-to-string
>> +		    (temp-eieio-override-prin1
>> +		     (aref thing i))))))
>> +	 (prin1 thing))
>
> This looks wrong, OTOH:
> - temp-eieio-override-prin1 does not exist in my copy of Emacs
> `master`.

That was a temporary name to prevent these changes from wrecking my Gnus
and EBDB. I guess I should be working in a separate Emacs.

> - you modify the object you received, so while the print part will
>   presumably work OK, the object left after that is unusable.

I should have realized that...

> Maybe a better option is to print something of the form
>
>    (vector <exp1> <exp2> ...)
>
> i.e. not something that looks like a vector.  Or otherwise, print the
> "standard" syntax of a vector/hash-table, but do it by hand rather than
> rely on prin1 to do it for us.

Would it be too "heavy" to just copy the object and modify the copy? It
seems like writing a list-like form will result in just as much
consing...?

Eric

bug#29220: 26.0.90; eieio-persistent-read fail to restore saved object.

[Stefan Monnier]

> Would it be too "heavy" to just copy the object and modify the copy?

No, that's fine as well.


        Stefan

bug#29220: 26.0.90; eieio-persistent-read fail to restore saved object.

[Eric Abrahamsen]

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


On 12/05/17 14:02 PM, Stefan Monnier wrote:
>> Would it be too "heavy" to just copy the object and modify the copy?
>
> No, that's fine as well.

Okay, the attached appears to work just fine. Pierre's recipe passes,
as do all the tests in eieio-test-persist. Pierre, maybe you could eval
this quickly and make sure that pcache works correctly again?

If this is okay, it's going into emacs-26, right?

Eric


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: persist-tables-vectors.diff --]
[-- Type: text/x-patch, Size: 2052 bytes --]

diff --git a/lisp/emacs-lisp/eieio-base.el b/lisp/emacs-lisp/eieio-base.el
index 58dcd09d7e..0241f27395 100644
--- a/lisp/emacs-lisp/eieio-base.el
+++ b/lisp/emacs-lisp/eieio-base.el
@@ -354,6 +354,26 @@ eieio-persistent-validate/fix-slot-value
 		   proposed-value))
 		 (t
 		  proposed-value))))
+        ;; For hash-tables and vectors, the top-level `read' will not
+        ;; "look inside" member values, so we need to do that
+        ;; explicitly.
+        ((hash-table-p proposed-value)
+         (maphash
+          (lambda (key value)
+            (when (class-p (car-safe value))
+              (setf (gethash key proposed-value)
+                    (eieio-persistent-convert-list-to-object
+                     value))))
+          proposed-value)
+         proposed-value)
+
+        ((vectorp proposed-value)
+         (dotimes (i (length proposed-value))
+           (when (class-p (car-safe (aref proposed-value i)))
+             (aset proposed-value i
+                   (eieio-persistent-convert-list-to-object
+                    (aref proposed-value i)))))
+         proposed-value)
 
 	 ((stringp proposed-value)
 	  ;; Else, check for strings, remove properties.
diff --git a/lisp/emacs-lisp/eieio.el b/lisp/emacs-lisp/eieio.el
index d0d2ff5145..ab96547f93 100644
--- a/lisp/emacs-lisp/eieio.el
+++ b/lisp/emacs-lisp/eieio.el
@@ -926,6 +926,25 @@ eieio-override-prin1
 	 (object-write thing))
 	((consp thing)
 	 (eieio-list-prin1 thing))
+	((hash-table-p thing)
+         (let ((copy (copy-hash-table thing)))
+	   (maphash
+	    (lambda (key val)
+	      (setf (gethash key copy)
+		    (read
+		     (with-output-to-string
+		       (eieio-override-prin1 val)))))
+	    copy)
+	   (prin1 copy)))
+	((vectorp thing)
+         (let ((copy (copy-sequence thing)))
+	  (dotimes (i (length copy))
+	    (aset copy i
+		  (read
+		   (with-output-to-string
+		     (eieio-override-prin1
+		      (aref copy i))))))
+	  (prin1 copy)))
 	((eieio--class-p thing)
 	 (princ (eieio--class-print-name thing)))
 	(t (prin1 thing))))

bug#29220: 26.0.90; eieio-persistent-read fail to restore saved object.

[Stefan Monnier]

> Okay, the attached appears to work just fine. Pierre's recipe passes,
> as do all the tests in eieio-test-persist. Pierre, maybe you could eval
> this quickly and make sure that pcache works correctly again?

Looks good to me.

[ While looking at this code, I notice that with cl-defmethod (which can
  dispatch not only on EIEIO objects, but other object types as well)
  we can now fold eieio-override-prin1 into object-write (i.e. make one
  into an alias for the other).  ]

> If this is okay, it's going into emacs-26, right?

I think so, but we're pretty late in the pretest, so better ask Eli&John
if they think it's OK for emacs-26.


        Stefan


> diff --git a/lisp/emacs-lisp/eieio-base.el b/lisp/emacs-lisp/eieio-base.el
> index 58dcd09d7e..0241f27395 100644
> --- a/lisp/emacs-lisp/eieio-base.el
> +++ b/lisp/emacs-lisp/eieio-base.el
> @@ -354,6 +354,26 @@ eieio-persistent-validate/fix-slot-value
>  		   proposed-value))
>  		 (t
>  		  proposed-value))))
> +        ;; For hash-tables and vectors, the top-level `read' will not
> +        ;; "look inside" member values, so we need to do that
> +        ;; explicitly.
> +        ((hash-table-p proposed-value)
> +         (maphash
> +          (lambda (key value)
> +            (when (class-p (car-safe value))
> +              (setf (gethash key proposed-value)
> +                    (eieio-persistent-convert-list-to-object
> +                     value))))
> +          proposed-value)
> +         proposed-value)
> +
> +        ((vectorp proposed-value)
> +         (dotimes (i (length proposed-value))
> +           (when (class-p (car-safe (aref proposed-value i)))
> +             (aset proposed-value i
> +                   (eieio-persistent-convert-list-to-object
> +                    (aref proposed-value i)))))
> +         proposed-value)
 
>  	 ((stringp proposed-value)
>  	  ;; Else, check for strings, remove properties.
> diff --git a/lisp/emacs-lisp/eieio.el b/lisp/emacs-lisp/eieio.el
> index d0d2ff5145..ab96547f93 100644
> --- a/lisp/emacs-lisp/eieio.el
> +++ b/lisp/emacs-lisp/eieio.el
> @@ -926,6 +926,25 @@ eieio-override-prin1
>  	 (object-write thing))
>  	((consp thing)
>  	 (eieio-list-prin1 thing))
> +	((hash-table-p thing)
> +         (let ((copy (copy-hash-table thing)))
> +	   (maphash
> +	    (lambda (key val)
> +	      (setf (gethash key copy)
> +		    (read
> +		     (with-output-to-string
> +		       (eieio-override-prin1 val)))))
> +	    copy)
> +	   (prin1 copy)))
> +	((vectorp thing)
> +         (let ((copy (copy-sequence thing)))
> +	  (dotimes (i (length copy))
> +	    (aset copy i
> +		  (read
> +		   (with-output-to-string
> +		     (eieio-override-prin1
> +		      (aref copy i))))))
> +	  (prin1 copy)))
>  	((eieio--class-p thing)
>  	 (princ (eieio--class-print-name thing)))
>  	(t (prin1 thing))))

bug#29220: 26.0.90; eieio-persistent-read fail to restore saved object.

[Pierre Téchoueyres]

Eric Abrahamsen <eric@ericabrahamsen.net> writes:

> On 12/05/17 14:02 PM, Stefan Monnier wrote:
>>> Would it be too "heavy" to just copy the object and modify the copy?
>>
>> No, that's fine as well.
>
> Okay, the attached appears to work just fine. Pierre's recipe passes,
> as do all the tests in eieio-test-persist. Pierre, maybe you could eval
> this quickly and make sure that pcache works correctly again?

Yes all my receipes tests are running fine. Many thanks.
I've tested also with the unicode-fonts package
(http://github.com/rolandwalker/unicode-fonts) from melpa.
Everything seem to work fine.

>
> If this is okay, it's going into emacs-26, right?
>
> Eric

bug#29220: 26.0.90; eieio-persistent-read fail to restore saved object.

[Eric Abrahamsen]

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


On 12/05/17 17:14 PM, Stefan Monnier wrote:

> Looks good to me.
>
> [ While looking at this code, I notice that with cl-defmethod (which can
>   dispatch not only on EIEIO objects, but other object types as well)
>   we can now fold eieio-override-prin1 into object-write (i.e. make one
>   into an alias for the other).  ]

Yup, I think there's a fair amount of modularization that can be done. I
can make that part of the patch on #29541.

>> If this is okay, it's going into emacs-26, right?
>
> I think so, but we're pretty late in the pretest, so better ask Eli&John
> if they think it's OK for emacs-26.

Okay: John and Eli, this patch is to handle changes in object
implementation in Emacs 26: they are now implemented with records, which
can't be round-tripped with prin1 and read, they have to be written and
read specially. The eieio-persistent mechanism handles that, but
previously did not look inside hash tables or vectors to see if there
were objects in there.

That's causing failure in the pcache library, which other libraries
depend on. I'm hoping we can sneak this in under the wire so that pcache
doesn't fail in Emacs 26.

[...]

On 12/05/17 23:20 PM, Pierre Téchoueyres wrote:
> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
>
>> On 12/05/17 14:02 PM, Stefan Monnier wrote:
>>>> Would it be too "heavy" to just copy the object and modify the copy?
>>>
>>> No, that's fine as well.
>>
>> Okay, the attached appears to work just fine. Pierre's recipe passes,
>> as do all the tests in eieio-test-persist. Pierre, maybe you could eval
>> this quickly and make sure that pcache works correctly again?
>
> Yes all my receipes tests are running fine. Many thanks.
> I've tested also with the unicode-fonts package
> (http://github.com/rolandwalker/unicode-fonts) from melpa.
> Everything seem to work fine.

Great! Thanks for testing.

Eric


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Handle-hash-tables-and-vectors-when-reading-writing-.patch --]
[-- Type: text/x-patch, Size: 2868 bytes --]

From 9f946a968f41e16f7279c72706ccb839dcb7757c Mon Sep 17 00:00:00 2001
From: Eric Abrahamsen <eric@ericabrahamsen.net>
Date: Tue, 5 Dec 2017 14:41:50 -0800
Subject: [PATCH] Handle hash tables and vectors when reading/writing EIEIO
 objects

* lisp/emacs-lisp/eieio.el (eieio-override-prin1): EIEIO objects
  printed with `prin1' can no longer be read with `read'. Make sure
  they are printed with object-write instead, even when they're inside
  hash tables and vectors.
* lisp/emacs-lisp/eieio-base.el (eieio-persistent-validate/fix-slot-value):
  Check for written representations of objects inside hash tables and
  vectors, and reconstruct them.
---
 lisp/emacs-lisp/eieio-base.el | 20 ++++++++++++++++++++
 lisp/emacs-lisp/eieio.el      | 19 +++++++++++++++++++
 2 files changed, 39 insertions(+)

diff --git a/lisp/emacs-lisp/eieio-base.el b/lisp/emacs-lisp/eieio-base.el
index 58dcd09d7e..0241f27395 100644
--- a/lisp/emacs-lisp/eieio-base.el
+++ b/lisp/emacs-lisp/eieio-base.el
@@ -354,6 +354,26 @@ eieio-persistent-validate/fix-slot-value
 		   proposed-value))
 		 (t
 		  proposed-value))))
+        ;; For hash-tables and vectors, the top-level `read' will not
+        ;; "look inside" member values, so we need to do that
+        ;; explicitly.
+        ((hash-table-p proposed-value)
+         (maphash
+          (lambda (key value)
+            (when (class-p (car-safe value))
+              (setf (gethash key proposed-value)
+                    (eieio-persistent-convert-list-to-object
+                     value))))
+          proposed-value)
+         proposed-value)
+
+        ((vectorp proposed-value)
+         (dotimes (i (length proposed-value))
+           (when (class-p (car-safe (aref proposed-value i)))
+             (aset proposed-value i
+                   (eieio-persistent-convert-list-to-object
+                    (aref proposed-value i)))))
+         proposed-value)
 
 	 ((stringp proposed-value)
 	  ;; Else, check for strings, remove properties.
diff --git a/lisp/emacs-lisp/eieio.el b/lisp/emacs-lisp/eieio.el
index d0d2ff5145..ab96547f93 100644
--- a/lisp/emacs-lisp/eieio.el
+++ b/lisp/emacs-lisp/eieio.el
@@ -926,6 +926,25 @@ eieio-override-prin1
 	 (object-write thing))
 	((consp thing)
 	 (eieio-list-prin1 thing))
+	((hash-table-p thing)
+         (let ((copy (copy-hash-table thing)))
+	   (maphash
+	    (lambda (key val)
+	      (setf (gethash key copy)
+		    (read
+		     (with-output-to-string
+		       (eieio-override-prin1 val)))))
+	    copy)
+	   (prin1 copy)))
+	((vectorp thing)
+         (let ((copy (copy-sequence thing)))
+	  (dotimes (i (length copy))
+	    (aset copy i
+		  (read
+		   (with-output-to-string
+		     (eieio-override-prin1
+		      (aref copy i))))))
+	  (prin1 copy)))
 	((eieio--class-p thing)
 	 (princ (eieio--class-print-name thing)))
 	(t (prin1 thing))))
-- 
2.15.1

bug#29220: 26.0.90; eieio-persistent-read fail to restore saved object.

[Eli Zaretskii]

> From: Eric Abrahamsen <eric@ericabrahamsen.net>
> Cc: pierre.techoueyres@free.fr (Pierre Téchoueyres),
>   29220@debbugs.gnu.org,  Noam Postavsky <npostavs@users.sourceforge.net>,
>  John Wiegley <jwiegley@gmail.com>, Eli Zaretskii <eliz@gnu.org>
> Date: Tue, 05 Dec 2017 14:58:45 -0800
> 
> > I think so, but we're pretty late in the pretest, so better ask Eli&John
> > if they think it's OK for emacs-26.
> 
> Okay: John and Eli, this patch is to handle changes in object
> implementation in Emacs 26: they are now implemented with records, which
> can't be round-tripped with prin1 and read, they have to be written and
> read specially. The eieio-persistent mechanism handles that, but
> previously did not look inside hash tables or vectors to see if there
> were objects in there.
> 
> That's causing failure in the pcache library, which other libraries
> depend on. I'm hoping we can sneak this in under the wire so that pcache
> doesn't fail in Emacs 26.

Fine with me to put this on emacs-26.

bug#29220: 26.0.90; eieio-persistent-read fail to restore saved object.

[Eric Abrahamsen]

On 12/08/17 12:41 PM, Eli Zaretskii wrote:
>> From: Eric Abrahamsen <eric@ericabrahamsen.net>
>> Cc: pierre.techoueyres@free.fr (Pierre Téchoueyres),
>>   29220@debbugs.gnu.org,  Noam Postavsky <npostavs@users.sourceforge.net>,
>>  John Wiegley <jwiegley@gmail.com>, Eli Zaretskii <eliz@gnu.org>
>> Date: Tue, 05 Dec 2017 14:58:45 -0800
>> 
>> > I think so, but we're pretty late in the pretest, so better ask Eli&John
>> > if they think it's OK for emacs-26.
>> 
>> Okay: John and Eli, this patch is to handle changes in object
>> implementation in Emacs 26: they are now implemented with records, which
>> can't be round-tripped with prin1 and read, they have to be written and
>> read specially. The eieio-persistent mechanism handles that, but
>> previously did not look inside hash tables or vectors to see if there
>> were objects in there.
>> 
>> That's causing failure in the pcache library, which other libraries
>> depend on. I'm hoping we can sneak this in under the wire so that pcache
>> doesn't fail in Emacs 26.
>
> Fine with me to put this on emacs-26.

Thanks -- done.

bug#29220: 26.0.90; eieio-persistent-read fail to restore saved object.

[Noam Postavsky]

tags 29220 fixed
close 29220 26.1
quit

Eric Abrahamsen <eric@ericabrahamsen.net> writes:

>> Fine with me to put this on emacs-26.
>
> Thanks -- done.

Closing.

bug#29220: 26.0.90; eieio-persistent-read fail to restore saved object.

[Pierre Téchoueyres]

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

Noam Postavsky <npostavs@users.sourceforge.net> writes:

> tags 29220 fixed
> close 29220 26.1
> quit
> [...]
>
> Closing.
>
Sorry but I not certain everything is working as expected.

I'm trying to add another test to the eieio part
(test/lisp/emacs-lisp/eieio-tests/eieio-test-persist.el) but when I did
try the simple receipt which Eric had posted some times ago I end in the
debugger.

#+BEGIN_SRC debugger
Debugger entered--Lisp error: (wrong-type-argument sequencep person)
  mapc(#f(compiled-function (elt) #<bytecode 0x237e645>) person)
  seq-do(#f(compiled-function (elt) #<bytecode 0x237e645>) person)
  seq-some(#f(compiled-function (elt) #<bytecode 0x237e299>) person)
  eieio-persistent-validate/fix-slot-value(#s(eieio--class :name classy :docstring nil :parents (#s(eieio--class :name eieio-persistent :docstring "This special class enables persistence through save files\nUse the `object-save' method to write this object to disk.  The save\nformat is Emacs Lisp code which calls the constructor for the saved\nobject.  For this reason, only slots which do not have an `:initarg'\nspecified will not be saved." :parents nil :slots [#s(cl-slot-descriptor :name file :initform unbound :type string :props ((:documentation . "The save file for this persistent object.\nThis must be a string, and must be specified when the new object is\ninstantiated.")))] :index-table #<hash-table eq 1/65 0x129c671> :children (classy pcache-repository) :initarg-tuples ((:file . file)) :class-slots [#s(cl-slot-descriptor :name do-backups :initform t :type boolean :props ((:documentation . "Saving this object should make backup files.\nSetting to nil will mean no backups are made."))) #s(cl-slot-descriptor :name file-header-line :initform ";; EIEIO PERSISTENT OBJECT" :type string :props ((:documentation . "Header line for the save file.\nThis is used with the `object-write' method."))) #s(cl-slot-descriptor :name extension :initform ".eieio" :type string :props ((:documentation . "Extension of files saved by this object.\nEnables auto-choosing nice file names based on name.")))] :class-allocation-values [t ";; EIEIO PERSISTENT OBJECT" ".eieio"] :default-object-cache #<eieio-persistent eieio-persistent> :options (:custom-groups nil :documentation "This special class enables persistence through save files\nUse the `object-save' method to write this object to disk.  The save\nformat is Emacs Lisp code which calls the constructor for the saved\nobject.  For this reason, only slots which do not have an `:initarg'\nspecified will not be saved." :abstract t))) :slots [#s(cl-slot-descriptor :name file :initform unbound :type string :props ((:documentation . "The save file for this persistent object.\nThis must be a string, and must be specified when the new object is\ninstantiated."))) #s(cl-slot-descriptor :name teacher :initform unbound :type person :props nil) #s(cl-slot-descriptor :name students :initform (make-hash-table) :type t :props nil)] :index-table #<hash-table eq 3/65 0x14d45a5> :children nil :initarg-tuples ((:file . file) (:teacher . teacher) (:students . students)) :class-slots [#s(cl-slot-descriptor :name extension :initform ".eieio" :type string :props ((:documentation . "Extension of files saved by this object.\nEnables auto-choosing nice file names based on name."))) #s(cl-slot-descriptor :name file-header-line :initform ";; EIEIO PERSISTENT OBJECT" :type string :props ((:documentation . "Header line for the save file.\nThis is used with the `object-write' method."))) #s(cl-slot-descriptor :name do-backups :initform t :type boolean :props ((:documentation . "Saving this object should make backup files.\nSetting to nil will mean no backups are made.")))] :class-allocation-values [".eieio" ";; EIEIO PERSISTENT OBJECT" t] :default-object-cache #<classy classy> :options (:custom-groups nil)) teacher (person "person" :name "Jane"))
  eieio-persistent-convert-list-to-object((classy "classy" :file "classy-26.0.90.eieio" :teacher (person "person" :name "Jane") :students #<hash-table eql 1/65 0x237e271>))
  eieio-persistent-read("classy-26.0.90.eieio" classy t)
  (let* ((jane (make-instance 'person :name "Jane")) (bob (make-instance 'person :name "Bob")) (class (make-instance 'classy :teacher jane :file (concat "classy-" emacs-version ".eieio")))) (puthash "Bob" bob (slot-value class 'students)) (eieio-persistent-save class (concat "classy-" emacs-version ".eieio")) (eieio-persistent-read (concat "classy-" emacs-version ".eieio") 'classy t))
  (progn (let* ((jane (make-instance 'person :name "Jane")) (bob (make-instance 'person :name "Bob")) (class (make-instance 'classy :teacher jane :file (concat "classy-" emacs-version ".eieio")))) (puthash "Bob" bob (slot-value class 'students)) (eieio-persistent-save class (concat "classy-" emacs-version ".eieio")) (eieio-persistent-read (concat "classy-" emacs-version ".eieio") 'classy t)))
  eval((progn (let* ((jane (make-instance 'person :name "Jane")) (bob (make-instance 'person :name "Bob")) (class (make-instance 'classy :teacher jane :file (concat "classy-" emacs-version ".eieio")))) (puthash "Bob" bob (slot-value class 'students)) (eieio-persistent-save class (concat "classy-" emacs-version ".eieio")) (eieio-persistent-read (concat "classy-" emacs-version ".eieio") 'classy t))) t)
  elisp--eval-last-sexp(nil)
  eval-last-sexp(nil)
  funcall-interactively(eval-last-sexp nil)
  call-interactively(eval-last-sexp nil nil)
  command-execute(eval-last-sexp)
#+END_SRC


[-- Attachment #2: simple eieio persistence test --]
[-- Type: text/plain, Size: 711 bytes --]

;;; -*- lexical-binding: t -*-
(require 'eieio)
(require 'eieio-base)

(defclass person ()
  ((name :type string :initarg :name)))

(defclass classy (eieio-persistent)
  ((teacher
    :type person
    :initarg :teacher)
   (students
    :initarg :students :initform (make-hash-table))))

(let* ((jane (make-instance 'person :name "Jane"))
       (bob  (make-instance 'person :name "Bob"))
       (class (make-instance 'classy
			     :teacher jane
			     :file (concat "classy-" emacs-version ".eieio"))))
  (puthash "Bob" bob (slot-value class 'students))
  (eieio-persistent-save class (concat "classy-" emacs-version ".eieio"))
  (eieio-persistent-read (concat "classy-" emacs-version ".eieio") 'classy t))

[-- Attachment #3: Tests for eieio-persistent class --]
[-- Type: text/plain, Size: 9394 bytes --]

;;; eieio-test-persist.el --- Tests for eieio-persistent class

;; Copyright (C) 2011-2017 Free Software Foundation, Inc.

;; Author: Eric M. Ludlam <eric@siege-engine.com>

;; This file is part of GNU Emacs.

;; GNU Emacs is free software: you can redistribute it and/or modify
;; it under the terms of the GNU General Public License as published by
;; the Free Software Foundation, either version 3 of the License, or
;; (at your option) any later version.

;; GNU Emacs is distributed in the hope that it will be useful,
;; but WITHOUT ANY WARRANTY; without even the implied warranty of
;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
;; GNU General Public License for more details.

;; You should have received a copy of the GNU General Public License
;; along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.

;;; Commentary:
;;
;; The eieio-persistent base-class provides a vital service, that
;; could be used to accidentally load in malicious code.  As such,
;; something as simple as calling eval on the generated code can't be
;; used.  These tests exercises various flavors of data that might be
;; in a persistent object, and tries to save/load them.

;;; Code:
(require 'eieio)
(require 'eieio-base)
(require 'ert)

(defun eieio--attribute-to-initarg (class attribute)
  "In CLASS, convert the ATTRIBUTE into the corresponding init argument tag.
This is usually a symbol that starts with `:'."
  (let ((tuple (rassoc attribute (eieio--class-initarg-tuples class))))
    (if tuple
	(car tuple)
      nil)))

(defun hash-equal (hash1 hash2)
  "Compare two hash tables to see whether they are equal."
  (and (= (hash-table-count hash1)
          (hash-table-count hash2))
       (catch 'flag (maphash (lambda (x y)
                               (or (equal (gethash x hash2) y)
                                   (throw 'flag nil)))
                             hash1)
              (throw 'flag t))))

(defun persist-test-save-and-compare (original)
  "Compare the object ORIGINAL against the one read fromdisk."

  (eieio-persistent-save original)

  (let* ((file (oref original file))
	 (class (eieio-object-class original))
	 (fromdisk (eieio-persistent-read file class))
	 (cv (cl--find-class class))
	 (slots  (eieio--class-slots cv)))

    (unless (object-of-class-p fromdisk class)
      (error "Persistent class %S != original class %S"
	     (eieio-object-class fromdisk)
	     class))

    (dotimes (i (length slots))
      (let* ((slot (aref slots i))
             (oneslot (cl--slot-descriptor-name slot))
	     (origvalue (eieio-oref original oneslot))
	     (fromdiskvalue (eieio-oref fromdisk oneslot))
	     (initarg-p (eieio--attribute-to-initarg
                         (cl--find-class class) oneslot)))

	(if initarg-p
	    (unless
		(cond ((and (hash-table-p origvalue) (hash-table-p fromdiskvalue))
		       (hash-equal origvalue fromdiskvalue))
		      (t (equal origvalue fromdiskvalue)))
	      (error "Slot %S Original Val %S != Persistent Val %S"
		     oneslot origvalue fromdiskvalue))
	  ;; Else !initarg-p
	  (let ((origval (cl--slot-descriptor-initform slot))
		(diskval fromdiskvalue))
	    (unless
		(cond ((and (hash-table-p origval) (hash-table-p diskval))
		       (hash-equal origval diskval))
		      (t (equal origval diskval)))
	    (error "Slot %S Persistent Val %S != Default Value %S"
		   oneslot diskval origvalue))))
	))))

;;; Simple Case
;;
;; Simplest case is a mix of slots with and without initargs.

(defclass persist-simple (eieio-persistent)
  ((slot1 :initarg :slot1
	  :type symbol
	  :initform moose)
   (slot2 :initarg :slot2
	  :initform "foo")
   (slot3 :initform 2))
  "A Persistent object with two initializable slots, and one not.")

(ert-deftest eieio-test-persist-simple-1 ()
  (let ((persist-simple-1
	 (persist-simple "simple 1" :slot1 'goose :slot2 "testing"
			 :file (concat default-directory "test-ps1.pt"))))
    (should persist-simple-1)

    ;; When the slot w/out an initarg has not been changed
    (persist-test-save-and-compare persist-simple-1)

    ;; When the slot w/out an initarg HAS been changed
    (oset persist-simple-1 slot3 3)
    (persist-test-save-and-compare persist-simple-1)
    (delete-file (oref persist-simple-1 file))))

;;; Slot Writers
;;
;; Replica of the test in eieio-tests.el -

(defclass persist-:printer (eieio-persistent)
  ((slot1 :initarg :slot1
	  :initform 'moose
	  :printer PO-slot1-printer)
   (slot2 :initarg :slot2
	  :initform "foo"))
  "A Persistent object with two initializable slots.")

(defun PO-slot1-printer (slotvalue)
  "Print the slot value SLOTVALUE to stdout.
Assume SLOTVALUE is a symbol of some sort."
  (princ "'")
  (princ (symbol-name slotvalue))
  (princ " ;; RAN PRINTER")
  nil)

(ert-deftest eieio-test-persist-printer ()
  (let ((persist-:printer-1
	 (persist-:printer "persist" :slot1 'goose :slot2 "testing"
			   :file (concat default-directory "test-ps2.pt"))))
    (should persist-:printer-1)
    (persist-test-save-and-compare persist-:printer-1)

    (let* ((find-file-hook nil)
	   (tbuff (find-file-noselect "test-ps2.pt"))
	   )
      (condition-case nil
	  (unwind-protect
	      (with-current-buffer tbuff
		(goto-char (point-min))
		(re-search-forward "RAN PRINTER"))
	    (kill-buffer tbuff))
	(error "persist-:printer-1's Slot1 printer function didn't work.")))
    (delete-file (oref persist-:printer-1 file))))

;;; Slot with Object
;;
;; A slot that contains another object that isn't persistent
(defclass persist-not-persistent ()
  ((slot1 :initarg :slot1
	  :initform 1)
   (slot2 :initform 2))
  "Class for testing persistent saving of an object that isn't
persistent.  This class is instead used as a slot value in a
persistent class.")

(defclass persistent-with-objs-slot (eieio-persistent)
  ((pnp :initarg :pnp
	:type (or null persist-not-persistent)
	:initform nil))
  "Class for testing the saving of slots with objects in them.")

(ert-deftest eieio-test-non-persistent-as-slot ()
  (let ((persist-wos
	 (persistent-with-objs-slot
	  "persist wos 1"
	  :pnp (persist-not-persistent "pnp 1" :slot1 3)
	  :file (concat default-directory "test-ps3.pt"))))

    (persist-test-save-and-compare persist-wos)
    (delete-file (oref persist-wos file))))

;;; Slot with Object child of :type
;;
;; A slot that contains another object that isn't persistent
(defclass persist-not-persistent-subclass (persist-not-persistent)
  ((slot3 :initarg :slot1
	  :initform 1)
   (slot4 :initform 2))
  "Class for testing persistent saving of an object subclass that isn't
persistent.  This class is instead used as a slot value in a
persistent class.")

(defclass persistent-with-objs-slot-subs (eieio-persistent)
  ((pnp :initarg :pnp
	:type (or null persist-not-persistent)
	:initform nil))
  "Class for testing the saving of slots with objects in them.")

(ert-deftest eieio-test-non-persistent-as-slot-child ()
  (let ((persist-woss
	 (persistent-with-objs-slot-subs
	  "persist woss 1"
	  :pnp (persist-not-persistent-subclass "pnps 1" :slot1 3)
	  :file (concat default-directory "test-ps4.pt"))))

    (persist-test-save-and-compare persist-woss)
    (delete-file (oref persist-woss file))))

;; A slot that can contain one of two different classes, to exercise
;; the `or' slot type.

(defclass persistent-random-class ()
  ())

(defclass persistent-multiclass-slot (eieio-persistent)
  ((slot1 :initarg :slot1
          :type (or persistent-random-class null persist-not-persistent))
   (slot2 :initarg :slot2
          :type (or persist-not-persistent persist-random-class null))))

(ert-deftest eieio-test-multiple-class-slot ()
  (let ((persist
         (persistent-multiclass-slot "random string"
          :slot1 (persistent-random-class)
          :slot2 (persist-not-persistent)
          :file (concat default-directory "test-ps5.pt"))))
    (unwind-protect
        (persist-test-save-and-compare persist)
     (ignore-errors (delete-file (oref persist file))))))

;;; Slot with a list of Objects
;;
;; A slot that contains another object that isn't persistent
(defclass persistent-with-objs-list-slot (eieio-persistent)
  ((pnp :initarg :pnp
	:type (list-of persist-not-persistent)
	:initform nil))
  "Class for testing the saving of slots with objects in them.")

(ert-deftest eieio-test-slot-with-list-of-objects ()
  (let ((persist-wols
	 (persistent-with-objs-list-slot
	  "persist wols 1"
	  :pnp (list (persist-not-persistent "pnp 1" :slot1 3)
		     (persist-not-persistent "pnp 2" :slot1 4)
		     (persist-not-persistent "pnp 3" :slot1 5))
	  :file (concat default-directory "test-ps5.pt"))))

    (persist-test-save-and-compare persist-wols)
    (delete-file (oref persist-wols file))))

(defclass person ()
  ((name :type string :initarg :name)))

(defclass classy (eieio-persistent)
  ((teacher
    :type person
    :initarg :teacher)
   (students
    :initarg :students :initform (make-hash-table))))

(ert-deftest eieio-test-persist-hash-and-objects ()
  (let* ((jane (make-instance 'person :name "Jane"))
         (bob  (make-instance 'person :name "Bob"))
         (class (make-instance 'classy
			       :teacher jane
			       :file (concat default-directory "classy-" emacs-version ".eieio"))))
    (puthash "Bob" bob (slot-value class 'students))
    (persist-test-save-and-compare class)
    (delete-file (oref class file))))


;;; eieio-test-persist.el ends here

bug#29220: 26.0.90; eieio-persistent-read fail to restore saved object.

[Pierre Téchoueyres]

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


Of course with the good example and patch this will be more obvious.


[-- Attachment #2: Simple tests eieio-persistent class --]
[-- Type: text/plain, Size: 724 bytes --]

;;; -*- lexical-binding: t -*-
(require 'eieio)
(require 'eieio-base)

(defclass person ()
  ((name :type string :initarg :name)))

(defclass classy (eieio-persistent)
  ((teacher
    :type person
    :initarg :teacher)
   (students
    :initarg :students :initform (make-hash-table :test 'equal))))

(let* ((jane (make-instance 'person :name "Jane"))
       (bob  (make-instance 'person :name "Bob"))
       (class (make-instance 'classy
			     :teacher jane
			     :file (concat "classy-" emacs-version ".eieio"))))
  (puthash "Bob" bob (slot-value class 'students))
  (eieio-persistent-save class (concat "classy-" emacs-version ".eieio"))
  (eieio-persistent-read (concat "classy-" emacs-version ".eieio") 'classy t))

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: Tests for eieio-persistent class --]
[-- Type: text/x-patch, Size: 4207 bytes --]

From 26341ec498e4ffdea1a3c8173c3831c01a745bb9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pierre=20T=C3=A9choueyres?= <pierre.techoueyres@free.fr>
Date: Fri, 15 Dec 2017 21:42:21 +0100
Subject: [PATCH] Add test for eieio persistence checks

* test/lisp/emacs-lisp/eieio-tests/eieio-test-persist.el:
(hash-equal): New comparaison test for hash-tables.
(persist-test-save-and-compare): specialize test for hash-tables.
(eieio-test-persist-hash-and-objects): New test.
---
 .../emacs-lisp/eieio-tests/eieio-test-persist.el   | 58 +++++++++++++++++++---
 1 file changed, 50 insertions(+), 8 deletions(-)

diff --git a/test/lisp/emacs-lisp/eieio-tests/eieio-test-persist.el b/test/lisp/emacs-lisp/eieio-tests/eieio-test-persist.el
index 738711c9c8..cfc51e42a2 100644
--- a/test/lisp/emacs-lisp/eieio-tests/eieio-test-persist.el
+++ b/test/lisp/emacs-lisp/eieio-tests/eieio-test-persist.el
@@ -1,4 +1,4 @@
-;;; eieio-persist.el --- Tests for eieio-persistent class
+;;; eieio-test-persist.el --- Tests for eieio-persistent class
 
 ;; Copyright (C) 2011-2017 Free Software Foundation, Inc.
 
@@ -40,6 +40,17 @@ eieio--attribute-to-initarg
 	(car tuple)
       nil)))
 
+(defun hash-equal (hash1 hash2)
+  "Compare two hash tables to see whether they are equal."
+  (and (= (hash-table-count hash1)
+          (hash-table-count hash2))
+       (catch 'flag
+         (maphash (lambda (x y)
+                    (or (equal (gethash x hash2) y)
+                        (throw 'flag nil)))
+                  hash1)
+         (throw 'flag t))))
+
 (defun persist-test-save-and-compare (original)
   "Compare the object ORIGINAL against the one read fromdisk."
 
@@ -49,8 +60,8 @@ persist-test-save-and-compare
 	 (class (eieio-object-class original))
 	 (fromdisk (eieio-persistent-read file class))
 	 (cv (cl--find-class class))
-	 (slots  (eieio--class-slots cv))
-	 )
+	 (slots  (eieio--class-slots cv)))
+
     (unless (object-of-class-p fromdisk class)
       (error "Persistent class %S != original class %S"
 	     (eieio-object-class fromdisk)
@@ -62,17 +73,24 @@ persist-test-save-and-compare
 	     (origvalue (eieio-oref original oneslot))
 	     (fromdiskvalue (eieio-oref fromdisk oneslot))
 	     (initarg-p (eieio--attribute-to-initarg
-                         (cl--find-class class) oneslot))
-	     )
+                         (cl--find-class class) oneslot)))
 
 	(if initarg-p
-	    (unless (equal origvalue fromdiskvalue)
+	    (unless
+		(cond ((and (hash-table-p origvalue) (hash-table-p fromdiskvalue))
+		       (hash-equal origvalue fromdiskvalue))
+		      (t (equal origvalue fromdiskvalue)))
 	      (error "Slot %S Original Val %S != Persistent Val %S"
 		     oneslot origvalue fromdiskvalue))
 	  ;; Else !initarg-p
-	  (unless (equal (cl--slot-descriptor-initform slot) fromdiskvalue)
+	  (let ((origval (cl--slot-descriptor-initform slot))
+		(diskval fromdiskvalue))
+	    (unless
+		(cond ((and (hash-table-p origval) (hash-table-p diskval))
+		       (hash-equal origval diskval))
+		      (t (equal origval diskval)))
 	    (error "Slot %S Persistent Val %S != Default Value %S"
-		   oneslot fromdiskvalue (cl--slot-descriptor-initform slot))))
+		   oneslot diskval origvalue))))
 	))))
 
 ;;; Simple Case
@@ -238,4 +256,28 @@ persistent-with-objs-list-slot
     (persist-test-save-and-compare persist-wols)
     (delete-file (oref persist-wols file))))
 
+(defclass person ()
+  ((name :type string :initarg :name)))
+
+(defclass classy (eieio-persistent)
+  ((teacher
+    :type person
+    :initarg :teacher)
+   (students
+    :initarg :students :initform (make-hash-table :test 'equal))))
+
+;;; Slot with a hash-table another with an non persistent Object
+;;
+;; see bug#29220
+(ert-deftest eieio-test-persist-hash-and-objects ()
+  (let* ((jane (make-instance 'person :name "Jane"))
+         (bob  (make-instance 'person :name "Bob"))
+         (class (make-instance 'classy
+			       :teacher jane
+			       :file (concat default-directory "classy-" emacs-version ".eieio"))))
+    (puthash "Bob" bob (slot-value class 'students))
+    (persist-test-save-and-compare class)
+    (delete-file (oref class file))))
+
+
 ;;; eieio-test-persist.el ends here
-- 
2.14.3

bug#29220: 26.0.90; eieio-persistent-read fail to restore saved object.

[Eric Abrahamsen]

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


On 12/15/17 23:26 PM, Pierre Téchoueyres wrote:
> Of course with the good example and patch this will be more obvious.

Oh bother, this is my fault, in c59ddb2120. The attached diff fixes
this, and I guess should be put in.

In #29541 I was hoping to reduce some of this complexity and make the
code easier to reason about, but those changes would go into master, and
this needs to be fixed in 26.

More tests are definitely in order!


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: quickfix.diff --]
[-- Type: text/x-patch, Size: 579 bytes --]

diff --git a/lisp/emacs-lisp/eieio-base.el b/lisp/emacs-lisp/eieio-base.el
index 0241f27395..a37cf7a1b3 100644
--- a/lisp/emacs-lisp/eieio-base.el
+++ b/lisp/emacs-lisp/eieio-base.el
@@ -349,7 +349,7 @@ eieio-persistent-validate/fix-slot-value
                        (seq-some
                         (lambda (elt)
                           (child-of-class-p (car proposed-value) elt))
-                        classtype))
+                        (if (consp classtype) classtype (list classtype))))
 		  (eieio-persistent-convert-list-to-object
 		   proposed-value))
 		 (t

bug#29220: 26.0.90; eieio-persistent-read fail to restore saved object.

[Eric Abrahamsen]

On 12/15/17 23:26 PM, Pierre Téchoueyres wrote:
> Of course with the good example and patch this will be more obvious.

And there's another problem here, manifesting in the Gnus registry.
Here's the issue:

(setq test-hash (make-hash-table :test #'equal))

(puthash "<msg@id>" '((sender "me")) test-hash)
(gethash "<msg@id>" test-hash)

(let (eval-expression-print-length)
  (prin1 test-hash))

=> Data is correct: ("<msg@id>" ((sender "me")))

(let (eval-expression-print-length)
  (eieio-override-prin1 test-hash))

=> Data is incorrect: ("<msg@id>" (quote ((sender "me"))))

The basic problem is that the code is really only set up to do
single-layer reading/writing, and nesting the process runs into
difficulties.

Hash table values are re-written with:

(maphash
   (lambda (key val)
     (setf (gethash key copy)
	   (read
	    (with-output-to-string
	      (eieio-override-prin1 val)))))
   copy)

If VAL is a list, it ends up in `eieio-list-prin1', which wraps it in a
call to `quote'.

The read process looks for these calls to `quote' and removes them, but
it only does it for top-level slot values. If the data is inside
something else (hash table values, in this case), the `quote' calls
remain in the code.

I'd really like to simplify this whole process (it should just walk the
whole tree and convert objects, nothing else -- why write quotes only to
delete them on read?) but in the meantime, I hope we can get a minimum
viable fix in for 26.

My feeling is that we could just remove the addition of the quote
character in `eieio-list-prin1'. The eieio-persistent load process only
`read's, doesn't `eval', so in theory the quote is superfluous. The
check for its presence would stay in.

Any thoughts on this? Obviously I will test better than I have been...

Eric

bug#29220: 26.0.90; eieio-persistent-read fail to restore saved object.

[Stefan Monnier]

> My feeling is that we could just remove the addition of the quote
> character in `eieio-list-prin1'.

Maybe it's needed to distinguish the printed value of "a list that
starts with the symbol `my-class`" from "an object of class` my-class`"?


        Stefan

bug#29220: 26.0.90; eieio-persistent-read fail to restore saved object.

[Eric Abrahamsen]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> My feeling is that we could just remove the addition of the quote
>> character in `eieio-list-prin1'.
>
> Maybe it's needed to distinguish the printed value of "a list that
> starts with the symbol `my-class`" from "an object of class` my-class`"?

Yup.

Clearly this has the potential to turn into a bigger mess. I just pushed
a branch to the repo called "fix/eieio-persistent", based off emacs-26.
I modified Pierre's patch to add more (failing) tests, and then added
another commit which fixes the easy error in handling the return value
of `eieio-persistent-slot-type-is-class-p'.

My feeling is that the process *ought* to be able to handle all the
failing test cases. But even if the issue of quoting is fixed, not all
the tests will pass, as the write/restore process doesn't descend into
lists.

As for the quoting thing, my current idea is to not add quotes to lists
when writing, then strip quotes on restore, if the car of the list is a
valid class symbol then try to restore an object, but wrap in
`condition-case' and return the plain list if an error is raised.

Seems pretty unlikely that someone would write a list that accidentally
happens to restore to a valid object.

Eric

bug#29220: 26.0.90; eieio-persistent-read fail to restore saved object.

[Stefan Monnier]

> As for the quoting thing, my current idea is to not add quotes to lists
> when writing, then strip quotes on restore, if the car of the list is a
> valid class symbol then try to restore an object, but wrap in
> `condition-case' and return the plain list if an error is raised.

I recommend you ask the opinion of Eric M. Ludlam <zappo@gnu.org>
(the original author of that code).

> Seems pretty unlikely that someone would write a list that accidentally
> happens to restore to a valid object.

It should(!) be easy to design the system such that we don't need to
make such assumptions.


        Stefan

bug#29220: 26.0.90; eieio-persistent-read fail to restore saved object.

[Eric Abrahamsen]

On 12/20/17 15:54 PM, Stefan Monnier wrote:
>> As for the quoting thing, my current idea is to not add quotes to lists
>> when writing, then strip quotes on restore, if the car of the list is a
>> valid class symbol then try to restore an object, but wrap in
>> `condition-case' and return the plain list if an error is raised.
>
> I recommend you ask the opinion of Eric M. Ludlam <zappo@gnu.org>
> (the original author of that code).

Okay!

Eric, I'm dragging you into this because we're having a little trouble
with the eieio persistence functionality. Sorry...

In Emacs 26 objects are implemented using records, which (long story
short) means that their representations from `prin1' are no longer
reconstructable using `read', which is causing problems for
eieio-persistent.

The current persistence read/write process basically only handles
top-level slot values. It looks at the value, does one of the following:

1. Writes an object using `object-write'
2. Writes a list of objects using a "list" plus `object-write'
3. Quotes a list and prints it
4. Prints value directly

If objects are inside lists, hash tables, or vectors (or anything else),
they are not written using `object-write'. This used to not be a problem
because their `prin1' representation was still readable.

This is no longer the case, which leads to failure for libraries that
put objects inside other data structures.

In this bug report I tried to do a simple fix for pcache, by looking for
objects inside hash tables and vectors, and using `object-write' for
them, too. That works (though it only goes one layer deeper), but it
caused new bugs in the Gnus registry, because now lists inside hash
tables are quoted, but not un-quoted during the restore process.

The persistence process is fundamentally only set up to do single-layer
processing of objects and their slot values. Probably, people are going
to continue to use eieio-persistent for more complex things (it's only
luck that I didn't run into trouble with EBDB), and I think
eieio-persistent should be able to handle whatever gets thrown at it.

My feeling is that both the write and restore process should walk the
entire tree of whatever object is being written. The write process
should turn objects into list representations, nothing more. The restore
process should restore objects, and strip strings of properties (I've
got the outline of this in bug#29541), and nothing more. If the restore
process was destructive, it would save some consing.

Three items for consideration:

1. What do you think about the above proposal? Have you had some ideas in
   this direction already?
2. Stefan pointed out that, if we don't quote lists, there's a potential
   ambiguity during restore time between lists that are lists, and lists
   that represent an object. Do you have any thoughts about this?
3. Emacs 26 is looming, and our current solutions either break pcache,
   or break the Gnus registry. We might want to do a stop-gap for Emacs
   26, and a more complete fix for master/27.

Sorry if I've over-explained, I'm using this opportunity to continue
thinking about the problem.

Eric

bug#29220: 26.0.90; eieio-persistent-read fail to restore saved object.

[Eric Ludlam]

On 12/20/2017 09:21 PM, Eric Abrahamsen wrote:
> On 12/20/17 15:54 PM, Stefan Monnier wrote:
>>> As for the quoting thing, my current idea is to not add quotes to lists
>>> when writing, then strip quotes on restore, if the car of the list is a
>>> valid class symbol then try to restore an object, but wrap in
>>> `condition-case' and return the plain list if an error is raised.
>> I recommend you ask the opinion of Eric M. Ludlam <zappo@gnu.org>
>> (the original author of that code).
> Okay!
>
> Eric, I'm dragging you into this because we're having a little trouble
> with the eieio persistence functionality. Sorry...
>
> In Emacs 26 objects are implemented using records, which (long story
> short) means that their representations from `prin1' are no longer
> reconstructable using `read', which is causing problems for
> eieio-persistent.
Hey, thanks for checking in.

I know a bunch of eieio was re-written to be faster and better, but I 
haven't been keeping up with the latest version.  For mysterious 
reasons, Ubuntu is still on Emacs 24 so I'm still on the old stuff.
> The current persistence read/write process basically only handles
> top-level slot values. It looks at the value, does one of the following:
>
> 1. Writes an object using `object-write'
> 2. Writes a list of objects using a "list" plus `object-write'
> 3. Quotes a list and prints it
> 4. Prints value directly
>
> If objects are inside lists, hash tables, or vectors (or anything else),
> they are not written using `object-write'. This used to not be a problem
> because their `prin1' representation was still readable.
>
> This is no longer the case, which leads to failure for libraries that
> put objects inside other data structures.

The original purpose of object-write on eieio objects was to prevent
circular references which kept popping up in EDE and SemanticDB. The first
version just dumped the old raw vector.  That's one of the reasons why it
doesn't try to be pedantic about carefully going down through every list.
It only had to deal with EDE and Semantic classes.

> My feeling is that both the write and restore process should walk the
> entire tree of whatever object is being written. The write process
> should turn objects into list representations, nothing more. The restore
> process should restore objects, and strip strings of properties (I've
> got the outline of this in bug#29541), and nothing more. If the restore
> process was destructive, it would save some consing.
I think that makes sense.  I was definitely taking short-cuts in letting
lists write themselves b/c I was lazy.  Based on what you describe, it
probably makes sense to walk entire structures and write each step.

It might be even better if that was a core Emacs feature, and not something
specific to a simple eieio utility base class.  :)

> Three items for consideration:
>
> 1. What do you think about the above proposal? Have you had some ideas in
>     this direction already?
> 2. Stefan pointed out that, if we don't quote lists, there's a potential
>     ambiguity during restore time between lists that are lists, and lists
>     that represent an object. Do you have any thoughts about this?
Objects, as a general rule, need to execute code of some sort to 
instantiate,
link themselves into a system (like semanticdb), or generate transient data
for slots that are derived (and not saved).

Thus, each layer in the save file needs some way to mark themselves as 
to what
they are so the restore process can either instantiate or just use the 
raw data.

 From the quote / not quote point of view, you could use any kind of 
keys you
want to indicate what is at each layer.

For example, you might say that any list that starts with a class symbol 
is an
object and everything else is data.  Of course, then you can't have a 
list of
class symbols, and definitely need to avoid having classes with simple 
names.

I used lists that could be read with 'read', but you could use xml or 
all sorts of
other save formats to ensure you know exactly what's going on.  I have a 
vague
recollection of writing an xml export for eieio, but I don't think I 
used it for
anything.
> 3. Emacs 26 is looming, and our current solutions either break pcache,
>     or break the Gnus registry. We might want to do a stop-gap for Emacs
>     26, and a more complete fix for master/27.

Makes sense to me.

Don't forget that objects can execute code in their 'object-write' to 
convert
an inconvenient data structure into something simple, and in the constructor
convert it back again.

For example, have a slot with the hash table be non-savable (no :initarg).
In the object-write, convert the hash table into a simple flat list on
a different slot with an :initarg.

On load, copy the content of the :initarg slot back into your hash table.

In this way, you may be able to have the 2 examples avoid having any 
problems
with the current system in E26.

Not that I don't know what the real issues are.  I'm just tossing some 
options out
there.

Good Luck
Eric

bug#29220: 26.0.90; eieio-persistent-read fail to restore saved object.

[Eric Abrahamsen]

On 12/23/17 21:18 PM, Eric Ludlam wrote:
> On 12/20/2017 09:21 PM, Eric Abrahamsen wrote:
>> On 12/20/17 15:54 PM, Stefan Monnier wrote:
>>>> As for the quoting thing, my current idea is to not add quotes to lists
>>>> when writing, then strip quotes on restore, if the car of the list is a
>>>> valid class symbol then try to restore an object, but wrap in
>>>> `condition-case' and return the plain list if an error is raised.
>>> I recommend you ask the opinion of Eric M. Ludlam <zappo@gnu.org>
>>> (the original author of that code).
>> Okay!
>>
>> Eric, I'm dragging you into this because we're having a little trouble
>> with the eieio persistence functionality. Sorry...
>>
>> In Emacs 26 objects are implemented using records, which (long story
>> short) means that their representations from `prin1' are no longer
>> reconstructable using `read', which is causing problems for
>> eieio-persistent.
> Hey, thanks for checking in.
>
> I know a bunch of eieio was re-written to be faster and better, but I
> haven't been keeping up with the latest version.  For mysterious
> reasons, Ubuntu is still on Emacs 24 so I'm still on the old stuff.
>> The current persistence read/write process basically only handles
>> top-level slot values. It looks at the value, does one of the following:
>>
>> 1. Writes an object using `object-write'
>> 2. Writes a list of objects using a "list" plus `object-write'
>> 3. Quotes a list and prints it
>> 4. Prints value directly
>>
>> If objects are inside lists, hash tables, or vectors (or anything else),
>> they are not written using `object-write'. This used to not be a problem
>> because their `prin1' representation was still readable.
>>
>> This is no longer the case, which leads to failure for libraries that
>> put objects inside other data structures.
>
> The original purpose of object-write on eieio objects was to prevent
> circular references which kept popping up in EDE and SemanticDB. The first
> version just dumped the old raw vector.  That's one of the reasons why it
> doesn't try to be pedantic about carefully going down through every list.
> It only had to deal with EDE and Semantic classes.

Thanks for the response! It's good to have a bit of background.

Would you mind pointing me in the direction of some tests or other code
that I can use to make sure the temporary fix works for CEDET and
family? It seems likely that what will happen for Emacs 26 is that I'll
do a limited set of fixes to ensure that no major packages break, and
then try to come up with a more comprehensive solution for Emacs 27.
Right now I've got tests aimed at pcache and the Gnus registry, but I'd
like to make sure EDE and Semantic work correctly, as well.

>> My feeling is that both the write and restore process should walk the
>> entire tree of whatever object is being written. The write process
>> should turn objects into list representations, nothing more. The restore
>> process should restore objects, and strip strings of properties (I've
>> got the outline of this in bug#29541), and nothing more. If the restore
>> process was destructive, it would save some consing.
> I think that makes sense.  I was definitely taking short-cuts in letting
> lists write themselves b/c I was lazy.  Based on what you describe, it
> probably makes sense to walk entire structures and write each step.
>
> It might be even better if that was a core Emacs feature, and not something
> specific to a simple eieio utility base class.  :)

Coincidentally, John Wiegley posted a possible generic structure-walking
function (called "traverse") to this list not too long ago.

[...]

> For example, you might say that any list that starts with a class
> symbol is an
> object and everything else is data.  Of course, then you can't have a
> list of
> class symbols, and definitely need to avoid having classes with simple
> names.

[...]

> Don't forget that objects can execute code in their 'object-write' to
> convert
> an inconvenient data structure into something simple, and in the constructor
> convert it back again.
>
> For example, have a slot with the hash table be non-savable (no :initarg).
> In the object-write, convert the hash table into a simple flat list on
> a different slot with an :initarg.
>
> On load, copy the content of the :initarg slot back into your hash table.
>
> In this way, you may be able to have the 2 examples avoid having any
> problems
> with the current system in E26.
>
> Not that I don't know what the real issues are.  I'm just tossing some
> options out
> there.

Okay, thanks very much for these responses. I've got some ideas about
the full fix, and will work on those in a separate bug report.

Thanks again,
Eric

bug#29220: 26.0.90; eieio-persistent-read fail to restore saved object.

[Pierre Téchoueyres]

Hi Eric,

pierre.techoueyres@free.fr (Pierre Téchoueyres) writes:

> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
>
>> On 12/05/17 14:02 PM, Stefan Monnier wrote:
>>>> Would it be too "heavy" to just copy the object and modify the copy?
>>>
>>> No, that's fine as well.
>>
>> Okay, the attached appears to work just fine. Pierre's recipe passes,
>> as do all the tests in eieio-test-persist. Pierre, maybe you could eval
>> this quickly and make sure that pcache works correctly again?
>
> Yes all my receipes tests are running fine. Many thanks.
> I've tested also with the unicode-fonts package
> (http://github.com/rolandwalker/unicode-fonts) from melpa.
> Everything seem to work fine.
>
>>
>> If this is okay, it's going into emacs-26, right?
>>
Sorry if I missed it but I think you didn't push this fix into the
emacs-26 branch, did you ?

bug#29220: 26.0.90; eieio-persistent-read fail to restore saved object.

[Eric Abrahamsen]

pierre.techoueyres@free.fr (Pierre Téchoueyres) writes:

> Hi Eric,
>
> pierre.techoueyres@free.fr (Pierre Téchoueyres) writes:
>
>> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
>>
>>> On 12/05/17 14:02 PM, Stefan Monnier wrote:
>>>>> Would it be too "heavy" to just copy the object and modify the copy?
>>>>
>>>> No, that's fine as well.
>>>
>>> Okay, the attached appears to work just fine. Pierre's recipe passes,
>>> as do all the tests in eieio-test-persist. Pierre, maybe you could eval
>>> this quickly and make sure that pcache works correctly again?
>>
>> Yes all my receipes tests are running fine. Many thanks.
>> I've tested also with the unicode-fonts package
>> (http://github.com/rolandwalker/unicode-fonts) from melpa.
>> Everything seem to work fine.
>>
>>>
>>> If this is okay, it's going into emacs-26, right?
>>>
> Sorry if I missed it but I think you didn't push this fix into the
> emacs-26 branch, did you ?

No, I haven't! Work has risen up and swallowed me; I'm also a little
worried about the potential for creating new screw-ups here. I'll try to
finish testing and the fixup branch this weekend...

bug#29220: 26.0.90; eieio-persistent-read fail to restore saved object.

[Pierre Téchoueyres]

Hello Eric,
Any news on this fix ?

Eric Abrahamsen <eric@ericabrahamsen.net> writes:

> On 12/15/17 23:26 PM, Pierre Téchoueyres wrote:
>> Of course with the good example and patch this will be more obvious.
>
> Oh bother, this is my fault, in c59ddb2120. The attached diff fixes
> this, and I guess should be put in.
>
> In #29541 I was hoping to reduce some of this complexity and make the
> code easier to reason about, but those changes would go into master, and
> this needs to be fixed in 26.
>
> More tests are definitely in order!
>
> diff --git a/lisp/emacs-lisp/eieio-base.el b/lisp/emacs-lisp/eieio-base.el
> index 0241f27395..a37cf7a1b3 100644
> --- a/lisp/emacs-lisp/eieio-base.el
> +++ b/lisp/emacs-lisp/eieio-base.el
> @@ -349,7 +349,7 @@ eieio-persistent-validate/fix-slot-value
>                         (seq-some
>                          (lambda (elt)
>                            (child-of-class-p (car proposed-value) elt))
> -                        classtype))
> +                        (if (consp classtype) classtype (list classtype))))
>  		  (eieio-persistent-convert-list-to-object
>  		   proposed-value))
>  		 (t
>

bug#29220: 26.0.90; eieio-persistent-read fail to restore saved object.

[Eric Abrahamsen]

pierre.techoueyres@free.fr (Pierre Téchoueyres) writes:

> Hello Eric,
> Any news on this fix ?

Hey, sorry again this is taking so long. I wanted to run CEDET's test
suite, as that seems to be the main library that relies on
eieio-persistent, and I wanted to make sure it passed with these
changes. I can't get CEDET to build on my machine, however (apparently
Arch has a too-new version of ImageMagick, and CEDET won't build against
it), and I haven't had any luck just faking it and running the tests
themselves -- everything's too dependent on a successful build.

I've only got two Arch machines to play with -- if anyone could
successfully build and test CEDET using the fix/eieio-persistent branch,
that would be very helpful...

Eric

bug#29220: 26.0.90; eieio-persistent-read fail to restore saved object.

[Pierre Téchoueyres]

Eric Abrahamsen <eric@ericabrahamsen.net> writes:

> pierre.techoueyres@free.fr (Pierre Téchoueyres) writes:
>
>> Hello Eric,
>> Any news on this fix ?
>
> Hey, sorry again this is taking so long. I wanted to run CEDET's test
> suite, as that seems to be the main library that relies on
> eieio-persistent, and I wanted to make sure it passed with these
> changes. I can't get CEDET to build on my machine, however (apparently
> Arch has a too-new version of ImageMagick, and CEDET won't build against
> it), and I haven't had any luck just faking it and running the tests
> themselves -- everything's too dependent on a successful build.
>
> I've only got two Arch machines to play with -- if anyone could
> successfully build and test CEDET using the fix/eieio-persistent branch,
> that would be very helpful...
>

I've been able to run the "itest-batch" and "utest-batch" from the CEDET
git repository (http://git.code.sf.net/p/cedet/git), but I don't know
how to interpret the results. Whatever the emacs version I use (25.3 or
26.0.91 + fix), I get errors. So I don't know what to check for the
fix/eieio-persistent branch.

> Eric
Pierre

bug#29220: 26.0.90; eieio-persistent-read fail to restore saved object.

[Eric Abrahamsen]

On 02/25/18 00:21 AM, Pierre Téchoueyres wrote:
> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
>
>> pierre.techoueyres@free.fr (Pierre Téchoueyres) writes:
>>
>>> Hello Eric,
>>> Any news on this fix ?
>>
>> Hey, sorry again this is taking so long. I wanted to run CEDET's test
>> suite, as that seems to be the main library that relies on
>> eieio-persistent, and I wanted to make sure it passed with these
>> changes. I can't get CEDET to build on my machine, however (apparently
>> Arch has a too-new version of ImageMagick, and CEDET won't build against
>> it), and I haven't had any luck just faking it and running the tests
>> themselves -- everything's too dependent on a successful build.
>>
>> I've only got two Arch machines to play with -- if anyone could
>> successfully build and test CEDET using the fix/eieio-persistent branch,
>> that would be very helpful...
>>
>
> I've been able to run the "itest-batch" and "utest-batch" from the CEDET
> git repository (http://git.code.sf.net/p/cedet/git), but I don't know
> how to interpret the results. Whatever the emacs version I use (25.3 or
> 26.0.91 + fix), I get errors. So I don't know what to check for the
> fix/eieio-persistent branch.

Bah, this is annoyingly difficult -- thank you for trying it out. I
suppose one terrible heuristic would be if the errors were different
between emacs-25, emacs-26, and fix/eieio-persistent. Would you mind
sending me the error output, in case anything jumps out at me?

Thanks again,
Eric

bug#29220: 26.0.90; eieio-persistent-read fail to restore saved object.

[Pierre Téchoueyres]

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

Eric Abrahamsen <eric@ericabrahamsen.net> writes:

> On 02/25/18 00:21 AM, Pierre Téchoueyres wrote:
>> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
>>
>>> pierre.techoueyres@free.fr (Pierre Téchoueyres) writes:
>>>
>>>> Hello Eric,
>>>> Any news on this fix ?
>>>
>>> Hey, sorry again this is taking so long. I wanted to run CEDET's test
>>> suite, as that seems to be the main library that relies on
>>> eieio-persistent, and I wanted to make sure it passed with these
>>> changes. I can't get CEDET to build on my machine, however (apparently
>>> Arch has a too-new version of ImageMagick, and CEDET won't build against
>>> it), and I haven't had any luck just faking it and running the tests
>>> themselves -- everything's too dependent on a successful build.
>>>
>>> I've only got two Arch machines to play with -- if anyone could
>>> successfully build and test CEDET using the fix/eieio-persistent branch,
>>> that would be very helpful...
>>>
>>
>> I've been able to run the "itest-batch" and "utest-batch" from the CEDET
>> git repository (http://git.code.sf.net/p/cedet/git), but I don't know
>> how to interpret the results. Whatever the emacs version I use (25.3 or
>> 26.0.91 + fix), I get errors. So I don't know what to check for the
>> fix/eieio-persistent branch.
>
> Bah, this is annoyingly difficult -- thank you for trying it out. I
> suppose one terrible heuristic would be if the errors were different
> between emacs-25, emacs-26, and fix/eieio-persistent. Would you mind
> sending me the error output, in case anything jumps out at me?
>
Hope this could help you.


[-- Attachment #2: itest-batch-25.3.log --]
[-- Type: application/x-xz, Size: 6472 bytes --]

[-- Attachment #3: utest-batch-25.3.log --]
[-- Type: application/x-xz, Size: 1292 bytes --]

[-- Attachment #4: itest-batch-26-eieio.log --]
[-- Type: application/x-xz, Size: 5756 bytes --]

[-- Attachment #5: utest-batch-26-eieio.log --]
[-- Type: application/x-xz, Size: 1328 bytes --]

bug#29220: 26.0.90; eieio-persistent-read fail to restore saved object.

[Eric Abrahamsen]

pierre.techoueyres@free.fr (Pierre Téchoueyres) writes:

> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
>
>> On 02/25/18 00:21 AM, Pierre Téchoueyres wrote:
>>> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
>>>
>>>> pierre.techoueyres@free.fr (Pierre Téchoueyres) writes:
>>>>
>>>>> Hello Eric,
>>>>> Any news on this fix ?
>>>>
>>>> Hey, sorry again this is taking so long. I wanted to run CEDET's test
>>>> suite, as that seems to be the main library that relies on
>>>> eieio-persistent, and I wanted to make sure it passed with these
>>>> changes. I can't get CEDET to build on my machine, however (apparently
>>>> Arch has a too-new version of ImageMagick, and CEDET won't build against
>>>> it), and I haven't had any luck just faking it and running the tests
>>>> themselves -- everything's too dependent on a successful build.
>>>>
>>>> I've only got two Arch machines to play with -- if anyone could
>>>> successfully build and test CEDET using the fix/eieio-persistent branch,
>>>> that would be very helpful...
>>>>
>>>
>>> I've been able to run the "itest-batch" and "utest-batch" from the CEDET
>>> git repository (http://git.code.sf.net/p/cedet/git), but I don't know
>>> how to interpret the results. Whatever the emacs version I use (25.3 or
>>> 26.0.91 + fix), I get errors. So I don't know what to check for the
>>> fix/eieio-persistent branch.
>>
>> Bah, this is annoyingly difficult -- thank you for trying it out. I
>> suppose one terrible heuristic would be if the errors were different
>> between emacs-25, emacs-26, and fix/eieio-persistent. Would you mind
>> sending me the error output, in case anything jumps out at me?
>>
> Hope this could help you.

Thanks. They all end in "Corrupt object on disk", which is an
eieio-persist problem. I'll keep working on this.

bug#29220: 26.0.90; eieio-persistent-read fail to restore saved object.

[Noam Postavsky]

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

Eric Abrahamsen <eric@ericabrahamsen.net> writes:

>>> Bah, this is annoyingly difficult -- thank you for trying it out. I
>>> suppose one terrible heuristic would be if the errors were different
>>> between emacs-25, emacs-26, and fix/eieio-persistent. Would you mind
>>> sending me the error output, in case anything jumps out at me?
>>>
>> Hope this could help you.
>
> Thanks. They all end in "Corrupt object on disk", which is an
> eieio-persist problem. I'll keep working on this.

So, I think it would be nice to have this fixed for 26.3.  It sounds
like the fix you're working on is too risky to have on a release branch
regardless of its current status, so here's a patch which makes eieio
objects use symbols as type tags, when eieio-backward-compatibility is
non-nil.  Folks who want the record-with-circular-references as type
tags can still get that by setting eieio-backward-compatibility to nil.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch --]
[-- Type: text/x-diff, Size: 4273 bytes --]

From f6cc4509073da4ff23a4227d48b33108b1e96828 Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
Date: Mon, 27 May 2019 19:05:56 -0400
Subject: [PATCH] Use plain symbols for eieio type descriptors (Bug#29220)

Since Emacs 26, eieio objects use a class record (with circular
references) as the type descriptor of the object record.  This causes
problems when reading back an object from a string, because the class
record is not `eq' to the canonical one (which means that read objects
don't satisfy the foo-p predicate).
* lisp/emacs-lisp/eieio.el (make-instance): As a (partial) fix, set
the record's type descriptor to a plain symbol for the type descriptor
when eieio-backward-compatibility is non-nil (the default).
* lisp/emacs-lisp/eieio-core.el (eieio--object-class): Call
eieio--class-object on the type tag when eieio-backward-compatibility
is non-nil.
(eieio-object-p): Use eieio--object-class instead of
eieio--object-class-tag.
* test/lisp/emacs-lisp/eieio-tests/eieio-test-persist.el
(eieio-test-persist-hash-and-vector)
(eieio-test-persist-interior-lists): `persist-test-save-and-compare'
no longer throws an error.
---
 lisp/emacs-lisp/eieio-core.el                          | 11 +++++++----
 lisp/emacs-lisp/eieio.el                               |  3 +++
 test/lisp/emacs-lisp/eieio-tests/eieio-test-persist.el |  6 ++----
 3 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/lisp/emacs-lisp/eieio-core.el b/lisp/emacs-lisp/eieio-core.el
index f879a3999f..4d55ed6e1d 100644
--- a/lisp/emacs-lisp/eieio-core.el
+++ b/lisp/emacs-lisp/eieio-core.el
@@ -117,9 +117,6 @@ (eval-and-compile
 (defsubst eieio--object-class-tag (obj)
   (aref obj 0))
 
-(defsubst eieio--object-class (obj)
-  (eieio--object-class-tag obj))
-
 \f
 ;;; Important macros used internally in eieio.
 
@@ -132,6 +129,12 @@ (defsubst eieio--class-object (class)
       (or (cl--find-class class) class)
     class))
 
+(defsubst eieio--object-class (obj)
+  (let ((tag (eieio--object-class-tag obj)))
+    (if eieio-backward-compatibility
+        (eieio--class-object tag)
+      tag)))
+
 (defun class-p (x)
   "Return non-nil if X is a valid class vector.
 X can also be is a symbol."
@@ -163,7 +166,7 @@ (defsubst eieio--class-option (class option)
 (defun eieio-object-p (obj)
   "Return non-nil if OBJ is an EIEIO object."
   (and (recordp obj)
-       (eieio--class-p (eieio--object-class-tag obj))))
+       (eieio--class-p (eieio--object-class obj))))
 
 (define-obsolete-function-alias 'object-p 'eieio-object-p "25.1")
 
diff --git a/lisp/emacs-lisp/eieio.el b/lisp/emacs-lisp/eieio.el
index 38436d1f94..864ac2616b 100644
--- a/lisp/emacs-lisp/eieio.el
+++ b/lisp/emacs-lisp/eieio.el
@@ -710,6 +710,9 @@ (cl-defmethod make-instance
     ;; Call the initialize method on the new object with the slots
     ;; that were passed down to us.
     (initialize-instance new-object slots)
+    (when eieio-backward-compatibility
+      ;; Use symbol as type descriptor, for backwards compatibility.
+      (aset new-object 0 class))
     ;; Return the created object.
     new-object))
 
diff --git a/test/lisp/emacs-lisp/eieio-tests/eieio-test-persist.el b/test/lisp/emacs-lisp/eieio-tests/eieio-test-persist.el
index dfaa031844..f8a0aa30dd 100644
--- a/test/lisp/emacs-lisp/eieio-tests/eieio-test-persist.el
+++ b/test/lisp/emacs-lisp/eieio-tests/eieio-test-persist.el
@@ -297,8 +297,7 @@ (ert-deftest eieio-test-persist-hash-and-vector ()
     (aset (car (slot-value class 'janitors)) 1 hans)
     (aset (nth 1 (slot-value class 'janitors)) 1 dierdre)
     (unwind-protect
-        ;; FIXME: This should not error.
-        (should-error (persist-test-save-and-compare class))
+        (persist-test-save-and-compare class)
       (delete-file (oref class file)))))
 
 ;; Extra quotation of lists inside other objects (Gnus registry), also
@@ -335,8 +334,7 @@ (ert-deftest eieio-test-persist-interior-lists ()
     (setf (nth 2 (cadar alst)) john
           (nth 2 (cadadr alst)) alexie)
     (unwind-protect
-        ;; FIXME: Should not error.
-        (should-error (persist-test-save-and-compare thing))
+        (persist-test-save-and-compare thing)
       (delete-file (slot-value thing 'file)))))
 
 ;;; eieio-test-persist.el ends here
-- 
2.11.0

bug#29220: 26.0.90; eieio-persistent-read fail to restore saved object.

[Eric Abrahamsen]

On 05/27/19 19:36 PM, Noam Postavsky wrote:
> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
>
>>>> Bah, this is annoyingly difficult -- thank you for trying it out. I
>>>> suppose one terrible heuristic would be if the errors were different
>>>> between emacs-25, emacs-26, and fix/eieio-persistent. Would you mind
>>>> sending me the error output, in case anything jumps out at me?
>>>>
>>> Hope this could help you.
>>
>> Thanks. They all end in "Corrupt object on disk", which is an
>> eieio-persist problem. I'll keep working on this.
>
> So, I think it would be nice to have this fixed for 26.3.  It sounds
> like the fix you're working on is too risky to have on a release branch
> regardless of its current status, so here's a patch which makes eieio
> objects use symbols as type tags, when eieio-backward-compatibility is
> non-nil.  Folks who want the record-with-circular-references as type
> tags can still get that by setting eieio-backward-compatibility to nil.

I agree that everything I came up with is too shaky for a release
branch. I haven't tried your solution, but it looks simple enough.

Despite much help from many people, this problem ultimately defeated me.
I was trying to come up with a universal solution that would work for
any object, and after a couple of months had something that worked for
restoring objects, but then realized that it wouldn't work for saving
them. Whereupon I deflated like a cheap balloon. This is just beyond my
skills right now.

Eric

bug#29220: 26.0.90; eieio-persistent-read fail to restore saved object.

[Noam Postavsky]

> Subject: [PATCH] Use plain symbols for eieio type descriptors (Bug#29220)
>
> Since Emacs 26, eieio objects use a class record (with circular
> references) as the type descriptor of the object record.  This causes
> problems when reading back an object from a string, because the class
> record is not `eq' to the canonical one (which means that read objects
> don't satisfy the foo-p predicate).
> * lisp/emacs-lisp/eieio.el (make-instance): As a (partial) fix, set
> the record's type descriptor to a plain symbol for the type descriptor
> when eieio-backward-compatibility is non-nil (the default).
> * lisp/emacs-lisp/eieio-core.el (eieio--object-class): Call
> eieio--class-object on the type tag when eieio-backward-compatibility
> is non-nil.
> (eieio-object-p): Use eieio--object-class instead of
> eieio--object-class-tag.

Pushed to emacs-26 (with the tests expanded to show expected fail with
eieio-backward-compatibility set to nil).  I'm leaving the bug open
since this still fails with eieio-backward-compatibility=nil.

5f01af6c8e 2019-05-30T18:46:07-04:00 "Use plain symbols for eieio type descriptors (Bug#29220)"
https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=5f01af6c8e0f7355f7a99a80ff32369071f65eda