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@xxxxxxxxxx> > 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 :| _______________________________________________ virt-tools-list mailing list virt-tools-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/virt-tools-list
