Re: [Xen-devel] [PATCH] [PATCH RFC] tools: rework uninstall target

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

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@xxxxxxxxxx>
> ---
>  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

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 *.

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

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.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel