launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/publish-distro-disable-steps into lp:launchpad.

Commit message:
Add publish-distro --disable-* options to allow entirely disabling individual steps.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/publish-distro-disable-steps/+merge/289658

Add publish-distro --disable-* options to allow entirely disabling individual steps.  This will let us e.g. run just step D over a set of archives without incurring the time cost of running step A (even in non-careful mode) on them all.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/publish-distro-disable-steps into lp:launchpad.

=== modified file 'lib/lp/archivepublisher/scripts/publishdistro.py'
--- lib/lp/archivepublisher/scripts/publishdistro.py	2016-03-17 17:08:49 +0000
+++ lib/lp/archivepublisher/scripts/publishdistro.py	2016-03-21 15:43:37 +0000
@@ -72,6 +72,26 @@
             help="Make the Release file generation process careful.")
 
         self.parser.add_option(
+            "--disable-publishing", action="store_false",
+            dest="enable_publishing", default=True,
+            help="Disable the package publishing process.")
+
+        self.parser.add_option(
+            "--disable-domination", action="store_false",
+            dest="enable_domination", default=True,
+            help="Disable the domination process.")
+
+        self.parser.add_option(
+            "--disable-apt", action="store_false",
+            dest="enable_apt", default=True,
+            help="Disable index generation (e.g. apt-ftparchive).")
+
+        self.parser.add_option(
+            "--disable-release", action="store_false",
+            dest="enable_release", default=True,
+            help="Disable the Release file generation process.")
+
+        self.parser.add_option(
             "--include-non-pending", action="store_true",
             dest="include_non_pending", default=False,
             help=(
@@ -270,29 +290,37 @@
         Commits transactions along the way.
         """
         publisher.setupArchiveDirs()
-        publisher.A_publish(self.isCareful(self.options.careful_publishing))
-        self.txn.commit()
-
-        # Flag dirty pockets for any outstanding deletions.
-        publisher.A2_markPocketsWithDeletionsDirty()
-        publisher.B_dominate(self.isCareful(self.options.careful_domination))
-        self.txn.commit()
-
-        # The primary and copy archives use apt-ftparchive to
-        # generate the indexes, everything else uses the newer
-        # internal LP code.
-        careful_indexing = self.isCareful(self.options.careful_apt)
-        if archive.purpose in (ArchivePurpose.PRIMARY, ArchivePurpose.COPY):
-            publisher.C_doFTPArchive(careful_indexing)
-        else:
-            publisher.C_writeIndexes(careful_indexing)
-        self.txn.commit()
-
-        publisher.D_writeReleaseFiles(self.isCareful(
-            self.options.careful_apt or self.options.careful_release))
-        # The caller will commit this last step.
-
-        publisher.createSeriesAliases()
+        if self.options.enable_publishing:
+            publisher.A_publish(
+                self.isCareful(self.options.careful_publishing))
+            self.txn.commit()
+
+        if self.options.enable_domination:
+            # Flag dirty pockets for any outstanding deletions.
+            publisher.A2_markPocketsWithDeletionsDirty()
+            publisher.B_dominate(
+                self.isCareful(self.options.careful_domination))
+            self.txn.commit()
+
+        if self.options.enable_apt:
+            # The primary and copy archives use apt-ftparchive to
+            # generate the indexes, everything else uses the newer
+            # internal LP code.
+            careful_indexing = self.isCareful(self.options.careful_apt)
+            if archive.purpose in (
+                    ArchivePurpose.PRIMARY, ArchivePurpose.COPY):
+                publisher.C_doFTPArchive(careful_indexing)
+            else:
+                publisher.C_writeIndexes(careful_indexing)
+            self.txn.commit()
+
+        if self.options.enable_release:
+            publisher.D_writeReleaseFiles(self.isCareful(
+                self.options.careful_apt or self.options.careful_release))
+            # The caller will commit this last step.
+
+        if self.options.enable_apt:
+            publisher.createSeriesAliases()
 
     def main(self):
         """See `LaunchpadScript`."""

=== modified file 'lib/lp/archivepublisher/tests/test_publishdistro.py'
--- lib/lp/archivepublisher/tests/test_publishdistro.py	2016-03-21 00:11:14 +0000
+++ lib/lp/archivepublisher/tests/test_publishdistro.py	2016-03-21 15:43:37 +0000
@@ -429,8 +429,10 @@
         self.assertThat(hoary_inrelease_path, Not(PathExists()))
         self.assertThat(breezy_inrelease_path, Not(PathExists()))
 
-        self.runPublishDistro(
-            ['--ppa', '--careful-release', '--include-non-pending'])
+        self.runPublishDistro([
+            '--ppa', '--careful-release', '--include-non-pending',
+            '--disable-publishing', '--disable-domination', '--disable-apt',
+            ])
         # hoary-test never had indexes created, so is untouched.
         self.assertThat(hoary_inrelease_path, Not(PathExists()))
         # breezy-autotest has its Release files rewritten.
@@ -896,6 +898,31 @@
         self.assertEqual(1, publisher.B_dominate.call_count)
         self.assertEqual(1, publisher.D_writeReleaseFiles.call_count)
 
+    def test_publishArchive_honours_disable_options(self):
+        # The various --disable-* options disable the corresponding
+        # publisher steps.
+        possible_options = {
+            "--disable-publishing": ["A_publish"],
+            "--disable-domination": [
+                "A2_markPocketsWithDeletionsDirty", "B_dominate",
+                ],
+            "--disable-apt": ["C_doFTPArchive"],
+            "--disable-release": ["D_writeReleaseFiles"],
+            }
+        for option in possible_options:
+            distro = self.makeDistro()
+            script = self.makeScript(distro, args=[option])
+            script.txn = FakeTransaction()
+            publisher = FakePublisher()
+            script.publishArchive(FakeArchive(), publisher)
+            for check_option, steps in possible_options.items():
+                for step in steps:
+                    publisher_step = getattr(publisher, step)
+                    if check_option == option:
+                        self.assertEqual(0, publisher_step.call_count)
+                    else:
+                        self.assertEqual(1, publisher_step.call_count)
+
     def test_publishArchive_uses_apt_ftparchive_for_main_archive(self):
         # For some types of archive, publishArchive invokes the
         # publisher's C_doFTPArchive method as a way of generating