launchpad-reviewers team mailing list archive

[Raphaël Victor Badin <raphael.badin@xxxxxxxxxxxxx>]

Raphaël Victor Badin has proposed merging lp:~rvb/launchpad/override-bug-812758 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #812758 in Launchpad itself: "Copying packages with binaries into a distro can crash the code"
  https://bugs.launchpad.net/launchpad/+bug/812758

For more details, see:
https://code.launchpad.net/~rvb/launchpad/override-bug-812758/+merge/68400

This branches fixes calculateBinaryOverrides so that it won't crash if it receives binaries=(name, arch) where arch is not a valid architecture for the distroseries.

= Q/A =

Initializing a series from a parent with two arches and selecting only one arch should work.

= Tests =

 ./bin/test -vvc test_overrides test_arch_not_in_distroseries
-- 
https://code.launchpad.net/~rvb/launchpad/override-bug-812758/+merge/68400
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/launchpad/override-bug-812758 into lp:launchpad.

=== modified file 'lib/lp/soyuz/adapters/overrides.py'
--- lib/lp/soyuz/adapters/overrides.py	2011-06-23 12:42:46 +0000
+++ lib/lp/soyuz/adapters/overrides.py	2011-07-19 15:13:22 +0000
@@ -235,9 +235,13 @@
 
         store = IStore(BinaryPackagePublishingHistory)
         expanded = calculate_target_das(distroseries, binaries)
+        filtered_expanded = [
+            (bpn, das) for bpn, das in expanded if das is not None]
+        if filtered_expanded == []:
+            return []
         candidates = (
             make_package_condition(archive, das, bpn)
-            for bpn, das in expanded)
+            for bpn, das in filtered_expanded)
         already_published = DecoratedResultSet(
             store.find(
                 (BinaryPackageRelease.binarypackagenameID,

=== modified file 'lib/lp/soyuz/adapters/tests/test_overrides.py'
--- lib/lp/soyuz/adapters/tests/test_overrides.py	2011-06-05 09:55:08 +0000
+++ lib/lp/soyuz/adapters/tests/test_overrides.py	2011-07-19 15:13:22 +0000
@@ -4,6 +4,7 @@
 """Test generic override policy classes."""
 
 from operator import attrgetter
+
 from testtools.matchers import Equals
 from zope.component import getUtility
 
@@ -136,7 +137,7 @@
         pocket = self.factory.getAnyPocket()
         for i in xrange(10):
             bpph = self.factory.makeBinaryPackagePublishingHistory(
-                distroarchseries=distroarchseries, 
+                distroarchseries=distroarchseries,
                 archive=distroseries.main_archive, pocket=pocket)
             bpns.append((bpph.binarypackagerelease.binarypackagename, None))
         flush_database_caches()
@@ -242,9 +243,26 @@
         overrides = policy.calculateBinaryOverrides(
             distroseries.main_archive, distroseries, pocket, bpns)
         self.assertEqual(5, len(overrides))
-        key=attrgetter("binary_package_name.name",
-            "distro_arch_series.architecturetag", 
+        key = attrgetter("binary_package_name.name",
+            "distro_arch_series.architecturetag",
             "component.name")
         sorted_expected = sorted(expected, key=key)
         sorted_overrides = sorted(overrides, key=key)
         self.assertEqual(sorted_expected, sorted_overrides)
+
+    def test_arch_not_in_distroseries(self):
+        # If calculateBinaryOverrides is passed with an archtag that
+        # does not correspond to an ArchSeries of the distroseries,
+        # an empty list is returned.
+        distroseries = self.factory.makeDistroSeries()
+        das = self.factory.makeDistroArchSeries(
+            architecturetag='amd64',
+            distroseries=distroseries)
+        distroseries.nominatedarchindep = das
+        bpn = self.factory.makeBinaryPackageName()
+        pocket = self.factory.getAnyPocket()
+        policy = FromExistingOverridePolicy()
+        overrides = policy.calculateBinaryOverrides(
+            distroseries.main_archive, distroseries, pocket, ((bpn, 'i386'),))
+
+        self.assertEqual([], overrides)