[Qemu-devel] [PATCH] configure: don't warn SDL abi if disabled

Peter Xu posted 1 patch 6 years ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20180410054034.20479-1-peterx@redhat.com
Test checkpatch passed
Test docker-build@min-glib passed
Test docker-mingw@fedora passed
Test s390x passed
configure | 83 ++++++++++++++++++++++++++++++++++-----------------------------
1 file changed, 45 insertions(+), 38 deletions(-)
[Qemu-devel] [PATCH] configure: don't warn SDL abi if disabled
Posted by Peter Xu 6 years ago
SDL has the same problem as GTK that we might get warnings on SDL ABI
version even if SDL is disabled.  Fix that by only probing SDL if SDL is
enabled.  Also this should let configure be a little bit faster since we
don't really need to probe SDL stuff when it's off.

CC: Paolo Bonzini <pbonzini@redhat.com>
CC: Gerd Hoffmann <kraxel@redhat.com>
CC: Peter Maydell <peter.maydell@linaro.org>
CC: Daniel P. Berrange <berrange@redhat.com>
CC: Fam Zheng <famz@redhat.com>
CC: "Philippe Mathieu-Daudé" <f4bug@amsat.org>
Signed-off-by: Peter Xu <peterx@redhat.com>
---
 configure | 83 ++++++++++++++++++++++++++++++++++-----------------------------
 1 file changed, 45 insertions(+), 38 deletions(-)

diff --git a/configure b/configure
index 752dd9ef32..f647026b8d 100755
--- a/configure
+++ b/configure
@@ -2836,49 +2836,52 @@ fi
 # Look for sdl configuration program (pkg-config or sdl-config).  Try
 # sdl-config even without cross prefix, and favour pkg-config over sdl-config.
 
-if test "$sdlabi" = ""; then
-    if $pkg_config --exists "sdl2"; then
-        sdlabi=2.0
-    elif $pkg_config --exists "sdl"; then
-        sdlabi=1.2
-    else
-        sdlabi=2.0
-    fi
-fi
+sdl_probe ()
+{
+  sdl_too_old=no
+  if test "$sdlabi" = ""; then
+      if $pkg_config --exists "sdl2"; then
+          sdlabi=2.0
+      elif $pkg_config --exists "sdl"; then
+          sdlabi=1.2
+      else
+          sdlabi=2.0
+      fi
+  fi
 
-if test $sdlabi = "2.0"; then
-    sdl_config=$sdl2_config
-    sdlname=sdl2
-    sdlconfigname=sdl2_config
-elif test $sdlabi = "1.2"; then
-    sdlname=sdl
-    sdlconfigname=sdl_config
-else
-    error_exit "Unknown sdlabi $sdlabi, must be 1.2 or 2.0"
-fi
+  if test $sdlabi = "2.0"; then
+      sdl_config=$sdl2_config
+      sdlname=sdl2
+      sdlconfigname=sdl2_config
+  elif test $sdlabi = "1.2"; then
+      sdlname=sdl
+      sdlconfigname=sdl_config
+  else
+      error_exit "Unknown sdlabi $sdlabi, must be 1.2 or 2.0"
+  fi
 
-if test "$(basename $sdl_config)" != $sdlconfigname && ! has ${sdl_config}; then
-  sdl_config=$sdlconfigname
-fi
+  if test "$(basename $sdl_config)" != $sdlconfigname && ! has ${sdl_config}; then
+    sdl_config=$sdlconfigname
+  fi
 
-if $pkg_config $sdlname --exists; then
-  sdlconfig="$pkg_config $sdlname"
-  sdlversion=$($sdlconfig --modversion 2>/dev/null)
-elif has ${sdl_config}; then
-  sdlconfig="$sdl_config"
-  sdlversion=$($sdlconfig --version)
-else
-  if test "$sdl" = "yes" ; then
-    feature_not_found "sdl" "Install SDL2-devel"
+  if $pkg_config $sdlname --exists; then
+    sdlconfig="$pkg_config $sdlname"
+    sdlversion=$($sdlconfig --modversion 2>/dev/null)
+  elif has ${sdl_config}; then
+    sdlconfig="$sdl_config"
+    sdlversion=$($sdlconfig --version)
+  else
+    if test "$sdl" = "yes" ; then
+      feature_not_found "sdl" "Install SDL2-devel"
+    fi
+    sdl=no
+    # no need to do the rest
+    return
+  fi
+  if test -n "$cross_prefix" && test "$(basename "$sdlconfig")" = sdl-config; then
+    echo warning: using "\"$sdlconfig\"" to detect cross-compiled sdl >&2
   fi
-  sdl=no
-fi
-if test -n "$cross_prefix" && test "$(basename "$sdlconfig")" = sdl-config; then
-  echo warning: using "\"$sdlconfig\"" to detect cross-compiled sdl >&2
-fi
 
-sdl_too_old=no
-if test "$sdl" != "no" ; then
   cat > $TMPC << EOF
 #include <SDL.h>
 #undef main /* We don't want SDL to override our main() */
@@ -2920,6 +2923,10 @@ EOF
     fi
     sdl=no
   fi # sdl compile test
+}
+
+if test "$sdl" != "no" ; then
+  sdl_probe
 fi
 
 if test "$sdl" = "yes" ; then
-- 
2.14.3


Re: [Qemu-devel] [PATCH] configure: don't warn SDL abi if disabled
Posted by Daniel P. Berrangé 6 years ago
On Tue, Apr 10, 2018 at 01:40:34PM +0800, Peter Xu wrote:
> SDL has the same problem as GTK that we might get warnings on SDL ABI
> version even if SDL is disabled.  Fix that by only probing SDL if SDL is
> enabled.  Also this should let configure be a little bit faster since we
> don't really need to probe SDL stuff when it's off.
> 
> CC: Paolo Bonzini <pbonzini@redhat.com>
> CC: Gerd Hoffmann <kraxel@redhat.com>
> CC: Peter Maydell <peter.maydell@linaro.org>
> CC: Daniel P. Berrange <berrange@redhat.com>
> CC: Fam Zheng <famz@redhat.com>
> CC: "Philippe Mathieu-Daudé" <f4bug@amsat.org>
> Signed-off-by: Peter Xu <peterx@redhat.com>

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH] configure: don't warn SDL abi if disabled
Posted by Fam Zheng 6 years ago
On Tue, 04/10 13:40, Peter Xu wrote:
> SDL has the same problem as GTK that we might get warnings on SDL ABI
> version even if SDL is disabled.  Fix that by only probing SDL if SDL is
> enabled.  Also this should let configure be a little bit faster since we
> don't really need to probe SDL stuff when it's off.
> 
> CC: Paolo Bonzini <pbonzini@redhat.com>
> CC: Gerd Hoffmann <kraxel@redhat.com>
> CC: Peter Maydell <peter.maydell@linaro.org>
> CC: Daniel P. Berrange <berrange@redhat.com>
> CC: Fam Zheng <famz@redhat.com>
> CC: "Philippe Mathieu-Daudé" <f4bug@amsat.org>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  configure | 83 ++++++++++++++++++++++++++++++++++-----------------------------
>  1 file changed, 45 insertions(+), 38 deletions(-)
> 
> diff --git a/configure b/configure
> index 752dd9ef32..f647026b8d 100755
> --- a/configure
> +++ b/configure
> @@ -2836,49 +2836,52 @@ fi
>  # Look for sdl configuration program (pkg-config or sdl-config).  Try
>  # sdl-config even without cross prefix, and favour pkg-config over sdl-config.
>  
> -if test "$sdlabi" = ""; then
> -    if $pkg_config --exists "sdl2"; then
> -        sdlabi=2.0
> -    elif $pkg_config --exists "sdl"; then
> -        sdlabi=1.2
> -    else
> -        sdlabi=2.0
> -    fi
> -fi
> +sdl_probe ()
> +{

We have had

    function_name()
    {
    ...

and

    function_name() {
    ...

and

    function_name () {
    ...

finally you invent the last in the family:

    function_name ()
    {

:)
.
> +  sdl_too_old=no
> +  if test "$sdlabi" = ""; then
> +      if $pkg_config --exists "sdl2"; then
> +          sdlabi=2.0
> +      elif $pkg_config --exists "sdl"; then
> +          sdlabi=1.2
> +      else
> +          sdlabi=2.0
> +      fi
> +  fi
>  
> -if test $sdlabi = "2.0"; then
> -    sdl_config=$sdl2_config
> -    sdlname=sdl2
> -    sdlconfigname=sdl2_config
> -elif test $sdlabi = "1.2"; then
> -    sdlname=sdl
> -    sdlconfigname=sdl_config
> -else
> -    error_exit "Unknown sdlabi $sdlabi, must be 1.2 or 2.0"
> -fi
> +  if test $sdlabi = "2.0"; then
> +      sdl_config=$sdl2_config
> +      sdlname=sdl2
> +      sdlconfigname=sdl2_config
> +  elif test $sdlabi = "1.2"; then
> +      sdlname=sdl
> +      sdlconfigname=sdl_config
> +  else
> +      error_exit "Unknown sdlabi $sdlabi, must be 1.2 or 2.0"
> +  fi
>  
> -if test "$(basename $sdl_config)" != $sdlconfigname && ! has ${sdl_config}; then
> -  sdl_config=$sdlconfigname
> -fi
> +  if test "$(basename $sdl_config)" != $sdlconfigname && ! has ${sdl_config}; then
> +    sdl_config=$sdlconfigname
> +  fi
>  
> -if $pkg_config $sdlname --exists; then
> -  sdlconfig="$pkg_config $sdlname"
> -  sdlversion=$($sdlconfig --modversion 2>/dev/null)
> -elif has ${sdl_config}; then
> -  sdlconfig="$sdl_config"
> -  sdlversion=$($sdlconfig --version)
> -else
> -  if test "$sdl" = "yes" ; then
> -    feature_not_found "sdl" "Install SDL2-devel"
> +  if $pkg_config $sdlname --exists; then
> +    sdlconfig="$pkg_config $sdlname"
> +    sdlversion=$($sdlconfig --modversion 2>/dev/null)
> +  elif has ${sdl_config}; then
> +    sdlconfig="$sdl_config"
> +    sdlversion=$($sdlconfig --version)
> +  else
> +    if test "$sdl" = "yes" ; then
> +      feature_not_found "sdl" "Install SDL2-devel"
> +    fi
> +    sdl=no
> +    # no need to do the rest
> +    return
> +  fi
> +  if test -n "$cross_prefix" && test "$(basename "$sdlconfig")" = sdl-config; then
> +    echo warning: using "\"$sdlconfig\"" to detect cross-compiled sdl >&2
>    fi
> -  sdl=no
> -fi
> -if test -n "$cross_prefix" && test "$(basename "$sdlconfig")" = sdl-config; then
> -  echo warning: using "\"$sdlconfig\"" to detect cross-compiled sdl >&2
> -fi
>  
> -sdl_too_old=no
> -if test "$sdl" != "no" ; then
>    cat > $TMPC << EOF
>  #include <SDL.h>
>  #undef main /* We don't want SDL to override our main() */
> @@ -2920,6 +2923,10 @@ EOF
>      fi
>      sdl=no
>    fi # sdl compile test
> +}
> +
> +if test "$sdl" != "no" ; then
> +  sdl_probe
>  fi
>  
>  if test "$sdl" = "yes" ; then
> -- 
> 2.14.3
> 

Reviewed-by: Fam Zheng <famz@redhat.com>


Re: [Qemu-devel] [PATCH] configure: don't warn SDL abi if disabled
Posted by Peter Xu 6 years ago
On Tue, Apr 10, 2018 at 04:49:50PM +0800, Fam Zheng wrote:

[...]

> We have had
> 
>     function_name()
>     {
>     ...
> 
> and
> 
>     function_name() {
>     ...
> 
> and
> 
>     function_name () {
>     ...
> 
> finally you invent the last in the family:
> 
>     function_name ()
>     {
> 
> :)

I was careful about whether we were using the "function" keyword, but
still that seems to be not enough... :-)

[...]

> Reviewed-by: Fam Zheng <famz@redhat.com>

Thanks!

-- 
Peter Xu