[libvirt] [PATCH 08/89] configure: use LIBVIRT_ARG_WITH(_ALT) macros

[Jiri Denemark]

On Fri, Dec 16, 2016 at 10:10:36 +0100, Pavel Hrdina wrote:
> Signed-off-by: Pavel Hrdina <phrdina at redhat.com>
> ---
>  configure.ac             | 381 ++++++++++++++---------------------------------
>  m4/virt-apparmor.m4      |   6 +-
>  m4/virt-driver-bhyve.m4  |   5 +-
>  m4/virt-driver-uml.m4    |   5 +-
>  m4/virt-driver-vz.m4     |   5 +-
>  m4/virt-host-validate.m4 |   5 +-
>  m4/virt-init-script.m4   |   8 +-
>  m4/virt-lib.m4           |  21 +--
>  m4/virt-login-shell.m4   |   5 +-
>  m4/virt-nss.m4           |   7 +-
>  m4/virt-selinux.m4       |   6 +-
>  m4/virt-wireshark.m4     |   7 +-
>  12 files changed, 134 insertions(+), 327 deletions(-)
> 
> diff --git a/configure.ac b/configure.ac
> index 3f8e785078..c4f0623b8c 100644
> --- a/configure.ac
> +++ b/configure.ac
...
> @@ -1033,16 +974,16 @@ LIBXML_CFLAGS=""
>  LIBXML_LIBS=""
>  LIBXML_FOUND="no"
>  
> -AC_ARG_WITH([libxml], [AS_HELP_STRING([--with-libxml=@<:@PFX@:>@],
> -   [libxml2 location])])
> +LIBVIRT_ARG_WITH_ALT([LIBXML], [libxml2 (>= 2.6.0) location], [check])
> +
>  if test "x$with_libxml" = "xno" ; then
>      AC_MSG_CHECKING(for libxml2 libraries >= $LIBXML_REQUIRED)
>      AC_MSG_ERROR([libxml2 >= $LIBXML_REQUIRED is required for libvirt])
> -elif test "x$with_libxml" = "x" && test "x$PKG_CONFIG" != "x" ; then
> +elif test "x$with_libxml" != "xno" && test "x$PKG_CONFIG" != "x" ; then
>      PKG_CHECK_MODULES(LIBXML, libxml-2.0 >= $LIBXML_REQUIRED, [LIBXML_FOUND=yes], [LIBXML_FOUND=no])
>  fi
>  if test "$LIBXML_FOUND" = "no" ; then
> -    if test "x$with_libxml" != "x" ; then
> +    if test "x$with_libxml" != "xyes" ; then
>  	LIBXML_CONFIG=$with_libxml/bin/$LIBXML_CONFIG
>      fi
>      AC_MSG_CHECKING(libxml2 $LIBXML_CONFIG >= $LIBXML_REQUIRED )

This hunk doesn't seem to be correct. First, "x$with_libxml" != "xno"
is always true in the else branch of "x$with_libxml" = "xno". But more
importantly, you would remove the option to override the path to libxml
using --with-libxml=/path/to/libxml in case another libxml library is
installed.

...
> @@ -2219,18 +2082,20 @@ else
>    default_qemu_group=root
>  fi
>  
> -AC_ARG_WITH([qemu-user],
> -  [AS_HELP_STRING([--with-qemu-user],
> -    [username to run QEMU system instance as
> -     @<:@default=platform dependent@:>@])],
> -  [QEMU_USER=${withval}],
> -  [QEMU_USER=${default_qemu_user}])
> -AC_ARG_WITH([qemu-group],
> -  [AS_HELP_STRING([--with-qemu-group],
> -    [groupname to run QEMU system instance as
> -     @<:@default=platform dependent@:>@])],
> -  [QEMU_GROUP=${withval}],
> -  [QEMU_GROUP=${default_qemu_group}])
> +LIBVIRT_ARG_WITH_ALT([QEMU_USER], [username to run QEMU system instance as],
> +                     ['platform dependent'])

Any reason why $default_qemu_user is not used as the default value
rather than 'platform dependent'?

> +LIBVIRT_ARG_WITH_ALT([QEMU_GROUP], [groupname to run QEMU system instance as],
> +                     ['platform dependent'])

And similarly here for $default_qemu_group.

> +if test "x$with_qemu_user" = "xplatform dependent" ; then
> +    QEMU_USER="$default_qemu_user"
> +else
> +    QEMU_USER="$with_qemu_user"
> +fi
> +if test "x$with_qemu_group" = "xplatform dependent" ; then
> +    QEMU_GROUP="$default_qemu_group"
> +else
> +    QEMU_GROUP="$with_qemu_group"
> +fi
>  AC_DEFINE_UNQUOTED([QEMU_USER], ["$QEMU_USER"], [QEMU user account])
>  AC_DEFINE_UNQUOTED([QEMU_GROUP], ["$QEMU_GROUP"], [QEMU group account])
>  
...
> @@ -2467,30 +2324,22 @@ test "x$lv_cv_static_analysis" = xyes && t=1
>  AC_DEFINE_UNQUOTED([STATIC_ANALYSIS], [$t],
>    [Define to 1 when performing static analysis.])
>  
> -AC_ARG_WITH([default-editor],
> -  [AS_HELP_STRING([--with-default-editor],
> -    [Editor to use for interactive commands
> -     @<:@default=vi@:>@])],
> -  [DEFAULT_EDITOR=${withval}],
> -  [DEFAULT_EDITOR=vi])
> -AC_DEFINE_UNQUOTED([DEFAULT_EDITOR], ["$DEFAULT_EDITOR"], [Default editor to use])
> -
> -AC_ARG_WITH([loader-nvram],
> -  [AS_HELP_STRING([--with-loader-nvram],
> -    [Pass list of pairs of <loader>:<nvram> paths. Both
> -     pairs and list items are separated by a colon.
> -     @<:default=paths to OVMF and its clones@:>@])],
> -     [if test "$withval" = "no"; then
> -        withval=""
> -      else
> -        l=`echo $withval | tr ':' '\n' | wc -l`
> -        if test "`expr $l % 2`" -ne 0; then
> -            AC_MSG_ERROR([Malformed --with-loader-nvram argument])
> -        fi
> -      fi
> -      AC_DEFINE_UNQUOTED([DEFAULT_LOADER_NVRAM],
> -                          ["$withval"],
> -                          [List of loader:nvram pairs])])
> +LIBVIRT_ARG_WITH_ALT([DEFAULT_EDITOR], [Editor to use for interactive commands], [vi])
> +AC_DEFINE_UNQUOTED([DEFAULT_EDITOR], ["$with_default_editor"], [Default editor to use])
> +
> +LIBVIRT_ARG_WITH_ALT([LOADER_NVRAM],
> +                     [Pass list of pairs of <loader>:<nvram> paths.
> +                      Both pairs and list items are separated by a colon.],
> +                     [''])
> +if test "x$with_loader_nvram" != "xno" && \
> +   test "x$with_loader_nvram" != "x" ; then
> +    l=$(echo $with_loader_nvram | tr ':' '\n' | wc -l)
> +    if test $(expr $l % 2) -ne 0 ; then

I believe `...` is more portable than $(...)

> +        AC_MSG_ERROR([Malformed --with-loader-nvram argument])
> +    fi
> +    AC_DEFINE_UNQUOTED([DEFAULT_LOADER_NVRAM], [$with_loader_nvram],
> +                       [List of loader:nvram pairs])
> +fi
>  
>  # Some GNULIB base64 symbols clash with a kerberos library
>  AC_DEFINE_UNQUOTED([isbase64],[libvirt_gl_isbase64],[Hack to avoid symbol clash])
...

Jirka