Merge lp:~robru/cupstream2distro/dont-publish-invalid-packagelist into lp:cupstream2distro

[Robert Bruce Park]

Raise a PublishError in PackageList.publish_source().

I've seen this problem a couple times now where the publish job succeeds but the dest package version field is missing, so copy2distro refuses to copy, but no indication is given to the user anywhere. In a later branch I'll fix this problem from happening, but at least for now we need the publish job to fail loudly if it's generating an invalid packagelist.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:933
http://jenkins.qa.ubuntu.com/job/cu2d-choo-choo-ci/577/
Executed test runs:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/cu2d-choo-choo-ci/577/rebuild

review: Approve (continuous-integration)

[Łukasz Zemczak (sil2100)]

Ok, I'm normally not +1 on hiding issues with quick-fixes, but sometimes when the bug is critical enough this makes sense. And I believe that you won't forget getting the root cause fixed ;) The workaround looks fine - and, come to think of it, it might help us with identifying any other possible issues with the rsync file! Besides, this makes sense as it's basically proper error-handling.

Anyway, thanks and +1!

review: Approve

=== modified file 'citrain/publisher.py'
--- citrain/publisher.py	2015-03-03 09:45:08 +0000
+++ citrain/publisher.py	2015-03-04 17:54:17 +0000
@@ -108,9 +108,12 @@
         self.silo = env.SILONAME.split('/')[-1]
 
     def publish_source(self, source):
-        """Publish one source package."""
+        """Publish one source package.
+
+        :param source: A launchpadlib source package object.
+        :raises: PublishError if any necessary data is missing."""
         name = source.source_package_name
-        self.lines.append(self.TEMPLATE.format(
+        line = self.TEMPLATE.format(
             src_ppa='ci-train-ppa-service/' + env.SILONAME,
             src_pocket=source.pocket,
             src_series=env.SERIES,
@@ -121,7 +124,12 @@
             version_in_dest=DotProject(name).dest_current_version,
             publisher_nick=env.PUBLISHER,
             target_distribution=env.DISTRO,
-        ))
+        )
+        if '\t\t' in line:
+            raise PublishError(
+                'Critical data missing from packagelist. '
+                'Perhaps try a WATCH_ONLY build.')
+        self.lines.append(line)
 
     def write(self):
         filename = self.FILENAME_FORMAT.format(self.silo, env.SERIES)

=== modified file 'tests/unit/test_script_publisher.py'
--- tests/unit/test_script_publisher.py	2015-03-03 09:45:08 +0000
+++ tests/unit/test_script_publisher.py	2015-03-04 17:54:17 +0000
@@ -121,6 +121,17 @@
              'ci-train-ppa-service/ubuntu/landing-789\tRelease\tvivid'
              '\tProposed\tvivid\tbar\t2.0-0ubuntu1\t0.9\trobru\tubuntu\n'])
 
+    def test_packagelist_publish_source_validation(self):
+        """Publishing is halted when packagelist is invalid."""
+        self.script.DotProject.return_value.dest_current_version = ''
+        env.PUBLISHER = 'robru'
+        packagelist = self.script.PackageList()
+        source = SourceFake(
+            'foo', '1.0-0ubuntu1',
+            'https://api.launchpad.net/devel/ubuntu/vivid')
+        with self.assertRaisesRegexp(PublishError, 'data missing'):
+            packagelist.publish_source(source)
+
     def test_packagelist_write(self):
         """Ensure we can correctly write the packagelist to disk."""
         packagelist = self.script.PackageList()