Bug#624716: [britney2] Does not properly undo failed migrations

["Adam D. Barratt" <adam@adam-barratt.org.uk>]

tag 624716 + patch
thanks

On Sat, 2011-04-30 at 21:39 +0100, Adam D. Barratt wrote:
> On Sun, 2011-04-10 at 10:29 +0000, Cron Daemon wrote:
> >   File "/srv/release.debian.org/britney/code/b2/britney.py", line 2039, in doop_source
> >     if binaries[parch][0][binary][SOURCE] != pkg_name: continue
> > KeyError: 'fixincludes'
[...]
> >From a bit of debugging, this appears to be an issue with cases where a
> particular binary package has either moved source packages (as in the
> "fixincludes" case, from gcc-4.5 to gcc-4.6) or moved from being
> arch-dependent to arch:all, and the package is considered during
> multiple phases of the b2 run (either as part of an "easy" and then
> during the main run, or in the main run and then in a "hint").

Or, indeed, in multiple hints.

> When deciding whether to migrate a package, britney removes all of the
> current binary packages - making an "undo" list - and then adds the
> binary packages from the new version.  If the new version is not
> accepted then the "undo" list is used to revert things to the previous
> state.  However, in the "fixincludes" case at least, that reversion
> seems not to be applied correctly.

I believe I've worked out what's happening here.  The undo process is
essentially:

  for each block of undo data
    undo source package changes
    remove newly added binary packages
    re-add removed binary packages
    revert changes to virtual packages

In each of the cases where a binary B has moved from source S1 to S2,
what ends up happening is that the undo data for S1 is processed first,
giving us:

  re-add binary "B", removed from source S1
  remove binary "B", newly added by source S2

with the net result that the binary list no longer contains B but the
sources dict lists it as being built by S1.  Restructuring the undo loop
so that all removals are processed before any re-introductions fixes the
crash and seems to produce the same logical result afaics.

I intend to apply the attached patch within the next few days unless
someone raises a problem with it.

Regards,

Adam

commit ef71f0e33a7c3d8ef223ec9ad5e9843777e68133
Author: Adam D. Barratt <adam@adam-barratt.org.uk>
Date:   Wed May 4 18:45:11 2011 +0000

    When undoing a hint, remove new binaries before re-adding those removed
    
    If new binary packages are removed from the system afer the original
    packages have been re-introduced, a binary package which has moved
    between source packages may be removed entirely.  See Debian bug #624716
    for more details.
    
    Signed-off-by: Adam D. Barratt <adam@adam-barratt.org.uk>

diff --git a/britney.py b/britney.py
index 7dd6e6e..b129d06 100755
--- a/britney.py
+++ b/britney.py
@@ -2483,6 +2483,7 @@ class Britney:
                         del self.sources['testing'][k[1:]]
                     else: self.sources['testing'][k] = undo['sources'][k]
 
+            for (undo, pkg, pkg_name, suite) in lundo:
                 # undo the changes (new binaries)
                 if pkg[0] != '-' and pkg_name in self.sources[suite]:
                     for p in self.sources[suite][pkg_name][BINARIES]:
@@ -2491,6 +2492,7 @@ class Britney:
                             del self.binaries['testing'][arch][0][binary]
                             self.systems[arch].remove_binary(binary)
 
+            for (undo, pkg, pkg_name, suite) in lundo:
                 # undo the changes (binaries)
                 for p in undo['binaries'].keys():
                     binary, arch = p.split("/")
@@ -2499,6 +2501,7 @@ class Britney:
                         self.systems[arch].remove_binary(binary[1:])
                     else: self.binaries['testing'][arch][0][binary] = undo['binaries'][p]
 
+            for (undo, pkg, pkg_name, suite) in lundo:
                 # undo the changes (virtual packages)
                 for p in undo['nvirtual']:
                     j, arch = p.split("/")