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

Bug#651925: marked as done (britney: refactor undo code into a method)



Your message dated Tue, 13 Dec 2011 22:49:13 +0100
with message-id <4EE7C859.5040706@thykier.net>
and subject line Re: Bug#651925: britney: refactor undo code into a method
has caused the Debian Bug report #651925,
regarding britney: refactor undo code into a method
to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
Bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact owner@bugs.debian.org
immediately.)


-- 
651925: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=651925
Debian Bug Tracking System
Contact owner@bugs.debian.org with problems
--- Begin Message ---
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


--- End Message ---
--- Begin Message ---
pushed as 3f4f1b4c23a154577b6c8bcb2962ce97e0412558




--- End Message ---

Reply to: