Merge lp:~cjwatson/launchpad/contents-fixes into lp:launchpad

[Colin Watson]

Fix a couple of bugs in updating Contents files.

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.

[William Grant (wgrant)]

review: Approve (code)

=== 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()