launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/immutable-contents into lp:launchpad.

Commit message:
Don't generate Contents files for immutable suites.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1448270 in Launchpad itself: "updateContentsFiles doesn't check for suite immutability"
  https://bugs.launchpad.net/launchpad/+bug/1448270

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/immutable-contents/+merge/272102

Don't generate Contents files for immutable suites.  We won't update their Release files and so checksums there will become outdated.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/immutable-contents into lp:launchpad.

=== modified file 'lib/lp/archivepublisher/publishing.py'
--- lib/lp/archivepublisher/publishing.py	2015-03-18 17:51:16 +0000
+++ lib/lp/archivepublisher/publishing.py	2015-09-23 13:33:45 +0000
@@ -2,6 +2,7 @@
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __all__ = [
+    'cannot_modify_suite',
     'FORMAT_TO_SUBCOMPONENT',
     'GLOBAL_PUBLISHER_LOCK',
     'Publisher',
@@ -191,6 +192,13 @@
         return os.path.join(component_root, subcomp, arch_path, "Packages")
 
 
+def cannot_modify_suite(archive, distroseries, pocket):
+    """Return True for Release pockets of stable series in primary archives."""
+    return (not distroseries.isUnstable() and
+            not archive.allowUpdatesToReleasePocket() and
+            pocket == PackagePublishingPocket.RELEASE)
+
+
 class I18nIndex(_multivalued):
     """Represents an i18n/Index file."""
     _multivalued_fields = {
@@ -482,7 +490,7 @@
         for distroseries, pocket in chain(source_suites, binary_suites):
             if self.isDirty(distroseries, pocket):
                 continue
-            if (self.cannotModifySuite(distroseries, pocket)
+            if (cannot_modify_suite(self.archive, distroseries, pocket)
                 or not self.isAllowed(distroseries, pocket)):
                 # We don't want to mark release pockets dirty in a
                 # stable distroseries, no matter what other bugs
@@ -725,12 +733,6 @@
         if separate_long_descriptions:
             translation_en.close()
 
-    def cannotModifySuite(self, distroseries, pocket):
-        """Return True if the distroseries is stable and pocket is release."""
-        return (not distroseries.isUnstable() and
-                not self.archive.allowUpdatesToReleasePocket() and
-                pocket == PackagePublishingPocket.RELEASE)
-
     def checkDirtySuiteBeforePublishing(self, distroseries, pocket):
         """Last check before publishing a dirty suite.
 
@@ -738,7 +740,7 @@
         in RELEASE pocket (primary archives) we certainly have a problem,
         better stop.
         """
-        if self.cannotModifySuite(distroseries, pocket):
+        if cannot_modify_suite(self.archive, distroseries, pocket):
             raise AssertionError(
                 "Oops, tainting RELEASE pocket of %s." % distroseries)
 

=== modified file 'lib/lp/archivepublisher/scripts/generate_contents_files.py'
--- lib/lp/archivepublisher/scripts/generate_contents_files.py	2015-02-13 10:43:03 +0000
+++ lib/lp/archivepublisher/scripts/generate_contents_files.py	2015-09-23 13:33:45 +0000
@@ -14,6 +14,7 @@
 from zope.component import getUtility
 
 from lp.archivepublisher.config import getPubConfig
+from lp.archivepublisher.publishing import cannot_modify_suite
 from lp.registry.interfaces.distribution import IDistributionSet
 from lp.registry.interfaces.pocket import PackagePublishingPocket
 from lp.services.command_spawner import (
@@ -133,9 +134,14 @@
 
     def getSuites(self):
         """Return suites that need Contents files."""
+        # XXX cjwatson 2015-09-23: This script currently only supports the
+        # primary archive.
+        archive = self.distribution.main_archive
         for series in self.distribution.getNonObsoleteSeries():
             for pocket in PackagePublishingPocket.items:
                 suite = series.getSuite(pocket)
+                if cannot_modify_suite(archive, series, pocket):
+                    continue
                 if file_exists(os.path.join(self.config.distsroot, suite)):
                     yield suite
 

=== modified file 'lib/lp/archivepublisher/scripts/publish_ftpmaster.py'
--- lib/lp/archivepublisher/scripts/publish_ftpmaster.py	2015-03-20 14:02:46 +0000
+++ lib/lp/archivepublisher/scripts/publish_ftpmaster.py	2015-09-23 13:33:45 +0000
@@ -18,7 +18,10 @@
 
 from lp.archivepublisher.config import getPubConfig
 from lp.archivepublisher.interfaces.publisherconfig import IPublisherConfigSet
-from lp.archivepublisher.publishing import GLOBAL_PUBLISHER_LOCK
+from lp.archivepublisher.publishing import (
+    cannot_modify_suite,
+    GLOBAL_PUBLISHER_LOCK,
+    )
 from lp.archivepublisher.scripts.processaccepted import ProcessAccepted
 from lp.archivepublisher.scripts.publishdistro import PublishDistro
 from lp.registry.interfaces.distribution import IDistributionSet
@@ -600,6 +603,8 @@
         for series in distribution.getNonObsoleteSeries():
             for pocket in PackagePublishingPocket.items:
                 suite = series.getSuite(pocket)
+                if cannot_modify_suite(archive, series, pocket):
+                    continue
                 updated = False
                 for arch in series.enabled_architectures:
                     if self.updateContentsFile(

=== modified file 'lib/lp/archivepublisher/scripts/publishdistro.py'
--- lib/lp/archivepublisher/scripts/publishdistro.py	2015-08-25 16:10:09 +0000
+++ lib/lp/archivepublisher/scripts/publishdistro.py	2015-09-23 13:33:45 +0000
@@ -13,6 +13,7 @@
 
 from lp.app.errors import NotFoundError
 from lp.archivepublisher.publishing import (
+    cannot_modify_suite,
     getPublisher,
     GLOBAL_PUBLISHER_LOCK,
     )
@@ -295,8 +296,8 @@
                     for suite in self.options.dirty_suites:
                         distroseries, pocket = self.findSuite(
                             distribution, suite)
-                        if not publisher.cannotModifySuite(
-                                distroseries, pocket):
+                        if not cannot_modify_suite(
+                                archive, distroseries, pocket):
                             publisher.markPocketDirty(distroseries, pocket)
                     self.publishArchive(archive, publisher)
                     work_done = True

=== modified file 'lib/lp/archivepublisher/tests/test_generate_contents_files.py'
--- lib/lp/archivepublisher/tests/test_generate_contents_files.py	2015-03-18 17:51:16 +0000
+++ lib/lp/archivepublisher/tests/test_generate_contents_files.py	2015-09-23 13:33:45 +0000
@@ -18,6 +18,7 @@
     )
 from lp.archivepublisher.scripts.publish_ftpmaster import PublishFTPMaster
 from lp.registry.interfaces.pocket import PackagePublishingPocket
+from lp.registry.interfaces.series import SeriesStatus
 from lp.services.log.logger import DevNullLogger
 from lp.services.osutils import write_file
 from lp.services.scripts.base import LaunchpadScriptFailure
@@ -190,21 +191,36 @@
         # pockets that have packages to publish.
         distro = self.makeDistro()
         distroseries = self.factory.makeDistroSeries(distribution=distro)
-        package = self.factory.makeSuiteSourcePackage(distroseries)
+        suite = distroseries.getSuite(PackagePublishingPocket.BACKPORTS)
         script = self.makeScript(distro)
-        os.makedirs(os.path.join(script.config.distsroot, package.suite))
-        self.assertEqual([package.suite], list(script.getSuites()))
+        os.makedirs(os.path.join(script.config.distsroot, suite))
+        self.assertEqual([suite], list(script.getSuites()))
 
     def test_getSuites_includes_release_pocket(self):
         # getSuites also includes the release pocket, which is named
         # after the distroseries without a suffix.
         distro = self.makeDistro()
-        distroseries = self.factory.makeDistroSeries(distribution=distro)
-        package = self.factory.makeSuiteSourcePackage(
-            distroseries, pocket=PackagePublishingPocket.RELEASE)
-        script = self.makeScript(distro)
-        os.makedirs(os.path.join(script.config.distsroot, package.suite))
-        self.assertEqual([package.suite], list(script.getSuites()))
+        distroseries = self.factory.makeDistroSeries(
+            distribution=distro, status=SeriesStatus.DEVELOPMENT)
+        script = self.makeScript(distro)
+        suite = distroseries.getSuite(PackagePublishingPocket.RELEASE)
+        os.makedirs(os.path.join(script.config.distsroot, suite))
+        self.assertEqual([suite], list(script.getSuites()))
+
+    def test_getSuites_excludes_immutable_suites(self):
+        # getSuites excludes suites that we would refuse to publish.
+        distro = self.makeDistro()
+        distroseries = self.factory.makeDistroSeries(
+            distribution=distro, status=SeriesStatus.CURRENT)
+        script = self.makeScript(distro)
+        pockets = [
+            PackagePublishingPocket.RELEASE,
+            PackagePublishingPocket.UPDATES,
+            ]
+        suites = [distroseries.getSuite(pocket) for pocket in pockets]
+        for suite in suites:
+            os.makedirs(os.path.join(script.config.distsroot, suite))
+        self.assertEqual([suites[1]], list(script.getSuites()))
 
     def test_writeAptContentsConf_writes_header(self):
         # writeAptContentsConf writes apt-contents.conf.  At a minimum

=== modified file 'lib/lp/archivepublisher/tests/test_publish_ftpmaster.py'
--- lib/lp/archivepublisher/tests/test_publish_ftpmaster.py	2015-03-20 14:02:46 +0000
+++ lib/lp/archivepublisher/tests/test_publish_ftpmaster.py	2015-09-23 13:33:45 +0000
@@ -927,7 +927,8 @@
         self.factory.makeArchive(
             distribution=distro, owner=distro.owner,
             purpose=ArchivePurpose.PARTNER)
-        distroseries = self.factory.makeDistroSeries(distribution=distro)
+        distroseries = self.factory.makeDistroSeries(
+            distribution=distro, status=SeriesStatus.DEVELOPMENT)
         das = self.factory.makeDistroArchSeries(distroseries=distroseries)
         script = self.makeScript(distro)
         script.setUp()
@@ -940,6 +941,25 @@
         self.assertEqual(
             expected_args, script.updateContentsFile.extract_args())
 
+    def test_updateContentsFiles_skips_immutable_suites(self):
+        # updateContentsFiles does not generate Contents files for immutable
+        # suites.
+        distro = self.makeDistroWithPublishDirectory()
+        distroseries = self.factory.makeDistroSeries(
+            distribution=distro, status=SeriesStatus.CURRENT)
+        das = self.factory.makeDistroArchSeries(distroseries=distroseries)
+        script = self.makeScript(distro)
+        script.setUp()
+        script.setUpDirs()
+        script.updateContentsFile = FakeMethod()
+        script.updateContentsFiles(distro)
+        expected_args = [
+            (distro.main_archive, distro, distroseries.getSuite(pocket), das)
+            for pocket in PackagePublishingPocket.items
+            if pocket != PackagePublishingPocket.RELEASE]
+        self.assertEqual(
+            expected_args, script.updateContentsFile.extract_args())
+
     def test_publish_always_returns_true_for_primary(self):
         script = self.makeScript()
         script.publishDistroUploads = FakeMethod()