Re: libvirt-php 0.5.1 uneeded files

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 27.11.2015 10:59, Remi Collet wrote:
> Le 27/11/2015 09:33, Remi Collet a écrit :
>> > Notice: I think it will be simpler to switch to the standard PHP
>> > extension build system (phpize; configure; make)
> See attached patch.
> This only manage the extension build (not the documentation)
> 
> TODO: add a --enable-libvirt-debug option to avoid having it always
> build in.
> 
> 
> Remi
> 
> 
> 0001-add-config.m4-to-allow-standard-PHP-extension-build-.patch
> 
> 
>>From 5b6e08f23f5eb5388c8ac7274786edd12a6695f0 Mon Sep 17 00:00:00 2001

Strange. Where does this line come from? I have to manually delete it to let git am through.

> From: Remi Collet <fedora@xxxxxxxxxxxxxxxxx>
> Date: Fri, 27 Nov 2015 10:57:32 +0100
> Subject: [PATCH] add config.m4 to allow standard PHP extension build system
> 
> ---
>  src/config.m4 | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 52 insertions(+)
>  create mode 100644 src/config.m4
> 
> diff --git a/src/config.m4 b/src/config.m4
> new file mode 100644
> index 0000000..ee2b47d
> --- /dev/null
> +++ b/src/config.m4

I guess the file should be at the top level instead of src/

> @@ -0,0 +1,52 @@
> +PHP_ARG_WITH(libvirt, for libvirt support,
> +[  --with-libvirt             Include varnish support])

Sort of misleading ... configure --with-libvirt for varnish support?

> +
> +if test "$PHP_LIBVIRT" != "no"; then
> +
> +  AC_PATH_PROG(PKG_CONFIG, pkg-config, no)
> +
> +  if test -x "$PKG_CONFIG" && $PKG_CONFIG varnishapi --exists ; then

Why? This module is a php module, not a varnish one. E.g. on my system, I do have php installed but no varnish.

> +    AC_MSG_CHECKING(libvirt version)
> +    if $PKG_CONFIG libvirt --atleast-version=1.2.8 ; then

I'd rather see this comparing to a variable, since you're repeating it throughout the code.

> +      LIBVIRT_INCLUDE=`$PKG_CONFIG libvirt --cflags`
> +      LIBVIRT_LIBRARY=`$PKG_CONFIG libvirt --libs`
> +      LIBVIRT_VERSION=`$PKG_CONFIG libvirt --modversion`
> +      AC_MSG_RESULT($LIBVIRT_VERSION)
> +    else
> +      AC_MSG_ERROR(version too old)
> +    fi
> +    PHP_EVAL_INCLINE($LIBVIRT_INCLUDE)
> +    PHP_EVAL_LIBLINE($LIBVIRT_LIBRARY, LIBVIRT_SHARED_LIBADD)

This is rather displeasing. I mean, on one hand we have grown powerful m4 language with all the nice macros and now we have to throw it away and write what would have fit on a single line into blocks like these. But I guess there's not a better way.

> +
> +    AC_MSG_CHECKING(libvirt-qemu version)
> +    if $PKG_CONFIG libvirt-qemu --atleast-version=1.2.8 ; then
> +      QEMU_INCLUDE=`$PKG_CONFIG libvirt-qemu --cflags`
> +      QEMU_LIBRARY=`$PKG_CONFIG libvirt-qemu --libs`
> +      QEMU_VERSION=`$PKG_CONFIG libvirt-qemu --modversion`
> +      AC_MSG_RESULT($QEMU_VERSION)
> +    else
> +      AC_MSG_ERROR(version too old)
> +    fi
> +    PHP_EVAL_INCLINE($QEMU_INCLUDE)
> +    PHP_EVAL_LIBLINE($QEMU_LIBRARY, LIBVIRT_SHARED_LIBADD)
> +
> +    AC_MSG_CHECKING(libxml version)
> +    if $PKG_CONFIG libxml-2.0 --atleast-version=1.2.8 ; then

See? this should have been 2.0.0

> +      LIBXML_INCLUDE=`$PKG_CONFIG libxml-2.0 --cflags`
> +      LIBXML_LIBRARY=`$PKG_CONFIG libxml-2.0 --libs`
> +      LIBXML_VERSION=`$PKG_CONFIG libxml-2.0 --modversion`
> +      AC_MSG_RESULT($LIBXML_VERSION)
> +    else
> +      AC_MSG_ERROR(version too old)
> +    fi
> +    PHP_EVAL_INCLINE($LIBXML_INCLUDE)
> +    PHP_EVAL_LIBLINE($LIBXML_LIBRARY, LIBVIRT_SHARED_LIBADD)
> +
> +    CFLAGS="$CFLAGS -DCOMPILE_DL_LIBVIRT=1"
> +
> +    PHP_SUBST(LIBVIRT_SHARED_LIBADD)
> +    PHP_NEW_EXTENSION(libvirt, libvirt-php.c sockets.c vncfunc.c, $ext_shared)
> +  else
> +    AC_MSG_ERROR(pkg-config not found)
> +  fi
> +fi
> -- 2.1.0

there's a plenty of other features missing - e.g. docs won't be built with this. I don't know about tests, because I am unable to get this working on my machine. I mean, this is what I've managed to get to:

libvirt-php.git $ make
/bin/sh /home/zippy/work/libvirt/libvirt-php.git/libtool --mode=link cc -DPHP_ATOM_INC -I/home/zippy/work/libvirt/libvirt-php.git/include -I/home/zippy/work/libvirt/libvirt-php.git/main -I/home/zippy/work/libvirt/libvirt-php.git -I/usr/lib64/php5.5/include/php -I/usr/lib64/php5.5/include/php/main -I/usr/lib64/php5.5/include/php/TSRM -I/usr/lib64/php5.5/include/php/Zend -I/usr/lib64/php5.5/include/php/ext -I/usr/lib64/php5.5/include/php/ext/date/lib -I/usr/include/libxml2  -DHAVE_CONFIG_H  -g -O2   -o libvirt.la -export-dynamic -avoid-version -prefer-pic -module -rpath /home/zippy/work/libvirt/libvirt-php.git/modules  src/.lo -lvirt -lvirt-qemu -lvirt -lxml2
libtool: Version mismatch error.  This is libtool 2.4.6, but the
libtool: definition of this LT_INIT comes from an older release.
libtool: You should recreate aclocal.m4 with macros from libtool 2.4.6
libtool: and run autoconf again.
Makefile:186: recipe for target 'libvirt.la' failed
make: *** [libvirt.la] Error 63


Frankly, I got frustrated by the lack of documentation on this, but that's not your fault.
This is what I have rewritten your patch into:

diff --git a/config.m4 b/config.m4
new file mode 100644
index 0000000..fef145f
--- /dev/null
+++ b/config.m4
@@ -0,0 +1,54 @@
+PHP_ARG_WITH(libvirt, for libvirt support,
+[  --with-libvirt           Include libvirt support], yes)
+
+LIBVIRT_REQUIRED=1.2.9
+LIBXML_REQUIRED=2.0.0
+
+if test "$PHP_LIBVIRT" != "no"; then
+    AC_PATH_PROG(PKG_CONFIG, pkg-config, no)
+
+    if ! test -x "$PKG_CONFIG" ; then
+        AC_MSG_ERROR(no pkg-config found)
+    fi
+
+    AC_MSG_CHECKING(for libvirt)
+    LIBVIRT_VERSION=`$PKG_CONFIG libvirt --modversion`
+    if $PKG_CONFIG libvirt --atleast-version=$LIBVIRT_REQUIRED ; then
+      LIBVIRT_INCLUDE=`$PKG_CONFIG libvirt --cflags`
+      LIBVIRT_LIBRARY=`$PKG_CONFIG libvirt --libs`
+      AC_MSG_RESULT($LIBVIRT_VERSION)
+    else
+      AC_MSG_ERROR($LIBVIRT_VERSION found, but $LIBVIRT_REQUIRED required)
+    fi
+
+    PHP_EVAL_INCLINE($LIBVIRT_INCLUDE)
+    PHP_EVAL_LIBLINE($LIBVIRT_LIBRARY, LIBVIRT_SHARED_LIBADD)
+
+    AC_MSG_CHECKING(libvirt-qemu version)
+    QEMU_VERSION=`$PKG_CONFIG libvirt-qemu --modversion`
+    if $PKG_CONFIG libvirt-qemu --atleast-version=$LIBVIRT_REQUIRED ; then
+      QEMU_INCLUDE=`$PKG_CONFIG libvirt-qemu --cflags`
+      QEMU_LIBRARY=`$PKG_CONFIG libvirt-qemu --libs`
+      AC_MSG_RESULT($QEMU_VERSION)
+    else
+      AC_MSG_ERROR(QEMU_VERSION found, but $LIBVIRT_REQUIRED required)
+    fi
+
+    PHP_EVAL_INCLINE($QEMU_INCLUDE)
+    PHP_EVAL_LIBLINE($QEMU_LIBRARY, LIBVIRT_SHARED_LIBADD)
+
+    AC_MSG_CHECKING(libxml version)
+    LIBXML_VERSION=`$PKG_CONFIG libxml-2.0 --modversion`
+    if $PKG_CONFIG libxml-2.0 --atleast-version=$LIBXML_REQUIRED ; then
+      LIBXML_INCLUDE=`$PKG_CONFIG libxml-2.0 --cflags`
+      LIBXML_LIBRARY=`$PKG_CONFIG libxml-2.0 --libs`
+      AC_MSG_RESULT($LIBXML_VERSION)
+    else
+      AC_MSG_ERROR($LIBXML_VERSION found, but $LIBXML_REQUIRED required)
+    fi
+    PHP_EVAL_INCLINE($LIBXML_INCLUDE)
+    PHP_EVAL_LIBLINE($LIBXML_LIBRARY, LIBVIRT_SHARED_LIBADD)
+
+    PHP_SUBST(LIBVIRT_SHARED_LIBADD)
+    PHP_NEW_EXTENSION(libvirt, src/, $ext_shared)
+fi

Which yet again is lacking a lot of features that are in configure.ac, but it could have been good starting point.

Anyway, we should either use phpize or configure.ac -- because phpize is unhappy with fact that we have the two at the same time.

Michal

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]