[PATCH util-macros 1/1] Turn on the selective -Werror=... CFLAGS for development builds only

[Gaetan Nadon]

On Mon, 2011-12-19 at 10:31 -0800, Jeremy Huddleston wrote:

> The problem is that --enable-strict-compilation adds -Werror to $CFLAGS which is a bit too strict to be used by default.
> 
> The problem is that without these -Werror=... flags, real bugs slip through the cracks and we ship broken code or don't fix drivers when API changes.  -Werror=implicit would've caught broken drivers years ago rather than when I added it in my tinderbox at the beginning of the year to find drivers broken by my API changes.

Only a dozen of drivers have CWARNFLAGS.

> 
> The util-macro changes added just a handful of select errors (like implicit) to catch these types of bad bugs.  I want them to be on for everyone because they will catch bugs.  I'm sorry that I hadn't tested kdrive on linux before shipping, but the tinderbox has been updated to build that case now, but I don't think that's a reason to disable them.
> 
> Would people prefer the --disable-selectve-werror option to turn them off if needed?  That's the other option that will allow users to easily get past it (and hopefully prompt a bug report if they run into issues).
> 

It looks you are getting close to finding the right balance. It usually
takes a few iterations. Adding a new configure option could be justified
for a few corner cases like older version of compiler or whatever else
we cannot think of. Situations that are rare where there is no easy
workaround. 

If too many people find they have to use it, then they will ask for the
"defaults" to change anyway. Keep in mind this affects all 200+ modules.

> On Dec 19, 2011, at 7:13 AM, Gaetan Nadon wrote:
> 
> > On Sun, 2011-12-18 at 16:19 -0800, Jeremy Huddleston wrote:
> > 
> >> For the more dangerous compiler warnings that we made into errors,
> >> use -Werror=... if .git exists and -W... if not.  This way we still
> >> force developers to trip on them but end users will just see
> >> warnings.
> >> 
> >> Signed-off-by: Jeremy Huddleston <jeremyhu at apple.com>
> >> ---
> >> xorg-macros.m4.in |   17 +++++++++++++++++
> >> 1 files changed, 17 insertions(+), 0 deletions(-)
> >> 
> >> 
> > 
> > I am concerned about creating a precedent here regarding the detection
> > of git and the assumption of a "use model".
> > 
> > We have been confronted with this on numerous occasions where we wanted
> > to do something different depending if the build is from git or from a
> > tarball. I, for one, have been reminded many times by reviewers not to
> > make assumptions about the role of the person who is building and the
> > reason why the build is performed, let alone matching roles with "git vs
> > tarball" builds.
> > 
> > The proposed behavior will appear to be erratic and unpredictable when
> > alternating between git/tarball builds until one realizes what is going
> > on. Some may report bugs or post patches to fix warnings just to be told
> > by others that they cannot reproduce the warning. Invariably we will
> > begin to see questions like "are you building from git or from tarball"?
> > 
> > The concept of a "development build" vs a "production build" where
> > compiler options are different is very common. This should not occur
> > without the builder's knowledge or consent. We can provide some
> > mechanism to help builder select the type of build desired, but we
> > cannot enforce it. The most obvious autotools features are configure
> > options (we have --enable-strict-compilation) and environment variables
> > that would be picked up by makefiles. 
> > 
> > There are no centralized builds and therefore no way to enforce a more
> > stringent build. The tinderbox can play a role and the documented
> > development process too.
> > 
> > Some of the readers may recall this (git vs tarball) issue being
> > discussed at length in other areas:
> > 
> >        ChangeLog: git is required to generate it.
> >        Special tools: like lex and yacc which may not be available when
> >        building from tarball which contains the special tool generated
> >        code
> > 
> > I know these cases are not quite the same, but they always raised the
> > question "if git do this, else do that" but it never worked out and we
> > had to find a different way without explicitly testing for git.
> > 
> > Would it not be possible to modify the content and semantic of
> > --enable-strict-compilation to mean "development build"? Make this
> > option a requirement in the server build process and on tinderbox. It
> > will take time for working habits to adapt, as for any change.
> > 
> > 
> > 
> > 
> > 
> 


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.x.org/archives/xorg-devel/attachments/20111219/13f1fbc0/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part
URL: <http://lists.x.org/archives/xorg-devel/attachments/20111219/13f1fbc0/attachment-0001.pgp>