launchpad-reviewers team mailing list archive

[Steve Kowalik <stevenk@xxxxxxxxxx>]

Steve Kowalik has proposed merging lp:~stevenk/launchpad/gina-supported-release-mutable into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1241334 in Launchpad itself: "gina does not understand immutable series"
  https://bugs.launchpad.net/launchpad/+bug/1241334

For more details, see:
https://code.launchpad.net/~stevenk/launchpad/gina-supported-release-mutable/+merge/191749

Add a new flag to some domination methods allowing IPublishingSet.requestDeletion() to bypass the immutable series check, allowing gina to delete sources from SUPPORTED series.
-- 
https://code.launchpad.net/~stevenk/launchpad/gina-supported-release-mutable/+merge/191749
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stevenk/launchpad/gina-supported-release-mutable into lp:launchpad.

=== modified file 'lib/lp/archivepublisher/domination.py'
--- lib/lp/archivepublisher/domination.py	2013-06-20 05:50:00 +0000
+++ lib/lp/archivepublisher/domination.py	2013-10-18 04:50:03 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2011 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2013 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Archive Domination class.
@@ -364,7 +364,8 @@
         self.logger = logger
         self.archive = archive
 
-    def dominatePackage(self, sorted_pubs, live_versions, generalization):
+    def dominatePackage(self, sorted_pubs, live_versions, generalization,
+                        immutable_check=True):
         """Dominate publications for a single package.
 
         The latest publication for any version in `live_versions` stays
@@ -432,7 +433,7 @@
                 # This publication is no longer live, but there is no
                 # newer version to supersede it either.  Therefore it
                 # must be deleted.
-                pub.requestDeletion(None)
+                pub.requestDeletion(None, immutable_check=immutable_check)
                 self.logger.debug2("Deleting version %s.", version)
             else:
                 # This publication is superseded.  This is what we're
@@ -759,7 +760,7 @@
         return query.order_by(Desc(SPR.version), Desc(SPPH.datecreated))
 
     def dominateSourceVersions(self, distroseries, pocket, package_name,
-                               live_versions):
+                               live_versions, immutable_check=True):
         """Dominate source publications based on a set of "live" versions.
 
         Active publications for the "live" versions will remain active.  All
@@ -778,7 +779,9 @@
         generalization = GeneralizedPublication(is_source=True)
         pubs = self.findPublishedSPPHs(distroseries, pocket, package_name)
         pubs = generalization.sortPublications(pubs)
-        self.dominatePackage(pubs, live_versions, generalization)
+        self.dominatePackage(
+            pubs, live_versions, generalization,
+            immutable_check=immutable_check)
 
     def judge(self, distroseries, pocket):
         """Judge superseded sources and binaries."""

=== modified file 'lib/lp/soyuz/interfaces/publishing.py'
--- lib/lp/soyuz/interfaces/publishing.py	2013-06-20 17:24:46 +0000
+++ lib/lp/soyuz/interfaces/publishing.py	2013-10-18 04:50:03 +0000
@@ -225,11 +225,14 @@
 class IPublishingEdit(Interface):
     """Base interface for writeable Publishing classes."""
 
-    def requestDeletion(removed_by, removal_comment=None):
+    def requestDeletion(removed_by, removal_comment=None,
+                        immutable_check=True):
         """Delete this publication.
 
         :param removed_by: `IPerson` responsible for the removal.
         :param removal_comment: optional text describing the removal reason.
+        :param immutable_check: Check if deletion will modify a non-current
+            series.
         """
 
     @call_with(removed_by=REQUEST_USER)

=== modified file 'lib/lp/soyuz/model/publishing.py'
--- lib/lp/soyuz/model/publishing.py	2013-09-24 05:45:06 +0000
+++ lib/lp/soyuz/model/publishing.py	2013-10-18 04:50:03 +0000
@@ -884,9 +884,11 @@
                     diff.diff_content, self.archive).http_url
         return None
 
-    def requestDeletion(self, removed_by, removal_comment=None):
+    def requestDeletion(self, removed_by, removal_comment=None,
+                        immutable_check=True):
         """See `IPublishing`."""
-        if not self.archive.canModifySuite(self.distroseries, self.pocket):
+        if (immutable_check and
+            not self.archive.canModifySuite(self.distroseries, self.pocket)):
             raise DeletionError(
                 "Cannot delete publications from suite '%s'" %
                 self.distroseries.getSuite(self.pocket))

=== modified file 'lib/lp/soyuz/scripts/gina/dominate.py'
--- lib/lp/soyuz/scripts/gina/dominate.py	2011-09-23 13:00:49 +0000
+++ lib/lp/soyuz/scripts/gina/dominate.py	2013-10-18 04:50:03 +0000
@@ -1,4 +1,4 @@
-# Copyright 2011 Canonical Ltd.  This software is licensed under the
+# Copyright 2011-2013 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Retirement of packages that are removed upstream."""
@@ -45,10 +45,12 @@
         # many Published publications as live versions, there is no
         # domination to do.  We skip these as an optimization.  Without
         # it, dominating a single Debian series takes hours.
+        # Set immutable_check to False, so we can change SUPPORTED series.
         if pub_count != len(live_versions):
             logger.debug("Dominating %s.", package_name)
             dominator.dominateSourceVersions(
-                series, pocket, package_name, live_versions)
+                series, pocket, package_name, live_versions,
+                immutable_check=False)
             txn.commit()
         else:
             logger.debug2(

=== modified file 'lib/lp/soyuz/scripts/tests/test_gina.py'
--- lib/lp/soyuz/scripts/tests/test_gina.py	2013-06-21 02:39:33 +0000
+++ lib/lp/soyuz/scripts/tests/test_gina.py	2013-10-18 04:50:03 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2013 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 from doctest import DocTestSuite
@@ -11,6 +11,7 @@
 
 from lp.archiveuploader.tagfiles import parse_tagfile
 from lp.registry.interfaces.pocket import PackagePublishingPocket
+from lp.registry.interfaces.series import SeriesStatus
 from lp.services.features.testing import FeatureFixture
 from lp.services.log.logger import DevNullLogger
 from lp.services.tarfile_helpers import LaunchpadWriteTarFile
@@ -49,6 +50,9 @@
 
     layer = ZopelessDatabaseLayer
 
+    def assertPublishingStates(self, spphs, states):
+        self.assertEqual(states, [pub.status for pub in spphs])
+
     def test_dominate_imported_source_packages_dominates_imports(self):
         # dominate_imported_source_packages dominates the source
         # packages that Gina imports.
@@ -91,14 +95,12 @@
         dominate_imported_source_packages(
             txn, logger, series.distribution.name, series.name, pocket,
             FakePackagesMap({package.name: [{'Version': '1.1.1'}]}))
-        self.assertEqual([
-            PackagePublishingStatus.SUPERSEDED,
-            PackagePublishingStatus.SUPERSEDED,
-            PackagePublishingStatus.PUBLISHED,
-            PackagePublishingStatus.DELETED,
-            PackagePublishingStatus.PENDING,
-            ],
-            [pub.status for pub in spphs])
+        states = [
+            PackagePublishingStatus.SUPERSEDED,
+            PackagePublishingStatus.SUPERSEDED,
+            PackagePublishingStatus.PUBLISHED, PackagePublishingStatus.DELETED,
+            PackagePublishingStatus.PENDING]
+        self.assertPublishingStates(spphs, states)
 
     def test_dominate_imported_source_packages_dominates_deletions(self):
         # dominate_imported_source_packages dominates the source
@@ -126,12 +128,34 @@
         # The older, superseded release stays superseded; but the
         # releases that dropped out of the imported Sources list without
         # known successors are marked deleted.
-        self.assertEqual([
-            PackagePublishingStatus.SUPERSEDED,
-            PackagePublishingStatus.DELETED,
-            PackagePublishingStatus.DELETED,
-            ],
-            [pub.status for pub in pubs])
+        self.assertPublishingStates(
+            pubs, [PackagePublishingStatus.SUPERSEDED,
+            PackagePublishingStatus.DELETED, PackagePublishingStatus.DELETED])
+
+    def test_dominate_imported_sources_dominates_supported_series(self):
+        series = self.factory.makeDistroSeries()
+        pocket = PackagePublishingPocket.RELEASE
+        package = self.factory.makeSourcePackageName()
+        pubs = [
+            self.factory.makeSourcePackagePublishingHistory(
+                archive=series.main_archive, distroseries=series,
+                pocket=pocket, status=PackagePublishingStatus.PUBLISHED,
+                sourcepackagerelease=self.factory.makeSourcePackageRelease(
+                    sourcepackagename=package, version=version))
+            for version in ['1.0', '1.1', '1.1a']]
+
+        # In this scenario, 1.0 is a superseded release.
+        pubs[0].supersede()
+        # Now set the series to SUPPORTED.
+        series.status = SeriesStatus.SUPPORTED
+        logger = DevNullLogger()
+        txn = FakeTransaction()
+        dominate_imported_source_packages(
+            txn, logger, series.distribution.name, series.name, pocket,
+            FakePackagesMap({}))
+        self.assertPublishingStates(
+            pubs, [PackagePublishingStatus.SUPERSEDED,
+            PackagePublishingStatus.DELETED, PackagePublishingStatus.DELETED])
 
 
 class TestSourcePackageData(TestCaseWithFactory):