Refactored the "undo" code into its own method

Signed-off-by: Niels Thykier <niels@thykier.net>

britney.py

@@ -2167,42 +2167,9 @@ class Britney:
                 extra.append(pkg)
                 if not mark_passed:
                     skipped.append(pkg)
-
+                single_undo = [(undo, item)]
-                # undo the changes (source)
+                # (local-scope) binaries is actually self.binaries["testing"] so we cannot use it here.
-                for k in undo['sources'].keys():
+                self.undo_changes(single_undo, systems, sources, self.binaries)
-                    if k[0] == '-':
-                        del sources['testing'][k[1:]]
-                    else: sources['testing'][k] = undo['sources'][k]
-
-                # undo the changes (new binaries)
-                if not item.is_removal and item.package in sources[item.suite]:
-                    for p in sources[item.suite][item.package][BINARIES]:
-                        binary, arch = p.split("/")
-                        if item.architecture in ['source', arch]:
-                            del binaries[arch][0][binary]
-                            self.systems[arch].remove_binary(binary)
-
-                # undo the changes (binaries)
-                for p in undo['binaries'].keys():
-                    binary, arch = p.split("/")
-                    if binary[0] == "-":
-                        del binaries[arch][0][binary[1:]]
-                        self.systems[arch].remove_binary(binary[1:])
-                    else:
-                        binaries[arch][0][binary] = undo['binaries'][p]
-                        self.systems[arch].remove_binary(binary)
-                        self.systems[arch].add_binary(binary, binaries[arch][0][binary][:PROVIDES] + \
-                            [", ".join(binaries[arch][0][binary][PROVIDES]) or None])
-
-                # undo the changes (virtual packages)
-                for p in undo['nvirtual']:
-                    j, arch = p.split("/")
-                    del binaries[arch][1][j]
-                for p in undo['virtual']:
-                    j, arch = p.split("/")
-                    if j[0] == '-':
-                        del binaries[arch][1][j[1:]]
-                    else: binaries[arch][1][j] = undo['virtual'][p]
 
         # if we are processing hints, return now
         if hint:
@@ -2302,47 +2269,77 @@ class Britney:
             self.output_write("FAILED\n")
             if not undo: return
 
-            # undo all the changes
+            self.undo_changes(lundo, self.systems, self.sources, self.binaries)
+
+
+    def undo_changes(self, lundo, systems, sources, binaries):
+        """Undoes one or more changes to testing
+
+        * lundo is a list of (undo, item)-tuples
+        * systems is the britney-py.c system
+        * sources is the table of all source packages for all suites
+        * binaries is the table of all binary packages for all suites
+          and architectures
+        """
+
+        # We do the undo process in "4 steps" and each step must be
+        # fully completed for each undo-item before starting on the
+        # next.
+        #
+        # see commit:ef71f0e33a7c3d8ef223ec9ad5e9843777e68133 and
+        # #624716 for the issues we had when we did not do this.
+
+
+        # STEP 1
+        # undo all the changes for sources
         for (undo, item) in lundo:
-                # undo the changes (source)
             for k in undo['sources'].keys():
                 if k[0] == '-':
-                        del self.sources['testing'][k[1:]]
+                    del sources["testing"][k[1:]]
-                    else: self.sources['testing'][k] = undo['sources'][k]
+                else:
+                    sources["testing"][k] = undo['sources'][k]
 
+        # STEP 2
+        # undo all new binaries (consequence of the above)
         for (undo, item) in lundo:
-                # undo the changes (new binaries)
+            if not item.is_removal and item.package in sources[item.suite]:
-                if not item.is_removal and item.package in self.sources[item.suite]:
+                for p in sources[item.suite][item.package][BINARIES]:
-                    for p in self.sources[item.suite][item.package][BINARIES]:
                     binary, arch = p.split("/")
                     if item.architecture in ['source', arch]:
-                            del self.binaries['testing'][arch][0][binary]
+                        del binaries["testing"][arch][0][binary]
-                            self.systems[arch].remove_binary(binary)
+                        systems[arch].remove_binary(binary)
+
 
+        # STEP 3
+        # undo all other binary package changes (except virtual packages)
         for (undo, item) in lundo:
-                # undo the changes (binaries)
             for p in undo['binaries'].keys():
                 binary, arch = p.split("/")
                 if binary[0] == "-":
-                        del self.binaries['testing'][arch][0][binary[1:]]
+                    del binaries['testing'][arch][0][binary[1:]]
-                        self.systems[arch].remove_binary(binary[1:])
+                    systems[arch].remove_binary(binary[1:])
                 else:
-                        binaries = self.binaries['testing'][arch][0]
+                    binaries_t_a = binaries['testing'][arch][0]
-                        binaries[binary] = undo['binaries'][p]
+                    binaries_t_a[binary] = undo['binaries'][p]
-                        self.systems[arch].remove_binary(binary)
+                    systems[arch].remove_binary(binary)
-                        self.systems[arch].add_binary(binary, binaries[binary][:PROVIDES] + \
+                    systems[arch].add_binary(binary, binaries_t_a[binary][:PROVIDES] + \
-                            [", ".join(binaries[binary][PROVIDES]) or None])
+                         [", ".join(binaries_t_a[binary][PROVIDES]) or None])
-
+
+        # STEP 4
+        # undo all changes to virtual packages
         for (undo, item) in lundo:
-                # undo the changes (virtual packages)
             for p in undo['nvirtual']:
                 j, arch = p.split("/")
-                    del self.binaries['testing'][arch][1][j]
+                del binaries['testing'][arch][1][j]
             for p in undo['virtual']:
                 j, arch = p.split("/")
                 if j[0] == '-':
-                        del self.binaries['testing'][arch][1][j[1:]]
+                    del binaries['testing'][arch][1][j[1:]]
-                    else: self.binaries['testing'][arch][1][j] = undo['virtual'][p]
+                else:
+                    binaries['testing'][arch][1][j] = undo['virtual'][p]
+
+
+
 
     def upgrade_testing(self):
         """Upgrade testing using the unstable packages