GNU bug report logs - #44981
28.0.50; Restore nnimap-split-download-body?

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 44981 in the body.
You can then email your comments to 44981 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

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

From: Eric Abrahamsen <eric <at> ericabrahamsen.net>
To: bug-gnu-emacs <at> gnu.org
Subject: 28.0.50; Restore nnimap-split-download-body?
Date: Mon, 30 Nov 2020 20:12:13 -0800

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

When splitting IMAP messages in Gnus' nnimap, there is a defvar called
`nnimap-split-download-body-default', which is consulted to decide
whether to download the whole message body during splitting, or only
consider its headers.

It appears that there used to be a defcustom called
`nnimap-split-download-body', which was advertised to the user, and this
defvar was used to hold... its default? It's odd.

In 2010, in commit 20a673b2d, a bunch of external Gnus code was merged
in, changing a lot of nnimap.el, and in the course of that the defcustom
was deleted. The defvar still remains, and is still effective. The
manual still refers to the customization option.

This looks unintentional, and I'd like to apply the attached diff,
restoring `nnimap-split-download-body' as a user option, and getting rid
of the default variable, which seems unnecessary. If this seems okay
I'll do up a proper commit, and make sure the documentation is all
accurate.

The same user who reported this also expressed a desire to be able to
download message bodies conditionally, based on the headers, only if
a split rule required it. I'm not at all sure about the feasibility of
that, but wanted to see if anyone else had an opinion.

Eric

[nnimap-split-body.diff (text/x-patch, attachment)]

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Eric Abrahamsen <eric <at> ericabrahamsen.net>
Cc: 44981 <at> debbugs.gnu.org
Subject: Re: bug#44981: 28.0.50; Restore nnimap-split-download-body?
Date: Wed, 02 Dec 2020 11:28:16 +0100

Eric Abrahamsen <eric <at> ericabrahamsen.net> writes:

> This looks unintentional, and I'd like to apply the attached diff,
> restoring `nnimap-split-download-body' as a user option, and getting rid
> of the default variable, which seems unnecessary. If this seems okay
> I'll do up a proper commit, and make sure the documentation is all
> accurate.

I think it's unintentional, yes, so that sounds like a good plan.  You
can't get rid of the variable, though (there'll be external packages
relying on it), but you can make it obsolete.

> The same user who reported this also expressed a desire to be able to
> download message bodies conditionally, based on the headers, only if
> a split rule required it. I'm not at all sure about the feasibility of
> that, but wanted to see if anyone else had an opinion.

I don't see any way to do that without making splitting unbearably slow.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

From: "Basil L. Contovounesios" <contovob <at> tcd.ie>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: Eric Abrahamsen <eric <at> ericabrahamsen.net>, 44981 <at> debbugs.gnu.org
Subject: Re: bug#44981: 28.0.50; Restore nnimap-split-download-body?
Date: Wed, 02 Dec 2020 21:01:19 +0000

Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> You can't get rid of the variable, though (there'll be external
> packages relying on it), but you can make it obsolete.

Of course, though in fairness its docstring starts with
"Internal variable..."

-- 
Basil

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

From: Eric Abrahamsen <eric <at> ericabrahamsen.net>
To: "Basil L. Contovounesios" <contovob <at> tcd.ie>
Cc: Lars Ingebrigtsen <larsi <at> gnus.org>, 44981 <at> debbugs.gnu.org
Subject: Re: bug#44981: 28.0.50; Restore nnimap-split-download-body?
Date: Wed, 02 Dec 2020 13:18:37 -0800

"Basil L. Contovounesios" <contovob <at> tcd.ie> writes:

> Lars Ingebrigtsen <larsi <at> gnus.org> writes:
>
>> You can't get rid of the variable, though (there'll be external
>> packages relying on it), but you can make it obsolete.
>
> Of course, though in fairness its docstring starts with
> "Internal variable..."

I also can't picture any way in which an external library would actually
make use of this. On the other hand, typing
`define-obsolete-variable-alias' is very little work.

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

From: Eric Abrahamsen <eric <at> ericabrahamsen.net>
To: bug-gnu-emacs <at> gnu.org
Subject: Re: bug#44981: 28.0.50; Restore nnimap-split-download-body?
Date: Wed, 02 Dec 2020 15:56:22 -0800

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

Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> Eric Abrahamsen <eric <at> ericabrahamsen.net> writes:
>
>> This looks unintentional, and I'd like to apply the attached diff,
>> restoring `nnimap-split-download-body' as a user option, and getting rid
>> of the default variable, which seems unnecessary. If this seems okay
>> I'll do up a proper commit, and make sure the documentation is all
>> accurate.
>
> I think it's unintentional, yes, so that sounds like a good plan.  You
> can't get rid of the variable, though (there'll be external packages
> relying on it), but you can make it obsolete.
>
>> The same user who reported this also expressed a desire to be able to
>> download message bodies conditionally, based on the headers, only if
>> a split rule required it. I'm not at all sure about the feasibility of
>> that, but wanted to see if anyone else had an opinion.
>
> I don't see any way to do that without making splitting unbearably slow.

No, me neither.

Here's a proper commit. I moved the bulk of the documentation to the
section on IMAP client-side splitting. Originally it was only mentioned
in the docs for the Spam package, with a link to IMAP splitting, but
then not actually mentioned in the IMAP splitting section.

Eric

[0001-Restore-nnimap-split-download-body-as-a-customizatio.patch (text/x-patch, attachment)]

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Eric Abrahamsen <eric <at> ericabrahamsen.net>
Cc: "Basil L. Contovounesios" <contovob <at> tcd.ie>, 44981 <at> debbugs.gnu.org
Subject: Re: bug#44981: 28.0.50; Restore nnimap-split-download-body?
Date: Thu, 03 Dec 2020 09:03:50 +0100

Eric Abrahamsen <eric <at> ericabrahamsen.net> writes:

> I also can't picture any way in which an external library would actually
> make use of this. On the other hand, typing
> `define-obsolete-variable-alias' is very little work.

It's mentioned in spam.el, at least, so it's probably out there.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Eric Abrahamsen <eric <at> ericabrahamsen.net>
Cc: 44981 <at> debbugs.gnu.org
Subject: Re: bug#44981: 28.0.50; Restore nnimap-split-download-body?
Date: Thu, 03 Dec 2020 09:10:24 +0100

Eric Abrahamsen <eric <at> ericabrahamsen.net> writes:

> Here's a proper commit.

Looks good to me.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

From: Eric Abrahamsen <eric <at> ericabrahamsen.net>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 44981-done <at> debbugs.gnu.org
Subject: Re: bug#44981: 28.0.50; Restore nnimap-split-download-body?
Date: Thu, 03 Dec 2020 10:20:01 -0800

Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> Eric Abrahamsen <eric <at> ericabrahamsen.net> writes:
>
>> Here's a proper commit.
>
> Looks good to me.

Thanks, done.

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

From: "Basil L. Contovounesios" <contovob <at> tcd.ie>
To: eric <at> ericabrahamsen.net
Cc: 44981 <at> debbugs.gnu.org
Subject: Re: bug#44981: 28.0.50; Restore nnimap-split-download-body?
Date: Fri, 04 Dec 2020 10:45:12 +0000

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

Thanks for cleaning this up, Eric.  Could someone please look into the
following byte-compilation warning?

  In spam-setup-widening:
  gnus/spam.el:1234:11: Warning: ‘nnimap-split-download-body-default’ is an
      obsolete variable (as of 28.1); use ‘nnimap-split-download-body’ instead.

I was thinking of the following change:

[spam.diff (text/x-diff, inline)]

diff --git a/lisp/gnus/spam.el b/lisp/gnus/spam.el
index 96a7da2313..8634fa680d 100644
--- a/lisp/gnus/spam.el
+++ b/lisp/gnus/spam.el
@@ -44,12 +44,9 @@
 ;;; for the definitions of group content classification and spam processors
 (require 'gnus)
 
-(eval-when-compile (require 'hashcash))
-
-;; for nnimap-split-download-body-default
-(eval-when-compile (require 'nnimap))
-
-(eval-when-compile (require 'cl-lib))
+(eval-when-compile
+  (require 'cl-lib)
+  (require 'hashcash))
 
 ;; autoload query-dig
 (autoload 'query-dig "dig")
@@ -1230,8 +1227,9 @@ spam-generic-score
 
 ;;; set up IMAP widening if it's necessary
 (defun spam-setup-widening ()
+  (defvar nnimap-split-download-body)
   (when (spam-widening-needed-p)
-    (setq nnimap-split-download-body-default t)))
+    (setq nnimap-split-download-body t)))
 
 (defun spam-widening-needed-p (&optional force-symbols)
   (let (found)

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

But Ted explicitly changed spam-setup-widening in 2003 to use
nnimap-split-download-body-default instead of
nnimap-split-download-body, to avoid modifying a user option.

I don't know what spam-setup-widening is meant to do, and by extension I
don't know what TRT to do here is.  Either way, loading nnimap in
eval-when-compile "for a variable" is definitely dubious.  Ideas?

Thanks,

-- 
Basil

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

From: Eric Abrahamsen <eric <at> ericabrahamsen.net>
To: "Basil L. Contovounesios" <contovob <at> tcd.ie>
Cc: 44981 <at> debbugs.gnu.org
Subject: Re: bug#44981: 28.0.50; Restore nnimap-split-download-body?
Date: Fri, 04 Dec 2020 10:39:41 -0800

"Basil L. Contovounesios" <contovob <at> tcd.ie> writes:

> Thanks for cleaning this up, Eric.  Could someone please look into the
> following byte-compilation warning?
>
>   In spam-setup-widening:
>   gnus/spam.el:1234:11: Warning: ‘nnimap-split-download-body-default’ is an
>       obsolete variable (as of 28.1); use ‘nnimap-split-download-body’ instead.
>
> I was thinking of the following change:
>
> diff --git a/lisp/gnus/spam.el b/lisp/gnus/spam.el
> index 96a7da2313..8634fa680d 100644
> --- a/lisp/gnus/spam.el
> +++ b/lisp/gnus/spam.el
> @@ -44,12 +44,9 @@
>  ;;; for the definitions of group content classification and spam processors
>  (require 'gnus)
>  
> -(eval-when-compile (require 'hashcash))
> -
> -;; for nnimap-split-download-body-default
> -(eval-when-compile (require 'nnimap))
> -
> -(eval-when-compile (require 'cl-lib))
> +(eval-when-compile
> +  (require 'cl-lib)
> +  (require 'hashcash))
>  
>  ;; autoload query-dig
>  (autoload 'query-dig "dig")
> @@ -1230,8 +1227,9 @@ spam-generic-score
>  
>  ;;; set up IMAP widening if it's necessary
>  (defun spam-setup-widening ()
> +  (defvar nnimap-split-download-body)
>    (when (spam-widening-needed-p)
> -    (setq nnimap-split-download-body-default t)))
> +    (setq nnimap-split-download-body t)))
>  
>  (defun spam-widening-needed-p (&optional force-symbols)
>    (let (found)
>
>
> But Ted explicitly changed spam-setup-widening in 2003 to use
> nnimap-split-download-body-default instead of
> nnimap-split-download-body, to avoid modifying a user option.
>
> I don't know what spam-setup-widening is meant to do, and by extension I
> don't know what TRT to do here is.  Either way, loading nnimap in
> eval-when-compile "for a variable" is definitely dubious.  Ideas?

I should have known it wouldn't be this simple!

It looks like widening is used if the spam backend in use does
statistical "learning", e.g. a Bayesian system that you can train on
spam/ham data sets. In that case the backend needs access to the full
message text in order to train properly. For local mail, all that's
needed is to "widen" the message buffer so that the body is available
for training, not just the headers. For IMAP the message body isn't
present, so "widening" takes on the additional meaning of "download the
message body and widen to it".

I'm not sure why spam.el would need to work on the default value of
`nnimap-split-download-body'. If spam.el determines that widening is
needed, it's going to download message bodies across the board anyway.
The only reason to preserve the user's own customization would be if the
user later unloaded spam.el -- then it should restore the previous
value. But how often would that actually be useful?

In principle, TRT would be to have spam.el let-bind a variable around
the splitting process. But I don't think the code is set up that way:
all the spam.el stuff happens "inside" the splitting process, not around
it.

Eric

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

From: "Basil L. Contovounesios" <contovob <at> tcd.ie>
To: Eric Abrahamsen <eric <at> ericabrahamsen.net>
Cc: Lars Ingebrigtsen <larsi <at> gnus.org>, 44981 <at> debbugs.gnu.org
Subject: Re: bug#44981: 28.0.50; Restore nnimap-split-download-body?
Date: Sat, 05 Dec 2020 15:57:17 +0000

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

Eric Abrahamsen <eric <at> ericabrahamsen.net> writes:

> I'm not sure why spam.el would need to work on the default value of
> `nnimap-split-download-body'. If spam.el determines that widening is
> needed, it's going to download message bodies across the board anyway.
> The only reason to preserve the user's own customization would be if the
> user later unloaded spam.el -- then it should restore the previous
> value. But how often would that actually be useful?
>
> In principle, TRT would be to have spam.el let-bind a variable around
> the splitting process. But I don't think the code is set up that way:
> all the spam.el stuff happens "inside" the splitting process, not around
> it.

How about the attached kludgy but conservative dance?

-- 
Basil

[0001-Stop-using-deprecated-nnimap-variable-in-spam.el.patch (text/x-diff, attachment)]

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

From: Eric Abrahamsen <eric <at> ericabrahamsen.net>
To: "Basil L. Contovounesios" <contovob <at> tcd.ie>
Cc: Lars Ingebrigtsen <larsi <at> gnus.org>, 44981 <at> debbugs.gnu.org
Subject: Re: bug#44981: 28.0.50; Restore nnimap-split-download-body?
Date: Sat, 05 Dec 2020 11:48:55 -0800

"Basil L. Contovounesios" <contovob <at> tcd.ie> writes:

> Eric Abrahamsen <eric <at> ericabrahamsen.net> writes:
>
>> I'm not sure why spam.el would need to work on the default value of
>> `nnimap-split-download-body'. If spam.el determines that widening is
>> needed, it's going to download message bodies across the board anyway.
>> The only reason to preserve the user's own customization would be if the
>> user later unloaded spam.el -- then it should restore the previous
>> value. But how often would that actually be useful?
>>
>> In principle, TRT would be to have spam.el let-bind a variable around
>> the splitting process. But I don't think the code is set up that way:
>> all the spam.el stuff happens "inside" the splitting process, not around
>> it.
>
> How about the attached kludgy but conservative dance?

It seems like a lot of work for something no one's likely to even
notice! But since you've already done the work I wouldn't object :)

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

From: "Basil L. Contovounesios" <contovob <at> tcd.ie>
To: Eric Abrahamsen <eric <at> ericabrahamsen.net>
Cc: Lars Ingebrigtsen <larsi <at> gnus.org>, 44981 <at> debbugs.gnu.org
Subject: Re: bug#44981: 28.0.50; Restore nnimap-split-download-body?
Date: Sun, 06 Dec 2020 12:06:37 +0000

Eric Abrahamsen <eric <at> ericabrahamsen.net> writes:

> "Basil L. Contovounesios" <contovob <at> tcd.ie> writes:
>
>> How about the attached kludgy but conservative dance?
>
> It seems like a lot of work for something no one's likely to even
> notice! But since you've already done the work I wouldn't object :)

Thanks, done.  :)

Avoid modifying nnimap user option in spam.el
e84a1ffde9 2020-12-06 11:45:52 +0000
https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=e84a1ffde9047c1ca0acb9abcd6d31e3bfba457d

-- 
Basil

Previous Next

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