[Piuparts-devel] Bug#588313: [PATCH] simplify diversion modification checks

Andreas Beckmann debian at abeckmann.de
Sat Nov 5 22:33:36 UTC 2011


(this patch applies on top of Scott's patch in git)
Simplify the diversion checks, fix a logic error that had switched
added/removed.
Diversion state is no longer tracked globally but each of the three
tests collects the initial diversion information at appropriate time
to better cooperate with hook scripts etc.
The actual diversion checking is performed from check_results.

An even better solution would be to collect and store the diversion
info together with root_info.

FIXME: needs --save-end-meta support

Signed-off-by: Andreas Beckmann <debian at abeckmann.de>

---
 piuparts.py |   63 +++++++++++++++++++++++++++++------------------------------
 1 files changed, 31 insertions(+), 32 deletions(-)

diff --git a/piuparts.py b/piuparts.py
index 5d73d88..0064a22 100644
--- a/piuparts.py
+++ b/piuparts.py
@@ -574,8 +574,6 @@ class Chroot:
     
     def __init__(self):
         self.name = None
-        self.pre_install_diversions = None
-        self.post_install_diversions = None
         
     def create_temp_dir(self):
         """Create a temporary directory for the chroot."""
@@ -613,8 +611,6 @@ class Chroot:
                 if (sfile.startswith("post_") or sfile.startswith("pre_")) and os.path.isfile(os.path.join((settings.scriptsdir), sfile)):
                     shutil.copy(os.path.join((settings.scriptsdir), sfile), dest) 
 
-        self.pre_install_diversions = self.get_diversions()
-
         # Run custom scripts after creating the chroot.
         if settings.scriptsdir is not None: 
             self.run_scripts("post_setup")
@@ -869,30 +865,19 @@ class Chroot:
         return vdict
 
     def get_diversions(self):
-    	"""Get current dpkg-divert --list in a chroot."""
+        """Get current dpkg-divert --list in a chroot."""
         if not settings.check_broken_diversions:
             return
         (status, output) = self.run(["dpkg-divert", "--list"])
-        lines = []
-        for line in output.split("\n"):
-            lines.append(line)
-        return lines
-
+        return output.split("\n")
 
-    def check_for_broken_diversions(self):
-        """Check that diversions in chroot are identical (though potenttially reordered)."""
-        if not settings.check_broken_diversions:
-            return
-        if self.pre_install_diversions and self.post_install_diversions:
-            added = [ln for ln in self.pre_install_diversions if not ln in self.post_install_diversions]
-            removed = [ln for ln in self.post_install_diversions if not ln in self.pre_install_diversions]
-            if added:
-                logging.error("Error: Installed diversions (dpkg-divert) not removed by purge:\n%s" %  
-                              indent_string("\n".join(added)))
-            if removed:
-                logging.error("Error: Existing diversions (dpkg-divert) removed/modified:\n%s" %
-                              indent_string("\n".join(removed)))
-    	
+    def get_modified_diversions(self, pre_install_diversions, post_install_diversions=None):
+        """Check that diversions in chroot are identical (though potentially reordered)."""
+        if post_install_diversions is None:
+            post_install_diversions = self.get_diversions()
+        removed = [ln for ln in pre_install_diversions if not ln in post_install_diversions]
+        added = [ln for ln in post_install_diversions if not ln in pre_install_diversions]
+        return (removed, added)
 
     def remove_or_purge(self, operation, packages):
         """Remove or purge packages in a chroot."""
@@ -951,8 +936,6 @@ class Chroot:
         # Finally, purge actual packages.
         self.remove_or_purge("purge", nondeps_to_purge)
 
-        self.post_install_diversions = self.get_diversions()
-
         # remove logrotate and it's depends 
         #    (this is a fix for #602409 introduced by #566597 
         #    - search for the latter bug number in this file)
@@ -1644,7 +1627,7 @@ def process_changes(changes):
     return package_list
 
 
-def check_results(chroot, root_info, file_owners, deps_info=None):
+def check_results(chroot, root_info, file_owners, deps_info=None, diversion_info=None):
     """Check that current chroot state matches 'root_info'.
     
     If settings.warn_on_others is True and deps_info is not None, then only
@@ -1655,6 +1638,18 @@ def check_results(chroot, root_info, file_owners, deps_info=None):
     installed.)
     """
 
+    ok = True
+    if settings.check_broken_diversions and diversion_info is not None:
+        (removed, added) = chroot.get_modified_diversions(diversion_info)
+        if added:
+            logging.error("FAIL: Installed diversions (dpkg-divert) not removed by purge:\n%s" %
+                          indent_string("\n".join(added)))
+            ok = False
+        if removed:
+            logging.error("FAIL: Existing diversions (dpkg-divert) removed/modified:\n%s" %
+                          indent_string("\n".join(removed)))
+            ok = False
+
     current_info = chroot.save_meta_data()
     if settings.warn_on_others and deps_info is not None:
         (new, removed, modified) = diff_meta_data(root_info, current_info)
@@ -1668,7 +1663,6 @@ def check_results(chroot, root_info, file_owners, deps_info=None):
     else:
         (new, removed, modified) = diff_meta_data(root_info, current_info)
 
-    ok = True
     if new:
         if settings.warn_on_leftovers_after_purge:
           logging.info("Warning: Package purging left files on system:\n" +
@@ -1714,6 +1708,8 @@ def install_purge_test(chroot, root_info, selections, package_list, packages):
        Assume 'root' is a directory already populated with a working
        chroot, with packages in states given by 'selections'."""
 
+    diversions = chroot.get_diversions()
+
     # Install packages into the chroot.
 
     if settings.warn_on_others:
@@ -1776,16 +1772,17 @@ def install_purge_test(chroot, root_info, selections, package_list, packages):
     changes = diff_selections(chroot, selections)
     chroot.restore_selections(changes, packages)
     
-    chroot.check_for_broken_diversions()
     chroot.check_for_broken_symlinks()
 
-    return check_results(chroot, root_info, file_owners, deps_info=deps_info)
+    return check_results(chroot, root_info, file_owners, deps_info=deps_info, diversion_info=diversions)
 
 
 def install_upgrade_test(chroot, root_info, selections, package_list, package_names):
     """Install package via apt-get, then upgrade from package files.
     Return True if successful, False if not."""
 
+    diversions = chroot.get_diversions()
+
     # First install via apt-get.
     chroot.install_packages_by_name(package_names)
     
@@ -1807,7 +1804,7 @@ def install_upgrade_test(chroot, root_info, selections, package_list, package_na
     chroot.check_for_no_processes()
     chroot.check_for_broken_symlinks()
 
-    return check_results(chroot, root_info, file_owners)
+    return check_results(chroot, root_info, file_owners, diversion_info=diversions)
 
 
 def save_meta_data(filename, root_info, selections):
@@ -1866,6 +1863,7 @@ def install_and_upgrade_between_distros(filenames, packages):
     if settings.end_meta:
         # load root_info and selections
         root_info, selections = load_meta_data(settings.end_meta)
+        diversions = None # FIXME: how can the diversions be stored?
     else:
         chroot.upgrade_to_distros(settings.debian_distros[1:], [])
         chroot.run(["apt-get", "clean"])
@@ -1873,6 +1871,7 @@ def install_and_upgrade_between_distros(filenames, packages):
         # set root_info and selections
         root_info = chroot.save_meta_data()
         selections = chroot.get_selections()
+        diversions = chroot.get_diversions()
         
         if settings.save_end_meta:
             # save root_info and selections
@@ -1911,7 +1910,7 @@ def install_and_upgrade_between_distros(filenames, packages):
     # use root_info and selections
     changes = diff_selections(chroot, selections)
     chroot.restore_selections(changes, packages)
-    result = check_results(chroot, root_info, file_owners)
+    result = check_results(chroot, root_info, file_owners, diversion_info=diversions)
 
     chroot.check_for_no_processes()
     
-- 
tg: (2a7cfa9..) t/feat/diversions2 (depends on: origin/feature/588313)





More information about the Piuparts-devel mailing list