You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@spamassassin.apache.org by bu...@bugzilla.spamassassin.org on 2005/02/24 12:21:38 UTC
[Bug 4153] New: Spamc patch for config file
http://bugzilla.spamassassin.org/show_bug.cgi?id=4153
Summary: Spamc patch for config file
Product: Spamassassin
Version: 3.0.2
Platform: PC
URL: http://maddenj.skynet.ie/useful/patches/spamc.patch
OS/Version: Linux
Status: NEW
Severity: enhancement
Priority: P1
Component: spamc/spamd
AssignedTo: dev@spamassassin.apache.org
ReportedBy: maddenj+spamassassin@skynet.ie
I wrote this patch because we (UL Computer Society -- www.skynet.ie) needed to
move spamd off one server onto another one, but we didn't want to have to change
all users' .procmailrc files to specify a host on the command line.
This patch is very Linux specific at the moment, but I'll work on it. It uses
/etc/mail/spamassassin/spamc.conf for the config file, and the only
configuration setting supported is hostname = server.tld, simply because that's
all we needed.
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
[Bug 4153] [review] RFE : Spamc patch for config file
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4153
------- Additional Comments From maddenj+spamassassin@skynet.ie 2005-05-12 09:59 -------
(In reply to comment #37)
> yeah, I initially left out spamc.conf accidentally, but then decided against
> bundling it anyway, since it's not something that's easy to install (where does
> it go? what if it overwrites existing config files on 'make install'?). given
> that it's not necessary by default, and the documentation is good enough to show
> people how to write one, I just left it out.
Cool. Just need to change spamc.pod so. It's got a reference to spamc/spamc.conf
in it.
Actually, looking now, the documentation for it isn't up to scratch (doesn't
mention the name or default location of the conf file, apart from referencing
the sample spamc/spamc.conf). I'll fix that up now, and attach the patch here.
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.
[Bug 4153] [review] RFE : Spamc patch for config file
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4153
------- Additional Comments From jm@jmason.org 2005-05-19 10:55 -------
the 'cd $srcdir/spamc; perl configure.pl && make' case isn't supported;
configure is only ever to be run from a top-level 'make'. so if that breaks, so
be it, people aren't supposed to be building it like that ;) I wouldn't worry
about that case at all.
regarding supporting this as a way to let people build spamc without spamd --
don't worry about that. in that case it's expected that they'd built the lot,
but just install the spamc files.
'I've just done some poking, and it's possible, using a preprocessor
macro, to define CONFIG_FILE to @sysconfdir@/mail/spamassassin/spamc.conf
_only_if_ the first character of @sysconfdir@ isn't '$', otherwise use
@prefix@/etc/spamc.conf. This can all be done before compilation, and so
will remove the need to check each time spamc is run. The major problem
would be if --sysconfdir isn't passed to configure, spamc (by default) is
installed in /usr/local; there's no easy way to check that so that the
default sysconfdirs can be used (ie. /etc/mail/spamassassin for /usr/*,
/etc/opt for /opt/* etc.).'
do we need to have any of that logic in spamc? can't that all be dealt with by
Makefile.PL, and it just passes in a single string for SYSCONFDIR, which spamc uses?
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.
[Bug 4153] [review] RFE : Spamc patch for config file
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4153
------- Additional Comments From jm@jmason.org 2005-05-10 09:36 -------
disappointing!
yeah, I guess the separate parser is the way to go, then; IMO it'd be best to
just use that for both config-file and CLI (instead of getopt) so at least we
can share that code.
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.
[Bug 4153] [review] RFE : Spamc patch for config file
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4153
------- Additional Comments From maddenj+spamassassin@skynet.ie 2005-05-19 11:19 -------
Subject: Re: [review] RFE : Spamc patch for config file
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On (19/05/05 10:55), bugzilla-daemon@bugzilla.spamassassin.org didst pronounce:
> the 'cd $srcdir/spamc; perl configure.pl && make' case isn't supported;
> configure is only ever to be run from a top-level 'make'. so if that
> breaks, so be it, people aren't supposed to be building it like that ;)
> I wouldn't worry about that case at all.
>
Ah, ok. That makes things immensely easier so. I was working on the
basis that this _would_ be a supported build method for spamc.
> do we need to have any of that logic in spamc? can't that all be dealt
> with by Makefile.PL, and it just passes in a single string for SYSCONFDIR,
> which spamc uses?
>
Yes. Without the need to support building spamc on its own, all this is
covered already. Makefile.PL passes the relevant sysconfdir to
spamc/configure, so it'll be statically set in spamc.h. This means the
config file is #define'd to a single location, and can only be changed
using the -F command line option, and no funky preprocessing is needed.
If people want to build spamc separately, and need a config file
somewhere else, it can be changed in spamc.h before compilation. If you
want to semi-support building spamc separately, the macro in spamc.h.in
would cover it (for a case where --sysconfdir isn't passed to configure
by the person compiling it) for a single scenario -- the config file
would be defined to ${prefix}/etc/spamc.conf. It's not a necessary
addition, but it doesn't hurt either.
Sorted so. Without the changes to spamc.h.in, patch #2879 covers the
above. With the changes to spamc.h.in in #2879, it also covers the case
where someone builds spamc as `perl configure.pl && make', and it sets the
default config to ${prefix}/etc/spamc.conf. Each is set at compile time,
so spamc doesn't do any extra searching for a config file. Either method
is fine with me, and will work fine for the reason I had in mind for the
patch in the first place.
And, just to confirm, Makefile.PL doesn't need any modification.
- --sysconfdir is passed to spamc/configure during the default (perl
Makefile.PL && make) build.
Any thoughts on the Windows case? Is it easiest to not support (by
default) a config for Windows? I can't see many (if any) large Windows
installations having need for a global config file for spamc anyway.
It's a simple hack for the user to add the config file support should
they want it.
- --
Chat ya later,
John.
- --
BOFH excuse #1: clock speed
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.0 (GNU/Linux)
iD8DBQFCjNqJQBw+ZtKOvTIRAiwfAJsEomqgDuIV1WZxw436Zctmt0faGwCfQcB1
Ez7RTeQm0w71nZ9Sg9evfxA=
=RxND
-----END PGP SIGNATURE-----
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.
[Bug 4153] [review] RFE : Spamc patch for config file
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4153
jm@jmason.org changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|RESOLVED |REOPENED
Resolution|FIXED |
------- Additional Comments From jm@jmason.org 2005-05-18 18:15 -------
'This is reset to NULL in the program if it contains '${prefix}' (ie.
spamc was compiled without passing --sysconfdir to configure). This is
where the code to search for a config file comes in.'
hmm. Makefile.PL should always be telling the code where sysconfdir is supposed
to be; if it's not passing that info to configure we should probably fix that ;)
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.
[Bug 4153] [review] RFE : Spamc patch for config file
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4153
------- Additional Comments From maddenj+spamassassin@skynet.ie 2005-05-10 15:18 -------
> any luck with the CLA? if it's in, I'll apply this.
Daniel has sent me his fax and postal address. I'll try the fax route tomorrow,
but if I can't get use of one, I'll post it.
I'll get a full patch together for this tomorrow aswell, and attach it to this
bug. That'll include the documentation, sample config, configure.in and
Makefile.PL changes needed. I haven't figured out the t/ script for it yet
though. I was going to use a simple config file, and test a connect to spamd on
a non-default port. This keeps running spamc with -d and -p arguments though, so
isn't working. It looks (from SATest.pm) that if spamc doesn't have either -U or
-p argument, it's given one.
I've just realised though, that I can use -h or -V in the test config, and not
need spamd at all, so I'll put that together tomorrow aswell.
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.
[Bug 4153] [review] RFE : Spamc patch for config file
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4153
------- Additional Comments From maddenj+spamassassin@skynet.ie 2005-05-10 16:09 -------
> and then change the script to run
>
> ok (spamcrun ("-F log/spamc_test.cf < data/spam/001", \&patterns_run_cb));
>
Running this using a test config with -U log/spamd.sock ends up with :
../spamc/spamc -d localhost -p 57398 -F data/spamc_test.cf < data/spam/001
I think it's because of the following check in SATest.pm
if($args !~ /\b(?:-p\s*[0-9]+|-U)\b/)
{
$spamcargs = "$spamc -d $spamdhost -p $spamdport $args";
}else
{
$spamcargs = "$spamc $args";
}
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.
[Bug 4153] [review] RFE : Spamc patch for config file
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4153
------- Additional Comments From automasschecker@jmason.org 2005-05-10 16:23 -------
Subject: Re: [review] RFE : Spamc patch for config file
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
bugzilla-daemon@bugzilla.spamassassin.org writes:
> http://bugzilla.spamassassin.org/show_bug.cgi?idA53
>
> ------- Additional Comments From maddenj+spamassassin@skynet.ie 2005-05-10 16:09 -------
> > and then change the script to run
> >
> > ok (spamcrun ("-F log/spamc_test.cf < data/spam/001", \&patterns_run_cb));
> >
>
> Running this using a test config with -U log/spamd.sock ends up with :
> ../spamc/spamc -d localhost -p 57398 -F data/spamc_test.cf < data/spam/001
>
> I think it's because of the following check in SATest.pm
>
> if($args !~ /\b(?:-p\s*[0-9]+|-U)\b/)
> {
> $spamcargs = "$spamc -d $spamdhost -p $spamdport $args";
> }else
> {
> $spamcargs = "$spamc $args";
> }
ick. yeah, I think it'd be fine to change that to accept '|-F|-U)'
instead of just '|-U)' to avoid that.
- --j.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.5 (GNU/Linux)
Comment: Exmh CVS
iD8DBQFCgUJBMJF5cimLx9ARAgMvAJ48iTYs3Clf1omL4D2uOex47iBMhQCfcsNV
V8xU8+nFq1uEvnwkbTlgb3k=
=VpMo
-----END PGP SIGNATURE-----
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.
[Bug 4153] Spamc patch for config file
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4153
jm@jmason.org changed:
What |Removed |Added
----------------------------------------------------------------------------
Target Milestone|Undefined |3.1.0
------- Additional Comments From jm@jmason.org 2005-04-13 10:11 -------
John noted (in bug 3934) that a CLA has been sent -- hasn't shown up in the ASF
committers list yet though.
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.
[Bug 4153] Spamc patch for config file
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4153
maddenj+spamassassin@skynet.ie changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #2666 is|0 |1
obsolete| |
------- Additional Comments From maddenj+spamassassin@skynet.ie 2005-02-25 09:43 -------
Created an attachment (id=2669)
--> (http://bugzilla.spamassassin.org/attachment.cgi?id=2669&action=view)
Updated patch
I've updated the patch to search for a config file in the following order :
/etc/mail/spamassassin/spamc.conf
/etc/spamassassin/spamc.conf
$HOME/.spamassassin/spamc.conf
../etc/mail/spamassassin/spamc.conf
../etc/spamassassin/spamc.conf
I have no idea where the configs are kept under Windows, so I haven't added any
code to find them yet.
I've also added a command line switch (-C) to specify a configuration file on
the command line.
This patch has been tested under Linux only.
If anyone has any ideas for other configuration options or just general fixes,
send them on to me.
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
[Bug 4153] [review] RFE : Spamc patch for config file
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4153
------- Additional Comments From maddenj+spamassassin@skynet.ie 2005-05-21 09:25 -------
Subject: Re: [review] RFE : Spamc patch for config file
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On (21/05/05 08:08), bugzilla-daemon@bugzilla.spamassassin.org didst pronounce:
> Unless a specific define as been set at compile time, spamc will never
> attempt to stat a config file, unless that config file was passed to it
> via -F <somefile>.
>
Understood - but should the #define be on or off by default? Ie., should
it #define CONFIG_FILE "@sysconfdir@/spamc.conf" or #define CONFIG_FILE
NULL by default?
> If so desired, the person who compiles spamc can set the define that
> will a) cause it to stat a single file in the system config directory
> (what the current proposed patch now allows for), or b) go search
> through several directories to find a config file (what the current code
> in trunk allows for), or c) use whatever was passed in via -F <somefile>.
>
Wait, I think I may have misunderstood you. What you're saying is
something like :
#define CONFIG_FILE "@sysconfdir@/spamc.conf" // default config
#define USE_CONFIG 0 // don't use a config file
#define USE_CONFIG 1 // use a single config file
#define USE_CONFIG 2 // search if default isn't found
Then, in spamc.c check which "option" was chosen and do what the admin
has requested during compilation. Have I got what you mean this time?
> My thinking is, why take the stat overhead if it's never going to be
> used? It should be easy enough to provide that functionality to a local
> admin if they so desire, I see the usefulness of allowing a config file
> in multiple places but its overhead is too great IMHO.
>
Yes, I agree. I never timed a run of spamc with the config patch to see
what kind of overhead it added. I really don't see the need for any
admin to need spamc.conf in multiple places, so while the functionality
for searching for a config _could_ be useful, maybe a prompt from
Makefile.PL to specify the location of a spamc.conf would be handier?
> I'll backtrack a little with the following thought, this does create a
> situation where one spamc will behave differently from another spamc,
> will this cause support nightmares? How do you tell how a particular
> spamc was built? Maybe -V should print out the compile time information
> (see mutt -v) as an example.
>
It really shouldn't behave very differently, since the config file is
parsed in the same way as the command line arguments. The main
difference will be whether or not a config file is being used. This
could add options that users don't know about (but, obviously, the admin
would), and could lead to misleading bug reports if a user reports
directly to SA instead of to the admin running their local SA install.
There'll also be the extra overhead of reading spamc.conf, if it's
enabled.
That said, it'd be easy enough to add to the -V argument, and state what
config option is being used, if any.
Let me know what you think, and I'll work on it. I'm away all next week
though, so I really only have today to work on it. Personally, I would
think patch #2879 should cover anything an admin would want, without the
overhead of searching for a config file.
- --
Chat ya later,
John.
- --
BOFH excuse #1: clock speed
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.0 (GNU/Linux)
iD8DBQFCj2K4QBw+ZtKOvTIRAvLoAJsHXUB7i2foyitHtTICFW+hG1+00QCeP2QX
/gfd9O0dP/0vdNX9R00UAhQ=
=26Pv
-----END PGP SIGNATURE-----
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.
[Bug 4153] [review] RFE : Spamc patch for config file
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4153
------- Additional Comments From maddenj+spamassassin@skynet.ie 2005-05-05 11:29 -------
> 1. the patch seems to change pretty much every line in spamc.c -- could you
> check why that is? perhaps use the "-b" switch to diff to ignore whitespace
> changes.
>
Ya, it removes spamc.c and adds spamc.c.in (most of the code is the exact same,
but I added -N to diff which "blanks" the removed file). That can be changed
easily enough. Main reason for it was to change the source tree into a sort of
"proposed" source tree with all the changes necessary.
> 2. no sign of the CLA yet :( -- any chance you could resend it via fax to the
> ASF? damn CLAs :(
>
I'll see if I can get use of a fax machine in the next few days.
> 3. 'The config file supports host, port, socket, max-size, ssl, bsmtp and
> report.' I'd suggest "-H", "-l", "-x" would also be useful.
>
It already supports "-x" -- just forgot to mention it :) Actually, I just
noticed a problem with the "-x" support (whereby turning it on explicitly in the
config will result it in being turned off!). Will fix that shortly.
> 4. btw, I think the alternative form of parsing is a little bit overkill -- it
> may be possible to refactor the getopt() block of code and reuse that instead.
> basically, change the config file format so it looks like:
>
> # this is a spamc config file
> # comment...
> -H
> # another comment...
> -U /var/run/spamd.sock
> -x
> [other options and comments]
>
> IOW, ignore comment lines, treat whitespace and newlines as separators between
> "words", split into an argv-like array of strings, and use that as input to an
> abstracted form of the getopt() function that doesn't work directly on
> argc/argv.
>
Ya, I understand what you're saying. I'll take a look and see how feasible it is.
> that way you avoid having two parsers, and two grammars in the documentation,
> for the same thing...
>
Well, usually CLI is simply "switches" where configs tend to use words to
represent each option. I'll look into it though, and see if getopt can be
persuaded to parse a config file!
> 5. the only thing it's missing is a "t" script to test it ;) that'd be
> pretty easy though -- copy spamd_unix.t and change it to create a config file
> with the path, instead of passing it via command line. if spamc can connect
> and scan a message, it read the config file correctly.
>
Will add that aswell then.
> 6. finally, yep, command line should take precedence over config file.
>
Ok. That's pretty much a one line change, just to read the config before
checking command line arguments. However, are there any arguments that an admin
may want to set globally and _NOT_ allow be overridden? If there is, there's two
options :
1) Don't allow only those options to be overridden (ie. if they're set in the
config file, they cannot be reset elsewhere -- just requires extra checking in
the command line parsing code - not a big deal)
2) Have a separate config option (AllowOverRide?) to either allow or disallow it
> looks pretty cool overall. I wonder where that damn CLA got to!
>
Ah, stuff gets lost! I'll send another one :)
Thanks for the feedback Justin. Always helpful :)
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.
[Bug 4153] [review] RFE : Spamc patch for config file
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4153
------- Additional Comments From parkerm@pobox.com 2005-05-11 19:07 -------
CLA Received
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.
[Bug 4153] [review] RFE : Spamc patch for config file
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4153
------- Additional Comments From maddenj+spamassassin@skynet.ie 2005-05-19 02:12 -------
Subject: Re: [review] RFE : Spamc patch for config file
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
> 'No, a define that specifies if it should go off and try to find the
> config file. That gives the local admin the power to do what you want
> without penalizing the folks that have no need.'
>
> ok, I can go for that.
>
> combine that with a fix for Makefile.PL and configure to ensure that
> sysconfdir is actually set during build, so that in almost all cases it
> will just attempt to open one file
> (${sysconfdir}/mail/spamassassin/spamc.conf), and I think we'd
> be good here. Michael?
>
Justin, Makefile.PL _always_ sets sysconfdir, but if spamc is built on
its own (ie. cd $srcdir/spamc; perl configure.pl && make) _then_
sysconfdir isn't set.
I also realised something last night -- I didn't do any work on making
it compile under Windows, so spamc.h.in never gets parsed into spamc.h,
which obviously means the vars aren't #define'd and spamc won't compile.
Basically, the extra code to search for spamc.conf was put in to cover
the outside case where spamc is built separate to spamd. So, what I'd
suggest is a prompt within configure.pl to run _only_ if configure.pl is
being run directly (and not being called via Makefile.PL). This prompt
asks where spamc.conf is located, with a default set of
$sysconfdir/mail/SA/spamc.conf (in much the same way as Makefile.PL
prompts for the email address to use), which can then be #define'd in
spamc.h.
I've just done some poking, and it's possible, using a preprocessor
macro, to define CONFIG_FILE to @sysconfdir@/mail/spamassassin/spamc.conf
_only_if_ the first character of @sysconfdir@ isn't '$', otherwise use
@prefix@/etc/spamc.conf. This can all be done before compilation, and so
will remove the need to check each time spamc is run. The major problem
would be if --sysconfdir isn't passed to configure, spamc (by default) is
installed in /usr/local; there's no easy way to check that so that the
default sysconfdirs can be used (ie. /etc/mail/spamassassin for /usr/*,
/etc/opt for /opt/* etc.).
So, do any of the above fit the bill? Either way, if this code is to
remain, the compilation for Windows has to be sorted out.
- --
Chat ya later,
John.
- --
BOFH excuse #158: Defunct processes
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.0 (GNU/Linux)
iD8DBQFCjFoxQBw+ZtKOvTIRAlrBAKCHi/kukhQeME4ZeoSVxd1UUcVEHQCeMfCk
/6jfGIjh2MkZ/9AZLYG/D/s=
=msii
-----END PGP SIGNATURE-----
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.
[Bug 4153] [review] RFE : Spamc patch for config file
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4153
maddenj+spamassassin@skynet.ie changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #2861 is|0 |1
obsolete| |
------- Additional Comments From maddenj+spamassassin@skynet.ie 2005-05-10 13:54 -------
Created an attachment (id=2862)
--> (http://bugzilla.spamassassin.org/attachment.cgi?id=2862&action=view)
CLI options via config file patch
Ignore the last one -- was missing an else {} bit in combine_args()
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.
[Bug 4153] [review] RFE : Spamc patch for config file
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4153
maddenj+spamassassin@skynet.ie changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #2862 is|0 |1
obsolete| |
------- Additional Comments From maddenj+spamassassin@skynet.ie 2005-05-11 05:44 -------
Created an attachment (id=2864)
--> (http://bugzilla.spamassassin.org/attachment.cgi?id=2864&action=view)
Patch against trunk r169632
This patch _includes_ changes to spamc/configure after running autoconf with
changes to configure.in
Another patch will follow which leaves out that section of the patch.
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.
[Bug 4153] [review] RFE : Spamc patch for config file
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4153
------- Additional Comments From jm@jmason.org 2005-05-23 00:10 -------
Michael, thanks!
John, nice benchmarking ;)
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.
[Bug 4153] Spamc patch for config file
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4153
maddenj+spamassassin@skynet.ie changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #2671 is|0 |1
obsolete| |
------- Additional Comments From maddenj+spamassassin@skynet.ie 2005-04-14 11:19 -------
Created an attachment (id=2793)
--> (http://bugzilla.spamassassin.org/attachment.cgi?id=2793&action=view)
Adds support for unix socket
For bug #3934
If the hostname in the config file starts with a '/', it's presumed to be a
unix socket, and set accordingly.
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.
[Bug 4153] [review] RFE : Spamc patch for config file
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4153
------- Additional Comments From parkerm@pobox.com 2005-05-18 17:09 -------
Subject: Re: [review] RFE : Spamc patch for config file
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
>
>
> That wouldn't fix the issue of a sysadmin wanting to set system wide defaults
> for users, without user intervention (which was my initial need for the config
> file).
>
How about a compile time define?
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.2 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org
iD8DBQFCi9kQG4km+uS4gOIRAuRXAJ9L245K7aTtClUocYfNFzg4XJFzjgCgiZCk
bLcEuoQG+htSOuskBflGWAI=
=bdGq
-----END PGP SIGNATURE-----
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.
[Bug 4153] [review] RFE : Spamc patch for config file
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4153
------- Additional Comments From jm@jmason.org 2005-05-05 11:44 -------
I get you about the spamc.c{,.in} change, that's what it was alright. here's
another option -- have a .h.in file that #defines strings for the @foo@ data,
and just #include that from spamc.c. that way we avoid rewriting the entire
file, which (a) scares me a bit and (b) is a bit messy for SVN history.
'Ok. That's pretty much a one line change, just to read the config before
checking command line arguments. However, are there any arguments that an admin
may want to set globally and _NOT_ allow be overridden? If there is, there's two
options :
1) Don't allow only those options to be overridden (ie. if they're set in the
config file, they cannot be reset elsewhere -- just requires extra checking in
the command line parsing code - not a big deal)
2) Have a separate config option (AllowOverRide?) to either allow or disallow it'
I don't think I'd worry about that just now, to be honest. if the admin needs
that much control, a setuid wrapper for spamc that strictly enforces their
policies in a custom form is probably a better way to do it.
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.
[Bug 4153] [review] RFE : Spamc patch for config file
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4153
------- Additional Comments From maddenj+spamassassin@skynet.ie 2005-05-18 17:19 -------
Subject: Re: [review] RFE : Spamc patch for config file
> How about a compile time define?
>
It's there already (spamc.h.in defines it as @SYSCONFDIR@/spamc.conf).
This is reset to NULL in the program if it contains '${prefix}' (ie.
spamc was compiled without passing --sysconfdir to configure). This is
where the code to search for a config file comes in.
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.
[Bug 4153] [review] RFE : Spamc patch for config file
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4153
------- Additional Comments From maddenj+spamassassin@skynet.ie 2005-05-19 08:05 -------
In relation to building spamc under Windows, is it enough to do the following
(instead of blindly #include "spamc.h") :
#ifndef _WIN32
#include "spamc.h"
#else
#define CONFIG_FILE NULL
#endif
This means a default config file isn't supported for Windows, and will only work
if -F is passed. If people want a default Windows config file, then where should
it be on the filesystem?
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.
[Bug 4153] [review] RFE : Spamc patch for config file
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4153
------- Additional Comments From parkerm@pobox.com 2005-05-18 14:52 -------
> only check for a config file if the -F command line switch is present;
> in other words, no default config file. that doesn't fix the reported
> bug, either.
This was actually my understanding of the patch, well, I didn't read it but
someone told me that was how it worked.
I'd be ok with that, don't do anything with a config file unless -F is passed in.
Michael
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.
[Bug 4153] Spamc patch for config file
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4153
maddenj+spamassassin@skynet.ie changed:
What |Removed |Added
----------------------------------------------------------------------------
AssignedTo|dev@spamassassin.apache.org |maddenj+spamassassin@skynet.
| |ie
------- Additional Comments From maddenj+spamassassin@skynet.ie 2005-02-28 10:52 -------
(In reply to comment #3)
> Subject: Re: Spamc patch for config file
>
> Why not using the "sysconfdir" variable provided by the configure script?
>
> Rename spamc.c to spamc.c.in, just write a test for "@sysconfdir@/spamc.conf"
> (and remove all those other unnecessary cases), add
> "AC_CONFIG_FILES([spamc.c])" at the end of configure.in and recreate
> the configure script with autoconf.
>
I've done all this (new patches to follow). However, in the case that spamc is
built on it's own (ie. not using Makefile.PL from the source base-dir), the
directory will need to be figured out. What I've done for this case is :
if ${prefix} starts with /usr, use /etc/mail/spamassassin/spamc.conf
if ${prefix} starts with /opt, use /etc/opt/mail/spamassassin/spamc.conf
else, use ${prefix}/etc/spamc.conf
> Why do you use a while-loop when you are looking for spamc.conf? What
> happens if "config_file" remains NULL because there is no such file?
>
I've removed the while loop, since it's no longer necessary with the above changes.
In the case of spamc not finding a config file, it returns the transport type to
TRANSPORT_LOCALHOST and sets the hostname to NULL, and will continue normally
using spamd on localhost.
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
[Bug 4153] Spamc patch for config file
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4153
jm@jmason.org changed:
What |Removed |Added
----------------------------------------------------------------------------
OtherBugsDependingO| |3934
nThis| |
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.
[Bug 4153] [review] RFE : Spamc patch for config file
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4153
jm@jmason.org changed:
What |Removed |Added
----------------------------------------------------------------------------
Target Milestone|3.2.0 |3.1.0
------- Additional Comments From jm@jmason.org 2005-05-11 18:36 -------
cool, I think that's good to go ;)
only one last thing: any news on the CLA?
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.
[Bug 4153] Spamc patch for config file
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4153
maddenj+spamassassin@skynet.ie changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |ASSIGNED
------- Additional Comments From maddenj+spamassassin@skynet.ie 2005-05-01 09:30 -------
I'm still waiting on word about the CLA, but in the meantime, I'll work on
incorporating most (if not all) the spamc command line options as config options
aswell.
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.
[Bug 4153] [review] RFE : Spamc patch for config file
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4153
jm@jmason.org changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|ASSIGNED |RESOLVED
Resolution| |FIXED
------- Additional Comments From jm@jmason.org 2005-05-11 20:11 -------
excellent! checked in as r169750.
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.
[Bug 4153] [review] RFE : Spamc patch for config file
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4153
------- Additional Comments From maddenj+spamassassin@skynet.ie 2005-05-12 01:55 -------
(In reply to comment #35)
> excellent! checked in as r169750.
Justin, the checkin left out spamc/spamc.conf.
If this is on purpose, then spamc.pod needs to be changed to remove the
reference to it.
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.
[Bug 4153] [review] RFE : Spamc patch for config file
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4153
maddenj+spamassassin@skynet.ie changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #2838 is|0 |1
obsolete| |
------- Additional Comments From maddenj+spamassassin@skynet.ie 2005-05-10 13:50 -------
Created an attachment (id=2861)
--> (http://bugzilla.spamassassin.org/attachment.cgi?id=2861&action=view)
CLI options via config file patch
This implements Justin's idea of using command line switches in the config
file, and then using the same (getopt) parser for both sets of options.
Command line can override anything in config file by just adding the argv[]
options after the config file ones.
I've ommitted spamc.h.in from this, but that just #defines CONFIG_FILE and
PREFIX.
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.
[Bug 4153] [review] RFE : Spamc patch for config file
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4153
------- Additional Comments From parkerm@pobox.com 2005-05-21 08:08 -------
Subject: Re: [review] RFE : Spamc patch for config file
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
I think things are going in two different paths, closely related, but
still different.
Let me make restate how I believe things should work.
Unless a specific define as been set at compile time, spamc will never
attempt to stat a config file, unless that config file was passed to it
via -F <somefile>.
If so desired, the person who compiles spamc can set the define that
will a) cause it to stat a single file in the system config directory
(what the current proposed patch now allows for), or b) go search
through several directories to find a config file (what the current code
in trunk allows for), or c) use whatever was passed in via -F <somefile>.
My thinking is, why take the stat overhead if it's never going to be
used? It should be easy enough to provide that functionality to a local
admin if they so desire, I see the usefulness of allowing a config file
in multiple places but its overhead is too great IMHO.
I'll backtrack a little with the following thought, this does create a
situation where one spamc will behave differently from another spamc,
will this cause support nightmares? How do you tell how a particular
spamc was built? Maybe -V should print out the compile time information
(see mutt -v) as an example.
BTW, for those wanting to run a benchmark, here is the benchmark code:
#!/usr/bin/perl -w
use strict;
use Benchmark qw(:all);
timethese(10000, {
'spamc.old' => '`/usr/bin/spamc.old < /dev/null`',
'spamc.new' => '`/usr/bin/spamc < /dev/null`',
});
spamc.old was just copied over from spamc and the new version was
installed in place.
Michael
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.2 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org
iD8DBQFCj07gG4km+uS4gOIRAgcxAJwKgSqS88lksZz5gdkDs8iMUKzhsgCfbFP0
CezZWdQ+vjA5n2E5zSYDTSc=
=dZ+E
-----END PGP SIGNATURE-----
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.
[Bug 4153] [review] RFE : Spamc patch for config file
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4153
------- Additional Comments From maddenj+spamassassin@skynet.ie 2005-05-11 05:48 -------
Created an attachment (id=2865)
--> (http://bugzilla.spamassassin.org/attachment.cgi?id=2865&action=view)
Patch against trunk r169632 (without configure)
This patch has all the changes needed, but without running autoconf in spamc/.
In other words, trunk won't build without running autoconf in spamc/. Previous
patch (#2864) should leave trunk in a buildable state without needing to run
autoconf.
Note :: I changed SATest.pm to allow -F as a single argument to spamc, so -d
and -p aren't added. The test passes for me here (Debian AMD64).
There was also a minor change to read_args to accept -F as an argument, so
getopt() didn't complain when it came across it. Other than that, it does
nothing.
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.
[Bug 4153] Spamc patch for config file
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4153
------- Additional Comments From klaus.heinz@onlinehome.de 2005-02-27 10:50 -------
Subject: Re: Spamc patch for config file
bugzilla-daemon@bugzilla.spamassassin.org wrote:
> I've updated the patch to search for a config file in the following order :
> /etc/mail/spamassassin/spamc.conf
> /etc/spamassassin/spamc.conf
> $HOME/.spamassassin/spamc.conf
> ../etc/mail/spamassassin/spamc.conf
> ../etc/spamassassin/spamc.conf
Why not using the "sysconfdir" variable provided by the configure script?
Rename spamc.c to spamc.c.in, just write a test for "@sysconfdir@/spamc.conf"
(and remove all those other unnecessary cases), add
"AC_CONFIG_FILES([spamc.c])" at the end of configure.in and recreate
the configure script with autoconf.
Why do you use a while-loop when you are looking for spamc.conf? What
happens if "config_file" remains NULL because there is no such file?
ciao
Klaus
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
[Bug 4153] [review] RFE : Spamc patch for config file
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4153
------- Additional Comments From jm@jmason.org 2005-05-19 00:19 -------
'How about an environment variable that will merge with the command line
options? Fairly traditional trick for compilers.'
nah, last thing I want to do is add yet *another* way for options to be set...
that'd really confuse matters.
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.
[Bug 4153] Spamc patch for config file
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4153
maddenj+spamassassin@skynet.ie changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #2793 is|0 |1
obsolete| |
------- Additional Comments From maddenj+spamassassin@skynet.ie 2005-05-04 07:21 -------
Created an attachment (id=2838)
--> (http://bugzilla.spamassassin.org/attachment.cgi?id=2838&action=view)
Updated spamc config file patch
This is a cumulative patch against current trunk (r168130). It does the
following :
patches Makefile.PL (remove spamc.c on make clean)
patches spamc/configure.in (AC_CONFIG_FILES([spamc.c]))
patches spamc/configure (changes from above, after running autoconf)
removes spamc/spamc.c
creates spamc/spamc.c.in
creates spamc/spamc.conf (sample config file)
patches spamc/spamc.pod (documentation for config file)
It builds cleanly, and seems to work ok (at least, it has for the tests I've
run on it). The only problem, which is really only cosmetic and I couldn't
figure out Makefile.PL to fix, is that it complains about spamc.c being missing
after running perl Makefile.PL. This is on a debian-amd64 machine (sarge).
The config file supports host, port, socket, max-size, ssl, bsmtp and report.
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.
[Bug 4153] [review] RFE : Spamc patch for config file
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4153
------- Additional Comments From jm@jmason.org 2005-05-10 00:20 -------
'Of course, that probably makes startup be a can of worms, especially if one
of the command line options is the location of the config file to
override...'
btw, in case John hasn't seen how to deal with that yet -- the way to get around
that chicken+egg problem is to do
someoption = UNSET
process command line opts:
if command-line opt found) {
someoption = value
}
process config-file opts:
if (config-file opt found && someoption != UNSET) {
ignore it, command-line set it
} else {
someoption = value
}
finally, set defaults:
if (someoption == UNSET) {
someoption = defaultvalue
}
in other words, use some way to indicate that the value was not set by a
previous step.
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.
[Bug 4153] [review] RFE : Spamc patch for config file
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4153
------- Additional Comments From parkerm@pobox.com 2005-05-10 10:35 -------
Subject: Re: [review] RFE : Spamc patch for config file
If someone wanted to, I wouldn't mind if we could get long options (ie
--whatever) added to the command line parser.
It would make adding options to spamc much easier.
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.
[Bug 4153] [review] RFE : Spamc patch for config file
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4153
------- Additional Comments From lwilton@earthlink.net 2005-05-06 04:41 -------
Subject: Re: [review] RFE : Spamc patch for config file
> First major question : should the global config file override user
command-line
> options?
Thinking from the user viewpoint, the config file is more static than
command line options, so logically the command line should override the
config file, not the other way around.
Of course, that probably makes startup be a can of worms, especially if one
of the command line options is the location of the config file to
override...
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.
[Bug 4153] [review] RFE : Spamc patch for config file
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4153
------- Additional Comments From maddenj+spamassassin@skynet.ie 2005-05-10 10:11 -------
(In reply to comment #20)
> disappointing!
>
> yeah, I guess the separate parser is the way to go, then; IMO it'd be best to
> just use that for both config-file and CLI (instead of getopt) so at least we
> can share that code.
Only other option I can think of is to generate our own argc / argv pair from
the CLI and config opts, and pass that to getopt. This would require awkward
parsing of all options first though, to make sure duplicates aren't passed in.
Of course, this would mean that checking _after_ option parsing isn't necessary,
since no duplicates were passed in in the first place. Having not tested this,
I'm not sure how it'd work out though.
The easiest option, for the moment anyway, is the two separate parsers. Any
preferences on which one I should work on to get working?
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.
[Bug 4153] [review] RFE : Spamc patch for config file
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4153
------- Additional Comments From maddenj+spamassassin@skynet.ie 2005-05-18 15:42 -------
> This was actually my understanding of the patch, well, I didn't read it but
> someone told me that was how it worked.
>
> I'd be ok with that, don't do anything with a config file unless -F is
> passed in.
>
That wouldn't fix the issue of a sysadmin wanting to set system wide defaults
for users, without user intervention (which was my initial need for the config
file).
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.
[Bug 4153] Spamc patch for config file
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4153
parkerm@pobox.com changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |dev@spamassassin.apache.org
------- Additional Comments From parkerm@pobox.com 2005-02-28 10:55 -------
John, please make sure to keep dev@spamassassin.apache.org in the loop (ie CC)
when you assign a bug away from that address. Otherwise we lose visability on
the dev list.
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.
[Bug 4153] Spamc patch for config file
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4153
------- Additional Comments From maddenj+spamassassin@skynet.ie 2005-02-24 03:23 -------
Created an attachment (id=2666)
--> (http://bugzilla.spamassassin.org/attachment.cgi?id=2666&action=view)
patch for spamc (SA version 3.0.2)
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
[Bug 4153] [review] RFE : Spamc patch for config file
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4153
------- Additional Comments From parkerm@pobox.com 2005-05-18 14:51 -------
Transfering some the discussion from the dev list:
>From parkerm@pobox.com:
ust an FYI, I'm considering a veto on the spamc config file patch.
Admittedly, I got very busy and wasn't able to review the patch as
closely as I would have liked. This is a feature that will NOT be used
by 99% of the spamc users and it was my understanding for those 99% it
was going to be a NOP with nothing done at all with the config file (not
even a half dozen checks to see if the file exists).
Now, it looks like the functionality has switched to always on and
always checking for the existence of the config file. I understand the
usefulness of the feature but it should not affect everyone else using
spamc. At the very least, we shouldn't be issuing an error when the
config file can't be found, talk about a support nightmare, everyone and
their dog will be filing bugs, asking on the mailing lists and in IRC
about that file.
Like I said, I'm about -.9 on this patch at the moment, but am willing
to be convinced that it's the right thing to do going forward.
Michael
>From John Madden:
I suppose since I want the patch in, I should follow up on this.
The logging can be turned off or require debugging or something - that's
not a problem. I can't see an easy way around searching for the log file
though, unless it's a compile time variable. Currently it's a bit of
both : if SA is built through the usual perl Makefile.PL && make
scenario, then the config file is #define'd and doesn't need to be
searched for. If it doesn't exist, operation continues as normal. If
spamc is built on its own, without --sysconfdir passed to configure,
then @SYSCONFDIR@ contains ${prefix}, which is why the extra parsing and
searching is needed.
The checks can be reduced, so that if a _single_ file doesn't exist,
then continue processing and don't look anywhere else. It can even be
removed completely, and if @SYSCONFDIR@ contains ${prefix}, then don't
search for a config at all, and skip it completely.
I think its uses for the 1% outweigh the negative for the 99%. If it's
over-searching or over-logging, then that code can all be removed, and
leave a simple single search for one config file in its place. I know
for our setup, a global config file would be very useful. As it is, I'm
running a patched version of spamc to point to a host other than
localhost, so we don't have to force around 400 users to change their
.procmailrc files. This feature, even if not the exact patch submitted,
has definite uses, and I'd definitely like to see it in a future
release.
>From Michael Parker:
Here is a quick benchmark to illustrate my point, spamc.old was the revision
immediately before the conf patch, spamc.new is the conf patch revision:
Benchmark: timing 10000 iterations of spamc.new, spamc.old...
spamc.new: 97 wallclock secs ( 0.61 usr 2.70 sys + 14.37 cusr 26.04 csys =
43.72 CPU) @ 3021.15/s (n=10000)
spamc.old: 39 wallclock secs ( 0.48 usr 2.36 sys + 13.73 cusr 23.52 csys =
40.09 CPU) @ 3521.13/s (n=10000)
Michael
>From Justin Mason:
ouch. that *is* slow.
OK, we have the following options imo:
- leave it that it will look for a default config file in one location
only, with one stat() operation. This is my preferred option.
- same as above, with a possible $SPAMC_CONFIG env variable allowed
to set the location. still only one stat() op, so it's fast,
but it does allow people with odd installs to specify the file
location, if that's a desirable feature. in my opinion, this isn't
really required, since they can just use -F.
- remove the code. I'm not too happy with this, since it's a long-desired
feature and fixes another reported bug.
- only check for a config file if the -F command line switch is present;
in other words, no default config file. that doesn't fix the reported
bug, either.
ps: yes, that "config file cannot be found" warning has to go; there's a
bug open about that already.
pps: would probably have been better to discuss this on the bugzilla entry
btw.
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.
[Bug 4153] [review] RFE : Spamc patch for config file
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4153
------- Additional Comments From maddenj+spamassassin@skynet.ie 2005-05-10 05:31 -------
> 4. btw, I think the alternative form of parsing is a little bit overkill -- it
> may be possible to refactor the getopt() block of code and reuse that instead.
> basically, change the config file format so it looks like:
>
It's possible to use getopt to do one or the other -- CLI or config. However, it
can't do both apparently. I put together a simple test to print one option from
a config file, and another from CLI. It will print the CLI one if it's provided,
and the config one if the CLI is not set. Using the same kind of test with spamc
(after patching it to treat the config file as CLI switches), it segfaults when
given more than one option (ie. I pass it a switch to point to the config file,
and anyother switch, and it segfaults).
I suspect the easiest way is to go with the separate parser, with extra checking
to make sure the option hasn't been set already.
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.
[Bug 4153] [review] RFE : Spamc patch for config file
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4153
maddenj+spamassassin@skynet.ie changed:
What |Removed |Added
----------------------------------------------------------------------------
Summary|Spamc patch for config file |[review] RFE : Spamc patch
| |for config file
------- Additional Comments From maddenj+spamassassin@skynet.ie 2005-05-05 09:05 -------
Looking for some feedback on this, in case there's more to be done.
First major question : should the global config file override user command-line
options? Currently (with latest patch - #2838) read_config runs after read_args,
and doesn't check any of the variables before setting them to the config values.
So, if a user uses a command line argument for any setting configured in the
config file, it gets overridden by the config value.
Secondly, are there any extra config options that would be useful? Or, are there
any that are there that shouldn't be, or that are useless?
Any feedback graciously accepted :)
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.
[Bug 4153] [review] RFE : Spamc patch for config file
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4153
------- Additional Comments From jm@jmason.org 2005-05-12 09:54 -------
yeah, I initially left out spamc.conf accidentally, but then decided against
bundling it anyway, since it's not something that's easy to install (where does
it go? what if it overwrites existing config files on 'make install'?). given
that it's not necessary by default, and the documentation is good enough to show
people how to write one, I just left it out.
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.
[Bug 4153] [review] RFE : Spamc patch for config file
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4153
parkerm@pobox.com changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|REOPENED |RESOLVED
Resolution| |FIXED
------- Additional Comments From parkerm@pobox.com 2005-05-22 09:05 -------
Ok, I'm convinced. I changed a couple of other things and left the CONFIG_FILE
define at @sysconfdir@/spamc.conf (we can add support of allowing it to be
changed in a default install later if folks see the need). I also added a
warning if a user defined config file was not found.
Committed revision 171339.
I believe this should make everyone happy and obviously removes my near veto.
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.
[Bug 4153] [review] RFE : Spamc patch for config file
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4153
------- Additional Comments From maddenj+spamassassin@skynet.ie 2005-05-19 04:28 -------
Created an attachment (id=2879)
--> (http://bugzilla.spamassassin.org/attachment.cgi?id=2879&action=view)
Speed up spamc + config
I haven't got a collection of spam messages, so I timed this running (in
t/data/spam/) :
time for i in `echo *`;do cat $i | spamc;done
spamc above was replaced with current trunk, current trunk with attached patch,
and r169749 (pre-config revision). Times were :
Current trunk : 57.597s
Current trunk w/patch : 29.895s
r169749 : 31.607s
I restarted spamd between each, so nothing was cached. spamc from each release
was built with : cd $srcdir/spamc; perl configure.pl && make
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.
[Bug 4153] [review] RFE : Spamc patch for config file
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4153
------- Additional Comments From jm@jmason.org 2005-05-05 10:44 -------
a couple of things:
1. the patch seems to change pretty much every line in spamc.c -- could you
check why that is? perhaps use the "-b" switch to diff to ignore whitespace
changes.
2. no sign of the CLA yet :( -- any chance you could resend it via fax to the
ASF? damn CLAs :(
3. 'The config file supports host, port, socket, max-size, ssl, bsmtp and
report.' I'd suggest "-H", "-l", "-x" would also be useful.
4. btw, I think the alternative form of parsing is a little bit overkill -- it
may be possible to refactor the getopt() block of code and reuse that instead.
basically, change the config file format so it looks like:
# this is a spamc config file
# comment...
-H
# another comment...
-U /var/run/spamd.sock
-x
[other options and comments]
IOW, ignore comment lines, treat whitespace and newlines as separators between
"words", split into an argv-like array of strings, and use that as input to an
abstracted form of the getopt() function that doesn't work directly on argc/argv.
that way you avoid having two parsers, and two grammars in the documentation,
for the same thing...
If you do it this way, then #3 isn't a problem -- just parse all the switches
equally in both file and command-line!
5. the only thing it's missing is a "t" script to test it ;) that'd be pretty
easy though -- copy spamd_unix.t and change it to create a config file with the
path, instead of passing it via command line. if spamc can connect and scan a
message, it read the config file correctly.
6. finally, yep, command line should take precedence over config file.
looks pretty cool overall. I wonder where that damn CLA got to!
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.
[Bug 4153] [review] RFE : Spamc patch for config file
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4153
------- Additional Comments From jm@jmason.org 2005-05-10 10:31 -------
'The easiest option, for the moment anyway, is the two separate parsers. Any
preferences on which one I should work on to get working?'
I would go with one that does the same thing as the getopt() code currently
does. In other words use the command-line switch style in the config file as
well, so the parsing code can be shared as much as possible. as mentioned in
http://bugzilla.spamassassin.org/show_bug.cgi?id=4153#c13 point 4.
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.
[Bug 4153] [review] RFE : Spamc patch for config file
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4153
------- Additional Comments From parkerm@pobox.com 2005-05-18 18:17 -------
Subject: Re: [review] RFE : Spamc patch for config file
> It's there already (spamc.h.in defines it as @SYSCONFDIR@/spamc.conf).
> This is reset to NULL in the program if it contains '${prefix}' (ie.
> spamc was compiled without passing --sysconfdir to configure). This is
> where the code to search for a config file comes in.
No, a define that specifies if it should go off and try to find the
config file. That gives the local admin the power to do what you want
without penalizing the folks that have no need.
Michael
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.
[Bug 4153] Spamc patch for config file
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4153
maddenj+spamassassin@skynet.ie changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #2669 is|0 |1
obsolete| |
------- Additional Comments From maddenj+spamassassin@skynet.ie 2005-02-28 10:59 -------
Created an attachment (id=2671)
--> (http://bugzilla.spamassassin.org/attachment.cgi?id=2671&action=view)
Updated spamc configuration file patch
spamc.c needs to be renamed to spamc.c.in after this patch has been applied,
and AC_CONFIG_FILES([spamc.c]) needs to be added to the end of configure.in,
and run autoconf to rebuild the configure script.
I have repackaged SA with these changes (and one other to Makefile.PL to remove
spamc.c on make clean), downloadable from
http://maddenj.skynet.ie/useful/patches/Mail-Spamassassin-3.0.2-spamc-config.tar.gz
or the patches are available individually :
http://maddenj.skynet.ie/useful/patches/spamc.patch
http://maddenj.skynet.ie/useful/patches/Makefile.PL.patch (one liner)
http://maddenj.skynet.ie/useful/patches/configure.in.patch (one liner)
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.
[Bug 4153] Spamc patch for config file
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4153
quinlan@pathname.com changed:
What |Removed |Added
----------------------------------------------------------------------------
Target Milestone|3.1.0 |3.2.0
------- Additional Comments From quinlan@pathname.com 2005-04-30 20:15 -------
punting to 3.2.0
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.
[Bug 4153] [review] RFE : Spamc patch for config file
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4153
------- Additional Comments From lwilton@earthlink.net 2005-05-18 18:17 -------
Subject: Re: [review] RFE : Spamc patch for config file
> > That wouldn't fix the issue of a sysadmin wanting to set system wide
defaults
> > for users, without user intervention (which was my initial need for the
config
> > file).
How about an environment variable that will merge with the command line
options? Fairly traditional trick for compilers.
Loren
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.
[Bug 4153] [review] RFE : Spamc patch for config file
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4153
------- Additional Comments From maddenj+spamassassin@skynet.ie 2005-05-22 03:28 -------
Subject: Re: [review] RFE : Spamc patch for config file
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On (21/05/05 23:53), bugzilla-daemon@bugzilla.spamassassin.org didst pronounce:
> By default, CONFIG_FILE will be NULL. We need some Makefile.PL magic to allow
> you to specify (it's not gonna ask) what the default config file should be:
> perl Makefile.PL SPAMC_CONFIG_FILE=/etc/mail/spamassassin/spamc.conf
>
I just ran a benchmark (using the code you posted here -- thanks :) ).
With your patch, and CONFIG_FILE == NULL by default, it's only 1 second
quicker over 10000 iterations than with CONFIG_FILE ==
"@sysconfdir@/spamc.conf" (ie. with my patch). I did make some changes
to the version patched with my patch, mainly changed the order of the
combine_args call to the same layout as in your patch. Essentially, both
patches leave the code in the same state and just change the defined
CONFIG_FILE. I didn't have a spamc.conf in @sysconfdir@ for the
benchmark, so both benchmarks tested default behaviour.
That said, I've no problem with this feature being off by default.
Here's the benchmark output :
Benchmark: timing 10000 iterations of spamc.me, spamc.parker...
spamc.me: 61 wallclock secs ( 0.65 usr 1.71 sys + 36.36 cusr 19.63
csys = 58.35 CPU) @ 4237.29/s (n=10000)
spamc.parker: 60 wallclock secs ( 0.60 usr 1.76 sys + 36.26 cusr
19.43 csys = 58.05 CPU) @ 4237.29/s (n=10000)
- --
Chat ya later,
John.
- --
BOFH excuse #1: clock speed
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.0 (GNU/Linux)
iD8DBQFCkGCCQBw+ZtKOvTIRAsS9AKCMngf0LH0C7m85PtyCb1KoBTNkNwCdG0S5
PZFTYAvVqjV4G44qiDv+mec=
=h7jz
-----END PGP SIGNATURE-----
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.
[Bug 4153] [review] RFE : Spamc patch for config file
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4153
------- Additional Comments From jm@jmason.org 2005-05-10 15:26 -------
btw on the test script thing -- just copy one of the existing ones (I'd suggest
spamd_unix.t) and change it so the command-line switches it runs spamc with are
loaded from a config file, instead of from the command line. e.g.:
ok (spamcrun ("-U log/spamd.sock < data/spam/001", \&patterns_run_cb));
you'd want to create a file "log/spamc_test.cf" containing
-U log/spamd.sock
and then change the script to run
ok (spamcrun ("-F log/spamc_test.cf < data/spam/001", \&patterns_run_cb));
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.
[Bug 4153] [review] RFE : Spamc patch for config file
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4153
------- Additional Comments From parkerm@pobox.com 2005-05-21 23:53 -------
Created an attachment (id=2890)
--> (http://bugzilla.spamassassin.org/attachment.cgi?id=2890&action=view)
Another option, minus the necessary Makefile.PL portion
Here is my vision. It's missing one piece. Basically, if CONFIG_FILE == NULL
then we aren't going to do anything, unless you specify a config file with -F.
By default, CONFIG_FILE will be NULL. We need some Makefile.PL magic to allow
you to specify (it's not gonna ask) what the default config file should be:
perl Makefile.PL SPAMC_CONFIG_FILE=/etc/mail/spamassassin/spamc.conf
Someone want to do the Makefile.PL/configure/etc magic?
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.
[Bug 4153] [review] RFE : Spamc patch for config file
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4153
------- Additional Comments From maddenj+spamassassin@skynet.ie 2005-05-12 10:08 -------
> Actually, looking now, the documentation for it isn't up to scratch (doesn't
> mention the name or default location of the conf file, apart from referencing
> the sample spamc/spamc.conf). I'll fix that up now, and attach the patch here.
Scratch all this. I just noticed that spamc.pod has been updated from the one I
posted here, and all looks well.
I think this is truly closed now :)
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.
[Bug 4153] [review] RFE : Spamc patch for config file
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4153
------- Additional Comments From jm@jmason.org 2005-05-19 00:24 -------
'No, a define that specifies if it should go off and try to find the
config file. That gives the local admin the power to do what you want
without penalizing the folks that have no need.'
ok, I can go for that.
combine that with a fix for Makefile.PL and configure to ensure that sysconfdir
is actually set during build, so that in almost all cases it will just attempt
to open one file (${sysconfdir}/mail/spamassassin/spamc.conf), and I think we'd
be good here. Michael?
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.
[Bug 4153] [review] RFE : Spamc patch for config file
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4153
------- Additional Comments From maddenj+spamassassin@skynet.ie 2005-05-05 12:19 -------
> I don't think I'd worry about that just now, to be honest. if the admin needs
> that much control, a setuid wrapper for spamc that strictly enforces their
> policies in a custom form is probably a better way to do it.
Ya, you're probably right. I was mainly thinking of the "user" option here,
where an admin may want all checking to be done under a specific account, and
it'd be easy to implement through the config. I can't really think of any others
that would need this sort of control though (maybe bsmtp and ssl?). I'm still
not fully up-to-speed on the SA way of things though, so I'll go with what the
longer-term developers think.
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.
[Bug 4153] [review] RFE : Spamc patch for config file
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4153
------- Additional Comments From jm@jmason.org 2005-05-10 14:24 -------
excellent! +1, looks good from here.
any luck with the CLA? if it's in, I'll apply this.
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.