Re: svn commit: rev 30793 - spamassassin/trunk/spamc

[Justin Mason <jm...@jmason.org>]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1


Daniel Quinlan writes:
> > bug 3599: Removed -Wall from the CFLAGS for now to make it compile
> > with non-GCC compilers. The file configure.in is currently broken and
> > needs some love for 3.1.
> 
> -1 the cure is worse than the disease
> 
> I think this is a bad idea, especially considering the problems we're
> having with the spamc code, 99% of the sites compiling spamc use GCC
> and we want -Wall for those.  Can you please revert this change?

Well, in my opinion we should ensure that the code can compile on
all platforms using the default settings.  So:

  - on non-gcc platforms it shouldn't use -Wall

  - on gcc, it should use -Wall by default *if it's possible to do that
    easily*; otherwise it should be easy for a user to do something
    like

        perl Makefile.PL CFLAGS="-g -Wall"

    which is the more "traditional" way to support this in packaging
    terms.

there's another point here -- that change needed R-T-C voting, in my
opinion.

- --j.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.4 (GNU/Linux)
Comment: Exmh CVS

iD8DBQFBBqi8QTcbUG5Y7woRAr9xAKCbKps3p0KLj4Vh9szfRM45OcdWSwCfZu7n
v1/S8yCIOT5imAr3q1vI+iA=
=W1ST
-----END PGP SIGNATURE-----

Re: svn commit: rev 30793 - spamassassin/trunk/spamc

[Sidney Markowitz <si...@sidney.com>]

Malte S. Stretz wrote:
> Unfortunately had the raised warning level (I think Sidney increased the 
> Windows /W-switch about that time, too) exactly the opposite effect :-/

I'm confused about whether "raised warning level" means more warnings or 
fewer warnings :-). As I recall, what I did was the equivalent of first 
adding -Wall and then turned off just a few specific warnings that are 
annoying and can't be worked around. You may be remembering that second 
step when you talk about the "opposite effect".

 > even though it produced probably a warning for Dallas and Nathan,
 > the initial size_t problem went on unnoticed

I'm in favor of building with the option to treat warnings as errors. 
But that may be too radical for everyone :-)

  -- sidney

Re: svn commit: rev 30793 - spamassassin/trunk/spamc

[Daniel Quinlan <qu...@pathname.com>]

"Malte S. Stretz" <ms...@gmx.net> writes:

> Not in mine but seems like I was wrong ;-) Let's use bug 3599 to vote on 
> this.

Given that I vetoed the change and gave what is a valid technical
objection, it really has to be reverted.  We can have a new vote on the
next patch.

-- 
Daniel Quinlan
http://www.pathname.com/~quinlan/

Re: svn commit: rev 30793 - spamassassin/trunk/spamc

["Malte S. Stretz" <ms...@gmx.net>]

On Tuesday 27 July 2004 21:10 CET Justin Mason wrote:
> Daniel Quinlan writes:
> > > bug 3599: Removed -Wall from the CFLAGS for now to make it compile
> > > with non-GCC compilers. The file configure.in is currently broken and
> > > needs some love for 3.1.
> >
> > -1 the cure is worse than the disease
> >
> > I think this is a bad idea, especially considering the problems we're
> > having with the spamc code, 99% of the sites compiling spamc use GCC
> > and we want -Wall for those.  Can you please revert this change?

Doh. To me this was a trivial fix as it doesn't have any impact on the code 
itself. Proven wrong again ;-)

About the issue: The -Wall switch was added by me maybe a month ago, to 
catch things like the ones discussed 3506.

Unfortunately had the raised warning level (I think Sidney increased the 
Windows /W-switch about that time, too) exactly the opposite effect :-/

And even though it produced probably a warning for Dallas and Nathan, the 
initial size_t problem went on unnoticed until the actual bug cropped up.

> Well, in my opinion we should ensure that the code can compile on
> all platforms using the default settings.  So:

ACK. What I planned to do was:
1. Remove that switch for 3.0.
2. Add some autoconf magic to the configure.in for 3.1 so that support of 
the -Wall switch is detected.
3. Add also -Werror if supported.

The current behaviour (don't warn) is exactly the one we had for 2.x and all 
warnings *we* see are gone. And obviously, nobody looks at the gcc warnings 
when the whole bunch of make output scrolls over the screen.

>   - on non-gcc platforms it shouldn't use -Wall

For the initial 3.0 release I think its better to support people who compile 
with non-gcc compilers (that UnixWare on or maybe the Intel compiler).

>   - on gcc, it should use -Wall by default *if it's possible to do that
>     easily*; [...]

The autoconf changes would be quite a large patch, not something I wanted to 
do at this stage.

> there's another point here -- that change needed R-T-C voting, in my
> opinion.

Not in mine but seems like I was wrong ;-) Let's use bug 3599 to vote on 
this.

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: rev 30793 - spamassassin/trunk/spamc

[Daniel Quinlan <qu...@pathname.com>]

jm@jmason.org (Justin Mason) writes:

> there's another point here -- that change needed R-T-C voting, in my
> opinion.

I think I covered that, let's give Malte a chance here.  ;-)

Also, note that I explicitly vetoed the chance and gave a technical
reason along with that veto explaining why I feel the change needed to
be reverted.  It's first and foremost that it was a bad change to the
code in my opinion and second that it was non-trivial (which it was).

Daniel

-- 
Daniel Quinlan
http://www.pathname.com/~quinlan/