bug#44981: 28.0.50; Restore nnimap-split-download-body?

[Eric Abrahamsen <eric@ericabrahamsen.net>]

"Basil L. Contovounesios" <contovob@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