Re: [Pio-temporary] Re: [ pio-Patches-1293678 ] Enable compiler warnings without debugging support

[Brian W. <bwe...@xb...>]

On Mon, 19 Sep 2005, Bas Wijnen wrote:

>>> Comment By: Brian Wellington (bwelling)
>> Date: 2005-09-18 20:56
>>
>> Roland - I don't think your patch will work, because the
>> specific warnings supported by gcc vary greatly by version
>> (and since many distributors shipped patched gcc, even
>> version checking isn't enough).
>
> Good point.  However, the warnings are not new: They are currently in
> rules.make, and can be enabled with --enable-debug.  I would like to compile
> with lots of warnings, but of course it shouldn't break the compilation.  Is
> there a solution for this?

It looked like the attached patch added more warnings than were currently 
in rules.make, but I could be wrong.

The solution we use in our build system at work is to attempt to determine 
the version of gcc, and conditionally enable warnings when appropriate. 
This works for us, but we also have a much more limited number of gcc 
versions.

It's basically:
  - always add: -W -Wall -Wcast-qual -Wwrite-strings -Wpointer-arith
  - unless building C++, add: -Wmissing-prototypes
  - if gcc >= 3.2, add: -Wno-format-y2k -Werror
  - if gcc >= 3.4 and not building C++, add: -Wdeclaration-after-statement


>> I'm a bit confused as to the intent of the original patch -
>> having debugging symbols in a binary doesn't really cause
>> any burden other than increased disk usage.  Given that hard
>> drives are effectively free (and a pioneers build with
>> debugging symbols is still going to use way less space than,
>> say, gnome) and debugging symbols are useful if there are
>> bugs, I don't know why they should be split.
>
> The difference is more than a factor of 3 for the client (I didn't check the
> others).  I don't think it is reasonable to ask people who aren't going to
> submit bugs to install that.  At least for Debian they will be stripped
> anyway, so that's no problem.  I don't know what other distributions do
> though.

It might be a factor of 3, but on my system (FC4/x86), the total size of 
the 6 installed binaries is only about 2.3M, and the total size of the 
other installed files is about 1.4M (and wouldn't be affected by this).
That said, I don't have any problem making this configurable.

> I have been building debugging versions of the debian packages, it might be a
> good idea to put them on the web somewhere to ask people to use them if we
> want more info.  Of course I can only build for my own architecture (although
> I can provide a package which people can use to build it on their machine),
> which is currently i386.

OK.

>> In my opinion, what should be split is the debug/warning
>> code and the deprecation flags.
>
> Sounds good.
>
>> Splitting off -DDEBUG from warnings could also be useful,
>> but I don't see any good reason why -DDEBUG and -g should be
>> tied together - they're completely unrelated.
>
> The reason they're together (and called --enable-debug) is that you'll want
> both when debugging.  I agree though that it makes sense to split them.

That's true, but I've also found cases where I've been trying to debug 
something in gdb, and the huge amount of data printed when DEBUG is set 
actually makes it harder to follow.

>> I'd suggest something like:
>>   - --enable-gcc-warnings (-Wall, etc)
>>   - --enable-debug (-DDEBUG)
>>   - --enable-deprecation-checks (or
>> --enable-glib-deprecation checks, etc)
>>   - possibly a way to set all of these with one flag
>
> Sounds good.
>
>>   - always build with -g unless CFLAGS is set, like most
>> other autoconf-based software.
>
> I don't know about this.  Perhaps it makes sense that people who compile from
> source have enough disk space anyway.  Distributions will strip the files
> before installing (at least I know I will do that for Debian), so that should
> be ok.

I'm not sure about debian, but a lot of RH/Fedora packages set CFLAGS 
before invoking configure as part of the build process.  I know that 
setting CFLAGS doesn't currently work for pioneers (or was that fixed? 
I'm a bit behind), but that's a fairly common solution.

>> I could try to come up with a patch to do this if there's
>> interest, but I'm not going to fight with autoconf unless
>> there's consensus.
>
> In cases like these, I think it is a good idea to apply a patch which makes
> the thing work, and move the report to the feature requests.  While my patch
> may not be the final solution, it does fix a problem (it's even more relevant
> for the AI-empty-bank-maritime-trade bug).  How about the default policy of
> fixing the bug first, and changing the thing later after we decided what's the
> Right Thing(tm) to do in that situation.  In particular for bugs, it's not
> acceptable to leave them open just because we don't know how we want to
> restructure the code, IMO.

Agreed.  Your patch is definitely an improvement over the current code, 
and should be applied.  The we can figure out what the best way to move 
forward from there.

Brian