Re: bug in recent version of nnml.el

Re: bug in recent version of nnml.el

[Katsumi Yamaoka]

>>>>> Phillip Lord wrote:

> On Gnu Emacs 23.0.60.1 there is a bug in nnml. I get this error on
> attempting to catchup in a group.

> Debugger entered--Lisp error: (wrong-type-argument stringp delete)
>   string-match("^\\([^:+]+\\)\\(?:\\+\\([^:]*\\)\\)?:" delete)
>   gnus-request-group(delete)
>   (or (gnus-request-group target) (gnus-request-create-group target))
>   (and target (or (gnus-request-group target) (gnus-request-create-group target)))
>   (if (and target (or ... ...)) (nnmail-expiry-target-group target group) (setq target nil))
>   (let (nnml-current-directory nnml-current-group nnml-article-file-alist) (when (functionp target) (setq target ...)) (if (and target ...) (nnmail-expiry-target-group target group) (setq target nil)))
>   (save-current-buffer (set-buffer temp-buffer) (nnml-request-article number group server (current-buffer)) (let (nnml-current-directory nnml-current-group nnml-article-file-alist) (when ... ...) (if ... ... ...)))
>   (with-current-buffer temp-buffer (nnml-request-article number group server (current-buffer)) (let (nnml-current-directory nnml-current-group nnml-article-file-alist) (when ... ...) (if ... ... ...)))
>   (unwind-protect (with-current-buffer temp-buffer (nnml-request-article number group server ...) (let ... ... ...)) (and (buffer-name temp-buffer) (kill-buffer temp-buffer)))
>   (let ((temp-buffer ...)) (unwind-protect (with-current-buffer temp-buffer ... ...) (and ... ...)))
>   (with-temp-buffer (nnml-request-article number group server (current-buffer)) (let (nnml-current-directory nnml-current-group nnml-article-file-alist) (when ... ...) (if ... ... ...)))
>   (if (eq target (quote delete)) nil (with-temp-buffer (nnml-request-article number group server ...) (let ... ... ...)) (nnml-possibly-change-directory group server))
>   (unless (eq target (quote delete)) (with-temp-buffer (nnml-request-article number group server ...) (let ... ... ...)) (nnml-possibly-change-directory group server))
>   (progn (setq target nnmail-expiry-target) (unless (eq target ...) (with-temp-buffer ... ...) (nnml-possibly-change-directory group server)) (if target (progn ... ... ... ...) (push number rest)))
>   (if (and (setq article ...) (setq mod-time ...) (nnml-deletable-article-p group number) (setq is-old ...)) (progn (setq target nnmail-expiry-target) (unless ... ... ...) (if target ... ...)) (push number rest))
>   (while (and articles is-old) (if (and ... ... ... ...) (progn ... ... ...) (push number rest)))
>   (let* ((file-name-coding-system nnmail-pathname-coding-system) (active-articles ...) (is-old t) (decoded ...) article rest mod-time number target) (nnmail-activate (quote nnml)) (setq active-articles (sort active-articles ...)) (setq articles (gnus-sorted-intersection articles active-articles)) (while (and articles is-old) (if ... ... ...)) (let (...) (when active ...) (nnmail-save-active nnml-group-alist nnml-active-file)) (nnml-save-nov) (nconc rest articles))
>   nnml-request-expire-articles((9224 9225 9226 9227 9228 9229 9230 9231 9232 9233 9234 9235 9236 9237 9238 9239 9240 9241 9242 9243 9244 9245 9246 9247 9248 9249 9250 9251 9252 9253 9254 9255 9256 9257 9258 9259 9260 9261 9262 9263 9264 9265 9266 9267 9268 9269 9270 9271 9272 9273 ...) "stormcock" "" nil)
>   gnus-request-expire-articles((9224 9225 9226 9227 9228 9229 9230 9231 9232 9233 9234 9235 9236 9237 9238 9239 9240 9241 9242 9243 9244 9245 9246 9247 9248 9249 9250 9251 9252 9253 9254 9255 9256 9257 9258 9259 9260 9261 9262 9263 9264 9265 9266 9267 9268 9269 9270 9271 9272 9273 ...) "nnml:stormcock")
>   gnus-summary-expire-articles()
>   run-hooks(gnus-summary-prepare-exit-hook)
>   apply(run-hooks gnus-summary-prepare-exit-hook)
>   gnus-run-hooks(gnus-summary-prepare-exit-hook)
>   gnus-summary-exit()
>   gnus-summary-catchup-and-exit(nil)
>   call-interactively(gnus-summary-catchup-and-exit nil nil)

> The problem comes from this bit of code here...

> 	    ;; Allow a special target group.
> 	    (setq target nnmail-expiry-target)
> 	    (unless (eq target 'delete)
> 	      (with-temp-buffer
> 		(nnml-request-article number group server (current-buffer))
> 		(let (nnml-current-directory
> 		      nnml-current-group
> 		      nnml-article-file-alist)
> 		  (when (functionp target)
> 		    (setq target (funcall target group)))
> 		  (if (and target
> 			   (or (gnus-request-group target)
> 			       (gnus-request-create-group target)))
> 		      (nnmail-expiry-target-group target group)
> 		    (setq target nil))))
> 	      ;; Maybe directory is changed during nnmail-expiry-target-group.
> 	      (nnml-possibly-change-directory group server))

> This statement assumes that target or the result of calling target is a group...

>   (if (and target
>      (or (gnus-request-group target)
> 	 (gnus-request-create-group target)))

> But it could be delete also as recognised just above.

> Moving the funcall to before the unless fixes the problem, as shown here.

> 	    ;; Allow a special target group.
> 	    (setq target nnmail-expiry-target)
>             (when (functionp target)
> 		    (setq target (funcall target group)))

> 	    (unless (eq target 'delete)
> 	      (with-temp-buffer
> 		(nnml-request-article number group server (current-buffer))
> 		(let (nnml-current-directory
> 		      nnml-current-group
> 		      nnml-article-file-alist)
> 		  (if (and target
> 			   (or (gnus-request-group target)
> 			       (gnus-request-create-group target)))
> 		      (nnmail-expiry-target-group target group)
> 		    (setq target nil))))
> 	      ;; Maybe directory is changed during nnmail-expiry-target-group.
> 	      (nnml-possibly-change-directory group server))

> Thanks for Gnus!

> Phil

Thank you for inquiring into the bug so closely.  I could reproduce
it using:

(setq nnmail-expiry-target (lambda (&rest args) 'delete))

Furthermore, I checked nnbabyl.el, nndiary.el, nnfolder.el,
nnimap.el, nnmbox.el and nnmh.el, and found the same problem in
nnfolder.el, too.  The others seem to be safe.  I've installed
the fix that is slightly modified from yours in the Gnus CVS
trunk (it will be merged to the Emacs trunk soon).

The changelog entry is as follows now but I'd like to replace
the entry line with your name.

2008-10-02  Katsumi Yamaoka  <yamaoka@jpl.org>

	* nnml.el (nnml-request-expire-articles): Expand nnmail-expiry-target
	to an actual expiry target first if it is a function.  Suggested by
	Phillip Lord <phillip.lord@newcastle.ac.uk>.

	* nnfolder.el (nnfolder-request-expire-articles): Ditto.

Mail me if you've already signed or when you sign a paper to FSF.

Regards,

Re: bug in recent version of nnml.el

[Katsumi Yamaoka]

>>>>> Katsumi Yamaoka wrote:
> 2008-10-02  Katsumi Yamaoka  <yamaoka@jpl.org>

>         * nnml.el (nnml-request-expire-articles): Expand nnmail-expiry-target
>         to an actual expiry target first if it is a function.  Suggested by
>         Phillip Lord <phillip.lord@newcastle.ac.uk>.

>         * nnfolder.el (nnfolder-request-expire-articles): Ditto.

I've reverted those changes.  Sorry.  I realized it causes another
error if `nnmail-expiry-target' is the symbol `delete' that is
the default.  It is not only a symbol but also a function symbol.
I'll look for another solution...

Re: bug in recent version of nnml.el

[Katsumi Yamaoka]

>>>>> Katsumi Yamaoka wrote:
>>>>>> Katsumi Yamaoka wrote:
>> 2008-10-02  Katsumi Yamaoka  <yamaoka@jpl.org>

>>         * nnml.el (nnml-request-expire-articles): Expand nnmail-expiry-target
>>         to an actual expiry target first if it is a function.  Suggested by
>>         Phillip Lord <phillip.lord@newcastle.ac.uk>.

>>         * nnfolder.el (nnfolder-request-expire-articles): Ditto.

> I've reverted those changes.  Sorry.  I realized it causes another
> error if `nnmail-expiry-target' is the symbol `delete' that is
> the default.  It is not only a symbol but also a function symbol.
> I'll look for another solution...

Fixed in the Gnus CVS trunk.  The change I made in nnml.el is as
follows.  Note that the function set to `nnmail-expiry-target'
should be executed in a buffer in which there is an article to
be examined by the function (your function might not care article
contents, though).

--8<---------------cut here---------------start------------->8---
--- nnml.el~	2008-06-11 21:40:59 +0000
+++ nnml.el	2008-10-03 01:26:37 +0000
@@ -364,11 +364,11 @@
 		      nnml-article-file-alist)
 		  (when (functionp target)
 		    (setq target (funcall target group)))
-		  (if (and target
-			   (or (gnus-request-group target)
-			       (gnus-request-create-group target)))
-		      (nnmail-expiry-target-group target group)
-		    (setq target nil))))
+		  (when (and target (not (eq target 'delete)))
+		    (if (or (gnus-request-group target)
+			    (gnus-request-create-group target))
+			(nnmail-expiry-target-group target group)
+		      (setq target nil)))))
 	      ;; Maybe directory is changed during nnmail-expiry-target-group.
 	      (nnml-possibly-change-directory group server))
 	    (if target
--8<---------------cut here---------------end--------------->8---

Regards,

Re: bug in recent version of nnml.el

[Phillip Lord]

>>>>> "KY" == Katsumi Yamaoka <yamaoka@jpl.org> writes:

  KY> Thank you for inquiring into the bug so closely.

No worries. It wanted to move my gnus emacs to 23 and this was stopping me, so
it was purely self-interest. 


  KY> Mail me if you've already signed or when you sign a paper to FSF.

Hmm. I have done this in the past, but I fear that they might be out of date. 
I don't know how to check this; any ideas? 

  KY> I've reverted those changes. Sorry. I realized it causes another error
  KY> if `nnmail-expiry-target' is the symbol `delete' that is the default. It
  KY> is not only a symbol but also a function symbol. I'll look for another
  KY> solution...


Unfortunate! Hadn't thought of that, of course. 

  KY> Fixed in the Gnus CVS trunk. The change I made in nnml.el is as follows.
  KY> Note that the function set to `nnmail-expiry-target' should be executed
  KY> in a buffer in which there is an article to be examined by the function
  KY> (your function might not care article contents, though).

Good stuff. My function just expires differently for different groups, so it
doesn't matter. In general, you are right though. This is what the
documentation says!

Phil

Re: bug in recent version of nnml.el

[Katsumi Yamaoka]

>>>>> phillip.lord@newcastle.ac.uk wrote:

>   KY> Mail me if you've already signed or when you sign a paper to FSF.

> Hmm. I have done this in the past, but I fear that they might be out of date.
> I don't know how to check this; any ideas?

Hm, I looked into the etc/AUTHORS file and didn't find your name,
but I've never heard that expiration is being done.  How about
inquiring to FSF?  (maybe <gnu(at)gnu.org> will help you.)

But you have nothing to do since the code was written by me in
this case.  Of course you should be praised for having discovered
the bug. ;-)

Regards,

Re: bug in recent version of nnml.el

[Reiner Steib]

On Fri, Oct 03 2008, Katsumi Yamaoka wrote:

>>>>>> phillip.lord@newcastle.ac.uk wrote:
>> Hmm. I have done this in the past, but I fear that they might be out of date.
>> I don't know how to check this; any ideas?
>
> Hm, I looked into the etc/AUTHORS file and didn't find your name,

The right place to look at is copyright.list on fencepost.

> but I've never heard that expiration is being done.  How about
> inquiring to FSF?  (maybe <gnu(at)gnu.org> will help you.)

[ Assuming that Phillip is <p.lord at russet.org.uk> ... ]
We have an assignment for "past changes" to Emacs (and two employer
disclaimers).  So we'd need a new assignment if the contributed code
is more than a "tiny change" (the limit is around 15 lines, IIRC.  I
can't remember the exact number).

> But you have nothing to do since the code was written by me in
> this case.  Of course you should be praised for having discovered
> the bug. ;-)

Bye, Reiner.
-- 
       ,,,
      (o o)
---ooO-(_)-Ooo---  |  PGP key available  |  http://rsteib.home.pages.de/

Re: bug in recent version of nnml.el

[Phillip Lord]

>>>>> "KY" == Katsumi Yamaoka <yamaoka@jpl.org> writes:

>>>>> phillip.lord@newcastle.ac.uk wrote:
  KY> Mail me if you've already signed or when you sign a paper to FSF.

  >> Hmm. I have done this in the past, but I fear that they might be out of
  >> date. I don't know how to check this; any ideas?

  KY> Hm, I looked into the etc/AUTHORS file and didn't find your name

I wrote the lazy loading support for desktop.el. I think that's my own bit in
core. 

  KY> but I've never heard that expiration is being done. How about inquiring
  KY> to FSF? (maybe <gnu(at)gnu.org> will help you.)

It's the employer waver of rights; my contract has changed since I last sent
papers in. 


  KY> But you have nothing to do since the code was written by me in this
  KY> case. Of course you should be praised for having discovered the bug. ;-)

Good point!

Phil

Re: bug in recent version of nnml.el

[Phillip Lord]

>>>>> "RS" == Reiner Steib <reinersteib+gmane@imap.cc> writes:

  RS> On Fri, Oct 03 2008, Katsumi Yamaoka wrote:
  >>>>>>> phillip.lord@newcastle.ac.uk wrote:
  >>> Hmm. I have done this in the past, but I fear that they might be out of
  >>> date. I don't know how to check this; any ideas?
  >> 
  >> Hm, I looked into the etc/AUTHORS file and didn't find your name,

  RS> The right place to look at is copyright.list on fencepost.

  >> but I've never heard that expiration is being done. How about inquiring
  >> to FSF? (maybe <gnu(at)gnu.org> will help you.)

  RS> [ Assuming that Phillip is <p.lord at russet.org.uk> ... ] We have an


That's me. 


  RS> assignment for "past changes" to Emacs (and two employer disclaimers).
  RS> So we'd need a new assignment if the contributed code is more than a
  RS> "tiny change" (the limit is around 15 lines, IIRC. I can't remember the
  RS> exact number).

Well, even my original fix was well under this. All clear then!

Phil