Re: [patch 0/9] [RFC] Introduction of pdfedit-dev package v2

[Martin P. <ma...@pe...>]

On 8/23/08, Michal Hocko <ms...@gm...> wrote:
> On Sat, Aug 23, 2008 at 03:43:20AM +0200, Martin Petricek wrote:
>  > >  Changelog v1 -> v2:
>  > >  ===================
>  > >  * rebased to the current CVS state
>  > >  * fix for buggy make uninstall-dev implementation (alarm that it sneaked
>  > >   through all reviews!!! - this means that nobody even tried that). The
>  > >   problem was that we have used
>  > >   $(DEL_FILE) $(INSTALL_ROOT)$(INCLUDE_PATH)/kernel/$(HEADERS)
>  > >   which was expanded to
>  > >   rm $(INSTALL_ROOT)$(INCLUDE_PATH)/kernel/static.h exceptions.h ...
>  > >   So that _ONLY ONE_ header in the destation is removed and the rest is
>  > >   removed from the current directory!!!
>  >
>  > I usually use rm -rf test_directory_where_i_put_the_result instead of
>  > uninstall ... :)
>
>
> make uninstall should be definitely one of the test cases. We want to
>  make sure that there is not big garbage after you try to get rid of
>  pdfedit and you are not using distribution package and then rm -rf is
>  not a case.
>  And on the other hand, this could be caught during patch review too ;)?
>  It was inattention from my side, but these reviews should be for such
>  errors, shouldn't they?
>
>  Please don't take it like complaining from my side (it is my fault after
>  all), but these patches are in devel mailing list for ages and nobody
>  has noticed that. How can we make this better?

Some framework for automated  testing? That will run ./configure
--prefix=/tmp/pdfedit-test or alike, then test make install/uninstall
and verify everything works as expected. Files left after uninstall?
bug :)

> Yeah, you are right. The problem is that if gui installation is enabled
>  (by default) it overwrites UNINSTALL_TARGET with "" and then it sets the
>  value for GUI. I have used
>  ./configure --disable-gui --enable-pdfedit-core-dev
>  too much, so I didn't catch that. Thanks for reporting (simple patch to

I have not specified --disable-gui

>  fix is attached - on top of the patchset - I don't want to spam you with
>  the whole patchset again. Don't remember to run autoconf after patching).

I'll test it

>  [ These are from dev package, but I think they are problem here, because
>   there were created as mkdir -p and these almost never got empty
>   (unless you have really exotic prefix - like the one above). I also

if you install or test as non-root, you'll always have "exotic" prefix
- on machines where I have no root access, I specify something like
prefix=$HOME/name_of_sw, then I'll just symlink binaries to $HOME/bin

In such case, I expect the make uninstall to clean up properly and
remove even the install_root if it is empty directory.

>   think that it is not worth fixing - should be remove recursively all
>   empty directories up to $INSTALL_ROOT? ]

Up to and including $INSTALL_ROOT.

>  [ this contained pdfedit/{LICENSE.GPL,Changelog, AUTHORS, README-* }
>   and uninstaller removed whole pdfedit directory. So in the end
>   PREFIX/share/doc remained and we have the same case like above -
>   should we remove standard directories? Again, I think it is not worth
>   the work. But we can discuss that

Most distros packaging systems does that - remove every directory
created by package (if not empty), no matter how "standard" it is or
not. They even remove the standard directories like /bin or /usr,
though they always fail, unless you do something like complete
uninstall or reinstall or alike. Also, some still quite commonly used
prefixes would be /usr/local (if only densely populated, pdfedit
install & uninstall can leave there many new empty directories there)
or often /opt/name_of_sw is used (especially if you have some other
version already installed in "standard location" and you want to try
"new beta" without uninstalling the old one first) ... so if someone
specified /opt/pdfedit as install_root, I expect it to be created
during install and then removed during uninstall (if empty)

Martin Petricek