| Message ID | 1342553056-11090-1-git-send-email-peter.maydell@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
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.
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 >
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"