You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@spamassassin.apache.org by "Malte S. Stretz" <ms...@gmx.net> on 2005/02/11 10:11:16 UTC
Re: svn commit: r153377 - in spamassassin/trunk: MANIFEST Makefile.PL build/do spamassassin spamassassin.pod
On Friday 11 February 2005 09:57 CET quinlan@apache.org wrote:
> Author: quinlan
> Date: Fri Feb 11 00:57:06 2005
> New Revision: 153377
>
> URL: http://svn.apache.org/viewcvs?view=rev&rev=153377
> Log:
> bug 4124: fix spamassassin symlink generation and pod installation
> location
*sigh* That commit is still wrong. Why don't you reply to the thread on the
list first?
Cheers,
Malte
--
[SGT] Simon G. Tatham: "How to Report Bugs Effectively"
<http://www.chiark.greenend.org.uk/~sgtatham/bugs.html>
[ESR] Eric S. Raymond: "How To Ask Questions The Smart Way"
<http://www.catb.org/~esr/faqs/smart-questions.html>
Re: svn commit: r153377 - in spamassassin/trunk: MANIFEST Makefile.PL build/do spamassassin spamassassin.pod
Posted by "Malte S. Stretz" <ms...@gmx.net>.
Sorry, I foobared my KDE yesterday, took me a while to fix that before I
could answer...
On Saturday 12 February 2005 05:02 CET Daniel Quinlan wrote:
> "Malte S. Stretz" <ms...@gmx.net> writes:
> > There was a discussion going on on this mailinglist on which you could
> > have asked if that what you committed would work. Which also included
> > a suggestion by me to split out the reporting stuff which would need a
> > spamassassin script (instead of a symlink) anyway.
>
> I think you might be overreacting a bit. Waiting for a long email
> discussion to finish while the installation was broken would not have
> made much sense, especially since I was willing to burn my evening
> trying to figure out the Makefiles.
But its not like we want to release 3.1 tomorrow and the installation wasn't
more broken than now :) It just installed a copy of the script instead of
a symlink.
> Anyway, if we split out reporting, then things will need to be
> redesigned much more. The problem with the current design around
> "spamassassin" is that various behaviors are implemented as flags rather
> than svn-style subcommands or razor-style commands.
>
> sa-filter [options] [ < mailmessage | path ... ]
> sa-filter -d [ < mailmessage | path ... ]
> sa-filter -r [-w addr] [ < mailmessage | path ... ]
> sa-filter -k [-w addr] [ < mailmessage | path ... ]
> sa-filter -W|-R [ < mailmessage | path ... ]
>
> This suggests several different possible breakdowns--I'm ignoring
> whether or not it's "sa-foo" or "sa foo" and please just consider these
> placeholder names.
>
> Obviously, one command, the main command, is "filter".
>
> The second command is "remove". This could be its own command or it
> could possibly be added to "filter", but I think separate is probably
> better since it's very simple.
>
> "report" and "revoke" are the next commands, maybe together in one
> command, although, again, I prefer separate commands here.
I also prefer separate commands, the other way "sa-report -r < foo" (ie.
"report but do the opposite) feels a bit odd.
> "whitelist" and "blacklist" -- these *really* belong in "sa-learn" since
> it's AWL/History, but "sa-learn" has been crufted up with lots of
> Bayes-specific stuff, so merging it with "sa-learn" will be non-trivial.
I've got no opinion on that. As a user it looks a bit weird to have to call
two commands if I want to learn a single message but OTOH is the
functionality really pretty different. Hmmm... two commands for learning
might indeed have some technical problmes as it might not always be
possible to call two commands via a single filter and each call will have
the perl-startup overhead.
> Basically, the command line tools are a confusing mess of disorganized
> crap. Compare "svn", "cvs", even "apt-get".
Indeed, that's why we should fix it before we move all that stuff to
sa-filter and have to maintain two backward compatible interfaces (which
are IMO, though that O might have change din the last year, part of our
public interface and must stay compatible for the whole 3.x branch).
> > Right, as long as we don't want to test the spamassassin
> > script/symlink.
>
> Maybe.
>
> > I'm not opposed to your approach, it didn't come to my mind, but if
> > you had asked I could have told you a few cases where it doesn't work
> > as you implemented it.
>
> Could you could more specific?
I did, below, just was too lazy to rephrase that sentence :)
> > No, it doesn't work for vendor installs, just for perl and site.
> > That's what all the I_FOO and B_FOO hackery in the Makefile.PL is for.
>
> Completely underdocumented hackery.
ACK. Even completely nderdocumented hackery based on even more
underdocumented internals of EU::MM.
> > But it won't work on systems where the single quote is not supported.
>
> Single quotes appear all over the Makefile.PL and the generated
> Makefile. If this is an issue, it's also not documented and you did not
> mention it recently.
I didn't mention it because you didn't ask :) Actually, I wonder why I used
single quotes at some places, too, maybe I just forgot about it myself back
then :( The code generated by EU::MM always uses the internal method
oneliner() to get it right. Unfortunately was there some bug in oneliner()
in some ersion and its semantics have changed at some point.
> > On the quoting of the dollar sign is also different on the platforms.
> > That's the reason why I moved the logic into a separate script. (I
> > think EU::MM has a function oneliner() for the correct quoting but I
> > faintly remember some bug in there.)
>
> If you're trying to paralyze me into inaction, it might be working, but
> it might be better to document things more and suggest alternate
> solutions instead of just raising issues with my implementation.
Ummm, no, I'm not trying to paralyze anybody. As you noted yourself, thats
all black magic and normally when I encounter some black magic I don't
fully understand, I first send a patch for review to the mailinglist
(especially if there's already a discussion going on) before two people
start to commit-and-revert on the same code block.
> Sorry, I'm a bit frustrated here...
Sorry, that was definitely not my intention.
Cheers,
Malte
--
[SGT] Simon G. Tatham: "How to Report Bugs Effectively"
<http://www.chiark.greenend.org.uk/~sgtatham/bugs.html>
[ESR] Eric S. Raymond: "How To Ask Questions The Smart Way"
<http://www.catb.org/~esr/faqs/smart-questions.html>
Re: svn commit: r153377 - in spamassassin/trunk: MANIFEST Makefile.PL build/do spamassassin spamassassin.pod
Posted by Michael Parker <pa...@pobox.com>.
On Tue, Feb 22, 2005 at 11:44:37AM -0800, Daniel Quinlan wrote:
> Justin Mason <jm...@jmason.org> writes:
>
> > I agree with Daniel that we should model the command line UI around the
> > use cases, not around the code subsystems involved.
>
> yay! ;-)
>
BTW, I don't think we're on different pages here. Or maybe I'm missing
something. But I agree with just about everything y'all said.
I do think we're in for a world of hurt if we remove things from the
'spamassassin' script for 3.1, we need to officially deprecate the
interface and warn on usage, maybe deprecate and wait til 3.1.1 to
warn.
Michael
Re: svn commit: r153377 - in spamassassin/trunk: MANIFEST Makefile.PL build/do spamassassin spamassassin.pod
Posted by Daniel Quinlan <qu...@pathname.com>.
Justin Mason <jm...@jmason.org> writes:
> I agree with Daniel that we should model the command line UI around the
> use cases, not around the code subsystems involved.
yay! ;-)
> But then "plugin-specific stuff like bayes" -- Daniel, I'm not sure
> what you mean here, this seems to contradict the previous idea (UI
> based on use-cases).
I mean that *if* there's some aspect that can't be generalized, then it
should go into a plugin-specific front-end. I think there are few, if
any, cases of that, though. For example, --backup and --restore could
become standard plugin APIs and the format could be extended to handle
more than just Bayes.
> On another aspect --- I like Michael's idea of leaving the
> "spamassassin" script's UI intact as it is now for a few 3.x
> releases. There is a lot of third-party code that hooks into
> SpamAssassin that will need to be updated before that interface can be
> removed safely without causing a lot of needless pain.
Well, it might be cleaner to junk the "sa-filter" idea and leave
"sa-filter" as a 100% clean future program. Move the code back into
"spamassassin", but document it under the meta document or in a separate
pod.
Daniel
--
Daniel Quinlan
http://www.pathname.com/~quinlan/
Re: svn commit: r153377 - in spamassassin/trunk: MANIFEST Makefile.PL build/do spamassassin spamassassin.pod
Posted by Daniel Quinlan <qu...@pathname.com>.
Michael Parker <pa...@pobox.com> writes:
> Now you're adding in a whole other subsystem. Using this logic, it is
> pretty easy to argue that everything should be in one executable, and
> not split out. A little cross use is allowed. Here is my proposed
> set of commands and their arguments:
> [...usages removed...]
> sa-filter
> sa-report
> sa-learn (maybe sa-bayes)
> sa-update
> sa-history/sa-awl (maybe sa-whitelist)
> sa-lint
Hmmm... my thinking is that we have several basic actions people
perform.
- filtering and removal of markup
- learning/forgetting
- reporting/revoking
- plugin-specific stuff that cannot be generalized
I'd strongly favor move the basic learn functionality (learn, forget)
into a single command regardless of the learning subsystem and the
plugin-specific stuff like bayes, whitelist, and history into
plugin-specific commands.
I think 3.2 is perhaps the right place to do a larger restructuring than
what is in 3.1.
Daniel
--
Daniel Quinlan
http://www.pathname.com/~quinlan/
Re: svn commit: r153377 - in spamassassin/trunk: MANIFEST Makefile.PL build/do spamassassin spamassassin.pod
Posted by Michael Parker <pa...@pobox.com>.
On Tue, Feb 15, 2005 at 03:07:52AM -0800, Daniel Quinlan wrote:
> Daniel Quinlan <qu...@pathname.com> writes:
>
> >> "whitelist" and "blacklist" -- these *really* belong in "sa-learn" since
> >> it's AWL/History, but "sa-learn" has been crufted up with lots of
> >> Bayes-specific stuff, so merging it with "sa-learn" will be non-trivial.
>
> Michael Parker <pa...@pobox.com> writes:
>
> > Please no, I'm -1 on this, I've worked very hard to separate the
> > whitelist/blacklist/awl stuff from bayes. sa-learn is bayes, period.
>
> Hmm... I agree it's been crufted up, but think about this:
>
> report calls learning stuff, so if you're going to report you don't
> need to call learn or whitelist stuff
>
> if you're not going to report, do you really want to require users to
> call two commands?
>
Now you're adding in a whole other subsystem. Using this logic, it is
pretty easy to argue that everything should be in one executable, and
not split out. A little cross use is allowed. Here is my proposed
set of commands and their arguments:
sa-filter
-L, --local Local tests only (no online tests)
-d, --remove-markup Remove spam reports from a message
-x, --nocreate-prefs Don't create user preferences file
-e, --exit-code Exit with a non-zero exit code if the tested message was spam
-t, --test-mode Pipe message through and add extra report to the bottom
--progress Show progress using progress bar
--mbox read in messages in mbox format
--mbx read in messages in UW mbx format
-f file,
--folders=file Read list of files/directories from file
-C path,
--configpath=path,
--config-file=path Path to standard configuration dir
-p prefs,
--prefspath=file,
--prefs-file=file Set user preferences file
--siteconfigpath=path Path for site configs (def: /etc/mail/spamassassin)
-D, --debug [area=n,...] Print debugging messages
-V, --version Print version
-h, --help Print usage message
sa-report
-k, --revoke Revoke message as spam
--progress Show progress using progress bar
--mbox read in messages in mbox format
--mbx read in messages in UW mbx format
-f file,
--folders=file Read list of files/directories from file
-C path,
--configpath=path,
--config-file=path Path to standard configuration dir
-p prefs,
--prefspath=file,
--prefs-file=file Set user preferences file
--siteconfigpath=path Path for site configs (def: /etc/mail/spamassassin)
-D, --debug [area=n,...] Print debugging messages
-V, --version Print version
-h, --help Print usage message
sa-learn (maybe sa-bayes)
--ham Learn messages as ham (non-spam)
--spam Learn messages as spam
--forget Forget a message
--use-ignores Use bayes_ignore_from and bayes_ignore_to
--sync Syncronize the database and the journal if needed
--no-sync Skip syncronizing the database and journal after learning
--force-expire Force a database sync and expiry run
--dbpath <path> Allows commandline override (in bayes_path form)
for where to read the Bayes DB from
--status Display the status of the Bayes database
--progress Show progress using progress bar
--import Migrate data from older version/non DB_File based databases
--clear Wipe out existing database
--backup Backup, to STDOUT, existing database
--restore <filename> Restore a database from filename
-u username,
--username=username Override username taken from the runtime environment
--mbox read in messages in mbox format
--mbx read in messages in UW mbx format
-f file,
--folders=file Read list of files/directories from file
-C path,
--configpath=path,
--config-file=path Path to standard configuration dir
-p prefs,
--prefspath=file,
--prefs-file=file Set user preferences file
--siteconfigpath=path Path for site configs (def: /etc/mail/spamassassin)
-D, --debug [area=n,...] Print debugging messages
-V, --version Print version
-h, --help Print usage message
sa-update
--updatedir path Directory to place updates, defaults to the SpamAssassin
site rules directory (def: /etc/mail/spamassassin)
--channel channel Retrieve updates from this channel
Use multiple times for multiple channels
--channelfile file Retrieve updates from the channels in the file
--gpgkey key Trust the key id to sign releases
Use multiple times for multiple keys
--gpgkeyfile file Trust the key ids in the file to sign releases
--usegpg Use GPG to verify updates
This is auto-enabled by use of the above
gpgkey and gpgkeyfile options.
-D, --debug [area=n,...] Print debugging messages
-V, --version Print version
-h, --help Print usage message
sa-history/sa-awl (maybe sa-whitelist)
-W,
--add-to-whitelist Add addresses in mail to persistent address whitelist
--add-to-blacklist Add addresses in mail to persistent address blacklist
-R,
--remove-from-whitelist Remove all addresses found in mail from persistent address list
--add-addr-to-whitelist=addr Add addr to persistent address whitelist
--add-addr-to-blacklist=addr Add addr to persistent address blacklist
--remove-addr-from-whitelist=addr Remove addr from persistent address list
--force-expire Force a database expiry run
--status Display the status of the database
--clear Wipe out existing database
--backup Backup, to STDOUT, existing database
--restore <filename> Restore a database from filename
-u username,
--username=username Override username taken from the runtime environment
--mbox read in messages in mbox format
--mbx read in messages in UW mbx format
-f file,
--folders=file Read list of files/directories from file
-C path,
--configpath=path,
--config-file=path Path to standard configuration dir
-p prefs,
--prefspath=file,
--prefs-file=file Set user preferences file
--siteconfigpath=path Path for site configs (def: /etc/mail/spamassassin)
-D, --debug [area=n,...] Print debugging messages
-V, --version Print version
-h, --help Print usage message
sa-lint
-C path,
--configpath=path,
--config-file=path Path to standard configuration dir
-p prefs,
--prefspath=file,
--prefs-file=file Set user preferences file
--siteconfigpath=path Path for site configs (def: /etc/mail/spamassassin)
-D, --debug [area=n,...] Print debugging messages
-V, --version Print version
-h, --help Print usage message
finally there is:
spamassassin
This exec calls the appropriate executable depending on the arguments,
and provides backward compatability. We should issue a warn before
calling exec saying that the spamassassin command is deprecated and
they should use command X instead. Then in 3.2 spamassassin becomes
nothing but a high level doc.
Michael
Re: svn commit: r153377 - in spamassassin/trunk: MANIFEST Makefile.PL build/do spamassassin spamassassin.pod
Posted by Daniel Quinlan <qu...@pathname.com>.
Daniel Quinlan <qu...@pathname.com> writes:
>> "whitelist" and "blacklist" -- these *really* belong in "sa-learn" since
>> it's AWL/History, but "sa-learn" has been crufted up with lots of
>> Bayes-specific stuff, so merging it with "sa-learn" will be non-trivial.
Michael Parker <pa...@pobox.com> writes:
> Please no, I'm -1 on this, I've worked very hard to separate the
> whitelist/blacklist/awl stuff from bayes. sa-learn is bayes, period.
Hmm... I agree it's been crufted up, but think about this:
report calls learning stuff, so if you're going to report you don't
need to call learn or whitelist stuff
if you're not going to report, do you really want to require users to
call two commands?
Daniel
--
Daniel Quinlan
http://www.pathname.com/~quinlan/
Re: svn commit: r153377 - in spamassassin/trunk: MANIFEST Makefile.PL build/do spamassassin spamassassin.pod
Posted by Michael Parker <pa...@pobox.com>.
On Fri, Feb 11, 2005 at 08:02:08PM -0800, Daniel Quinlan wrote:
>
> Anyway, if we split out reporting, then things will need to be
> redesigned much more. The problem with the current design around
> "spamassassin" is that various behaviors are implemented as flags rather
> than svn-style subcommands or razor-style commands.
>
FYI, I'm completely +1 for doing away with the command line options
and going with either single commands or subcommand style (we can save
that discussion).
>
> Obviously, one command, the main command, is "filter".
>
> The second command is "remove". This could be its own command or it
> could possibly be added to "filter", but I think separate is probably
> better since it's very simple.
>
Keep this part of filter.
> "report" and "revoke" are the next commands, maybe together in one
> command, although, again, I prefer separate commands here.
>
One command, think of them as opposing commands, yin and yang so to
speak.
> "whitelist" and "blacklist" -- these *really* belong in "sa-learn" since
> it's AWL/History, but "sa-learn" has been crufted up with lots of
> Bayes-specific stuff, so merging it with "sa-learn" will be non-trivial.
>
Please no, I'm -1 on this, I've worked very hard to separate the
whitelist/blacklist/awl stuff from bayes. sa-learn is bayes, period.
sa-history. I suppose we might have to support sa-awl, but maybe we
can get away with "emulation" via spamassassin til we remove AWL.
A quick aside, the current History plugin design emulates
whitelist_from and blacklist_from (user added whitelist/blacklist
entries will not decay over time). Might be worth merging the two in
some capacity.
Michael
Re: svn commit: r153377 - in spamassassin/trunk: MANIFEST Makefile.PL build/do spamassassin spamassassin.pod
Posted by Daniel Quinlan <qu...@pathname.com>.
"Malte S. Stretz" <ms...@gmx.net> writes:
> There was a discussion going on on this mailinglist on which you could
> have asked if that what you committed would work. Which also included
> a suggestion by me to split out the reporting stuff which would need a
> spamassassin script (instead of a symlink) anyway.
I think you might be overreacting a bit. Waiting for a long email
discussion to finish while the installation was broken would not have
made much sense, especially since I was willing to burn my evening
trying to figure out the Makefiles.
Anyway, if we split out reporting, then things will need to be
redesigned much more. The problem with the current design around
"spamassassin" is that various behaviors are implemented as flags rather
than svn-style subcommands or razor-style commands.
sa-filter [options] [ < mailmessage | path ... ]
sa-filter -d [ < mailmessage | path ... ]
sa-filter -r [-w addr] [ < mailmessage | path ... ]
sa-filter -k [-w addr] [ < mailmessage | path ... ]
sa-filter -W|-R [ < mailmessage | path ... ]
This suggests several different possible breakdowns--I'm ignoring
whether or not it's "sa-foo" or "sa foo" and please just consider these
placeholder names.
Obviously, one command, the main command, is "filter".
The second command is "remove". This could be its own command or it
could possibly be added to "filter", but I think separate is probably
better since it's very simple.
"report" and "revoke" are the next commands, maybe together in one
command, although, again, I prefer separate commands here.
"whitelist" and "blacklist" -- these *really* belong in "sa-learn" since
it's AWL/History, but "sa-learn" has been crufted up with lots of
Bayes-specific stuff, so merging it with "sa-learn" will be non-trivial.
Basically, the command line tools are a confusing mess of disorganized
crap. Compare "svn", "cvs", even "apt-get".
> Right, as long as we don't want to test the spamassassin script/symlink.
Maybe.
> I'm not opposed to your approach, it didn't come to my mind, but if
> you had asked I could have told you a few cases where it doesn't work
> as you implemented it.
Could you could more specific?
> No, it doesn't work for vendor installs, just for perl and site. That's
> what all the I_FOO and B_FOO hackery in the Makefile.PL is for.
Completely underdocumented hackery.
> But it won't work on systems where the single quote is not supported.
Single quotes appear all over the Makefile.PL and the generated
Makefile. If this is an issue, it's also not documented and you did not
mention it recently.
> On the quoting of the dollar sign is also different on the platforms.
> That's the reason why I moved the logic into a separate script. (I
> think EU::MM has a function oneliner() for the correct quoting but I
> faintly remember some bug in there.)
If you're trying to paralyze me into inaction, it might be working, but
it might be better to document things more and suggest alternate
solutions instead of just raising issues with my implementation.
Sorry, I'm a bit frustrated here...
Daniel
--
Daniel Quinlan
http://www.pathname.com/~quinlan/
Re: svn commit: r153377 - in spamassassin/trunk: MANIFEST Makefile.PL build/do spamassassin spamassassin.pod
Posted by "Malte S. Stretz" <ms...@gmx.net>.
On Friday 11 February 2005 20:06 CET Daniel Quinlan wrote:
> "Malte S. Stretz" <ms...@gmx.net> writes:
> > *sigh* That commit is still wrong. Why don't you reply to the thread
> > on the list first?
>
> Because nothing was happening and most of the symlink code was too
> complicated and unnecessary. I wanted to get the code *working* and it
> definitively works better now. The symbolic link was never installed
> for the code that was there prior to my commits last night.
There was a discussion going on on this mailinglist on which you could have
asked if that what you committed would work. Which also included a
suggestion by me to split out the reporting stuff which would need a
spamassassin script (instead of a symlink) anyway.
> Benefits of the current code:
>
> - the symbolic link only needs to be installed; it does not need to
> be put under trunk/blib or at trunk/spamassassin
Right, as long as we don't want to test the spamassassin script/symlink.
I'm not opposed to your approach, it didn't come to my mind, but if you had
asked I could have told you a few cases where it doesn't work as you
implemented it.
> - it fixed the problem of spamassassin being installed under man3
> (Makefile assumes that files ending in .pod are man3, it is much
> easier and okay to just remove the .pod extension)
Cool.
> - it works for both vendor and site installs (perhaps there is a
> better way to *simply* determine the script installation directory,
> but I could not find it)
No, it doesn't work for vendor installs, just for perl and site. That's
what all the I_FOO and B_FOO hackery in the Makefile.PL is for.
> - it will work on systems that do not support symlink
But it won't work on systems where the single quote is not supported. On
the quoting of the dollar sign is also different on the platforms. That's
the reason why I moved the logic into a separate script. (I think EU::MM
has a function oneliner() for the correct quoting but I faintly remember
some bug in there.)
> - it employs much less hackery than before
Yeah, cool.
Cheers,
Malte
--
[SGT] Simon G. Tatham: "How to Report Bugs Effectively"
<http://www.chiark.greenend.org.uk/~sgtatham/bugs.html>
[ESR] Eric S. Raymond: "How To Ask Questions The Smart Way"
<http://www.catb.org/~esr/faqs/smart-questions.html>
Re: svn commit: r153377 - in spamassassin/trunk: MANIFEST Makefile.PL build/do spamassassin spamassassin.pod
Posted by Daniel Quinlan <qu...@pathname.com>.
"Malte S. Stretz" <ms...@gmx.net> writes:
> *sigh* That commit is still wrong. Why don't you reply to the thread
> on the list first?
Because nothing was happening and most of the symlink code was too
complicated and unnecessary. I wanted to get the code *working* and it
definitively works better now. The symbolic link was never installed
for the code that was there prior to my commits last night.
Benefits of the current code:
- the symbolic link only needs to be installed; it does not need to
be put under trunk/blib or at trunk/spamassassin
- it fixed the problem of spamassassin being installed under man3
(Makefile assumes that files ending in .pod are man3, it is much
easier and okay to just remove the .pod extension)
- it works for both vendor and site installs (perhaps there is a
better way to *simply* determine the script installation directory,
but I could not find it)
- it will work on systems that do not support symlink
- it employs much less hackery than before
Daniel
--
Daniel Quinlan
http://www.pathname.com/~quinlan/