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)
next prev parent 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 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.