launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Diff comments:

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

This unfortunately ends up rather unreadable because nested comprehensions are painful.  For the record:

=== modified file 'lib/lp/archivepublisher/scripts/publishdistro.py'
--- lib/lp/archivepublisher/scripts/publishdistro.py    2015-03-18 17:49:57 +0000
+++ lib/lp/archivepublisher/scripts/publishdistro.py    2015-03-20 00:23:58 +0000
@@ -189,11 +189,10 @@ class PublishDistro(PublisherScript):

     def findAllowedSuites(self, distribution):
         """Find the selected suite(s)."""
-        suites = set()
-        for suite in self.options.suite:
-            series, pocket = self.findSuite(distribution, suite)
-            suites.add((series.name, pocket))
-        return suites
+        return set([
+            (series.name, pocket)
+            for suite in self.options.suite
+            for series, pocket in [self.findSuite(distribution, suite)]])

     def getCopyArchives(self, distribution):
         """Find copy archives for the selected distribution."""

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


-- 
https://code.launchpad.net/~cjwatson/launchpad/ftparchive-release-contents/+merge/253415
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.