Created attachment 2714 [details]
Proposed fix

Doesn't include any changes to Mail::SpamAssassin::PerMsgLearner.  I'm not sure
what it means to learn when Bayes is disabled.

bug with patch -> 3.1 milestone

+1 for me, tests fine for me in limited testing

Taking.

Eventually this will be a plugin, but not yet so this seems to make some sense.

punting to 3.2.0

John's CLA is noted as received at jimjag's page so this is CLA-ready.
I agree this should go into 3.2.0 though, as it could have big effects in terms
of stability and it's too late for 3.1.0 given that :(

I plan to pluginize Bayes and don't have time to test this before putting it in,
so someone else can grab it if they'd like to get it in.

+0.9 on HEAD only
-1 on 3.1

The reason I'm not fully +1 is because I think we'll plugin-ize Bayes before
3.2 and this will be useful as a reference/precursor work.  I would prefer to
just skip this step and go straight to plugin.

+1 for trunk
-1 for 3.1.0

agreed with Daniel.  I'm more ok with getting it into trunk than he is, though.

BTW there may be a safer way to do it without having to risk calls of methods on
an undef value (which is a die()), by making $main->{bayes_scanner} be an
implementation of an object which does no-ops for all the Mail::SA::Bayes
methods.   I think this is the Null Object pattern iirc?

i'll just throw in a reminder about bug 4448  ;)

so when you do implement this, please dont break SQL prefs where 'use_bayes 0'
is set in a flat file but some users have it 'use_bayes 1' via their SQL prefs...

thanks!

I'm not asking for this to go into 3.1, just the trunk.  A 3.0 version of this
patch has been running in production with bayes disabled for months.

Right, I think we're all in agreement that this should not go into 3.1 (it's
triaged for 3.2, and it's clearly too late).

Any committer is free to test and commit -- I'll do it if I get around to it
(haha... I've said that before) since we're CTR on trunk. (Just wanted to
clarify that we have no need for a vote)

Since we expressed the desire to have a short release cycle for 3.2, it would be
good to get this in soon so we can test it in case Bayes doesn't get plugininzed
quick enough -- I'm not sure how high a priority that is for Michael/Daniel(?).

I was going to apply this -- but Dallas' point is a good one.  Has anyone tested
that situation?   (reminder: system-wide settings have "use_bayes 0", user has
"use_bayes 1").

Committed revision 321240.

Bayes loading is tested in signal_user_changed, which is documented as being
called after reading any per-user configuration.