[Qemu-devel] [PATCH] configure: Don't run configure tests with -Werror enabled

[Qemu-devel] [PATCH] configure: Don't run configure tests with -Werror enabled

[Peter Maydell]

Don't run configure tests with -Werror in the compiler flags. The idea
of -Werror is that it makes problems very obvious to developers, so
they get fixed quickly. However, when running configure tests, failures
due to -Werror are far from obvious -- they simply result in the test
quietly failing when it should have passed. Not using -Werror is in
line with recommended practice in the Autoconf world.

This commit is essentially backing out the changes in commit 417c9d72.
Instead we fix the problem that commit was trying to address in a
different way: we add -Werror only for the test of the nss headers,
with a comment that this is specifically intended to detect a bug
in some releases of nss.

We also have to clean up a bug in the smartcard test where it was
trying to include smartcard_cflags in the test compile flags: this
would always result in a failure with -Werror, because they include
an escaped "$(SRC_PATH)" which is only valid when used in the final
makefile.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
Yeah, this is kind of breaking the "never say 'also' in a commit
message" rule :-)

 configure |   22 ++++++++++++++++++----
 1 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/configure b/configure
index 0a3896e..383fa3d 100755
--- a/configure
+++ b/configure
@@ -1156,9 +1156,10 @@ gcc_flags="-Wold-style-declaration -Wold-style-definition -Wtype-limits"
 gcc_flags="-Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers $gcc_flags"
 gcc_flags="-Wmissing-include-dirs -Wempty-body -Wnested-externs $gcc_flags"
 gcc_flags="-fstack-protector-all -Wendif-labels $gcc_flags"
-if test "$werror" = "yes" ; then
-    gcc_flags="-Werror $gcc_flags"
-fi
+# Note that we do not add -Werror to gcc_flags here, because that would
+# enable it for all configure tests. If a configure test failed due
+# to -Werror this would just silently disable some features,
+# so it's too error prone.
 cat > $TMPC << EOF
 int main(void) { return 0; }
 EOF
@@ -2656,8 +2657,16 @@ EOF
         smartcard_cflags="-I\$(SRC_PATH)/libcacard"
         libcacard_libs="$($pkg_config --libs nss 2>/dev/null) $glib_libs"
         libcacard_cflags="$($pkg_config --cflags nss 2>/dev/null) $glib_cflags"
+        test_cflags="$libcacard_cflags"
+        # The header files in nss < 3.13.3 have a bug which causes them to
+        # emit a warning. If we're going to compile QEMU with -Werror, then
+        # test that the headers don't have this bug. Otherwise we would pass
+        # the configure test but fail to compile QEMU later.
+        if test "$werror" = "yes"; then
+            test_cflags="-Werror $test_cflags"
+        fi
         if $pkg_config --atleast-version=3.12.8 nss >/dev/null 2>&1 && \
-          compile_prog "$smartcard_cflags $libcacard_cflags" "$libcacard_libs"; then
+          compile_prog "$test_cflags" "$libcacard_libs"; then
             smartcard_nss="yes"
             QEMU_CFLAGS="$QEMU_CFLAGS $smartcard_cflags $libcacard_cflags"
             libs_softmmu="$libcacard_libs $libs_softmmu"
@@ -2903,6 +2912,11 @@ if test -z "$zero_malloc" ; then
     fi
 fi
 
+# Now we've finished running tests it's OK to add -Werror to the compiler flags
+if test "$werror" = "yes"; then
+    QEMU_CFLAGS="-Werror $QEMU_CFLAGS"
+fi
+
 if test "$solaris" = "no" ; then
     if $ld --version 2>/dev/null | grep "GNU ld" >/dev/null 2>/dev/null ; then
         LDFLAGS="-Wl,--warn-common $LDFLAGS"
-- 
1.7.5.4

Re: [Qemu-devel] [PATCH] configure: Don't run configure tests with -Werror enabled

[Stefan Weil]

Am 17.07.2012 21:24, schrieb Peter Maydell:
> Don't run configure tests with -Werror in the compiler flags. The idea
> of -Werror is that it makes problems very obvious to developers, so
> they get fixed quickly. However, when running configure tests, failures
> due to -Werror are far from obvious -- they simply result in the test
> quietly failing when it should have passed. Not using -Werror is in
> line with recommended practice in the Autoconf world.
>
> This commit is essentially backing out the changes in commit 417c9d72.
> Instead we fix the problem that commit was trying to address in a
> different way: we add -Werror only for the test of the nss headers,
> with a comment that this is specifically intended to detect a bug
> in some releases of nss.
>
> We also have to clean up a bug in the smartcard test where it was
> trying to include smartcard_cflags in the test compile flags: this
> would always result in a failure with -Werror, because they include
> an escaped "$(SRC_PATH)" which is only valid when used in the final
> makefile.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> Yeah, this is kind of breaking the "never say 'also' in a commit
> message" rule :-)
>
>   configure |   22 ++++++++++++++++++----
>   1 files changed, 18 insertions(+), 4 deletions(-)
>
> diff --git a/configure b/configure
> index 0a3896e..383fa3d 100755
> --- a/configure
> +++ b/configure
> @@ -1156,9 +1156,10 @@ gcc_flags="-Wold-style-declaration -Wold-style-definition -Wtype-limits"
>   gcc_flags="-Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers $gcc_flags"
>   gcc_flags="-Wmissing-include-dirs -Wempty-body -Wnested-externs $gcc_flags"
>   gcc_flags="-fstack-protector-all -Wendif-labels $gcc_flags"
> -if test "$werror" = "yes" ; then
> -    gcc_flags="-Werror $gcc_flags"
> -fi
> +# Note that we do not add -Werror to gcc_flags here, because that would
> +# enable it for all configure tests. If a configure test failed due
> +# to -Werror this would just silently disable some features,
> +# so it's too error prone.
>   cat > $TMPC << EOF
>   int main(void) { return 0; }
>   EOF
> @@ -2656,8 +2657,16 @@ EOF
>           smartcard_cflags="-I\$(SRC_PATH)/libcacard"
>           libcacard_libs="$($pkg_config --libs nss 2>/dev/null) $glib_libs"
>           libcacard_cflags="$($pkg_config --cflags nss 2>/dev/null) $glib_cflags"
> +        test_cflags="$libcacard_cflags"
> +        # The header files in nss < 3.13.3 have a bug which causes them to
> +        # emit a warning. If we're going to compile QEMU with -Werror, then
> +        # test that the headers don't have this bug. Otherwise we would pass
> +        # the configure test but fail to compile QEMU later.
> +        if test "$werror" = "yes"; then
> +            test_cflags="-Werror $test_cflags"
> +        fi
>           if $pkg_config --atleast-version=3.12.8 nss >/dev/null 2>&1 && \
> -          compile_prog "$smartcard_cflags $libcacard_cflags" "$libcacard_libs"; then
> +          compile_prog "$test_cflags" "$libcacard_libs"; then
>               smartcard_nss="yes"
>               QEMU_CFLAGS="$QEMU_CFLAGS $smartcard_cflags $libcacard_cflags"
>               libs_softmmu="$libcacard_libs $libs_softmmu"
> @@ -2903,6 +2912,11 @@ if test -z "$zero_malloc" ; then
>       fi
>   fi
>   
> +# Now we've finished running tests it's OK to add -Werror to the compiler flags
> +if test "$werror" = "yes"; then
> +    QEMU_CFLAGS="-Werror $QEMU_CFLAGS"
> +fi
> +
>   if test "$solaris" = "no" ; then
>       if $ld --version 2>/dev/null | grep "GNU ld" >/dev/null 2>/dev/null ; then
>           LDFLAGS="-Wl,--warn-common $LDFLAGS"


Reviewed-by: Stefan Weil <sw@weilnetz.de>

Regards,
Stefan W.

Re: [Qemu-devel] [PATCH] configure: Don't run configure tests with -Werror enabled

[Alexander Graf]

On 17.07.2012, at 21:24, Peter Maydell wrote:

> Don't run configure tests with -Werror in the compiler flags. The idea
> of -Werror is that it makes problems very obvious to developers, so
> they get fixed quickly. However, when running configure tests, failures
> due to -Werror are far from obvious -- they simply result in the test
> quietly failing when it should have passed. Not using -Werror is in
> line with recommended practice in the Autoconf world.
> 
> This commit is essentially backing out the changes in commit 417c9d72.
> Instead we fix the problem that commit was trying to address in a
> different way: we add -Werror only for the test of the nss headers,
> with a comment that this is specifically intended to detect a bug
> in some releases of nss.
> 
> We also have to clean up a bug in the smartcard test where it was
> trying to include smartcard_cflags in the test compile flags: this
> would always result in a failure with -Werror, because they include
> an escaped "$(SRC_PATH)" which is only valid when used in the final
> makefile.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Works for me. I merely tried to get the default configure cycle to compile for me :). This patch has the same net effect to me.


Alex

> ---
> Yeah, this is kind of breaking the "never say 'also' in a commit
> message" rule :-)
> 
> configure |   22 ++++++++++++++++++----
> 1 files changed, 18 insertions(+), 4 deletions(-)
> 
> diff --git a/configure b/configure
> index 0a3896e..383fa3d 100755
> --- a/configure
> +++ b/configure
> @@ -1156,9 +1156,10 @@ gcc_flags="-Wold-style-declaration -Wold-style-definition -Wtype-limits"
> gcc_flags="-Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers $gcc_flags"
> gcc_flags="-Wmissing-include-dirs -Wempty-body -Wnested-externs $gcc_flags"
> gcc_flags="-fstack-protector-all -Wendif-labels $gcc_flags"
> -if test "$werror" = "yes" ; then
> -    gcc_flags="-Werror $gcc_flags"
> -fi
> +# Note that we do not add -Werror to gcc_flags here, because that would
> +# enable it for all configure tests. If a configure test failed due
> +# to -Werror this would just silently disable some features,
> +# so it's too error prone.
> cat > $TMPC << EOF
> int main(void) { return 0; }
> EOF
> @@ -2656,8 +2657,16 @@ EOF
>         smartcard_cflags="-I\$(SRC_PATH)/libcacard"
>         libcacard_libs="$($pkg_config --libs nss 2>/dev/null) $glib_libs"
>         libcacard_cflags="$($pkg_config --cflags nss 2>/dev/null) $glib_cflags"
> +        test_cflags="$libcacard_cflags"
> +        # The header files in nss < 3.13.3 have a bug which causes them to
> +        # emit a warning. If we're going to compile QEMU with -Werror, then
> +        # test that the headers don't have this bug. Otherwise we would pass
> +        # the configure test but fail to compile QEMU later.
> +        if test "$werror" = "yes"; then
> +            test_cflags="-Werror $test_cflags"
> +        fi
>         if $pkg_config --atleast-version=3.12.8 nss >/dev/null 2>&1 && \
> -          compile_prog "$smartcard_cflags $libcacard_cflags" "$libcacard_libs"; then
> +          compile_prog "$test_cflags" "$libcacard_libs"; then
>             smartcard_nss="yes"
>             QEMU_CFLAGS="$QEMU_CFLAGS $smartcard_cflags $libcacard_cflags"
>             libs_softmmu="$libcacard_libs $libs_softmmu"
> @@ -2903,6 +2912,11 @@ if test -z "$zero_malloc" ; then
>     fi
> fi
> 
> +# Now we've finished running tests it's OK to add -Werror to the compiler flags
> +if test "$werror" = "yes"; then
> +    QEMU_CFLAGS="-Werror $QEMU_CFLAGS"
> +fi
> +
> if test "$solaris" = "no" ; then
>     if $ld --version 2>/dev/null | grep "GNU ld" >/dev/null 2>/dev/null ; then
>         LDFLAGS="-Wl,--warn-common $LDFLAGS"
> -- 
> 1.7.5.4
>