All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] configure: Make C++ test work with --enable-werror
@ 2014-02-24 19:08 Peter Maydell
  2014-02-25  7:44 ` Thomas Huth
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Peter Maydell @ 2014-02-24 19:08 UTC (permalink / raw)
  To: qemu-devel
  Cc: Alexey Kardashevskiy, Thomas Huth, Andreas Färber, patches

gcc's C++ compiler complains about being passed some -W options
which make sense for C but not for C++. This means we mustn't try
a C++ compile with QEMU_CFLAGS, but only with a filtered version
that removes the offending options. This filtering was already being
done for uses of C++ in the build itself, but was omitted for the
"does C++ work?" configure test. This only showed up when doing
builds which explicitly enabled -Werror with --enable-werror,
because the "do the compilers work" tests were mistakenly placed
above the "default werror based on whether compiling from git" code.
Further, when the test did fail configure would plunge on regardless
of the error since we were running do_cc in a subshell. Fix this
complex of errors:

1. Move the default-werror code up so that there are no invocations
of compile_object and friends between it and the point where we
set $werror explicitly based on the --enable-werror command line
option.

2. Provide a mechanism for filtering QEMU_CFLAGS to create
QEMU_CXXFLAGS, and use it for the test we run here.

3. Provide a do_cxx function to run a test with the C++ compiler
rather than doing cute tricks with subshells and do_cc.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
This was pretty confusing to debug (mostly because I got tripped
up by the issue fixed by 1. above)...

We could probably roll do_libtool into do_compiler, but I'd rather
not mix that cleanup up with this build-breakage fix.

 configure | 70 +++++++++++++++++++++++++++++++++++++++++++++------------------
 1 file changed, 50 insertions(+), 20 deletions(-)

diff --git a/configure b/configure
index 00f9070..d5929d6 100755
--- a/configure
+++ b/configure
@@ -54,10 +54,13 @@ error_exit() {
     exit 1
 }
 
-do_cc() {
-    # Run the compiler, capturing its output to the log.
-    echo $cc "$@" >> config.log
-    $cc "$@" >> config.log 2>&1 || return $?
+do_compiler() {
+    # Run the compiler, capturing its output to the log. First argument
+    # is compiler binary to execute.
+    local compiler="$1"
+    shift
+    echo $compiler "$@" >> config.log
+    $compiler "$@" >> 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
@@ -71,14 +74,39 @@ do_cc() {
            return 0
         ;;
     esac
-    echo $cc -Werror "$@" >> config.log
-    $cc -Werror "$@" >> config.log 2>&1 && return $?
+    echo $compiler -Werror "$@" >> config.log
+    $compiler -Werror "$@" >> config.log 2>&1 && return $?
     error_exit "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."
 }
 
+do_cc() {
+    do_compiler "$cc" "$@"
+}
+
+do_cxx() {
+    do_compiler "$cxx" "$@"
+}
+
+update_cxxflags() {
+    # Set QEMU_CXXFLAGS from QEMU_CFLAGS by filtering out those
+    # options which some versions of GCC's C++ compiler complain about
+    # because they only make sense for C programs.
+    QEMU_CXXFLAGS=
+    for arg in $QEMU_CFLAGS; do
+        case $arg in
+            -Wstrict-prototypes|-Wmissing-prototypes|-Wnested-externs|\
+            -Wold-style-declaration|-Wold-style-definition|-Wredundant-decls)
+                ;;
+            *)
+                QEMU_CXXFLAGS=${QEMU_CXXFLAGS:+$QEMU_CXXFLAGS }$arg
+                ;;
+        esac
+    done
+}
+
 compile_object() {
   do_cc $QEMU_CFLAGS -c -o $TMPO $TMPC
 }
@@ -1320,6 +1348,19 @@ if test "$ARCH" = "unknown"; then
     fi
 fi
 
+# Consult white-list to determine whether to enable werror
+# by default.  Only enable by default for git builds
+z_version=`cut -f3 -d. $source_path/VERSION`
+
+if test -z "$werror" ; then
+    if test -d "$source_path/.git" -a \
+        "$linux" = "yes" ; then
+        werror="yes"
+    else
+        werror="no"
+    fi
+fi
+
 # check that the C compiler works.
 cat > $TMPC <<EOF
 int main(void) { return 0; }
@@ -1347,7 +1388,9 @@ extern "C" {
 int c_function(void) { return 42; }
 EOF
 
-    if (cc=$cxx do_cc $QEMU_CFLAGS -o $TMPE $TMPC $TMPO $LDFLAGS); then
+    update_cxxflags
+
+    if do_cxx $QEMU_CXXFLAGS -o $TMPE $TMPC $TMPO $LDFLAGS; then
         # C++ compiler $cxx works ok with C compiler $cc
         :
     else
@@ -1360,19 +1403,6 @@ else
     cxx=
 fi
 
-# Consult white-list to determine whether to enable werror
-# by default.  Only enable by default for git builds
-z_version=`cut -f3 -d. $source_path/VERSION`
-
-if test -z "$werror" ; then
-    if test -d "$source_path/.git" -a \
-        "$linux" = "yes" ; then
-        werror="yes"
-    else
-        werror="no"
-    fi
-fi
-
 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"
-- 
1.8.5

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [Qemu-devel] [PATCH] configure: Make C++ test work with --enable-werror
  2014-02-24 19:08 [Qemu-devel] [PATCH] configure: Make C++ test work with --enable-werror Peter Maydell
@ 2014-02-25  7:44 ` Thomas Huth
  2014-02-25  8:32 ` Andreas Färber
  2014-02-25 17:01 ` Peter Maydell
  2 siblings, 0 replies; 4+ messages in thread
From: Thomas Huth @ 2014-02-25  7:44 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Alexey Kardashevskiy, qemu-devel, Andreas Färber, patches

On Mon, 24 Feb 2014 19:08:18 +0000
Peter Maydell <peter.maydell@linaro.org> wrote:

> gcc's C++ compiler complains about being passed some -W options
> which make sense for C but not for C++. This means we mustn't try
> a C++ compile with QEMU_CFLAGS, but only with a filtered version
> that removes the offending options. This filtering was already being
> done for uses of C++ in the build itself, but was omitted for the
> "does C++ work?" configure test. This only showed up when doing
> builds which explicitly enabled -Werror with --enable-werror,
> because the "do the compilers work" tests were mistakenly placed
> above the "default werror based on whether compiling from git" code.
> Further, when the test did fail configure would plunge on regardless
> of the error since we were running do_cc in a subshell. Fix this
> complex of errors:
> 
> 1. Move the default-werror code up so that there are no invocations
> of compile_object and friends between it and the point where we
> set $werror explicitly based on the --enable-werror command line
> option.
> 
> 2. Provide a mechanism for filtering QEMU_CFLAGS to create
> QEMU_CXXFLAGS, and use it for the test we run here.
> 
> 3. Provide a do_cxx function to run a test with the C++ compiler
> rather than doing cute tricks with subshells and do_cc.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> This was pretty confusing to debug (mostly because I got tripped
> up by the issue fixed by 1. above)...
> 
> We could probably roll do_libtool into do_compiler, but I'd rather
> not mix that cleanup up with this build-breakage fix.
> 
>  configure | 70 +++++++++++++++++++++++++++++++++++++++++++++------------------
>  1 file changed, 50 insertions(+), 20 deletions(-)
> 
> diff --git a/configure b/configure
> index 00f9070..d5929d6 100755
> --- a/configure
> +++ b/configure
> @@ -54,10 +54,13 @@ error_exit() {
>      exit 1
>  }
> 
> -do_cc() {
> -    # Run the compiler, capturing its output to the log.
> -    echo $cc "$@" >> config.log
> -    $cc "$@" >> config.log 2>&1 || return $?
> +do_compiler() {
> +    # Run the compiler, capturing its output to the log. First argument
> +    # is compiler binary to execute.
> +    local compiler="$1"
> +    shift
> +    echo $compiler "$@" >> config.log
> +    $compiler "$@" >> 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
> @@ -71,14 +74,39 @@ do_cc() {
>             return 0
>          ;;
>      esac
> -    echo $cc -Werror "$@" >> config.log
> -    $cc -Werror "$@" >> config.log 2>&1 && return $?
> +    echo $compiler -Werror "$@" >> config.log
> +    $compiler -Werror "$@" >> config.log 2>&1 && return $?
>      error_exit "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."
>  }
> 
> +do_cc() {
> +    do_compiler "$cc" "$@"
> +}
> +
> +do_cxx() {
> +    do_compiler "$cxx" "$@"
> +}
> +
> +update_cxxflags() {
> +    # Set QEMU_CXXFLAGS from QEMU_CFLAGS by filtering out those
> +    # options which some versions of GCC's C++ compiler complain about
> +    # because they only make sense for C programs.
> +    QEMU_CXXFLAGS=
> +    for arg in $QEMU_CFLAGS; do
> +        case $arg in
> +            -Wstrict-prototypes|-Wmissing-prototypes|-Wnested-externs|\
> +            -Wold-style-declaration|-Wold-style-definition|-Wredundant-decls)
> +                ;;
> +            *)
> +                QEMU_CXXFLAGS=${QEMU_CXXFLAGS:+$QEMU_CXXFLAGS }$arg
> +                ;;
> +        esac
> +    done
> +}
> +
>  compile_object() {
>    do_cc $QEMU_CFLAGS -c -o $TMPO $TMPC
>  }
> @@ -1320,6 +1348,19 @@ if test "$ARCH" = "unknown"; then
>      fi
>  fi
> 
> +# Consult white-list to determine whether to enable werror
> +# by default.  Only enable by default for git builds
> +z_version=`cut -f3 -d. $source_path/VERSION`
> +
> +if test -z "$werror" ; then
> +    if test -d "$source_path/.git" -a \
> +        "$linux" = "yes" ; then
> +        werror="yes"
> +    else
> +        werror="no"
> +    fi
> +fi
> +
>  # check that the C compiler works.
>  cat > $TMPC <<EOF
>  int main(void) { return 0; }
> @@ -1347,7 +1388,9 @@ extern "C" {
>  int c_function(void) { return 42; }
>  EOF
> 
> -    if (cc=$cxx do_cc $QEMU_CFLAGS -o $TMPE $TMPC $TMPO $LDFLAGS); then
> +    update_cxxflags
> +
> +    if do_cxx $QEMU_CXXFLAGS -o $TMPE $TMPC $TMPO $LDFLAGS; then
>          # C++ compiler $cxx works ok with C compiler $cc
>          :
>      else
> @@ -1360,19 +1403,6 @@ else
>      cxx=
>  fi
> 
> -# Consult white-list to determine whether to enable werror
> -# by default.  Only enable by default for git builds
> -z_version=`cut -f3 -d. $source_path/VERSION`
> -
> -if test -z "$werror" ; then
> -    if test -d "$source_path/.git" -a \
> -        "$linux" = "yes" ; then
> -        werror="yes"
> -    else
> -        werror="no"
> -    fi
> -fi
> -
>  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"

Looks fine to me.
Reviewed-by: Thomas Huth <thuth@linux.vnet.ibm.com>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Qemu-devel] [PATCH] configure: Make C++ test work with --enable-werror
  2014-02-24 19:08 [Qemu-devel] [PATCH] configure: Make C++ test work with --enable-werror Peter Maydell
  2014-02-25  7:44 ` Thomas Huth
@ 2014-02-25  8:32 ` Andreas Färber
  2014-02-25 17:01 ` Peter Maydell
  2 siblings, 0 replies; 4+ messages in thread
From: Andreas Färber @ 2014-02-25  8:32 UTC (permalink / raw)
  To: Peter Maydell, qemu-devel; +Cc: Alexey Kardashevskiy, Thomas Huth, patches

Am 24.02.2014 20:08, schrieb Peter Maydell:
> gcc's C++ compiler complains about being passed some -W options
> which make sense for C but not for C++. This means we mustn't try
> a C++ compile with QEMU_CFLAGS, but only with a filtered version
> that removes the offending options. This filtering was already being
> done for uses of C++ in the build itself, but was omitted for the
> "does C++ work?" configure test. This only showed up when doing
> builds which explicitly enabled -Werror with --enable-werror,
> because the "do the compilers work" tests were mistakenly placed
> above the "default werror based on whether compiling from git" code.
> Further, when the test did fail configure would plunge on regardless
> of the error since we were running do_cc in a subshell. Fix this
> complex of errors:
> 
> 1. Move the default-werror code up so that there are no invocations
> of compile_object and friends between it and the point where we
> set $werror explicitly based on the --enable-werror command line
> option.
> 
> 2. Provide a mechanism for filtering QEMU_CFLAGS to create
> QEMU_CXXFLAGS, and use it for the test we run here.
> 
> 3. Provide a do_cxx function to run a test with the C++ compiler
> rather than doing cute tricks with subshells and do_cc.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Tested-by: Andreas Färber <afaerber@suse.de>

Thanks,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Qemu-devel] [PATCH] configure: Make C++ test work with --enable-werror
  2014-02-24 19:08 [Qemu-devel] [PATCH] configure: Make C++ test work with --enable-werror Peter Maydell
  2014-02-25  7:44 ` Thomas Huth
  2014-02-25  8:32 ` Andreas Färber
@ 2014-02-25 17:01 ` Peter Maydell
  2 siblings, 0 replies; 4+ messages in thread
From: Peter Maydell @ 2014-02-25 17:01 UTC (permalink / raw)
  To: QEMU Developers
  Cc: Alexey Kardashevskiy, Patch Tracking, Thomas Huth, Andreas Färber

On 24 February 2014 19:08, Peter Maydell <peter.maydell@linaro.org> wrote:
> gcc's C++ compiler complains about being passed some -W options
> which make sense for C but not for C++. This means we mustn't try
> a C++ compile with QEMU_CFLAGS, but only with a filtered version
> that removes the offending options. This filtering was already being
> done for uses of C++ in the build itself, but was omitted for the
> "does C++ work?" configure test. This only showed up when doing
> builds which explicitly enabled -Werror with --enable-werror,
> because the "do the compilers work" tests were mistakenly placed
> above the "default werror based on whether compiling from git" code.
> Further, when the test did fail configure would plunge on regardless
> of the error since we were running do_cc in a subshell. Fix this
> complex of errors:
>
> 1. Move the default-werror code up so that there are no invocations
> of compile_object and friends between it and the point where we
> set $werror explicitly based on the --enable-werror command line
> option.
>
> 2. Provide a mechanism for filtering QEMU_CFLAGS to create
> QEMU_CXXFLAGS, and use it for the test we run here.
>
> 3. Provide a do_cxx function to run a test with the C++ compiler
> rather than doing cute tricks with subshells and do_cc.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> This was pretty confusing to debug (mostly because I got tripped
> up by the issue fixed by 1. above)...
>
> We could probably roll do_libtool into do_compiler, but I'd rather
> not mix that cleanup up with this build-breakage fix.

Alas, this won't work for clang builds, because clang
does not like us building a .c file with the C++ compiler,
which the test does.

thanks
-- PMM

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2014-02-25 17:02 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-02-24 19:08 [Qemu-devel] [PATCH] configure: Make C++ test work with --enable-werror Peter Maydell
2014-02-25  7:44 ` Thomas Huth
2014-02-25  8:32 ` Andreas Färber
2014-02-25 17:01 ` Peter Maydell

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.