launchpad-reviewers team mailing list archive

["Jeroen T. Vermeulen" <jtv@xxxxxxxxxxxxx>]

Jeroen T. Vermeulen has proposed merging lp:~jtv/launchpad/db-bug-771727 into lp:launchpad/db-devel.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #771727 in Launchpad itself: "generate-contents-files.py ignores the release pocket"
  https://bugs.launchpad.net/launchpad/+bug/771727

For more details, see:
https://code.launchpad.net/~jtv/launchpad/db-bug-771727/+merge/59203

= Summary =

The new, python-based generate-contents-files (a conversion from a bash script) was not generating contents files for release pockets.

The cause: the old script used lp-query-distro to get the distribution's pocket suffixes *except the empty ones* (the release pocket doesn't have a suffix) and then computed the distribution's suites by adding the suite name, plus for each pocket suffix, the suite with the pocket suffix attached.  So effectively we had code to take the release pocket out of the list and then other code to put it back in.  In converting the bash code to python I missed this subtlety.


== Proposed fix ==

Don't use LpQueryDistro at all.  Use what it uses: pocketsuffix.values().


== Pre-implementation notes ==

Spotted by wgrant.  At first we thought the empty suffix was lost in split()ing the output from LpQueryDistro.  But since it would be (both in a python split() and in a shell script) the empty suffix wasn't even in there.


== Implementation details ==

The method wasn't individually tested, so that part is easy.  I added a test to prove that the release pocket is included, which duly fails without the fix.


== Tests ==

{{{
./bin/test -vvc lp.archivepublisher.tests.test_generate_contents_files
}}}


== Demo and Q/A ==

Apart from this, the script seems to work.  Both Julian and William have tried it on dogfood.


= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/archivepublisher/tests/test_generate_contents_files.py
  lib/lp/archivepublisher/scripts/generate_contents_files.py


Jeroen
-- 
https://code.launchpad.net/~jtv/launchpad/db-bug-771727/+merge/59203
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/launchpad/db-bug-771727 into lp:launchpad/db-devel.

=== modified file 'lib/lp/archivepublisher/scripts/generate_contents_files.py'
--- lib/lp/archivepublisher/scripts/generate_contents_files.py	2011-04-21 14:14:00 +0000
+++ lib/lp/archivepublisher/scripts/generate_contents_files.py	2011-04-27 11:15:43 +0000
@@ -15,6 +15,7 @@
 from canonical.config import config
 from lp.archivepublisher.config import getPubConfig
 from lp.registry.interfaces.distribution import IDistributionSet
+from lp.registry.interfaces.pocket import pocketsuffix
 from lp.services.command_spawner import (
     CommandSpawner,
     OutputLineHandler,
@@ -157,10 +158,6 @@
         query_distro.runAction(presenter=receiver)
         return receiver.argument
 
-    def getPocketSuffixes(self):
-        """Query the distribution's pocket suffixes."""
-        return self.queryDistro("pocket_suffixes").split()
-
     def getSuites(self):
         """Query the distribution's suites."""
         return self.queryDistro("supported").split()
@@ -168,7 +165,7 @@
     def getPockets(self):
         """Return suites that are actually supported in this distribution."""
         pockets = []
-        pocket_suffixes = self.getPocketSuffixes()
+        pocket_suffixes = pocketsuffix.values()
         for suite in self.getSuites():
             for pocket_suffix in pocket_suffixes:
                 pocket = suite + pocket_suffix

=== modified file 'lib/lp/archivepublisher/tests/test_generate_contents_files.py'
--- lib/lp/archivepublisher/tests/test_generate_contents_files.py	2011-04-21 12:26:00 +0000
+++ lib/lp/archivepublisher/tests/test_generate_contents_files.py	2011-04-27 11:15:43 +0000
@@ -20,6 +20,7 @@
     GenerateContentsFiles,
     move_file,
     )
+from lp.registry.interfaces.pocket import PackagePublishingPocket
 from lp.services.log.logger import DevNullLogger
 from lp.services.scripts.base import LaunchpadScriptFailure
 from lp.services.utils import file_exists
@@ -219,6 +220,17 @@
         os.makedirs(os.path.join(script.config.distsroot, package.suite))
         self.assertEqual([package.suite], script.getPockets())
 
+    def test_getPocket_includes_release_pocket(self):
+        # getPockets also includes the release pocket, which is named
+        # after the distroseries without a suffix.
+        distro = self.makeDistro()
+        distroseries = self.factory.makeDistroSeries(distribution=distro)
+        package = self.factory.makeSuiteSourcePackage(
+            distroseries, pocket=PackagePublishingPocket.RELEASE)
+        script = self.makeScript(distro)
+        os.makedirs(os.path.join(script.config.distsroot, package.suite))
+        self.assertEqual([package.suite], script.getPockets())
+
     def test_writeAptContentsConf_writes_header(self):
         # writeAptContentsConf writes apt-contents.conf.  At a minimum
         # this will include a header based on apt_conf_header.template,