Re: [PATCH] tests: factor st_ctime-comparison out of two headers

[Simon Josefsson]

Ben Pfaff <address@hidden> writes:

> Simon Josefsson <address@hidden> writes:
>
>> Bruno Haible <address@hidden> writes:
>>
>>> Jim Meyering wrote:
>>>> -W
>> ...
>>>> -Wwrite-strings
>>>> -fdiagnostics-show-option
>>>
>>> Thanks. I've updated my build script to include these for glibc/x86 builds
>>> (except -Werror, which causes a configuration error already in
>>> "checking whether the compiler works...").
>>
>> I recommend putting warning flags in a separate variable,
>> e.g. WARN_CFLAGS, which is not used during ./configure checks but only
>> when building (parts of) the project code.  That way, -Werror can be
>> used, which I find helpful since I can mentally ignore the compilation
>> output since I know I will be interrupted if there is anything real to
>> pay attention to.
>
> Another approach is to add -Werror last (this is what Eric Blake
> suggested on the autoconf mailing list a long time ago), e.g.:
>
> AC_DEFUN([OVS_ENABLE_WERROR],
>   [AC_ARG_ENABLE(
>      [Werror],
>      [AC_HELP_STRING([--enable-Werror], [Add -Werror to CFLAGS])],
>      [], [enable_Werror=no])
>    AC_CONFIG_COMMANDS_PRE(
>      [if test "X$enable_Werror" = Xyes; then
>         CFLAGS="$CFLAGS -Werror"
>       fi])])

But does that really work?  Don't you risk affecting ./configure checks
that often are written in a way that triggers warnings, and with the
-Werror flag, leads to compilation errors.  If the only reason for the
compilation to fail was the -Werror flag, then that ./configure check
has the wrong result.  I guess it may depend on where in the ./configure
run the statement above is placed, but that seems fragile to me.

A combination of the two approaches that I've used is this:

if test "$gl_gcc_warnings" = yes; then
  gl_WARN_ADD([-Werror], [WERROR_CFLAGS])
  gl_WARN_ADD([-Wframe-larger-than=112], [WSTACK_CFLAGS])

  nw="$nw -Wsystem-headers"         # Don't let system headers trigger warnings
  nw="$nw -Wpadded"                 # Struct in src/idn_cmd.h is not padded
  nw="$nw -Wformat"                 # Self tests and examples print size_t as %d
  nw="$nw -Wc++-compat"             # We don't care strongly about C++ compilers
  nw="$nw -Woverlength-strings"     # Some of our strings are too large
  nw="$nw -Wsign-conversion"        # Too many warnings for now
  nw="$nw -Wconversion"             # Too many warnings for now
  nw="$nw -Wtraditional"            # Warns on #elif which we use often
  nw="$nw -Wtraditional-conversion" # Too many warnings for now
  nw="$nw -Wmissing-noreturn"       # Too many warnings for now
  nw="$nw -Wunreachable-code"       # Too many false positives
  nw="$nw -Wlogical-op"             # Too many false positives

  gl_MANYWARN_ALL_GCC([ws])
  gl_MANYWARN_COMPLEMENT(ws, [$ws], [$nw])
  for w in $ws; do
    gl_WARN_ADD([$w])
  done

  gl_WARN_ADD([-Wno-format])
  gl_WARN_ADD([-Wno-missing-field-initializers])
  gl_WARN_ADD([-fdiagnostics-show-option])
fi

note the different variables, WERROR_CFLAGS, WSTACK_CFLAGS and
WARN_CFLAGS.  Then I can use $(WERROR_CFLAGS) $(WARN_CFLAGS)
$(WSTACK_CFLAGS) on my own code, and typically $(WARN_CFLAGS) on gnulib
code or similar.

/Simon