The original uninstall target in tools is a bit out-dated. The patch removes
code to delete things don''t exist any more and add lines to delete
things
should be deleted. Also rearrange lines to group similar things together.
To the best of my knowledge, libvhd and libfsimage are only distributed by Xen
so it is safe to delete them.
For things installed by ''python setup.py'', we record a list of
files installed
and use it when doing uninstall. This still leaves the directories there but
they won''t affect the system.
The changes are verified by doing ''cd tools; ./configure
--prefix=/tmp/tmpdir;
make install; make uninstall; find /tmp/tmpdir -type f''. All files are
removed. The only question is that do we remove too many things than
necessary.
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
tools/Makefile | 39 ++++++++++++++++++++++++++-------------
tools/pygrub/Makefile | 3 ++-
tools/python/Makefile | 3 ++-
3 files changed, 30 insertions(+), 15 deletions(-)
diff --git a/tools/Makefile b/tools/Makefile
index 8a30c83..8aeb811 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -74,34 +74,47 @@ install: subdirs-install
.PHONY: uninstall
uninstall: D=$(DESTDIR)
uninstall:
- rm -rf $(D)$(LIBDIR)/xen* $(D)$(BINDIR)/lomount
- rm -rf $(D)$(BINDIR)/cpuperf-perfcntr $(D)$(BINDIR)/cpuperf-xen
- rm -rf $(D)$(BINDIR)/xc_shadow
- rm -rf $(D)$(BINDIR)/pygrub
- rm -rf $(D)$(BINDIR)/setsize $(D)$(BINDIR)/tbctl
- rm -rf $(D)$(BINDIR)/xsls
rm -rf $(D)$(BINDIR)/xenstore* $(D)$(BINDIR)/xentrace*
rm -rf $(D)$(BINDIR)/xen-detect $(D)$(BINDIR)/xencons
- rm -rf $(D)$(BINDIR)/xenpvnetboot $(D)$(BINDIR)/qemu-*-xen
+ rm -rf $(D)$(BINDIR)/qemu-*-xen
+ rm -rf $(D)$(BINDIR)/remus
+ rm -rf $(D)$(BINDIR)/pygrub
rm -rf $(D)$(INCLUDEDIR)/xenctrl* $(D)$(INCLUDEDIR)/xenguest.h
rm -rf $(D)$(INCLUDEDIR)/xs_lib.h $(D)$(INCLUDEDIR)/xs.h
- rm -rf $(D)$(INCLUDEDIR)/xenstore-compat/xs_lib.h
$(D)$(INCLUDEDIR)/xenstore-compat/xs.h
+ rm -rf $(D)$(INCLUDEDIR)/xenstore-compat
rm -rf $(D)$(INCLUDEDIR)/xenstore_lib.h $(D)$(INCLUDEDIR)/xenstore.h
rm -rf $(D)$(INCLUDEDIR)/xen
rm -rf $(D)$(INCLUDEDIR)/_libxl* $(D)$(INCLUDEDIR)/libxl*
rm -rf $(D)$(INCLUDEDIR)/xenstat.h $(D)$(INCLUDEDIR)/xentoollog.h
- rm -rf $(D)$(LIBDIR)/libxenctrl* $(D)$(LIBDIR)/libxenguest*
+ rm -rf $(D)$(INCLUDEDIR)/blktaplib.h
+ rm -rf $(D)$(INCLUDEDIR)/libxenvchan.h
+ rm -rf $(D)$(INCLUDEDIR)/fsimage*
+ rm -rf $(D)$(LIBDIR)/xen*
+ rm -rf $(D)$(LIBDIR)/libxen*
rm -rf $(D)$(LIBDIR)/libxenstore* $(D)$(LIBDIR)/libxlutil*
- rm -rf $(D)$(LIBDIR)/python/xen $(D)$(LIBDIR)/python/grub
rm -rf $(D)$(LIBDIR)/xen/
- rm -rf $(D)$(LIBEXEC)/xen*
- rm -rf $(D)$(SBINDIR)/setmask
- rm -rf $(D)$(SBINDIR)/xen* $(D)$(SBINDIR)/netfix $(D)$(SBINDIR)/xm
+ rm -rf $(D)$(LIBDIR)/libblktap*
+ rm -rf $(D)$(LIBDIR)/fs/
+ rm -rf $(D)$(LIBDIR)/libfsimage*
+ rm -rf $(D)$(LIBDIR)/libvhd*
+ rm -rf $(D)$(SBINDIR)/xen*
+ rm -rf $(D)$(SBINDIR)/xm
+ rm -rf $(D)$(SBINDIR)/flask-*
+ rm -rf $(D)$(SBINDIR)/xl
+ rm -rf $(D)$(SBINDIR)/xsview
+ rm -rf $(D)$(SBINDIR)/tap-ctl $(D)$(SBINDIR)/tapdisk*
$(D)$(SBINDIR)/blktapctrl
+ rm -rf $(D)$(SBINDIR)/vhd-*
+ rm -rf $(D)$(SBINDIR)/gdbsx $(D)$(SBINDIR)/gtracestat
$(D)$(SBINDIR)/gtraceview $(D)$(SBINDIR)/kdd
+ rm -rf $(D)$(SBINDIR)/qcow2raw $(D)$(SBINDIR)/qcow-create
$(D)$(SBINDIR)/img2qcow
+ rm -rf $(D)$(SBINDIR)/td-util
+ rm -rf $(D)$(SBINDIR)/lock-util
rm -rf $(D)$(SHAREDIR)/doc/xen
rm -rf $(D)$(SHAREDIR)/xen
rm -rf $(D)$(SHAREDIR)/qemu-xen
rm -rf $(D)$(MAN1DIR)/xen*
rm -rf $(D)$(MAN8DIR)/xen*
+ if test -f python/installed-files.list; then cat python/installed-files.list |
xargs -I ''{}'' rm -f ''/{}''; fi
+ if test -f pygrub/installed-files.list; then cat pygrub/installed-files.list |
xargs -I ''{}'' rm -f ''/{}''; fi
.PHONY: clean
clean: subdirs-clean
diff --git a/tools/pygrub/Makefile b/tools/pygrub/Makefile
index 039f7f7..e63ebbd 100644
--- a/tools/pygrub/Makefile
+++ b/tools/pygrub/Makefile
@@ -12,7 +12,8 @@ build:
install: all
CC="$(CC)" CFLAGS="$(CFLAGS)" $(PYTHON) setup.py install \
$(PYTHON_PREFIX_ARG) --root="$(DESTDIR)" \
- --install-scripts=$(PRIVATE_BINDIR) --force
+ --install-scripts=$(PRIVATE_BINDIR) --force \
+ --record installed-files.list
$(INSTALL_DIR) $(DESTDIR)/var/run/xend/boot
set -e; if [ "`readlink -f $(DESTDIR)/$(BINDIR)`" != \
"`readlink -f $(PRIVATE_BINDIR)`" ]; then \
diff --git a/tools/python/Makefile b/tools/python/Makefile
index 9be1122..efc4fe3 100644
--- a/tools/python/Makefile
+++ b/tools/python/Makefile
@@ -21,7 +21,8 @@ build: genpath genwrap.py
$(XEN_ROOT)/tools/libxl/libxl_types.idl \
.PHONY: install
install: install-dtd
CC="$(CC)" CFLAGS="$(CFLAGS)" $(PYTHON) setup.py install \
- $(PYTHON_PREFIX_ARG) --root="$(DESTDIR)" --force
+ $(PYTHON_PREFIX_ARG) --root="$(DESTDIR)" --force \
+ --record installed-files.list
install-dtd: all
$(INSTALL_DIR) $(DESTDIR)$(SHAREDIR)/xen
--
1.7.10.4
I wrote this patch because the recent change set that switch to install tools in /usr/local by default exposes a problem (at least for me) that, if dev changes --prefix in ./configure without cleaning up previous installation, the loader might be picking up the wrong libs. Another problem (for me) with the "install to /usr/local" changeset is that I have to manually run ldconfig to point ld.so to the right position. It might be worth adding this to Makefile as well? But I don''t know whether this is specific to Debian Wheezy because I saw that patch has gone through to master which means it survived OSS-test. Another possibility is that OSS-test doesn''t delete the old libs and still uses them so we don''t see any error WRT that changeset. Wei.
On Sun, 2013-04-14 at 06:51 +0100, Wei Liu wrote:> The changes are verified by doing ''cd tools; ./configure --prefix=/tmp/tmpdir; > make install; make uninstall; find /tmp/tmpdir -type f''. All files are > removed.Could usefully be turned into a test case I think.> The only question is that do we remove too many things than > necessary.Or are they things which aren''t enabled in your build? e.g. LOMOUNT is obsolete but still subject to the CONFIG_LOMOUNT setting (disabled by default)> > Signed-off-by: Wei Liu <wei.liu2@citrix.com> > --- > tools/Makefile | 39 ++++++++++++++++++++++++++------------- > tools/pygrub/Makefile | 3 ++- > tools/python/Makefile | 3 ++- > 3 files changed, 30 insertions(+), 15 deletions(-) > > diff --git a/tools/Makefile b/tools/Makefile > index 8a30c83..8aeb811 100644 > --- a/tools/Makefile > +++ b/tools/Makefile > @@ -74,34 +74,47 @@ install: subdirs-install > .PHONY: uninstall > uninstall: D=$(DESTDIR) > uninstall: > - rm -rf $(D)$(LIBDIR)/xen* $(D)$(BINDIR)/lomount > - rm -rf $(D)$(BINDIR)/cpuperf-perfcntr $(D)$(BINDIR)/cpuperf-xen > - rm -rf $(D)$(BINDIR)/xc_shadow > - rm -rf $(D)$(BINDIR)/pygrub > - rm -rf $(D)$(BINDIR)/setsize $(D)$(BINDIR)/tbctl > - rm -rf $(D)$(BINDIR)/xsls > rm -rf $(D)$(BINDIR)/xenstore* $(D)$(BINDIR)/xentrace* > rm -rf $(D)$(BINDIR)/xen-detect $(D)$(BINDIR)/xencons > - rm -rf $(D)$(BINDIR)/xenpvnetboot $(D)$(BINDIR)/qemu-*-xen > + rm -rf $(D)$(BINDIR)/qemu-*-xen > + rm -rf $(D)$(BINDIR)/remus > + rm -rf $(D)$(BINDIR)/pygrub > rm -rf $(D)$(INCLUDEDIR)/xenctrl* $(D)$(INCLUDEDIR)/xenguest.h > rm -rf $(D)$(INCLUDEDIR)/xs_lib.h $(D)$(INCLUDEDIR)/xs.h > - rm -rf $(D)$(INCLUDEDIR)/xenstore-compat/xs_lib.h $(D)$(INCLUDEDIR)/xenstore-compat/xs.h > + rm -rf $(D)$(INCLUDEDIR)/xenstore-compatI''m not sure we can assume that the user (for better or worse) hasn''t put something of their own in this directory. I think removing the files and then trying to rmdir (failing if directory isn''t empty) is a better option. That said this class of issue is pretty big with this uninstall thing anyway given the existing use of *.> + if test -f python/installed-files.list; then cat python/installed-files.list | xargs -I ''{}'' rm -f ''/{}''; fi > + if test -f pygrub/installed-files.list; then cat pygrub/installed-files.list | xargs -I ''{}'' rm -f ''/{}''; fiI''m a little bit terrified of the possibility of {} being substituted with "" here e.g. if foo.list is somehow empty, or contains a blank line. Using --no-run-if-empty might help with the first case but not the blank line case. I don''t see an option which helps in that case :-( Of, it''s -f not -rf. That''s actually somewhat reassuring. http://bugs.python.org/issue4673 seems to suggest that setup.py ought to be able to uninstall stuff? But none of the docs seem to mention it so perhaps it is a phantasm. This won''t help users of "make dist" though since *.files will contain paths in the dist dir not the real filesystem. I suppose such people have installed by hand then and can be expected to undo it. Ian.
On Sun, 2013-04-14 at 07:04 +0100, Wei Liu wrote:> I wrote this patch because the recent change set that switch to install > tools in /usr/local by default exposes a problem (at least for me) that, > if dev changes --prefix in ./configure without cleaning up previous > installation, the loader might be picking up the wrong libs.Right, this is a bit of a pain during the transition. Not much can be done though I think :-( I''ve mentioned it in a mail here and it is referenced in the draft release notes, which I''ve just added a few more words to http://wiki.xen.org/wiki/Xen_4.3_Release_Notes> Another problem (for me) with the "install to /usr/local" changeset is > that I have to manually run ldconfig to point ld.so to the right > position. It might be worth adding this to Makefile as well?I added it to http://wiki.xen.org/wiki/Compiling_Xen_From_Source. Not sure about adding it to the Makefile, in that I''m not sure where it would need to be to be affective. It needs to be right at the end, or called in every library installing subdirectory. Not sure how harmful it is to call it too often (downside is slowing down the install phase I guess) I don''t know if this is something which projects normally do, it seems that libtool does it, which is as much as I could determine. You''d also need to be careful about not running it when doing a "make dist" as non root? Or would we, not sure.> But I don''t > know whether this is specific to Debian Wheezy because I saw that patch > has gone through to master which means it survived OSS-test. Another > possibility is that OSS-test doesn''t delete the old libs and still uses > them so we don''t see any error WRT that changeset.osstest does a fresh reinstall of the host every time. Ian.
On Mon, Apr 15, 2013 at 10:59:15AM +0100, Ian Campbell wrote:> On Sun, 2013-04-14 at 06:51 +0100, Wei Liu wrote: > > The changes are verified by doing ''cd tools; ./configure --prefix=/tmp/tmpdir; > > make install; make uninstall; find /tmp/tmpdir -type f''. All files are > > removed. > > Could usefully be turned into a test case I think. > > > The only question is that do we remove too many things than > > necessary. > > Or are they things which aren''t enabled in your build? e.g. LOMOUNT is > obsolete but still subject to the CONFIG_LOMOUNT setting (disabled by > default) >Probably, it is just a local patch for me and I''ve not tested all the cases. It is just RFC patch. If this patch is really necessary, I can try to respin it. But given it is code freeze now, it is probably better to leave it later.> >[...]> > rm -rf $(D)$(INCLUDEDIR)/xs_lib.h $(D)$(INCLUDEDIR)/xs.h > > - rm -rf $(D)$(INCLUDEDIR)/xenstore-compat/xs_lib.h $(D)$(INCLUDEDIR)/xenstore-compat/xs.h > > + rm -rf $(D)$(INCLUDEDIR)/xenstore-compat > > I''m not sure we can assume that the user (for better or worse) hasn''t > put something of their own in this directory. I think removing the files > and then trying to rmdir (failing if directory isn''t empty) is a better > option. > > That said this class of issue is pretty big with this uninstall thing > anyway given the existing use of *. >Nod.> > + if test -f python/installed-files.list; then cat python/installed-files.list | xargs -I ''{}'' rm -f ''/{}''; fi > > + if test -f pygrub/installed-files.list; then cat pygrub/installed-files.list | xargs -I ''{}'' rm -f ''/{}''; fi > > I''m a little bit terrified of the possibility of {} being substituted > with "" here e.g. if foo.list is somehow empty, or contains a blank > line. Using --no-run-if-empty might help with the first case but not the > blank line case. I don''t see an option which helps in that case :-( >Use sed to remove all blank lines can help with the first case.> Of, it''s -f not -rf. That''s actually somewhat reassuring. > > http://bugs.python.org/issue4673 seems to suggest that setup.py ought to > be able to uninstall stuff? But none of the docs seem to mention it so > perhaps it is a phantasm. >I don''t think it can. Even if setup.py can uninstall stuffs, this behavior is not clearly documented, i.e. I don''t find anything from ''python setup.py --help'', using undocumented command is dangerous. Wei.
On Mon, Apr 15, 2013 at 11:08:36AM +0100, Ian Campbell wrote:> On Sun, 2013-04-14 at 07:04 +0100, Wei Liu wrote: > > I wrote this patch because the recent change set that switch to install > > tools in /usr/local by default exposes a problem (at least for me) that, > > if dev changes --prefix in ./configure without cleaning up previous > > installation, the loader might be picking up the wrong libs. > > Right, this is a bit of a pain during the transition. Not much can be > done though I think :-( I''ve mentioned it in a mail here and it is > referenced in the draft release notes, which I''ve just added a few more > words to http://wiki.xen.org/wiki/Xen_4.3_Release_Notes > > > Another problem (for me) with the "install to /usr/local" changeset is > > that I have to manually run ldconfig to point ld.so to the right > > position. It might be worth adding this to Makefile as well? > > I added it to http://wiki.xen.org/wiki/Compiling_Xen_From_Source. >Could add similar things in Xen''s README as well? Wei.
On Mon, 2013-04-15 at 16:10 +0100, Wei Liu wrote:> On Mon, Apr 15, 2013 at 11:08:36AM +0100, Ian Campbell wrote: > > On Sun, 2013-04-14 at 07:04 +0100, Wei Liu wrote: > > > I wrote this patch because the recent change set that switch to install > > > tools in /usr/local by default exposes a problem (at least for me) that, > > > if dev changes --prefix in ./configure without cleaning up previous > > > installation, the loader might be picking up the wrong libs. > > > > Right, this is a bit of a pain during the transition. Not much can be > > done though I think :-( I''ve mentioned it in a mail here and it is > > referenced in the draft release notes, which I''ve just added a few more > > words to http://wiki.xen.org/wiki/Xen_4.3_Release_Notes > > > > > Another problem (for me) with the "install to /usr/local" changeset is > > > that I have to manually run ldconfig to point ld.so to the right > > > position. It might be worth adding this to Makefile as well? > > > > I added it to http://wiki.xen.org/wiki/Compiling_Xen_From_Source. > > > > Could add similar things in Xen''s README as well?Probably could link to the wiki for details as well as including the existing quickstart as well. I''m a little bit wary of "littering" the quickstart with workarounds and hacks for upgrading from old versions but I think /sbin/ldconfig is something which should always have been done but /usr/lib is treated specially by ld.so or something. Ian.