launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

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

Commit message:
List Contents-* in Release files if available.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #638498 in Launchpad itself: "Contents files are not listed with checksums"
  https://bugs.launchpad.net/launchpad/+bug/638498

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/ftparchive-release-contents/+merge/253415

List Contents-* in Release files if available.

This entailed a little refactoring.  We have to make sure that updated Contents files are in place before writing Release files, and we have to make sure that if Contents files change then we republish the suite even if there's nothing else to do.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/ftparchive-release-contents into lp:launchpad.

=== modified file 'lib/lp/archivepublisher/publishing.py'
--- lib/lp/archivepublisher/publishing.py	2014-10-31 12:48:39 +0000
+++ lib/lp/archivepublisher/publishing.py	2015-03-18 18:07:07 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2014 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2015 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __all__ = [
@@ -808,6 +808,7 @@
             # and pocket, don't!
             return
 
+        suite = distroseries.getSuite(pocket)
         all_components = [
             comp.name for comp in
             self.archive.getComponentsForSeries(distroseries)]
@@ -822,6 +823,12 @@
                     distroseries, pocket, component, architecture, all_files)
             self._writeSuiteI18n(
                 distroseries, pocket, component, all_files)
+        for architecture in all_architectures:
+            for contents_path in get_suffixed_indices(
+                    'Contents-' + architecture):
+                if os.path.exists(os.path.join(
+                        self._config.distsroot, suite, contents_path)):
+                    all_files.add(contents_path)
 
         drsummary = "%s %s " % (self.distro.displayname,
                                 distroseries.displayname)
@@ -830,7 +837,6 @@
         else:
             drsummary += pocket.name.capitalize()
 
-        suite = distroseries.getSuite(pocket)
         release_file = Release()
         release_file["Origin"] = self._getOrigin()
         release_file["Label"] = self._getLabel()

=== modified file 'lib/lp/archivepublisher/scripts/publish_ftpmaster.py'
--- lib/lp/archivepublisher/scripts/publish_ftpmaster.py	2015-03-06 07:50:53 +0000
+++ lib/lp/archivepublisher/scripts/publish_ftpmaster.py	2015-03-18 18:07:07 +0000
@@ -429,12 +429,14 @@
         publish_distro.main()
 
     def publishDistroArchive(self, distribution, archive,
-                             security_suites=None):
+                             security_suites=None, updated_suites=[]):
         """Publish the results for an archive.
 
         :param archive: Archive to publish.
         :param security_suites: An optional list of suites to restrict
             the publishing to.
+        :param updated_suites: An optional list of archive/suite pairs that
+            have been updated out of band and should be republished.
         """
         purpose = archive.purpose
         archive_config = self.configs[distribution][purpose]
@@ -449,6 +451,9 @@
         arguments = ['-R', temporary_dists]
         if archive.purpose == ArchivePurpose.PARTNER:
             arguments.append('--partner')
+        for updated_archive, updated_suite in updated_suites:
+            if archive == updated_archive:
+                arguments.extend(['--dirty-suite', updated_suite])
 
         os.rename(get_backup_dists(archive_config), temporary_dists)
         try:
@@ -544,18 +549,19 @@
             security_suites=security_suites)
         return True
 
-    def publishDistroUploads(self, distribution):
+    def publishDistroUploads(self, distribution, updated_suites=[]):
         """Publish the distro's complete uploads."""
         self.logger.debug("Full publication.  This may take some time.")
         for archive in get_publishable_archives(distribution):
             if archive.purpose in self.configs[distribution]:
                 # This, for the main archive, is where the script spends
                 # most of its time.
-                self.publishDistroArchive(distribution, archive)
+                self.publishDistroArchive(
+                    distribution, archive, updated_suites=updated_suites)
 
-    def updateContentsFile(self, distribution, suite, arch):
+    def updateContentsFile(self, archive, distribution, suite, arch):
         """Update a single Contents file if necessary."""
-        config = self.configs[distribution][ArchivePurpose.PRIMARY]
+        config = self.configs[distribution][archive.purpose]
         backup_dists = get_backup_dists(config)
         content_dists = os.path.join(
             config.distroroot, "contents-generation", distribution.name,
@@ -573,11 +579,16 @@
 
     def updateContentsFiles(self, distribution):
         """Pick up updated Contents files if necessary."""
-        for series in distribution.getNonObsoleteSeries():
-            for pocket in PackagePublishingPocket.items:
-                suite = series.getSuite(pocket)
-                for arch in series.enabled_architectures:
-                    self.updateContentsFile(distribution, suite, arch)
+        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))
+        return updated_suites
 
     def publish(self, distribution, security_only=False):
         """Do the main publishing work.
@@ -594,8 +605,9 @@
             if security_only:
                 has_published = self.publishSecurityUploads(distribution)
             else:
-                self.publishDistroUploads(distribution)
-                self.updateContentsFiles(distribution)
+                updated_suites = self.updateContentsFiles(distribution)
+                self.publishDistroUploads(
+                    distribution, updated_suites=updated_suites)
                 # Let's assume the main archive is always modified
                 has_published = True
 

=== modified file 'lib/lp/archivepublisher/scripts/publishdistro.py'
--- lib/lp/archivepublisher/scripts/publishdistro.py	2014-08-09 19:45:00 +0000
+++ lib/lp/archivepublisher/scripts/publishdistro.py	2015-03-18 18:07:07 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2013 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2015 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Publisher script class."""
@@ -70,6 +70,11 @@
             type='string', default=[], help='The suite to publish')
 
         self.parser.add_option(
+            "--dirty-suite", metavar="SUITE", dest="dirty_suites",
+            action="append", default=[],
+            help="Consider this suite dirty regardless of publications.")
+
+        self.parser.add_option(
             "-R", "--distsroot", dest="distsroot", metavar="SUFFIX",
             default=None,
             help=(
@@ -174,19 +179,21 @@
         """Find the named `suite` in the selected `Distribution`.
 
         :param suite: The suite name to look for.
-        :return: A tuple of distroseries name and pocket.
+        :return: A tuple of distroseries and pocket.
         """
         try:
             series, pocket = distribution.getDistroSeriesAndPocket(suite)
         except NotFoundError as e:
             raise OptionValueError(e)
-        return series.name, pocket
+        return series, pocket
 
     def findAllowedSuites(self, distribution):
         """Find the selected suite(s)."""
-        return set([
-            self.findSuite(distribution, suite)
-            for suite in self.options.suite])
+        suites = set()
+        for suite in self.options.suite:
+            series, pocket = self.findSuite(distribution, suite)
+            suites.add((series.name, pocket))
+        return suites
 
     def getCopyArchives(self, distribution):
         """Find copy archives for the selected distribution."""
@@ -285,6 +292,12 @@
                 if archive.status == ArchiveStatus.DELETING:
                     work_done = self.deleteArchive(archive, publisher)
                 elif archive.publish:
+                    for suite in self.options.dirty_suites:
+                        distroseries, pocket = self.findSuite(
+                            distribution, suite)
+                        if not publisher.cannotModifySuite(
+                                distroseries, pocket):
+                            publisher.markPocketDirty(distroseries, pocket)
                     self.publishArchive(archive, publisher)
                     work_done = True
                 else:

=== modified file 'lib/lp/archivepublisher/tests/test_generate_contents_files.py'
--- lib/lp/archivepublisher/tests/test_generate_contents_files.py	2015-02-13 10:33:30 +0000
+++ lib/lp/archivepublisher/tests/test_generate_contents_files.py	2015-03-18 18:07:07 +0000
@@ -5,6 +5,7 @@
 
 __metaclass__ = type
 
+import hashlib
 from optparse import OptionValueError
 import os
 
@@ -262,7 +263,8 @@
 
     def test_main(self):
         # If run end-to-end, the script generates Contents.gz files, and a
-        # following publisher run will put those files in their final place.
+        # following publisher run will put those files in their final place
+        # and include them in the Release file.
         distro = self.makeDistro()
         distroseries = self.factory.makeDistroSeries(distribution=distro)
         processor = self.factory.makeProcessor()
@@ -287,9 +289,21 @@
         publisher_script.txn = self.layer.txn
         publisher_script.logger = DevNullLogger()
         publisher_script.main()
-        self.assertTrue(file_exists(os.path.join(
+        contents_path = os.path.join(
             script.config.distsroot, suite,
-            "Contents-%s.gz" % das.architecturetag)))
+            "Contents-%s.gz" % das.architecturetag)
+        self.assertTrue(file_exists(contents_path))
+        with open(contents_path, "rb") as contents_file:
+            contents_bytes = contents_file.read()
+        release_path = os.path.join(script.config.distsroot, suite, "Release")
+        self.assertTrue(file_exists(release_path))
+        with open(release_path) as release_file:
+            release_lines = release_file.readlines()
+        self.assertIn(
+            " %s %16s Contents-%s.gz\n" % (
+                hashlib.md5(contents_bytes).hexdigest(), len(contents_bytes),
+                das.architecturetag),
+            release_lines)
 
     def test_run_script(self):
         # The script will run stand-alone.

=== modified file 'lib/lp/archivepublisher/tests/test_publish_ftpmaster.py'
--- lib/lp/archivepublisher/tests/test_publish_ftpmaster.py	2015-02-13 10:33:30 +0000
+++ lib/lp/archivepublisher/tests/test_publish_ftpmaster.py	2015-03-18 18:07:07 +0000
@@ -862,7 +862,8 @@
         os.makedirs(content_suite)
         write_marker_file(
             [content_suite, "%s-staged.gz" % contents_filename], "Contents")
-        script.updateContentsFile(distro, distroseries.name, das)
+        script.updateContentsFile(
+            distro.main_archive, distro, distroseries.name, das)
         self.assertEqual(
             "Contents",
             read_marker_file([backup_suite, "%s.gz" % contents_filename]))

=== modified file 'lib/lp/archivepublisher/tests/test_publisher.py'
--- lib/lp/archivepublisher/tests/test_publisher.py	2014-10-30 13:04:31 +0000
+++ lib/lp/archivepublisher/tests/test_publisher.py	2015-03-18 18:07:07 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2014 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2015 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Tests for publisher class."""
@@ -1820,6 +1820,33 @@
             for component in components:
                 self.assertIn(component + '/i18n/Translation-en.bz2', content)
 
+    def testReleaseFileForContents(self):
+        """Test Release file writing for Contents files."""
+        publisher = Publisher(
+            self.logger, self.config, self.disk_pool,
+            self.ubuntutest.main_archive)
+
+        # Put a Contents file in place, and force the publisher to republish
+        # that suite.
+        series_path = os.path.join(self.config.distsroot, 'breezy-autotest')
+        contents_path = os.path.join(series_path, 'Contents-i386.gz')
+        os.makedirs(os.path.dirname(contents_path))
+        with gzip.GzipFile(contents_path, 'wb'):
+            pass
+        publisher.markPocketDirty(
+            self.ubuntutest.getSeries('breezy-autotest'),
+            PackagePublishingPocket.RELEASE)
+
+        publisher.A_publish(False)
+        publisher.C_doFTPArchive(False)
+        publisher.D_writeReleaseFiles(False)
+
+        # The Contents file is listed correctly in Release.
+        release = self.parseRelease(os.path.join(series_path, 'Release'))
+        with open(contents_path, "rb") as contents_file:
+            self.assertReleaseContentsMatch(
+                release, 'Contents-i386.gz', contents_file.read())
+
     def testCreateSeriesAliasesNoAlias(self):
         """createSeriesAliases has nothing to do by default."""
         publisher = Publisher(