default warnings and default patching policy

[David Holland <dholland-pkgtech%netbsd.org@localhost>]

I have been wading through clang failures on and off for the last
couple weeks (because it's something that comes in small units and
doesn't require much brainpower) and I have some observations to make.

First of all, because clang objects to void main, and to functions
like this:

   foo()
   {
           if (bar())
                   return;
           baz();
   }

it has been scraping up all the ugly old legacy code in pkgsrc, so
I've been seeing a lot of cruft go by.

Second, clang has more warnings enabled by default than gcc does, so
most of the packages that fail on one of the above things (or
something else of a similar caliber) also spew tons of warnings.

A healthy fraction of those warnings turn out to reflect bugs -- not
subtle bugs or anything one has to look carefully at the source to
check if it's right or not, but stuff like calling malloc without
including stdlib.h, passing a literal 0 as the last argument to execl,
writing "var & flag == 0" without parentheses, failing to include
headers for functions that have been __RENAME'd, and so forth. The
fraction isn't 50% or even 20%, but I'd estimate it at something like
one in four or five failing packages, which is plenty to take action
on.

Many of these issues could have been fixed preemptively if we'd been a
little more aggressive about patching. Quite a few more could have
been detected by gcc if gcc had been asked to speak up.

So I'd like to propose the following things:

1. When the pkgsrc compiler is gcc, add -Wall -Wno-error to the
default CFLAGS. This will generate build noise, in some cases a lot of
build noise. This hurts nobody, though, and it has the potential to
catch a lot of problems.

2. Extend the intended scope of pkgsrc patches, as a matter of policy,
to include making packages build cleanly. Right now we tend to do the
minimum necessary to resolve overt problems; however, this means that
less overt problems don't get fixed, overt problems sometimes get lost
in a deluge of boring but easily silenced warnings, and any
significant change of environment (like to LP64) leads to lots of
stuff silently not working that could, at little cost, have been fixed
ahead of time.

This mostly matters for legacy packages; when upstream is active it
makes more sense to contribute such patches upstream. But we have lots
of packages that are, while not necessarily fully dead upstream
(though some are that), not exactly actively maintained. For these,
the cost of merging build cleanup into any future versions that might
perhaps appear is relatively small and the probability that the
package will at some point fail silently if not patched proactively is
fairly high.

This also assumes someone wants to do the work for any given package.
Making clean builds mandatory is a ludicrous proposition. :-/

(Although I'd encourage anyone patching any package to make a point of
adding all missing standard header files everywhere they're needed,
whether or not anything seems to be going overtly wrong. This is a
common problem with bodgy code and creates all kinds of failure
cases.)

3. Extend the bulk build reporting system so that warnings from
packages whose build did not actually error out are made available.


Thoughts?

-- 
David A. Holland
dholland%netbsd.org@localhost