[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#651925: britney: refactor undo code into a method



Package: release.debian.org
Severity: normal
Tags: patch
User: release.debian.org@packages.debian.org
Usertags: britney

Hi,

Attached is a patch that should do that.  I have tested it on
the britney test suite and live data from 2011-10-19 (reduced
to i386 and amd64).

That being said, I would appreciate a review or two just in case.

~Niels
>From fd6decda54a125a5bfcf751c818ee37caf7e5c5e Mon Sep 17 00:00:00 2001
From: Niels Thykier <niels@thykier.net>
Date: Tue, 13 Dec 2011 09:58:45 +0100
Subject: [PATCH] Refactored the "undo" code into its own method

Signed-off-by: Niels Thykier <niels@thykier.net>
---
 britney.py |  150 +++++++++++++++++++++++++++++------------------------------
 1 files changed, 74 insertions(+), 76 deletions(-)

diff --git a/britney.py b/britney.py
index 7a577d3..defba6f 100755
--- a/britney.py
+++ b/britney.py
@@ -2167,42 +2167,10 @@ class Britney:
                 extra.append(pkg)
                 if not mark_passed:
                     skipped.append(pkg)
-
-                # undo the changes (source)
-                for k in undo['sources'].keys():
-                    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]
+                single_undo = list()
+                single_undo.append((undo, item))
+                # (local-scope) binaries is actually self.binaries["testing"] so we cannot use it here.
+                self.undo_changes(single_undo, systems, sources, self.binaries)
 
         # if we are processing hints, return now
         if hint:
@@ -2302,47 +2270,77 @@ class Britney:
             self.output_write("FAILED\n")
             if not undo: return
 
-            # undo all the changes
-            for (undo, item) in lundo:
-                # undo the changes (source)
-                for k in undo['sources'].keys():
-                    if k[0] == '-':
-                        del self.sources['testing'][k[1:]]
-                    else: self.sources['testing'][k] = undo['sources'][k]
-
-            for (undo, item) in lundo:
-                # undo the changes (new binaries)
-                if not item.is_removal and item.package in self.sources[item.suite]:
-                    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]
-                            self.systems[arch].remove_binary(binary)
-
-            for (undo, item) in lundo:
-                # undo the changes (binaries)
-                for p in undo['binaries'].keys():
+            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:
+            for k in undo['sources'].keys():
+                if k[0] == '-':
+                    del sources["testing"][k[1:]]
+                else:
+                    sources["testing"][k] = undo['sources'][k]
+
+        # STEP 2
+        # undo all new binaries (consequence of the above)
+        for (undo, item) in lundo:
+            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 binary[0] == "-":
-                        del self.binaries['testing'][arch][0][binary[1:]]
-                        self.systems[arch].remove_binary(binary[1:])
-                    else:
-                        binaries = self.binaries['testing'][arch][0]
-                        binaries[binary] = undo['binaries'][p]
-                        self.systems[arch].remove_binary(binary)
-                        self.systems[arch].add_binary(binary, binaries[binary][:PROVIDES] + \
-                            [", ".join(binaries[binary][PROVIDES]) or None])
-
-            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]
-                for p in undo['virtual']:
-                    j, arch = p.split("/")
-                    if j[0] == '-':
-                        del self.binaries['testing'][arch][1][j[1:]]
-                    else: self.binaries['testing'][arch][1][j] = undo['virtual'][p]
+                    if item.architecture in ['source', arch]:
+                        del binaries["testing"][arch][0][binary]
+                        systems[arch].remove_binary(binary)
+
+
+        # STEP 3
+        # undo all other binary package changes (except virtual packages)
+        for (undo, item) in lundo:
+            for p in undo['binaries'].keys():
+                binary, arch = p.split("/")
+                if binary[0] == "-":
+                    del binaries['testing'][arch][0][binary[1:]]
+                    systems[arch].remove_binary(binary[1:])
+                else:
+                    binaries_t_a = binaries['testing'][arch][0]
+                    binaries_t_a[binary] = undo['binaries'][p]
+                    systems[arch].remove_binary(binary)
+                    systems[arch].add_binary(binary, binaries_t_a[binary][:PROVIDES] + \
+                         [", ".join(binaries_t_a[binary][PROVIDES]) or None])
+
+        # STEP 4
+        # undo all changes to virtual packages
+        for (undo, item) in lundo:
+            for p in undo['nvirtual']:
+                j, arch = p.split("/")
+                del binaries['testing'][arch][1][j]
+            for p in undo['virtual']:
+                j, arch = p.split("/")
+                if j[0] == '-':
+                    del binaries['testing'][arch][1][j[1:]]
+                else:
+                    binaries['testing'][arch][1][j] = undo['virtual'][p]
+
+
+
 
     def upgrade_testing(self):
         """Upgrade testing using the unstable packages
-- 
1.7.7.3


Reply to: