qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Stefan Weil <sw@weilnetz.de>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: patches@linaro.org, Stefan Weil <sw@weilnetz.de>,
	Alexander Graf <agraf@suse.de>,
	qemu-devel@nongnu.org, Blue Swirl <blauwirbel@gmail.com>,
	Eric Blake <eblake@redhat.com>
Subject: Re: [Qemu-devel] [PATCH] configure: Don't run configure tests with -Werror enabled
Date: Tue, 17 Jul 2012 21:34:35 +0200	[thread overview]
Message-ID: <5005BE4B.4030009@weilnetz.de> (raw)
In-Reply-To: <1342553056-11090-1-git-send-email-peter.maydell@linaro.org>

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.

  reply	other threads:[~2012-07-17 19:34 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-07-17 19:24 [Qemu-devel] [PATCH] configure: Don't run configure tests with -Werror enabled Peter Maydell
2012-07-17 19:34 ` Stefan Weil [this message]
2012-07-17 19:35 ` Alexander Graf

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5005BE4B.4030009@weilnetz.de \
    --to=sw@weilnetz.de \
    --cc=agraf@suse.de \
    --cc=blauwirbel@gmail.com \
    --cc=eblake@redhat.com \
    --cc=patches@linaro.org \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).