Created attachment 3363 [details]
full port from 3.2

this version is a full port of sa-update which includes changes for
local_state_dir.

Created attachment 3364 [details]
minimal backport

this is just a patch set against sa-update, ignoring 3.2's local_state_dir
stuff.	it has the original behavior of shoving things into
/etc/mail/spamassassin, and doesn't expect a full rule load from
updates.sa.org.  this may be more useful for 3.1, then let people shift in 3.2.
 otherwise people will have to use "sa-update --updatedir
/etc/mail/spamassassin" for 3.1, then change to normal "sa-update" for 3.2 so
that local_state_dir will work as expected.

setting dependency on solving the lint issue in bug 4789, and we can then copy that over to the 
backported sa-update version.

We in particular will not be able to use spamassasin-3.1.1's sa-update if it
puts things into /etc/mail/spamassassin instead of local_state_dir due to
various reasons including SELinux policies.  This also is not good because it
does not conform to FHS.

Why not use versioned local_state_dir?  Wouldn't doing this upon initial
production launch of this capability in 3.1.1 minimize future migration issues
and keep things inform?

Am I the only one who finds adding a major new feature in a maintenance release
a little odd?  Isn't this sort of thing what major releases are for?

I think it is less clear when the "major new feature" is mostly an add-on. It is
"major" in terms of how useful it is, but not a major change to the existing
code. Since the purpose is to help mitigate the overly long wait between new
full versions, I am very happy to see users not have to wait for 3.2 in order to
get the benefits of it.

Perhaps we should make explicit the criteria for putting things in a full
(tenth) version release instead of a maintenance release. That would include
things like, perhaps, changing db structures, config file paths and formats,
outside module dependencies, etc., as well as changes that need more testing and
tester feedback than could be done in the timeframe of a point release.

But I think that sa-update is suitable for inclusion in a maintenance release.

(In reply to comment #4)
> We in particular will not be able to use spamassasin-3.1.1's sa-update if it
> puts things into /etc/mail/spamassassin instead of local_state_dir due to
> various reasons including SELinux policies.  This also is not good because it
> does not conform to FHS.

FWIW, the current 3.1.0 sa-update puts things in /etc/mail/spamassassin.

> Why not use versioned local_state_dir?  Wouldn't doing this upon initial
> production launch of this capability in 3.1.1 minimize future migration issues
> and keep things inform?

The main issue is that in 3.2 (and with the patch in this ticket), local_state_dir overrides def_rules_dir, 
which won't work with the way 3.1 currently does things (come to think of it, it may not work in 3.2 
either...  have to investigate.)  It's also, as jgm points out, a relatively major change to 3.1 since it 
actually touches and changes the behavior of the main code.

(In reply to comment #5)
> Am I the only one who finds adding a major new feature in a maintenance release
> a little odd?  Isn't this sort of thing what major releases are for?

It's a yes/no type of thing.  This isn't a new feature -- sa-update is included in 3.1 already.  This ticket is 
about backporting bugfixes and such from 3.2 where more development has made it into a more usable 
piece of code, thereby allowing us to publish updates for 3.1.

However, the local_state_dir stuff can be seen as adding new functionality, which is why I made one patch 
with it and one patch without it.

> But I think that sa-update is suitable for inclusion in a maintenance release.

Sigh, that was typed before morning coffee. I think that making sa-update more
useful is a suitable goal for 3.1.1, since it is supposed to reduce the
painfullness of waiting so long for 3.2. Because of that I'm inclined to include
the patches in 3.1.1 unless they would make the 3.1 to 3.1.1 upgrade process
painful for users.

My thought is this...

sa-update may exist in 3.1.0, but it isn't operational.  3.1.1 is to have the
first working sa-update meant for production usage.  I believe we should either
decide to do it 100% now in 3.1.1, or defer it entirely until 3.2 if this is too
much risk for 3.1.1.  A half-way job would be undesirable for several stated
reasons.

It comes down to a question of priorities.  If this important enough to us for
our users to have rapid rule/score update capability long before the release of
3.2.0, then we should do it 100%.  If this isn't that important, then we should
defer it.

Of course upstream is free to make any other decision.  My participation here is
only to point out that updating files in /etc is a bad design and we in
particular wont be able to use the software if it relies on that location.  It
makes a lot more sense to update versioned directories in a location recommended
by FHS.  For Fedora and RHEL, this is an extra complication because it actually
matters in runtime due to restrictions within SELinux policies.

If 3.1.1 is shipped with production sa-update but without the location,
Fedora/RHEL may ship 3.1.1 at some point, however I will be forced to make a
decision like 1) Disable sa-update or 2) Patch in the 3.2 behavior in our
shipped package.  Both of these options are very undesirable.

Comment on attachment 3364 [details]
minimal backport

after further thought, etc, it does make sense to do a full backport complete
with the local_state_dir stuff.  we'll then have to setup the 3.1 update plans
on the backend a little differently, but that's fine.

Created attachment 3378 [details]
new full port from 3.2

updated to r378965

+1

I'd prefer to just release 3.2.0 early, but I understand that may not be so easy ;)

adding 4790 as a dep

Created attachment 3382 [details]
new version

includes gpghomedir updates.  after applying the patch, the following needs to
be run before committing (from 3.1 root dir):

svn cp
https://svn.apache.org/repos/asf/spamassassin/trunk/rules/sa-update-pubkey.txt
rules

(In reply to comment #13)
> +1
> 
> I'd prefer to just release 3.2.0 early, but I understand that may not be so easy ;)

Yeah.  As I've said before, we need to get into a habit of backporting fixes and such while we do 
development.  If we released 3.2.0 right now, we'd start working on 3.3.0 and be in the same situation.  

Ideally, someone would volunteer to be "maintenance release manager" and deal with the issues we've 
been having in that area... ;)

ok, I'm putting this in review state.  from some quick testing, things seem to be working appropriately in 
3.1 with this patch (feel free to test out sa.kluge.net as a channel -- it's a tiny blank ruleset), but I'd like to 
see other folks test the patch out since it's not the smallest of patches.

+1 

A few issues with attachment 3382 [details]:

- line 699 of spamd.raw is missing a comma that breaks spamd and thus make test
- there's no handling of missing dependencies, such as Archive::Tar
- it appears that --usegpg is enabled no matter what, thus I can't test any
  further than downloading the update from sa.kluge.net since I don't have a GPG
  key for it

Is this a typo in the patch? That would explain why usegpg is always enabled.

-  'updatedir=s'				=> \$site_rules_path,
-  'usegpg'				=> \$GPG_ENABLED,
+  'updatedir=s'				=> \$opt{'updatedir'},
+  'gpg!'				=> \$GPG_ENABLED,

(In reply to comment #19)
> A few issues with attachment 3382 [details] [edit]:
> 
> - line 699 of spamd.raw is missing a comma that breaks spamd and thus make test

hrm.  suboptimal.  I'll put up a new patch with the comma in it (how'd I miss that?)

> - there's no handling of missing dependencies, such as Archive::Tar

I'm not sure what you mean.  We don't have any special handling for any missing but required modules.

> - it appears that --usegpg is enabled no matter what, thus I can't test any
>   further than downloading the update from sa.kluge.net since I don't have a GPG
>   key for it

use "--nogpg"

(In reply to comment #20)
> Is this a typo in the patch? That would explain why usegpg is always enabled.
> 
> -  'updatedir=s'				=> \$site_rules_path,
> -  'usegpg'				=> \$GPG_ENABLED,
> +  'updatedir=s'				=> \$opt{'updatedir'},
> +  'gpg!'				=> \$GPG_ENABLED,

nope.  we decided to drop usegpg and switch to --gpg (enabled by default) and --nogpg.

(In reply to comment #21)
> (In reply to comment #19)

> > - there's no handling of missing dependencies, such as Archive::Tar
> 
> I'm not sure what you mean.  We don't have any special handling for any
missing but required modules.

Fair enough, I just thought that warnings from perl itself that it couldn't find
a module was a little odd.  I had long forgotten that the rest of SA doesn't
provide cleaner warnings either.

Since Archive::Tar (and others?) aren't a prerequisite for SA itself, should the
prerequisite section of the POD not include these required modules?


> > - it appears that --usegpg is enabled no matter what, thus I can't test any
> >   further than downloading the update from sa.kluge.net since I don't have a GPG
> >   key for it
> 
> use "--nogpg"

Though it works, there's no mention in the POD of --nogpg.

(In reply to comment #23)
> Since Archive::Tar (and others?) aren't a prerequisite for SA itself, should the
> prerequisite section of the POD not include these required modules?

We have been working on the concept that the use of sa-update is optional, and therefore any of its 
requirements are optional.

> > > - it appears that --usegpg is enabled no matter what, thus I can't test any
> > >   further than downloading the update from sa.kluge.net since I don't have a GPG
> > >   key for it
> > 
> > use "--nogpg"
> 
> Though it works, there's no mention in the POD of --nogpg.

hrm.  the POD is a little out of date unfortunately. :(   I'll make some updates with the new "comma rich" 
patch.  it's worth noting that documentation updates are CTR so we can continue to update after the 
full patch is committed, btw.

BTW: why don't you have my GPG key downloaded? ;)

Created attachment 3404 [details]
latest version

same as before, but fixes the spamd "missing comma" issue, updates
documentation for sa-update.

Created attachment 3405 [details]
latest latest version

want to backport the find_rule_support_file() code so we don't need to put
languages and triplets.txt into the rule updates for 3.1.

also, still need to do:
svn cp
https://svn.apache.org/repos/asf/spamassassin/trunk/rules/sa-update-pubkey.txt
rules

also also, if this is applied, change the comment in 3.2's
M::SA::find_rule_support_file() to say the API was added in for 3.1.1

> We have been working on the concept that the use of sa-update is optional, and
> therefore any of its requirements are optional.

I missed that the optional prerequisites were already listed in INSTALL. 
Apparently I suck as much as Justin says Solaris sucks. :)


> BTW: why don't you have my GPG key downloaded? ;)

OK, I suck as much as Solaris on Intel!

OK... trying out the latest latest patch:

- Make test passes
- Everything works normally prior to running sa-update --channel sa.kluge.net

After getting an update:

dbg: config: using "/var/lib/spamassassin/3.001001" for sys rules pre files
dbg: config: using "/var/lib/spamassassin/3.001001" for default rules dir


/var/lib/spamassassin/3.001001 doesn't contain any rules... everything is still
in /usr/share/spamassassin

I guess we need to copy the current ruleset to the 3.1.1 update channel before
we release 3.1.1.


Also (after sa-update is run):

The only header added to processed mail is: X-Spam-Checker-Version

Not good.

> Also (after sa-update is run):
> 
> The only header added to processed mail is: X-Spam-Checker-Version
> 
> Not good.

Of course that's because there's nothing in /var/lib/spamassassin/3.001001/ yet.

*smacks head*

So... I'm +1 on 3405 assuming that the 3.1.1 update channel is setup before
release of 3.1.1.

(In reply to comment #29)
> > Also (after sa-update is run):
> > The only header added to processed mail is: X-Spam-Checker-Version
> > Not good.
> 
> Of course that's because there's nothing in /var/lib/spamassassin/3.001001/ yet.
> 
> *smacks head*
> 
> So... I'm +1 on 3405 assuming that the 3.1.1 update channel is setup before
> release of 3.1.1.

:)  Two things:

1) the update channel doesn't actually need to be setup -- if there are no updates available, sa-update 
doesn't create the directories and there's no problem.  but yes, the full ruleset needs to be available 
once we start.  I've already got a working area (https://svn.apache.org/repos/asf/spamassassin/rules/
branches/3.1) which is manually modified to get an update released.

2) I didn't think about it when the changes were being made to sa-update recently, but the new way of 
doing things (use local_state_dir for all rules) instead of using sa-update for just updates to rules in 
released versions means that people *have* to use sa-update for updates.sa.org if they want to use any 
channels at all.  I think this isn't terrible for 3.1, but we'll want to think more about this for 3.2.  I'll 
make another ticket to discuss it.


BTW: I changed the sa.kluge.net channel to have the full (current) 3.1 rule distribution, so it should 
actually function like it would "in the real world"

(In reply to comment #30)
> (In reply to comment #29)
> 2) I didn't think about it when the changes were being made to sa-update
recently, but the new way of 
> doing things (use local_state_dir for all rules) instead of using sa-update
for just updates to rules in 
> released versions means that people *have* to use sa-update for updates.sa.org
if they want to use any 
> channels at all.  I think this isn't terrible for 3.1, but we'll want to think
more about this for 3.2.  I'll 
> make another ticket to discuss it.

OK, as long as nobody (SARE?) beats us to setting up an update channel for 3.1.1
I'm happy.


Can somebody else please vote on the latest latest patch?

Created attachment 3406 [details]
updated to include bayes fix

while doing some testing, I noticed that since bayes is enabled by default, and
we're only looking at the config that comes with the update, sync and expire
try to happen which could cause problems.  so kluge in a bayes disable when
we're linting.

(In reply to comment #31)
> OK, as long as nobody (SARE?) beats us to setting up an update channel for 3.1.1
> I'm happy.

Just to make everyone happy, I'll make a 3.1.* (not 3.1.0) update available with the current files.

+1 on patch 3406

I guess that means we are _still_ one vote short? :-)

+1 on 3406


Thanks for the 3.1.* update... I just didn't want to see breakage if SARE starts
doing updates before we do.

(In reply to comment #35)
> +1 on 3406

cool, thanks.  I'm going to leave the patch up for another day to let others give it some review/testing 
before committing and closing the ticket.

> Thanks for the 3.1.* update... I just didn't want to see breakage if SARE starts
> doing updates before we do.

no problem.  it had to get done at some point anyway. ;)   I'm not too worried about SARE BTW.  They 
seem to be pretty happy with the RDJ stuff, so I don't think there's a strong desire to make a channel.

> I'm going to leave the patch up for another day to
> let others give it some review/testing

This is the last open bug targetted for 3.1.1. If you close it does that mean
that you get to cut the 3.1.1 release? :-)

+1

ok, patch committed:

Sending        MANIFEST
Sending        Makefile.PL
Sending        lib/Mail/SpamAssassin/EvalTests.pm
Sending        lib/Mail/SpamAssassin.pm
Adding         rules/sa-update-pubkey.txt
Sending        sa-update.raw
Sending        spamassassin.raw
Sending        spamd/spamd.raw
Transmitting file data .......
Committed revision 384884.

also updated the 3.2 M::SA POD as necessary
Sending        lib/Mail/SpamAssassin.pm
Transmitting file data .
Committed revision 384885.

:)

> I'm not too worried about SARE BTW.  They seem to be pretty happy with the RDJ
stuff, so I don't think there's a strong desire to make a channel.

I /think/ I can speak for SARE that a) yes, we'll definitely want to make a
channel, but b) we most definitely don't want to cause any problems for SA
users, and will gladly hold off until we can do so without causing problems. 

(In reply to comment #40)
> I /think/ I can speak for SARE that a) yes, we'll definitely want to make a
> channel, but b) we most definitely don't want to cause any problems for SA
> users, and will gladly hold off until we can do so without causing problems. 

:)  3.1.1 got released earlier tonight, and I've had updates out there, so the two important things to 
note are:

1) people who want to use a SARE channel needs to also do the updates.sa.org channel (I don't think 
this is a problem)

2) for 3.1, you'd probably want to make updates only for 3.1.1 and later (3.1.0's sa-update should 
work, and doesn't suffer from the issue causing #1 above, but it's lacking in several areas...)