launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

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

Commit message:
Fix a couple of bugs in updating Contents files.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

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

Fix a couple of bugs in updating Contents files:

Only update Contents files for the primary archive, to match the behaviour of GenerateContentsFiles.

Only force suites to be updated if we actually update their Contents files, not merely if we check for updates.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/contents-fixes into lp:launchpad.

=== modified file 'lib/lp/archivepublisher/scripts/publish_ftpmaster.py'
--- lib/lp/archivepublisher/scripts/publish_ftpmaster.py	2015-03-18 17:51:16 +0000
+++ lib/lp/archivepublisher/scripts/publish_ftpmaster.py	2015-03-20 11:42:01 +0000
@@ -560,7 +560,10 @@
                     distribution, archive, updated_suites=updated_suites)
 
     def updateContentsFile(self, archive, distribution, suite, arch):
-        """Update a single Contents file if necessary."""
+        """Update a single Contents file if necessary.
+
+        :return: True if a file was updated, otherwise False.
+        """
         config = self.configs[distribution][archive.purpose]
         backup_dists = get_backup_dists(config)
         content_dists = os.path.join(
@@ -576,18 +579,25 @@
                 "Installing new Contents file for %s/%s.", suite,
                 arch.architecturetag)
             shutil.copy2(new_contents, current_contents)
+            return True
+        return False
 
     def updateContentsFiles(self, distribution):
         """Pick up updated Contents files if necessary."""
         updated_suites = []
-        for archive in get_publishable_archives(distribution):
-            for series in distribution.getNonObsoleteSeries():
-                for pocket in PackagePublishingPocket.items:
-                    suite = series.getSuite(pocket)
-                    for arch in series.enabled_architectures:
-                        self.updateContentsFile(
-                            archive, distribution, suite, arch)
-                        updated_suites.append((archive, suite))
+        # XXX cjwatson 2015-03-20: GenerateContentsFiles currently only
+        # supports the primary archive.
+        archive = distribution.main_archive
+        for series in distribution.getNonObsoleteSeries():
+            for pocket in PackagePublishingPocket.items:
+                suite = series.getSuite(pocket)
+                updated = False
+                for arch in series.enabled_architectures:
+                    if self.updateContentsFile(
+                            archive, distribution, suite, arch):
+                        updated = True
+                if updated:
+                    updated_suites.append((archive, suite))
         return updated_suites
 
     def publish(self, distribution, security_only=False):

=== modified file 'lib/lp/archivepublisher/tests/test_publish_ftpmaster.py'
--- lib/lp/archivepublisher/tests/test_publish_ftpmaster.py	2015-03-18 17:51:16 +0000
+++ lib/lp/archivepublisher/tests/test_publish_ftpmaster.py	2015-03-20 11:42:01 +0000
@@ -862,12 +862,58 @@
         os.makedirs(content_suite)
         write_marker_file(
             [content_suite, "%s-staged.gz" % contents_filename], "Contents")
-        script.updateContentsFile(
-            distro.main_archive, distro, distroseries.name, das)
+        self.assertTrue(script.updateContentsFile(
+            distro.main_archive, distro, distroseries.name, das))
         self.assertEqual(
             "Contents",
             read_marker_file([backup_suite, "%s.gz" % contents_filename]))
 
+    def test_updateContentsFiles_updated_suites(self):
+        # updateContentsFiles returns a list of suites for which it updated
+        # Contents files.
+        distro = self.makeDistroWithPublishDirectory()
+        distroseries = self.factory.makeDistroSeries(distribution=distro)
+        das = self.factory.makeDistroArchSeries(distroseries=distroseries)
+        script = self.makeScript(distro)
+        script.setUp()
+        script.setUpDirs()
+        archive_config = getPubConfig(distro.main_archive)
+        contents_filename = "Contents-%s" % das.architecturetag
+        backup_suite = os.path.join(
+            archive_config.archiveroot + "-distscopy", "dists",
+            distroseries.name)
+        os.makedirs(backup_suite)
+        content_suite = os.path.join(
+            archive_config.distroroot, "contents-generation", distro.name,
+            "dists", distroseries.name)
+        os.makedirs(content_suite)
+        write_marker_file(
+            [content_suite, "%s-staged.gz" % contents_filename], "Contents")
+        self.assertEqual(
+            [(distro.main_archive, distroseries.name)],
+            script.updateContentsFiles(distro))
+
+    def test_updateContentsFiles_only_primary_archive(self):
+        # updateContentsFiles only considers the primary archive.  (This
+        # will need to change if GenerateContentsFiles ever gains support
+        # for other archive purposes.)
+        distro = self.makeDistroWithPublishDirectory()
+        self.factory.makeArchive(
+            distribution=distro, owner=distro.owner,
+            purpose=ArchivePurpose.PARTNER)
+        distroseries = self.factory.makeDistroSeries(distribution=distro)
+        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]
+        self.assertEqual(
+            expected_args, script.updateContentsFile.extract_args())
+
     def test_publish_always_returns_true_for_primary(self):
         script = self.makeScript()
         script.publishDistroUploads = FakeMethod()