Re: [Qemu-devel] [PATCH 2/3] configure: Default to enable module build

[Fam Zheng]

On Mon, 01/12 11:26, Andreas Färber wrote:
> 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 <address@hidden>
> 
> 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?
> 

Will remove the above error_exit and leave to caller to do flow control and
error reporting.

Thanks!

Fam