diff mbox

[11/11] configure: Check for -Werror causing failures when compiling tests

Message ID 1342620628-12751-12-git-send-email-peter.maydell@linaro.org
State Accepted
Commit 8dc38a78d0f74e1561c1fe4d276150a1a8a12c4c
Headers show

Commit Message

Peter Maydell July 18, 2012, 2:10 p.m. UTC
Add support for checking whether test case code can compile without
warnings, by recompiling each successful test with -Werror. If the
-Werror version doesn't pass, we bail out. This gives us the same
level of visibility of warnings in test code as --enable-werror
provides for the main compile.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 configure |   32 ++++++++++++++++++++++++++++----
 1 files changed, 28 insertions(+), 4 deletions(-)

Comments

Blue Swirl July 28, 2012, 9:04 a.m. UTC | #1
On Wed, Jul 18, 2012 at 2:10 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
> Add support for checking whether test case code can compile without
> warnings, by recompiling each successful test with -Werror. If the
> -Werror version doesn't pass, we bail out. This gives us the same
> level of visibility of warnings in test code as --enable-werror
> provides for the main compile.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  configure |   32 ++++++++++++++++++++++++++++----
>  1 files changed, 28 insertions(+), 4 deletions(-)
>
> diff --git a/configure b/configure
> index 8140464..1939bdb 100755
> --- a/configure
> +++ b/configure
> @@ -27,16 +27,40 @@ printf " '%s'" "$0" "$@" >> config.log
>  echo >> config.log
>  echo "#" >> config.log
>
> +do_cc() {
> +    # Run the compiler, capturing its output to the log.
> +    echo $cc "$@" >> config.log
> +    $cc "$@" >> config.log 2>&1 || return $?
> +    # Test passed. If this is an --enable-werror build, rerun
> +    # the test with -Werror and bail out if it fails. This
> +    # makes warning-generating-errors in configure test code
> +    # obvious to developers.
> +    if test "$werror" != "yes"; then
> +        return 0
> +    fi
> +    # Don't bother rerunning the compile if we were already using -Werror
> +    case "$*" in
> +        *-Werror*)
> +           return 0
> +        ;;
> +    esac
> +    echo $cc -Werror "$@" >> config.log
> +    $cc -Werror "$@" >> config.log 2>&1 && return $?
> +    echo "ERROR: configure test passed without -Werror but failed with -Werror."
> +    echo "This is probably a bug in the configure script. The failing command"
> +    echo "will be at the bottom of config.log."
> +    echo "You can run configure with --disable-werror to bypass this check."
> +    exit 1

This should be degraded to a warning, I'm getting configure breakage
in some cases:
config-host.mak is out-of-date, running configure
ERROR: configure test passed without -Werror but failed with -Werror.
This is probably a bug in the configure script. The failing command
will be at the bottom of config.log.
You can run configure with --disable-werror to bypass this check.

gcc -Werror -m32 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64
-D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall
-Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing
-fstack-protector-all -Wendif-labels -Wmissing-include-dirs
-Wempty-body -Wnested-externs -Wformat-security -Wformat-y2k
-Winit-self -Wignored-qualifiers -Wold-style-declaration
-Wold-style-definition -Wtype-limits -I/usr/include/libpng12 -o
/tmp/qemu-conf--24538-.exe /tmp/qemu-conf--24538-.c -m32 -g
cc1: warnings being treated as errors
/tmp/qemu-conf--24538-.c:2: error: unknown option after '#pragma GCC
diagnostic' kind

> +}
> +
>  compile_object() {
> -  echo $cc $QEMU_CFLAGS -c -o $TMPO $TMPC >> config.log
> -  $cc $QEMU_CFLAGS -c -o $TMPO $TMPC >> config.log 2>&1
> +  do_cc $QEMU_CFLAGS -c -o $TMPO $TMPC
>  }
>
>  compile_prog() {
>    local_cflags="$1"
>    local_ldflags="$2"
> -  echo $cc $QEMU_CFLAGS $local_cflags -o $TMPE $TMPC $LDFLAGS $local_ldflags >> config.log
> -  $cc $QEMU_CFLAGS $local_cflags -o $TMPE $TMPC $LDFLAGS $local_ldflags >> config.log 2>&1
> +  do_cc $QEMU_CFLAGS $local_cflags -o $TMPE $TMPC $LDFLAGS $local_ldflags
>  }
>
>  # symbolically link $1 to $2.  Portable version of "ln -sf".
> --
> 1.7.5.4
>
>
Peter Maydell July 28, 2012, 10:57 a.m. UTC | #2
On 28 July 2012 10:04, Blue Swirl <blauwirbel@gmail.com> wrote:
> On Wed, Jul 18, 2012 at 2:10 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
>> +    echo "ERROR: configure test passed without -Werror but failed with -Werror."
>> +    echo "This is probably a bug in the configure script. The failing command"
>> +    echo "will be at the bottom of config.log."
>> +    echo "You can run configure with --disable-werror to bypass this check."
>> +    exit 1
>
> This should be degraded to a warning

The idea is that by running configure with --enable-werror (either
explicitly or implicitly for a git build) you've asked for compile
warnings to cause build failure. We're just doing that in configure
as well as for the main build.

>, I'm getting configure breakage
> in some cases:

The thesis of the patch series is that these represent bugs to be fixed.

-- PMM
Blue Swirl July 28, 2012, 12:09 p.m. UTC | #3
On Sat, Jul 28, 2012 at 10:57 AM, Peter Maydell
<peter.maydell@linaro.org> wrote:
> On 28 July 2012 10:04, Blue Swirl <blauwirbel@gmail.com> wrote:
>> On Wed, Jul 18, 2012 at 2:10 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
>>> +    echo "ERROR: configure test passed without -Werror but failed with -Werror."
>>> +    echo "This is probably a bug in the configure script. The failing command"
>>> +    echo "will be at the bottom of config.log."
>>> +    echo "You can run configure with --disable-werror to bypass this check."
>>> +    exit 1
>>
>> This should be degraded to a warning
>
> The idea is that by running configure with --enable-werror (either
> explicitly or implicitly for a git build) you've asked for compile
> warnings to cause build failure. We're just doing that in configure
> as well as for the main build.
>
>>, I'm getting configure breakage
>> in some cases:
>
> The thesis of the patch series is that these represent bugs to be fixed.

Yes, but then the errors should be fixed by the series. When adding
new GCC warning flags, I also fixed the causes for the warning first.
Committing known build breakages is not OK.

>
> -- PMM
Peter Maydell July 28, 2012, 12:14 p.m. UTC | #4
On 28 July 2012 13:09, Blue Swirl <blauwirbel@gmail.com> wrote:
> On Sat, Jul 28, 2012 at 10:57 AM, Peter Maydell
> <peter.maydell@linaro.org> wrote:
>> On 28 July 2012 10:04, Blue Swirl <blauwirbel@gmail.com> wrote:
>>>, I'm getting configure breakage in some cases:
>>
>> The thesis of the patch series is that these represent bugs to be fixed.
>
> Yes, but then the errors should be fixed by the series. When adding
> new GCC warning flags, I also fixed the causes for the warning first.
> Committing known build breakages is not OK.

I fixed all the ones I saw on my system, obviously. I'll have a look
at this one.

I think it would be worth committing patches 1-10 now anyway : those fix
configure issues (patch 1 in particular fixes the problem of tests
silently failing when they should pass).

-- PMM
Blue Swirl July 28, 2012, 12:20 p.m. UTC | #5
On Sat, Jul 28, 2012 at 12:14 PM, Peter Maydell
<peter.maydell@linaro.org> wrote:
> On 28 July 2012 13:09, Blue Swirl <blauwirbel@gmail.com> wrote:
>> On Sat, Jul 28, 2012 at 10:57 AM, Peter Maydell
>> <peter.maydell@linaro.org> wrote:
>>> On 28 July 2012 10:04, Blue Swirl <blauwirbel@gmail.com> wrote:
>>>>, I'm getting configure breakage in some cases:
>>>
>>> The thesis of the patch series is that these represent bugs to be fixed.
>>
>> Yes, but then the errors should be fixed by the series. When adding
>> new GCC warning flags, I also fixed the causes for the warning first.
>> Committing known build breakages is not OK.
>
> I fixed all the ones I saw on my system, obviously. I'll have a look
> at this one.
>
> I think it would be worth committing patches 1-10 now anyway : those fix
> configure issues (patch 1 in particular fixes the problem of tests
> silently failing when they should pass).

OK, I'll test those.

>
> -- PMM
Blue Swirl July 28, 2012, 12:31 p.m. UTC | #6
On Sat, Jul 28, 2012 at 12:14 PM, Peter Maydell
<peter.maydell@linaro.org> wrote:
> On 28 July 2012 13:09, Blue Swirl <blauwirbel@gmail.com> wrote:
>> On Sat, Jul 28, 2012 at 10:57 AM, Peter Maydell
>> <peter.maydell@linaro.org> wrote:
>>> On 28 July 2012 10:04, Blue Swirl <blauwirbel@gmail.com> wrote:
>>>>, I'm getting configure breakage in some cases:
>>>
>>> The thesis of the patch series is that these represent bugs to be fixed.
>>
>> Yes, but then the errors should be fixed by the series. When adding
>> new GCC warning flags, I also fixed the causes for the warning first.
>> Committing known build breakages is not OK.
>
> I fixed all the ones I saw on my system, obviously. I'll have a look
> at this one.
>
> I think it would be worth committing patches 1-10 now anyway : those fix
> configure issues (patch 1 in particular fixes the problem of tests
> silently failing when they should pass).

I'm getting this error, probably because now Valgrind support is enabled:
  CC    coroutine-ucontext.o
cc1: warnings being treated as errors
/src/qemu/coroutine-ucontext.c:204: error: unknown option after
'#pragma GCC diagnostic' kind
/src/qemu/coroutine-ucontext.c:209: error: unknown option after
'#pragma GCC diagnostic' kind

Maybe the compiler does not understand this pragma and Valgrind check
should also fail in that case.

$ gcc -v
Using built-in specs.
Target: x86_64-linux-gnu
Configured with: ../src/configure -v --with-pkgversion='Debian
4.4.5-8' --with-bugurl=file:///usr/share/doc/gcc-4.4/README.Bugs
--enable-languages=c,c++,fortran,objc,obj-c++ --prefix=/usr
--program-suffix=-4.4 --enable-shared --enable-multiarch
--enable-linker-build-id --with-system-zlib --libexecdir=/usr/lib
--without-included-gettext --enable-threads=posix
--with-gxx-include-dir=/usr/include/c++/4.4 --libdir=/usr/lib
--enable-nls --enable-clocale=gnu --enable-libstdcxx-debug
--enable-objc-gc --with-arch-32=i586 --with-tune=generic
--enable-checking=release --build=x86_64-linux-gnu
--host=x86_64-linux-gnu --target=x86_64-linux-gnu
Thread model: posix
gcc version 4.4.5 (Debian 4.4.5-8)

>
> -- PMM
Peter Maydell July 28, 2012, 1:48 p.m. UTC | #7
On 28 July 2012 13:31, Blue Swirl <blauwirbel@gmail.com> wrote:
> I'm getting this error, probably because now Valgrind support is enabled:
>   CC    coroutine-ucontext.o
> cc1: warnings being treated as errors
> /src/qemu/coroutine-ucontext.c:204: error: unknown option after
> '#pragma GCC diagnostic' kind
> /src/qemu/coroutine-ucontext.c:209: error: unknown option after
> '#pragma GCC diagnostic' kind
>
> Maybe the compiler does not understand this pragma and Valgrind check
> should also fail in that case.

Yeah, I think this is one of the few tests which want to explicitly
check "is this construct going to provoke a compiler warning" --
fix is for that test to explictly put -Werror in the cflags in
the compile_prog line.

-- PMM
Blue Swirl Aug. 11, 2012, 7:12 p.m. UTC | #8
Thanks, applied.


On Wed, Jul 18, 2012 at 2:10 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
> Add support for checking whether test case code can compile without
> warnings, by recompiling each successful test with -Werror. If the
> -Werror version doesn't pass, we bail out. This gives us the same
> level of visibility of warnings in test code as --enable-werror
> provides for the main compile.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  configure |   32 ++++++++++++++++++++++++++++----
>  1 files changed, 28 insertions(+), 4 deletions(-)
>
> diff --git a/configure b/configure
> index 8140464..1939bdb 100755
> --- a/configure
> +++ b/configure
> @@ -27,16 +27,40 @@ printf " '%s'" "$0" "$@" >> config.log
>  echo >> config.log
>  echo "#" >> config.log
>
> +do_cc() {
> +    # Run the compiler, capturing its output to the log.
> +    echo $cc "$@" >> config.log
> +    $cc "$@" >> config.log 2>&1 || return $?
> +    # Test passed. If this is an --enable-werror build, rerun
> +    # the test with -Werror and bail out if it fails. This
> +    # makes warning-generating-errors in configure test code
> +    # obvious to developers.
> +    if test "$werror" != "yes"; then
> +        return 0
> +    fi
> +    # Don't bother rerunning the compile if we were already using -Werror
> +    case "$*" in
> +        *-Werror*)
> +           return 0
> +        ;;
> +    esac
> +    echo $cc -Werror "$@" >> config.log
> +    $cc -Werror "$@" >> config.log 2>&1 && return $?
> +    echo "ERROR: configure test passed without -Werror but failed with -Werror."
> +    echo "This is probably a bug in the configure script. The failing command"
> +    echo "will be at the bottom of config.log."
> +    echo "You can run configure with --disable-werror to bypass this check."
> +    exit 1
> +}
> +
>  compile_object() {
> -  echo $cc $QEMU_CFLAGS -c -o $TMPO $TMPC >> config.log
> -  $cc $QEMU_CFLAGS -c -o $TMPO $TMPC >> config.log 2>&1
> +  do_cc $QEMU_CFLAGS -c -o $TMPO $TMPC
>  }
>
>  compile_prog() {
>    local_cflags="$1"
>    local_ldflags="$2"
> -  echo $cc $QEMU_CFLAGS $local_cflags -o $TMPE $TMPC $LDFLAGS $local_ldflags >> config.log
> -  $cc $QEMU_CFLAGS $local_cflags -o $TMPE $TMPC $LDFLAGS $local_ldflags >> config.log 2>&1
> +  do_cc $QEMU_CFLAGS $local_cflags -o $TMPE $TMPC $LDFLAGS $local_ldflags
>  }
>
>  # symbolically link $1 to $2.  Portable version of "ln -sf".
> --
> 1.7.5.4
>
>
diff mbox

Patch

diff --git a/configure b/configure
index 8140464..1939bdb 100755
--- a/configure
+++ b/configure
@@ -27,16 +27,40 @@  printf " '%s'" "$0" "$@" >> config.log
 echo >> config.log
 echo "#" >> config.log
 
+do_cc() {
+    # Run the compiler, capturing its output to the log.
+    echo $cc "$@" >> config.log
+    $cc "$@" >> config.log 2>&1 || return $?
+    # Test passed. If this is an --enable-werror build, rerun
+    # the test with -Werror and bail out if it fails. This
+    # makes warning-generating-errors in configure test code
+    # obvious to developers.
+    if test "$werror" != "yes"; then
+        return 0
+    fi
+    # Don't bother rerunning the compile if we were already using -Werror
+    case "$*" in
+        *-Werror*)
+           return 0
+        ;;
+    esac
+    echo $cc -Werror "$@" >> config.log
+    $cc -Werror "$@" >> config.log 2>&1 && return $?
+    echo "ERROR: configure test passed without -Werror but failed with -Werror."
+    echo "This is probably a bug in the configure script. The failing command"
+    echo "will be at the bottom of config.log."
+    echo "You can run configure with --disable-werror to bypass this check."
+    exit 1
+}
+
 compile_object() {
-  echo $cc $QEMU_CFLAGS -c -o $TMPO $TMPC >> config.log
-  $cc $QEMU_CFLAGS -c -o $TMPO $TMPC >> config.log 2>&1
+  do_cc $QEMU_CFLAGS -c -o $TMPO $TMPC
 }
 
 compile_prog() {
   local_cflags="$1"
   local_ldflags="$2"
-  echo $cc $QEMU_CFLAGS $local_cflags -o $TMPE $TMPC $LDFLAGS $local_ldflags >> config.log
-  $cc $QEMU_CFLAGS $local_cflags -o $TMPE $TMPC $LDFLAGS $local_ldflags >> config.log 2>&1
+  do_cc $QEMU_CFLAGS $local_cflags -o $TMPE $TMPC $LDFLAGS $local_ldflags
 }
 
 # symbolically link $1 to $2.  Portable version of "ln -sf".