* fix 571:

- refactor remove operation for a builder pattern
  - smart remove
    - first write, then remove leftover files
    - don't remove entries from db until necessary
  - tested, but needs more testing to see if it has
    not broken anything
* unremoved stuff:
  - give a warning
  - something's fishy there. what's the deal with all
    the directories that are remnant of removed packages?

pisi/install.py

@@ -49,7 +49,7 @@ class Installer:
         self.ask_reinstall = ask_reinstall
         self.check_requirements()
         self.check_relations()
-        self.reinstall()
+        self.check_reinstall()
         self.extract_install()
         self.store_pisi_files()
         if ctx.comard:
@@ -81,11 +81,12 @@ class Installer:
                              ' not satisfied')
                 raise InstallError("Package not installable")
 
-    def reinstall(self):
-        "check reinstall, confirm action, and remove package if reinstall"
+    def check_reinstall(self):
+        "check reinstall, confirm action, and schedule reinstall"
 
         pkg = self.pkginfo
 
+        self.reinstall = False
         if ctx.installdb.is_installed(pkg.name): # is this a reinstallation?
             (iversion, irelease, ibuild) = ctx.installdb.get_version(pkg.name)
 
@@ -130,8 +131,10 @@ class Installer:
                     if not ctx.ui.confirm(x):
                         raise InstallError('Package downgrade declined')
 
-            # remove old package then
-            operations.remove_single(pkg.name)
+            # schedule for reinstall
+            self.old_files = ctx.installdb.files(pkg.name)
+            self.reinstall = True
+            operations.run_preremove(pkg.name)
 
     def extract_install(self):
         "unzip package in place"
@@ -139,6 +142,17 @@ class Installer:
         ctx.ui.info('Extracting files,')
         self.package.extract_dir_flat('install', ctx.config.destdir)
 
+        if self.reinstall:
+            # remove left over files
+            new = set(map(lambda x: str(x.path), self.files.list))
+            old = set(map(lambda x: str(x.path), self.old_files.list))
+            leftover = old - new
+            old_fileinfo = {}
+            for fileinfo in self.old_files.list:
+                old_fileinfo[str(fileinfo.path)] = fileinfo
+            for path in leftover:
+                operations.remove_file( old_fileinfo[path] )
+
     def store_pisi_files(self):
         """put files.xml, metadata.xml, actions.py and COMAR scripts
         somewhere in the file system. We'll need these in future..."""
@@ -179,6 +193,9 @@ class Installer:
     def update_databases(self):
         "update databases"
 
+        if self.reinstall:
+            operations.remove_db(self.metadata.package.name)
+
         # installdb
         ctx.installdb.install(self.metadata.package.name,
                           self.metadata.package.version,

pisi/operations.py

@@ -21,16 +21,28 @@ from pisi.uri import URI
 
 # single package operations
 
-def remove_single(package_name):
-    """Remove a single package"""
-    inst_packagedb = packagedb.inst_packagedb
+# remove stuff
 
-    #TODO: check dependencies
+def remove_file(fileinfo):
+    fpath = os.path.join(ctx.config.destdir, fileinfo.path)
+    # TODO: We have to store configuration files for futher
+    # usage. Currently we'are doing it like rpm does, saving
+    # with a prefix and leaving the user to edit it. In the future
+    # we'll have a plan for these configuration files.
+    if fileinfo.type == ctx.const.conf:
+        if os.path.isfile(fpath):
+            os.rename(fpath, fpath + ".pisi")
+    else:
+        # check if file is removed manually.
+        # And we don't remove directories!
+        # TODO: remove directory if there is nothing under it?
+        if os.path.isfile(fpath) or os.path.islink(fpath):
+            os.unlink(fpath)
+        else:
+            ctx.ui.warning('Not removing non-file, non-link %d' % fpath)
+
+def run_preremove(package_name):
 
-    ctx.ui.info('Removing package %s' % package_name)
-    if not ctx.installdb.is_installed(package_name):
-        raise Exception('Trying to remove nonexistent package '
-                        + package_name)
     if ctx.comard:
         com = ctx.comard
 
@@ -44,25 +56,27 @@ def remove_single(package_name):
             elif reply[1] == com.ERROR:
                 raise Error, "COMAR.remove failed!"
 
-    for fileinfo in ctx.installdb.files(package_name).list:
-        fpath = os.path.join(ctx.config.destdir, fileinfo.path)
-        # TODO: We have to store configuration files for futher
-        # usage. Currently we'are doing it like rpm does, saving
-        # with a prefix and leaving the user to edit it. In the future
-        # we'll have a plan for these configuration files.
-        if fileinfo.type == ctx.const.conf:
-            if os.path.isfile(fpath):
-                os.rename(fpath, fpath + ".pisi")
-        else:
-            # check if file is removed manually.
-            # And we don't remove directories!
-            # FIXME: should give a warning if it is...
-            if os.path.isfile(fpath) or os.path.islink(fpath):
-                os.unlink(fpath)
-
-
+def remove_db(package_name):
     ctx.installdb.remove(package_name)
-    packagedb.remove_package(package_name)
+    packagedb.remove_package(package_name)  #FIXME: this looks like a mistake!
+
+def remove_single(package_name):
+    """Remove a single package"""
+    inst_packagedb = packagedb.inst_packagedb
+
+    #TODO: check dependencies
+
+    ctx.ui.info('Removing package %s' % package_name)
+    if not ctx.installdb.is_installed(package_name):
+        raise Exception('Trying to remove nonexistent package '
+                        + package_name)
+        
+    run_preremove(package_name)
+        
+    for fileinfo in ctx.installdb.files(package_name).list:
+        remove_file(fileinfo)
+
+    remove_db(package_name)
 
 def install_single(pkg, upgrade = False):
     """install a single package from URI or ID"""