Milter-greylist

Here is our alpha6

ftp://ftp.espci.fr/pub/milter-greylist/milter-greylist-4.0a6.tgz
MD5 (milter-greylist-4.0a6.tgz) = fb469a3947a6821519a62644a4128Â27

Changes since alpha5:
        Avoid GeoIP reporting "--" for broken domain (Bernhard Schneider )
        Fix run-away string (AIDA Shinra)
        Check for libbind9 if libbind cannot be found (Fabien Tassin)
        Document ACL on TLS DN (Fabien Tassin)
        Avoid spurious exit on DATA stage ACL match for multiple recipients

-- 
Emmanuel Dreyfus
http://hcpnet.free.fr/pubz
manu@...

According to manu@...:
> 
> Here is our alpha6
> 
> ftp://ftp.espci.fr/pub/milter-greylist/milter-greylist-4.0a6.tgz
> MD5 (milter-greylist-4.0a6.tgz) = fb469a3947a6821519a62644a4128\ufffd27
> 
> Changes since alpha5:
>         Avoid GeoIP reporting "--" for broken domain (Bernhard Schneider )
>         Fix run-away string (AIDA Shinra)
>         Check for libbind9 if libbind cannot be found (Fabien Tassin)
>         Document ACL on TLS DN (Fabien Tassin)
>         Avoid spurious exit on DATA stage ACL match for multiple recipients

there's an issue with configure. I've already faced it with libbind, that why
I've initially proposed the patch mentionned above, but it's a more general
issue.

On a new server, I've compiled m-g as I usually do, so on my cmd line,
I used (amongst other things) --with-libcurl=/usr.

Configure stopped there:

checking for pthread_create in -lpthread... no
checking for pthread_create in -lc_r... no
checking for pthread_create in -lpthreads -lpthread... no
Required libpthread not found. Use --with-libpthread

that was unexpected on Debian.
Looking at config.log, I found out this has nothing to do this pthread:

==
configure:5632: checking for pthread_create in -lpthread
configure:5662: i486-linux-gnu-gcc -o conftest -Wall -g -O2 -Wall -I/usr/include -I/usr/include -DUSE_CURL  -Wl,-z,defs -L/usr/lib -Wl,--rpath=/usr/lib -L/usr/lib -Wl,--rpath=/usr/lib conftest.c -lpthread  -lcurl  >&5
/usr/bin/ld: cannot find -lcurl
collect2: ld returned 1 exit status
==

I just don't have libcurl on that box. That's my fault but configure should
behave differently. It's the same thing with other optionnal features.

Instead of providing a patch that moves that libcurl test before the
pthreads tests, I think it's wise to take a step back and think.

With the current model, it could be tricky to find the right order of tests
to have something robust.
We could also change the model a little bit and test the optional features
independently.

What do you think ? which way is preferable ?

/Fabien

Fabien Tassin <fta+miltergreylist@...> wrote:

> We could also change the model a little bit and test the optional features
> independently.

That seems a good solution. Are you going to work on that change?

-- 
Emmanuel Dreyfus
http://hcpnet.free.fr/pubz
manu@...

According to manu@...:
> 
> > We could also change the model a little bit and test the optional features
> > independently.
> 
> That seems a good solution. Are you going to work on that change?

once I have time, yes. Unless someone beat me to it :)

/Fabien

According to Fabien Tassin:
> 
> According to manu@...:
> > 
> > > We could also change the model a little bit and test the optional features
> > > independently.
> > 
> > That seems a good solution. Are you going to work on that change?
> 
> once I have time, yes. Unless someone beat me to it :)

Just had a look. Seems easy but current code is a little bit messy.
Some tests are performed twice, and lib tests are spread everywhere.
Would you accept a patch that moves blocs around ?
(I know some reject that considering it's cosmetic).

I'd like to add a few comments in there too (1 line explaining what the
next bloc is about). Do you mind ?
 
/Fabien

Fabien Tassin <fta+miltergreylist@...> wrote:

> Just had a look. Seems easy but current code is a little bit messy.
> Some tests are performed twice, and lib tests are spread everywhere.
> Would you accept a patch that moves blocs around ?
> (I know some reject that considering it's cosmetic).

Well, let's hope that won't break everything...

> I'd like to add a few comments in there too (1 line explaining what the
> next bloc is about). Do you mind ?

Go ahead.


-- 
Emmanuel Dreyfus
http://hcpnet.free.fr/pubz
manu@...

damn yahoo. retry without the patch.

http://www.sofaraway.org/tmp/milter-greylist/milter-greylist-4.0a6-configure.patch

/Fabien

===
Date: Fri, 15 Jun 2007 11:47:27 +0200

From: Fabien Tassin <fta+miltergreylist@...>
To: milter-greylist@yahoogroups.com
Subject: Re: [milter-greylist] milter-greylist 4.0 alpha6 is available

According to Fabien Tassin:
> According to manu@...:
> > Fabien Tassin <fta+miltergreylist@...> wrote:
> > 
> > > Just had a look. Seems easy but current code is a little bit messy.
> > > Some tests are performed twice, and lib tests are spread everywhere.
> > > Would you accept a patch that moves blocs around ?
> > > (I know some reject that considering it's cosmetic).
> > 
> > Well, let's hope that won't break everything...
> > 
> > > I'd like to add a few comments in there too (1 line explaining what the
> > > next bloc is about). Do you mind ?
> > 
> > Go ahead.

done.

I haven't merged the tests as I said I would as it makes the code
unnecessarily complex.

What I did is:
- reordered everything so that all tests are now grouped by feature
- added real checks for libcurl, libgeoip and berkeley db
- added support for --with-package in addition to the already existing
  --with-package=DIR
- added a few comments
- re-aligned --help message
- added an additional test for libmilter on debian/ubuntu using
  /usr/lib/libmilter as it is not possible to use --with-libmilter=DIR
  because it end up with /usr/lib/libmilter/lib/libmilter.so (no good).
  [ I'm not sure it's not a bug in those distro. I've opened a case
    on Ubuntu to check ]

remaining:
- real check for drac. really useful ? seems drac in m-g is only a db lookup
so it only has to to test berkeley db. correct ?
- --with-geoip-db=PATH similar to --with-drac-db=PATH

I've tested tons of situations and it seems ok.

/Fabien

Hi,

any comment on that patch ? did anyone try it?

/Fabien

According to Fabien Tassin:

> 
> http://www.sofaraway.org/tmp/milter-greylist/milter-greylist-4.0a6-configure.patch

> What I did is:
> - reordered everything so that all tests are now grouped by feature
> - added real checks for libcurl, libgeoip and berkeley db
> - added support for --with-package in addition to the already existing
>   --with-package=DIR
> - added a few comments
> - re-aligned --help message
> - added an additional test for libmilter on debian/ubuntu using
>   /usr/lib/libmilter as it is not possible to use --with-libmilter=DIR
>   because it end up with /usr/lib/libmilter/lib/libmilter.so (no good).
>   [ I'm not sure it's not a bug in those distro. I've opened a case
>     on Ubuntu to check ]
> 
> remaining:
> - real check for drac. really useful ? seems drac in m-g is only a db lookup
> so it only has to to test berkeley db. correct ?
> - --with-geoip-db=PATH similar to --with-drac-db=PATH
> 
> I've tested tons of situations and it seems ok.
> 
> /Fabien

Fabien Tassin <fta+miltergreylist@...> wrote:

> any comment on that patch ? did anyone try it?

Sorry for being late:

$ ./configure
...
checking for smfi_register in -lmilter... no
checking for smfi_register in -lmilter -lsm... no
checking for smfi_register in -lmilter -lsmutil... no
redo the test with -L/usr/lib/libmilter and smfi_addheader()
checking for smfi_addheader in -lmilter... no
Required libmilter not found. Use --with-libmilter
$ locate libmilter.a   
/usr/lib/libmilter.a

We agree this is not acceptable, right?

Here is the root of the error:
configure:7658: gcc -o conftest -g -O2 -Wall   conftest.c -lmilter
-lresolv >&5 /usr/lib/libmilter.a(signal.o)(.text+0x167): In function
`mi_clean_signals': undefined reference to `pthread_create'

I guess you need to find libpthread before libmilter.

-- 
Emmanuel Dreyfus
http://hcpnet.free.fr/pubz
manu@...