qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Andreas Färber" <afaerber@suse.de>
To: Fam Zheng <famz@redhat.com>, qemu-devel@nongnu.org
Cc: Brian Jackson <iggy@theiggy.com>,
	Peter Maydell <peter.maydell@linaro.org>,
	Michael Tokarev <mjt@tls.msk.ru>,
	Stefan Hajnoczi <stefanha@redhat.com>,
	Bharata B Rao <bharata.rao@gmail.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Richard Henderson <rth@twiddle.net>
Subject: Re: [Qemu-devel] [PATCH 2/3] configure: Default to enable module build
Date: Mon, 12 Jan 2015 11:26:24 +0100	[thread overview]
Message-ID: <54B3A150.3060507@suse.de> (raw)
In-Reply-To: <1421037791-32021-3-git-send-email-famz@redhat.com>

Am 12.01.2015 um 05:43 schrieb Fam Zheng:
> We have module build support around for a while, but also had it bitrot
> several times. It probably makes sense to enable it by default so that
> people can notice and use it.
> 
> Counterpart to --enable-modules, which is turned as default,
> --disable-modules is added to suppress it. If both are omitted, the
> support is guesses as usual.
> 
> Signed-off-by: Fam Zheng <famz@redhat.com>

General idea seems okay, however an error handling nit below.

> ---
>  configure | 96 ++++++++++++++++++++++++++++++++++++++++++++-------------------
>  1 file changed, 67 insertions(+), 29 deletions(-)
> 
> diff --git a/configure b/configure
> index 7539645..2015179 100755
> --- a/configure
> +++ b/configure
> @@ -271,7 +271,7 @@ gcov_tool="gcov"
>  EXESUF=""
>  DSOSUF=".so"
>  LDFLAGS_SHARED="-shared"
> -modules="no"
> +modules=""
>  prefix="/usr/local"
>  mandir="\${prefix}/share/man"
>  datadir="\${prefix}/share"
> @@ -768,6 +768,9 @@ for opt do
>    --enable-modules)
>        modules="yes"
>    ;;
> +  --disable-modules)
> +      modules="no"
> +  ;;
>    --cpu=*)
>    ;;
>    --target-list=*) target_list="$optarg"
> @@ -1259,7 +1262,8 @@ Advanced options (experts only):
>    --sysconfdir=PATH        install config in PATH$confsuffix
>    --localstatedir=PATH     install local state in PATH (set at runtime on win32)
>    --with-confsuffix=SUFFIX suffix for QEMU data inside datadir/libdir/sysconfdir [$confsuffix]
> -  --enable-modules         enable modules support
> +  --enable-modules         enable modules support (default)
> +  --disable-modules        enable modules support
>    --enable-debug-tcg       enable TCG debugging
>    --disable-debug-tcg      disable TCG debugging (default)
>    --enable-debug-info      enable debugging information (default)
> @@ -2699,22 +2703,26 @@ if test "$mingw32" = yes; then
>  else
>      glib_req_ver=2.12
>  fi
> -glib_modules=gthread-2.0
> -if test "$modules" = yes; then
> -    glib_modules="$glib_modules gmodule-2.0"
> -fi
>  
> -for i in $glib_modules; do
> -    if $pkg_config --atleast-version=$glib_req_ver $i; then
> -        glib_cflags=`$pkg_config --cflags $i`
> -        glib_libs=`$pkg_config --libs $i`
> -        CFLAGS="$glib_cflags $CFLAGS"
> -        LIBS="$glib_libs $LIBS"
> -        libs_qga="$glib_libs $libs_qga"
> -    else
> -        error_exit "glib-$glib_req_ver $i is required to compile QEMU"
> -    fi
> -done
> +glib_module_try_config()
> +{
> +  if $pkg_config --atleast-version=$glib_req_ver $1; then
> +    local probe_cflags=$($pkg_config --cflags $1)
> +    local probe_libs=$($pkg_config --libs $1)
> +    CFLAGS="$probe_cflags $CFLAGS"
> +    LIBS="$probe_libs $LIBS"
> +    libs_qga="$probe_libs $libs_qga"
> +    glib_cflags="$probe_cflags $glib_cflags"
> +    glib_libs="$probe_libs $glib_libs"
> +    return 0
> +  else
> +    return 1
> +    error_exit "glib-$glib_req_ver $i is required to compile QEMU"

Is this error_exit ever executed? I.e., shouldn't the two lines be
reordered?

> +  fi
> +}
> +
> +glib_module_try_config gthread-2.0 || \
> +  error_exit "glib-$glib_req_ver gthread-2.0 is required to compile QEMU"

Depending on the above, we might drop this error_exit duplication?

Regards,
Andreas

>  
>  # g_test_trap_subprocess added in 2.38. Used by some tests.
>  glib_subprocess=yes
> @@ -2723,19 +2731,49 @@ if ! $pkg_config --atleast-version=2.38 glib-2.0; then
>  fi
>  
>  ##########################################
> -# SHA command probe for modules
> -if test "$modules" = yes; then
> -    shacmd_probe="sha1sum sha1 shasum"
> -    for c in $shacmd_probe; do
> -        if has $c; then
> -            shacmd="$c"
> -            break
> -        fi
> -    done
> -    if test "$shacmd" = ""; then
> -        error_exit "one of the checksum commands is required to enable modules: $shacmd_probe"
> +# SHA command and gmodule-2.0 probe for modules
> +# return 0 if probe succeeds
> +# $1: true - force mode, exit if probe fail
> +#     false - optoinal mode, return 1 if probe fail
> +module_try_enable()
> +{
> +  force=$1
> +  shacmd_probe="sha1sum sha1 shasum"
> +  for c in $shacmd_probe; do
> +    if has $c; then
> +      shacmd="$c"
> +      break
>      fi
> -fi
> +  done
> +  if test "$shacmd" = ""; then
> +    if $force; then
> +      error_exit "one of the checksum commands is required to enable modules: $shacmd_probe"
> +    else
> +      modules="no"
> +      return
> +    fi
> +  fi
> +  if ! glib_module_try_config gmodule-2.0; then
> +    if $force; then
> +      error_exit "glib-$glib_req_ver gthread-2.0 is required to compile QEMU"
> +    else
> +      modules="no"
> +      return
> +    fi
> +  fi
> +  modules="yes"
> +}
> +
> +case "$modules" in
> +  yes)
> +    module_try_enable true
> +    ;;
> +  "")
> +    module_try_enable false
> +    ;;
> +  no)
> +    ;;
> +esac
>  
>  ##########################################
>  # pixman support probe

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Jennifer Guild, Dilip Upmanyu,
Graham Norton; HRB 21284 (AG Nürnberg)

  reply	other threads:[~2015-01-12 10:26 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-01-12  4:43 [Qemu-devel] [PATCH 0/3] buildsys: Fix and enable module build Fam Zheng
2015-01-12  4:43 ` [Qemu-devel] [PATCH 1/3] rules.mak: Fix " Fam Zheng
2015-01-12  4:43 ` [Qemu-devel] [PATCH 2/3] configure: Default to enable " Fam Zheng
2015-01-12 10:26   ` Andreas Färber [this message]
2015-01-13  8:49     ` Fam Zheng
2015-01-12  4:43 ` [Qemu-devel] [PATCH 3/3] .travis.yml: Add "--disable-modules" Fam Zheng
2015-01-12  6:04 ` [Qemu-devel] [PATCH 0/3] buildsys: Fix and enable module build Bharata B Rao
2015-01-12 10:30 ` Paolo Bonzini

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=54B3A150.3060507@suse.de \
    --to=afaerber@suse.de \
    --cc=bharata.rao@gmail.com \
    --cc=famz@redhat.com \
    --cc=iggy@theiggy.com \
    --cc=mjt@tls.msk.ru \
    --cc=pbonzini@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=rth@twiddle.net \
    --cc=stefanha@redhat.com \
    /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).