On 21/08/2015 21:04 , Andreas Pokorny wrote:
Hi,
The sole purpose of Werror is to keep warnings out. But libinput already
did a good job without Wall + Werror being enabled.
yeah, fwiw, the general rule is that patches that introduce warnings
won't get merged.
On Fri, Aug 21, 2015 at 4:05 AM, Peter Hutterer
<[email protected] <mailto:[email protected]>> wrote:
On Thu, Aug 20, 2015 at 02:32:23PM +0300, Pekka Paalanen wrote:
> On Thu, 20 Aug 2015 12:51:38 +0200
> Andreas Pokorny <[email protected]
<mailto:[email protected]>> wrote:
>
> > Just a small change in test is necessary to enable -Wall -Werror.
> >
> > Signed-off-by: Andreas Pokorny <[email protected]
<mailto:[email protected]>>
> > ---
> > configure.ac <http://configure.ac> | 4 ++--
> > test/litest.c | 8 ++++++--
> > 2 files changed, 8 insertions(+), 4 deletions(-)
> >
> > diff --git a/configure.ac <http://configure.ac> b/configure.ac
<http://configure.ac>
> > index 885cb39..b7597f0 100644
> > --- a/configure.ac <http://configure.ac>
> > +++ b/configure.ac <http://configure.ac>
> > @@ -87,8 +87,8 @@ AC_CHECK_LIB([m], [atan2])
> > AC_CHECK_LIB([rt], [clock_gettime])
> >
> > if test "x$GCC" = "xyes"; then
> > - GCC_CXXFLAGS="-Wall -Wextra -Wno-unused-parameter -g
-fvisibility=hidden"
> > - GCC_CFLAGS="$GCC_CXXFLAGS -Wmissing-prototypes
-Wstrict-prototypes"
> > + GCC_CXXFLAGS="-Wall -Werror -Wextra -Wno-unused-parameter
-g -fvisibility=hidden"
> > + GCC_CFLAGS="$GCC_CXXFLAGS -Wall -Werror
-Wmissing-prototypes -Wstrict-prototypes"
>
> Hi,
>
> are you sure you want to force -Werror on everyone? Even distribution
> builds?
Rather force it on the project. I think that developers are usually
ahead of distributions in terms of compiler versions.
but you can't guarantee that the warnings you see are the same as
someone else does. Sometimes you get different warnings depending on
whether you use -O0 or -O2, you can get different warnings for different
versions of your dependencies. case in point: I have -Wall in my $CFLAGS
yet the warning you fixed never showed up. So either you have an older
or a newer header file.
as I said above, we effectively enforce a no-warning policy anyway, but
it does require them to trigger on one of the machines I run the builds on.
> New compilers come out with new warnings, new or old headers might
> cause warnings...
>
> While checking with colleagues, I was pointed at
> https://www.gnu.org/software/autoconf-archive/ax_compiler_flags.html
> Maybe that could inspire something?
>
yeah, I agree with Pekka here. -Werror is almost always wrong unless you
added it yourself to the $CFLAGS on the host you're building on.
Worse, -Werror doesn't actually add anything - the reason these warnings
haven't been caught before was because they didn't show up. some of them
depend on compiler versions, library versions, etc.
The second warning showed up because I added -Wall which before was only
present in the CXXFLAGS.
from configure.ac:
GCC_CFLAGS="$GCC_CXXFLAGS -Wmissing-prototypes -Wstrict-prototypes"
so -Wall should always be enabled, right?
You get something out of Werror if you enforce it, and sure Werror does
not add additional warnings.
no, but it can 'arbitrarily' break the build, and the more dependencies
you have the greater the risk. That's ok when you're planning for it,
but not so much when the packager doesn't know how to fix the issue
immediately. And as I said above, by having it in configure.ac you're
forcing the packagers to patch it out rather than just set/unset an
environment variable.
So by all means, put -Werror into your local $CFLAGS, but let's not
enforce it on downstreams.
Cheers,
Peter
AX_COMPILER_FLAGS looks interesting, but unsure on the result. the term
"useful warnings" can be quite project-dependent and adding #pragmas
to turn
some of them off isn't helpful either. Haven't tried it yet though.
Merged patch 1/2 though, thanks.
Thanks.
regards
Andreas
_______________________________________________
wayland-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
