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

2011-12-13 Thread Niels Thykier
Package: release.debian.org
Severity: normal
Tags: patch
User: release.debian@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 ni...@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 ni...@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 

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

2011-12-13 Thread Adam D. Barratt
On Tue, 2011-12-13 at 10:03 +0100, Niels Thykier wrote:
 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.

After a certain amount of munging to confirm that the two current hunks
of code are effectively identical to both each other and your proposed
new method, I've convinced myself that they are. :-)

I'd be tempted to hack on the comments a tad, but I can always do that
after it's committed if I give in...

+single_undo = list()
+single_undo.append((undo, item))

Any particular reason why not simply: single_undo = [(undo, item)]

+self.undo_changes(single_undo, systems, sources, self.binaries)

I was going to grumble about this using systems where the existing
code always uses self.systems, but then noticed that we do actually
declare the convenience variable already and then just never use it for
the undo code.  *mutter*

Regards,

Adam




-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org



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

2011-12-13 Thread Niels Thykier
On 2011-12-13 21:03, Adam D. Barratt wrote:
 On Tue, 2011-12-13 at 10:03 +0100, Niels Thykier wrote:
 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.
 
 After a certain amount of munging to confirm that the two current hunks
 of code are effectively identical to both each other and your proposed
 new method, I've convinced myself that they are. :-)
 

Awesome, :)

 I'd be tempted to hack on the comments a tad, but I can always do that
 after it's committed if I give in...
 

Sure.

 +single_undo = list()
 +single_undo.append((undo, item))
 
 Any particular reason why not simply: single_undo = [(undo, item)]
 
 +self.undo_changes(single_undo, systems, sources, 
 self.binaries)
 

Lack of experience with Python, I suppose.  :)

 I was going to grumble about this using systems where the existing
 code always uses self.systems, but then noticed that we do actually
 declare the convenience variable already and then just never use it for
 the undo code.  *mutter*
 

XD

 Regards,
 
 Adam
 

~Niels




-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org