On 11/22/2013 09:59 AM, Daniel P. Berrange wrote: > From: "Daniel P. Berrange" <berrange@xxxxxxxxxx> > > The python binding now lives in > > http://libvirt.org/git/libvirt-python.git > > that repo also provides an RPM which is upgrade compatible > with the old libvirt-python sub-RPM. > > Signed-off-by: Daniel P. Berrange <berrange@xxxxxxxxxx> > --- > .gitignore | 11 - > Makefile.am | 7 +- > autobuild.sh | 6 +- > cfg.mk | 7 +- > configure.ac | 86 - These changes are important... > libvirt.spec.in | 36 +- > m4/virt-compile-warnings.m4 | 8 - > mingw-libvirt.spec.in | 1 - and these... > run.in | 9 - and this (hmm, do we need a ./run counterpart added to libvirt-python to make it easy to import a built but uninstalled python bindings for testing purposes?) > 37 files changed, 7 insertions(+), 13369 deletions(-) The rest of the patch is mechanical 'git rm'; it might have been nice to use git's -O/path/to/file to order the patch to float the interesting parts up top and trim the rest of the email. grepping for python still had hits after your patch; here's my analysis: ChangeLog-old - good to keep; we aren't changing details of historical releases bootstrap.conf - unsure: Does bootstrap.conf still need to worry about finding python-config, or is that a thing of the past? cfg.mk - keep; we still have .py files for docs docs/Makefile.am docs/apibuild.py docs/apps.html.in docs/bindings.html.in docs/errors.html.in docs/hooks.html.in docs/index.py docs/library.xen docs/news.html.in docs/python.html.in docs/sitemap.html.in docs/testsuites.html.in docs/windows.html.in - good to keep all docs stuff for doc generation libvirt.spec.in - good to keep; only mentions the historical %changelog src/esx/esx_vi_generator.py src/hyperv/hyperv_wmi_generator.py src/util/virconf.c src/util/virkeycode-mapgen.py - good to keep all the C code generators We also need to scrub the code base for references to 1.1.5 and instead refer to 1.2.0. > diff --git a/.gitignore b/.gitignore > index 6b024e7..faabd33 100644 > --- a/.gitignore > +++ b/.gitignore > @@ -91,17 +91,6 @@ > /mkinstalldirs > /po/* > /proxy/ > -/python/generated.stamp > -/python/generator.py.stamp > -/python/libvirt-export.c > -/python/libvirt-lxc-export.c > -/python/libvirt-lxc.[ch] > -/python/libvirt-qemu-export.c > -/python/libvirt-qemu.[ch] > -/python/libvirt.[ch] > -/python/libvirt.py > -/python/libvirt_lxc.py > -/python/libvirt_qemu.py Oddly enough, removing these lines will mean that someone with an incremental tree that likes to switch branches between master and any earlier branch (say v1.0.5-maint) will now see git complaining about untracked files. I personally tend to avoid removing lines from .gitignore (if we've ever ignored a file in the past, then branch switching could leave the file around to still be ignored); but I won't complain too much if you make the deletion (I can always re-add the lines in my .git/info/exclude for my own environment). > +++ b/Makefile.am > @@ -74,8 +73,6 @@ check-local: all tests > > tests: > @(cd docs/examples ; $(MAKE) MAKEFLAGS+=--silent tests) > - @(if [ "$(pythondir)" != "" ] ; then cd python ; \ > - $(MAKE) MAKEFLAGS+=--silent tests ; fi) Did configure.ac clean up the setting of $(pythondir)? /me looks some more Wow - we never set $(pythondir) anywhere pre-patch, so it only ever did something if you invoked 'make pythondir=...', and since autobuild wasn't doing it, I think this was dead code. > +++ b/autobuild.sh > @@ -86,8 +86,7 @@ if test -x /usr/bin/i686-w64-mingw32-gcc ; then > --prefix="$AUTOBUILD_INSTALL_ROOT/i686-w64-mingw32/sys-root/mingw" \ > --enable-expensive-tests \ > --enable-werror \ > - --without-libvirtd \ > - --without-python > + --without-libvirtd Hmm. Was --without-python disabling _just_ the python bindings, or was it _also_ disabling our attempts to use $(PYTHON) for building things in our docs/ subtree? We may STILL want to keep this setting for controlling whether to attempt to build generated docs (vs. using the pre-generated version shipped in the tarball). > +++ b/configure.ac > @@ -2012,83 +2012,6 @@ fi > AM_CONDITIONAL([WITH_HYPERV], [test "$with_hyperv" = "yes"]) > > > - AC_MSG_NOTICE(Found python in $with_python/bin/python) > - PYTHON="$with_python/bin/python" > - with_python=yes > - else > - if test -x "$with_python" > - then > - AC_MSG_NOTICE(Found python in $with_python) > - PYTHON="$with_python" > - with_python=yes > - else > - if test -x "$PYTHON" > - then > - AC_MSG_NOTICE(Found python in environment PYTHON=$PYTHON) > - with_python=yes > - fi > - fi > - fi > - > - if test "$with_python" = "yes" ; then > - AM_PATH_PYTHON(,, [:]) Ouch. Deleting this may break uses of $(PYTHON) when building our docs. > - > - if test "$PYTHON" != : ; then > - PYTHON_CONFIG="$PYTHON-config" Meanwhile, this part is probably safe to drop, which goes back to my question on getting rid of the check for python-config in bootstrap.conf. > - fi > - else > - AC_MSG_ERROR([You must install python to build Python bindings]) > - fi > - else > - AC_MSG_NOTICE([Could not find python in $with_python, disabling bindings]) > - with_python=no > - fi > -fi So it looks like pre-patch, --without-python attempted to disable only the bindings, but may have also accidentally disabled the setting of $(PYTHON)... > -AM_CONDITIONAL([WITH_PYTHON], [test "$with_python" = "yes"]) > -AC_SUBST([PYTHON_VERSION]) > -AC_SUBST([PYTHON_INCLUDES]) > - > dnl Allow perl overrides > AC_PATH_PROG([PERL], [perl]) ...maybe all we need is a simple AC_PATH_PROG([PYTHON], [python]) for the sake of still using $(PYTHON) during doc generation? And some of this goes back to whether deleting --without-python from autobuild makes sense. > --- a/examples/domain-events/events-python/event-test.py Giant snip :) > +++ b/libvirt.spec.in > @@ -126,7 +126,6 @@ > %define with_libssh2 0%{!?_without_libssh2:0} > > # Non-server/HV driver defaults which are always enabled > -%define with_python 0%{!?_without_python:1} > %define with_sasl 0%{!?_without_sasl:1} > > > @@ -425,7 +424,6 @@ BuildRequires: gettext-devel > BuildRequires: libtool > BuildRequires: /usr/bin/pod2man > %endif > -BuildRequires: python-devel Ouch. Don't we still need a buildreq on 'python' for doc generation purposes, possibly conditionally based on whether we have patches applied to a downstream rpm that warrant doc regeneration? > -%if %{with_python} > -%package python > -Summary: Python bindings for the libvirt library > -Group: Development/Libraries > -Requires: %{name}-client = %{version}-%{release} All of this transferred over to the libvirt-python.git, so good to see it go here. > -%if ! %{with_python} > - %define _without_python --without-python > -%endif > - > %if ! %{with_libvirtd} > %define _without_libvirtd --without-libvirtd > %endif > @@ -1378,7 +1359,6 @@ of recent versions of Linux (and other OSes). > %{?_without_sasl} \ > %{?_without_avahi} \ > %{?_without_polkit} \ > - %{?_without_python} \ Same question as in autobuild on whether --without-python still matters for suppressing doc regeneration, or whether it was intended only for suppressing bindings. > --- a/python/Makefile.am > +++ /dev/null Another giant snip :) > +++ b/run.in > @@ -58,15 +58,6 @@ export LIBVIRT_LOCK_MANAGER_PLUGIN_DIR="$b/src/.libs" > export VIRTLOCKD_PATH="$b/src/virtlockd" > export LIBVIRTD_PATH="$b/daemon/libvirtd" > > -# For Python. > -export PYTHON=@PYTHON@ > -if [ -z "$PYTHONPATH" ]; then > - PYTHONPATH="$b/python:$b/python/.libs" > -else > - PYTHONPATH="$b/python:$b/python/.libs:$PYTHONPATH" > -fi > -export PYTHONPATH Fine. If you can answer the comments I raised above, and if everyone is okay with your libvirt-python.git patches, then I'm okay with this patch. Let's get it polished and in before freeze! I also just kicked off a 'make distcheck' and will report back later if I saw anything fishy that needs fixing. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list