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

[Wei Liu <wei.liu2@xxxxxxxxxx>]

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.

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