← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~wgrant/launchpad/ddeb-domination into lp:launchpad

 

William Grant has proposed merging lp:~wgrant/launchpad/ddeb-domination into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  #604425 DDEBs need to be superseded when their DEBs are
  https://bugs.launchpad.net/bugs/604425


This branch alters binary domination to fix bug #604425.

DDEBs need to be as unintrusive as possible -- people shouldn't have to change how they deal with archive administration. To help with this, DDEB publications should be superseded as soon as their DEB is. This will, for example, eliminate the need for archive admins to manually NBS them out if they stop being built.

This is implemented much the same as atomic arch-indep domination: BinaryPackagePublishingHistory.supersede locates corresponding DDEB publications with the same overrides, and supersedes them.

Because of this change, it's no longer important for DDEBs to supersede each other (their DEBs will handle it all). I've filtered them out of the Dominator query, and added an assertion to confirm that no DDEB ever accidentally supersedes something else.
-- 
https://code.launchpad.net/~wgrant/launchpad/ddeb-domination/+merge/30928
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/ddeb-domination into lp:launchpad.
=== modified file 'lib/lp/archivepublisher/domination.py'
--- lib/lp/archivepublisher/domination.py	2010-07-11 13:58:56 +0000
+++ lib/lp/archivepublisher/domination.py	2010-07-26 12:04:48 +0000
@@ -66,6 +66,7 @@
     clear_current_connection_cache)
 
 from canonical.launchpad.interfaces import PackagePublishingStatus
+from lp.soyuz.interfaces.binarypackagerelease import BinaryPackageFormat
 
 
 def clear_cache():
@@ -310,10 +311,12 @@
                 AND binarypackagepublishinghistory.status = %s AND
                 binarypackagepublishinghistory.binarypackagerelease =
                     binarypackagerelease.id
+                AND binarypackagerelease.binpackageformat != %s
                 AND binarypackagerelease.binarypackagename IN (
                     SELECT name FROM PubDomHelper WHERE count > 1)"""
                 % sqlvalues(distroarchseries, self.archive,
-                            pocket, PackagePublishingStatus.PUBLISHED),
+                            pocket, PackagePublishingStatus.PUBLISHED,
+                            BinaryPackageFormat.DDEB),
                 clauseTables=['BinaryPackageRelease'])
 
             self.debug("Dominating binaries...")

=== modified file 'lib/lp/archivepublisher/tests/test_dominator.py'
--- lib/lp/archivepublisher/tests/test_dominator.py	2010-07-22 11:59:40 +0000
+++ lib/lp/archivepublisher/tests/test_dominator.py	2010-07-26 12:04:48 +0000
@@ -18,13 +18,14 @@
 class TestDominator(TestNativePublishingBase):
     """Test Dominator class."""
 
-    def createSourceAndBinaries(self, version):
+    def createSourceAndBinaries(self, version, with_debug=False,
+                                archive=None):
         """Create a source and binaries with the given version."""
         source = self.getPubSource(
-            version=version,
+            version=version, archive=archive,
             status=PackagePublishingStatus.PUBLISHED)
         binaries = self.getPubBinaries(
-            pub_source=source,
+            pub_source=source, with_debug=with_debug,
             status=PackagePublishingStatus.PUBLISHED)
         return (source, binaries)
 
@@ -107,6 +108,36 @@
             [foo_10_source] + foo_10_binaries,
             PackagePublishingStatus.SUPERSEDED)
 
+    def testJudgeAndDominateWithDDEBs(self):
+        """Verify that judgeAndDominate ignores DDEBs correctly.
+
+        DDEBs are superseded by their corresponding DEB publications, so they
+        are forbidden from superseding publications (an attempt would result
+        in an AssertionError), and shouldn't be directly considered for
+        superseding either.
+        """
+        ppa = self.factory.makeArchive()
+        foo_10_source, foo_10_binaries = self.createSourceAndBinaries(
+            '1.0', with_debug=True, archive=ppa)
+        foo_11_source, foo_11_binaries = self.createSourceAndBinaries(
+            '1.1', with_debug=True, archive=ppa)
+        foo_12_source, foo_12_binaries = self.createSourceAndBinaries(
+            '1.2', with_debug=True, archive=ppa)
+
+        dominator = Dominator(self.logger, ppa)
+        dominator.judgeAndDominate(
+            foo_10_source.distroseries, foo_10_source.pocket, self.config)
+
+        self.checkPublications(
+            [foo_12_source] + foo_12_binaries,
+            PackagePublishingStatus.PUBLISHED)
+        self.checkPublications(
+            [foo_11_source] + foo_11_binaries,
+            PackagePublishingStatus.SUPERSEDED)
+        self.checkPublications(
+            [foo_10_source] + foo_10_binaries,
+            PackagePublishingStatus.SUPERSEDED)
+
     def testEmptyDomination(self):
         """Domination asserts for not empty input list."""
         dominator = Dominator(self.logger, self.ubuntutest.main_archive)

=== modified file 'lib/lp/soyuz/model/publishing.py'
--- lib/lp/soyuz/model/publishing.py	2010-07-21 14:14:54 +0000
+++ lib/lp/soyuz/model/publishing.py	2010-07-26 12:04:48 +0000
@@ -50,6 +50,7 @@
 from lp.registry.interfaces.person import validate_public_person
 from lp.registry.interfaces.pocket import PackagePublishingPocket
 from lp.services.worlddata.model.country import Country
+from lp.soyuz.interfaces.binarypackagerelease import BinaryPackageFormat
 from lp.soyuz.model.binarypackagename import BinaryPackageName
 from lp.soyuz.model.binarypackagerelease import (BinaryPackageRelease,
     BinaryPackageReleaseDownloadCount)
@@ -975,6 +976,26 @@
                 section=self.section,
                 priority=self.priority)
 
+    def _getCorrespondingDDEBPublications(self):
+        """Return remaining publications of the corresponding DDEB.
+
+        Only considers binary publications in the corresponding debug
+        archive with the same distroarchseries, pocket, component, section
+        and priority.
+        """
+        return IMasterStore(BinaryPackagePublishingHistory).find(
+                BinaryPackagePublishingHistory,
+                BinaryPackagePublishingHistory.status.is_in(
+                    [PUBLISHED, PENDING]),
+                BinaryPackagePublishingHistory.distroarchseries ==
+                    self.distroarchseries,
+                binarypackagerelease=self.binarypackagerelease.debug_package,
+                archive=self.archive.debug_archive,
+                pocket=self.pocket,
+                component=self.component,
+                section=self.section,
+                priority=self.priority)
+
     def supersede(self, dominant=None, logger=None):
         """See `IBinaryPackagePublishingHistory`."""
         # At this point only PUBLISHED (ancient versions) or PENDING (
@@ -993,6 +1014,16 @@
         super(BinaryPackagePublishingHistory, self).supersede()
 
         if dominant is not None:
+            # DDEBs cannot themselves be dominant; they are always dominated
+            # by their corresponding DEB. Any attempt to dominate with a
+            # dominant DDEB is a bug.
+            assert (
+                dominant.binarypackagerelease.binpackageformat !=
+                    BinaryPackageFormat.DDEB), (
+                "Should not dominate with %s (%s); DDEBs cannot dominate" % (
+                    dominant.binarypackagerelease.title,
+                    dominant.distroarchseries.architecturetag))
+
             dominant_build = dominant.binarypackagerelease.build
             distroarchseries = dominant_build.distro_arch_series
             if logger is not None:
@@ -1010,7 +1041,10 @@
             # between releases.
             self.supersededby = dominant_build
 
-        # If this is architecture-independet, all publications with the same
+        for dominated in self._getCorrespondingDDEBPublications():
+            dominated.supersede(dominant, logger)
+
+        # If this is architecture-independent, all publications with the same
         # context and overrides should be dominated simultaneously.
         if not self.binarypackagerelease.architecturespecific:
             for dominated in self._getOtherPublications():

=== modified file 'lib/lp/soyuz/tests/test_publishing.py'
--- lib/lp/soyuz/tests/test_publishing.py	2010-07-23 12:19:26 +0000
+++ lib/lp/soyuz/tests/test_publishing.py	2010-07-26 12:04:48 +0000
@@ -32,7 +32,7 @@
 from lp.soyuz.model.processor import ProcessorFamily
 from lp.soyuz.model.publishing import (
     SourcePackagePublishingHistory, BinaryPackagePublishingHistory)
-from lp.soyuz.interfaces.archive import ArchivePurpose
+from lp.soyuz.interfaces.archive import ArchivePurpose, IArchiveSet
 from lp.soyuz.interfaces.archivearch import IArchiveArchSet
 from lp.soyuz.interfaces.binarypackagename import IBinaryPackageNameSet
 from lp.soyuz.interfaces.binarypackagerelease import BinaryPackageFormat
@@ -275,7 +275,8 @@
                        version='666',
                        architecturespecific=False,
                        builder=None,
-                       component='main'):
+                       component='main',
+                       with_debug=False):
         """Return a list of binary publishing records."""
         if distroseries is None:
             distroseries = self.distroseries
@@ -302,11 +303,25 @@
         published_binaries = []
         for build in builds:
             build.builder = builder
+            pub_binaries = []
+            if with_debug:
+                binarypackagerelease_ddeb = self.uploadBinaryForBuild(
+                    build, binaryname + '-dbgsym', filecontent, summary,
+                    description, shlibdep, depends, recommends, suggests,
+                    conflicts, replaces, provides, pre_depends, enhances,
+                    breaks, BinaryPackageFormat.DDEB)
+                pub_binaries += self.publishBinaryInArchive(
+                    binarypackagerelease_ddeb, archive.debug_archive, status,
+                    pocket, scheduleddeletiondate, dateremoved)
+            else:
+                binarypackagerelease_ddeb = None
+
             binarypackagerelease = self.uploadBinaryForBuild(
                 build, binaryname, filecontent, summary, description,
                 shlibdep, depends, recommends, suggests, conflicts, replaces,
-                provides, pre_depends, enhances, breaks, format)
-            pub_binaries = self.publishBinaryInArchive(
+                provides, pre_depends, enhances, breaks, format,
+                binarypackagerelease_ddeb)
+            pub_binaries += self.publishBinaryInArchive(
                 binarypackagerelease, archive, status, pocket,
                 scheduleddeletiondate, dateremoved)
             published_binaries.extend(pub_binaries)
@@ -1144,6 +1159,48 @@
         self.checkSuperseded([bin], super_bin)
         self.assertEquals(super_date, bin.datesuperseded)
 
+    def testSupersedesCorrespondingDDEB(self):
+        """Check that supersede() takes with it any corresponding DDEB.
+
+        DDEB publications should be superseded when their corresponding DEB
+        is.
+        """
+        getUtility(IArchiveSet).new(
+            purpose=ArchivePurpose.DEBUG, owner=self.ubuntutest.owner,
+            distribution=self.ubuntutest)
+
+        # Each of these will return (i386 deb, i386 ddeb, hppa deb,
+        # hppa ddeb).
+        bins = self.getPubBinaries(
+            architecturespecific=True, with_debug=True)
+        super_bins = self.getPubBinaries(
+            architecturespecific=True, with_debug=True)
+
+        bins[0].supersede(super_bins[0])
+        self.checkSuperseded(bins[:2], super_bins[0])
+        self.checkPublications(bins[2:], PackagePublishingStatus.PENDING)
+        self.checkPublications(super_bins, PackagePublishingStatus.PENDING)
+
+        bins[2].supersede(super_bins[2])
+        self.checkSuperseded(bins[:2], super_bins[0])
+        self.checkSuperseded(bins[2:], super_bins[2])
+        self.checkPublications(super_bins, PackagePublishingStatus.PENDING)
+
+    def testDDEBsCannotSupersede(self):
+        """Check that DDEBs cannot supersede other publications.
+
+        Since DDEBs are superseded when their DEBs are, there's no need to
+        for them supersede anything themselves. Any such attempt is an error.
+        """
+        getUtility(IArchiveSet).new(
+            purpose=ArchivePurpose.DEBUG, owner=self.ubuntutest.owner,
+            distribution=self.ubuntutest)
+
+        # This will return (i386 deb, i386 ddeb, hppa deb, hppa ddeb).
+        bins = self.getPubBinaries(
+            architecturespecific=True, with_debug=True)
+        self.assertRaises(AssertionError, bins[0].supersede, bins[1])
+
 
 class TestBinaryGetOtherPublications(TestNativePublishingBase):
     """Test BinaryPackagePublishingHistory._getOtherPublications() works."""