[VM] bug with labels in virtual folders

[Julian Bradfield]

After a year (or maybe two) of elapsed time and probably tens of hours
staring at the lisp debugger, I have finally tracked down the most
annoying (for me) outstanding bug in vm. That is, I've fixed it, but I
don't understand the fix or the bug!

My vm is Uday's private 8.2.X branch of 6 February 2012, but I think
this bug is still in trunk (haven't tried it, though).

Here's the bug description:

Create a folder (say /tmp/I) containing a single message.
Visit the folder, and add the label "a" to the message.
Quit the folder.

Now do
(setq vm-virtual-folder-alist
'(
("V" (("/tmp/I") (any)))))

Visit the virtual folder V.
Add the label "b" to the message.
The summary mode line will show the message as having labels "a" and
"b", as expected; but in fact the message has only the new label "b".
To see this, save the folder, quit, and re-visit the real folder (or
the virtual folder).

The thing that happened this morning was that I spent so long staring
at one line in the debugger that the flush timer fired while I was
doing so, which magically made the bug not happen.

The following patch appears to fix the bug:

*** vm-undo.el 2012/11/26 15:09:10 1.1
--- vm-undo.el 2013/01/08 08:27:02 1.2
***************
*** 597,603 ****
(vm-set-labels-of m labels)
(vm-set-label-string-of m nil)
(vm-mark-for-summary-update m)
! (if (eq vm-flush-interval t)
(vm-stuff-virtual-message-data m)
(vm-set-stuff-flag-of m t))))))

--- 597,604 ----
(vm-set-labels-of m labels)
(vm-set-label-string-of m nil)
(vm-mark-for-summary-update m)
! ;; deferring stuffing breaks labels in virtual folders
! (if t ;(eq vm-flush-interval t)
(vm-stuff-virtual-message-data m)
(vm-set-stuff-flag-of m t))))))


This does raise the question in my mind as to whether the other
similarly deferred uses of vm-stuff-virtual-message-data are
dangerous.

[John Hein]

Post by Julian Bradfield
After a year (or maybe two) of elapsed time and probably tens of hours
staring at the lisp debugger, I have finally tracked down the most
annoying (for me) outstanding bug in vm. That is, I've fixed it, but I
don't understand the fix or the bug!
My vm is Uday's private 8.2.X branch of 6 February 2012, but I think
this bug is still in trunk (haven't tried it, though).

I can't reproduce this with emacs 24.2.1 and vm trunk rev 1476.
Revisiting after 'l b', quit has both labels a & b.

Post by Julian Bradfield
Create a folder (say /tmp/I) containing a single message.
Visit the folder, and add the label "a" to the message.
Quit the folder.
Now do
(setq vm-virtual-folder-alist
'(
("V" (("/tmp/I") (any)))))
Visit the virtual folder V.
Add the label "b" to the message.
The summary mode line will show the message as having labels "a" and
"b", as expected; but in fact the message has only the new label "b".
To see this, save the folder, quit, and re-visit the real folder (or
the virtual folder).
The thing that happened this morning was that I spent so long staring
at one line in the debugger that the flush timer fired while I was
doing so, which magically made the bug not happen.
*** vm-undo.el 2012/11/26 15:09:10 1.1
--- vm-undo.el 2013/01/08 08:27:02 1.2
***************
*** 597,603 ****
(vm-set-labels-of m labels)
(vm-set-label-string-of m nil)
(vm-mark-for-summary-update m)
! (if (eq vm-flush-interval t)
(vm-stuff-virtual-message-data m)
(vm-set-stuff-flag-of m t))))))
--- 597,604 ----
(vm-set-labels-of m labels)
(vm-set-label-string-of m nil)
(vm-mark-for-summary-update m)
! ;; deferring stuffing breaks labels in virtual folders
! (if t ;(eq vm-flush-interval t)
(vm-stuff-virtual-message-data m)
(vm-set-stuff-flag-of m t))))))
This does raise the question in my mind as to whether the other
similarly deferred uses of vm-stuff-virtual-message-data are
dangerous.

[Julian Bradfield]

Post by John Hein
I can't reproduce this with emacs 24.2.1 and vm trunk rev 1476.

Interesting. However, I can't try trunk, as vm is no longer compatible
with XEmacs, owing to Uday's fixation with making use of every
incompatible new feature introduced by RMS ;-)
(or at least the second argument to #'truncate)

[Uday Reddy]

Post by Julian Bradfield
Interesting. However, I can't try trunk, as vm is no longer compatible
with XEmacs, owing to Uday's fixation with making use of every
incompatible new feature introduced by RMS ;-)
(or at least the second argument to #'truncate)

If you tell me whatever is incompatible with XEmacs, I am happy to change
it.

Does XEmacs have a function to calculate the integer quotient?

Cheers,
Uday

[Julian Bradfield]

Post by Uday Reddy
Does XEmacs have a function to calculate the integer quotient?

?

If I believe the FSFmacs documentation
(truncate ARG DIVISOR)
should be equivalent to
(truncate (/ (float ARG) DIVISOR))

XEmacs thinks that if you want an integer quotient, you should use
(floor ARG DIVISOR)
which rounds towards -∞ rather than towards zero, so that it plays
nicely with mod.
I haven't looked to see what your use of it is!

[Uday Reddy]

Post by Julian Bradfield
My vm is Uday's private 8.2.X branch of 6 February 2012, but I think
this bug is still in trunk (haven't tried it, though).

Confirmed that this bug is present in the 8.2.x branch, but not the trunk.
It is either fixed in the trunk or perhaps became harder to reproduce. So,
I will have to investigate why it is happening in the 8.2.x branch.

Cheers,
Uday

[Julian Bradfield]

So the patch that I thought fixed it is in fact nothing to do with the
root cause, it just hides the symptoms in some cases. I noticed on
further investigation that basically handling of multiple labels per
message is completely screwed.
After another two hours with edebug, the real problem turns out to be
very simple (but a pig to find):

*** vm-summary.el 2012/06/13 17:49:27 1.2
--- vm-summary.el 2013/01/09 10:55:24 1.3
***************
*** 1716,1722 ****
(or (vm-label-string-of m)
(vm-set-label-string-of
m
! (mapconcat 'identity (sort (vm-labels-of m) 'string-lessp) ","))
(vm-label-string-of m)))

(defun vm-make-folder-summary ()
--- 1716,1722 ----
(or (vm-label-string-of m)
(vm-set-label-string-of
m
! (mapconcat 'identity (sort (copy-sequence (vm-labels-of m)) 'string-lessp) ","))
(vm-label-string-of m)))

(defun vm-make-folder-summary ()


Presumably Uday's already noticed this at some point, since trunk does
have the protective copy-sequence.