[virt-tools-list] [PATCH virt-viewer] configure: Check for all libvirt dependencies

Daniel P. Berrange berrange at redhat.com
Mon Apr 24 08:55:53 UTC 2017


On Mon, Apr 24, 2017 at 10:09:24AM +0200, Christophe Fergeau wrote:
> On Mon, Apr 24, 2017 at 09:42:43AM +0200, Pavel Grunt wrote:
> > Build with libvirt only when libvirt-glib is present
> 
> There is already such a check right after the part you modified:
> 
> AS_IF([test "x$with_libvirt" != "xno" && test "x$with_libvirt" != "xyes"],
>       [PKG_CHECK_EXISTS([libvirt >= $LIBVIRT_REQUIRED],
>                         [with_libvirt=yes], [with_libvirt=no])])
> 
> AS_IF([test "x$with_libvirt" = "xyes"],
>       [PKG_CHECK_MODULES(LIBVIRT, [libvirt >= $LIBVIRT_REQUIRED] [libvirt-glib-1.0 >= $LIBVIRT_GLIB_REQUI
>       [AC_DEFINE([HAVE_LIBVIRT], 1, [Have libvirt?])]
> )
> AM_CONDITIONAL([HAVE_LIBVIRT], [test "x$with_libvirt" = "xyes"])
> 
> The check you modified is when we autodetect libvirt presence rather than
> having an explicit --with[out]-libvirt.
> In such a situation, we expect libvirt support to be silently enabled/disabled
> depending on what is installed. This fails when libvirt is detected, but
> libvirt-glib is missing/too old. In this situation, without your change,
> configure would error out when we expect it to just disable libvirt support.
> Your patch should fix this case, and disable libvirt support in this situation.
> 
> If my understanding is correct,
> Acked-by: Christophe Fergeau <cfergeau at redhat.com>
> with a more detailed commit log.

NACK, to this. It is crazy to have us calling pkg-config twice for the same
modules. It should be possible to rewrite this to remove the duplicated check
entirely 


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 :|




More information about the virt-tools-list mailing list