launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad-buildd/missing-changes into lp:launchpad-buildd.

Commit message:
If the expected .changes file doesn't exist, consider this as a package build failure rather than crashing.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #993642 in launchpad-buildd: "Breaks if the build doesn't produce the expected changes file"
  https://bugs.launchpad.net/launchpad-buildd/+bug/993642

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad-buildd/missing-changes/+merge/184591

The build slave crashes if the expected .changes file doesn't exist at the end of a build.  We should just consider this a package failure instead.
-- 
https://code.launchpad.net/~cjwatson/launchpad-buildd/missing-changes/+merge/184591
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad-buildd/missing-changes into lp:launchpad-buildd.

=== modified file 'debian/changelog'
--- debian/changelog	2013-08-30 02:48:14 +0000
+++ debian/changelog	2013-09-09 14:50:35 +0000
@@ -2,6 +2,8 @@
 
   [ Colin Watson ]
   * Remove obsolete BuilderStatus.ABORTED.
+  * If the expected .changes file doesn't exist, consider this as a package
+    build failure rather than crashing (LP: #993642).
 
   [ Adam Conrad ]
   * Tidy up log formatting of the "Already reaped..." message. 

=== modified file 'lpbuildd/binarypackage.py'
--- lpbuildd/binarypackage.py	2013-07-25 17:26:10 +0000
+++ lpbuildd/binarypackage.py	2013-09-09 14:50:35 +0000
@@ -127,7 +127,12 @@
             self.doReapProcesses(self._state)
         else:
             print("Returning build status: OK")
-            self.gatherResults()
+            try:
+                self.gatherResults()
+            except Exception, e:
+                print("Failed to gather results: %s" % e)
+                self._slave.buildFail()
+                self.alreadyfailed = True
             self.doReapProcesses(self._state)
 
     def iterateReap_SBUILD(self, success):

=== modified file 'lpbuildd/tests/test_binarypackage.py'
--- lpbuildd/tests/test_binarypackage.py	2013-07-26 18:22:25 +0000
+++ lpbuildd/tests/test_binarypackage.py	2013-09-09 14:50:35 +0000
@@ -204,3 +204,43 @@
         self.assertEqual(expected_command, self.buildmanager.commands[-1])
         self.assertEqual(
             self.buildmanager.iterate, self.buildmanager.iterators[-1])
+
+    def test_missing_changes(self):
+        # The build manager recovers if the expected .changes file does not
+        # exist, and considers it a package build failure.
+        self.startBuild()
+
+        log_path = os.path.join(self.buildmanager._cachepath, 'buildlog')
+        log = open(log_path, 'w')
+        log.write("I am a build log.")
+        log.close()
+
+        changes_path = os.path.join(
+            self.buildmanager.home, 'build-%s' % self.buildid,
+            'foo_2_i386.changes')
+        changes = open(changes_path, 'w')
+        changes.write("I am a changes file.")
+        changes.close()
+
+        # After building the package, reap processes.
+        self.buildmanager.iterate(0)
+        expected_command = [
+            'processscanpath', 'processscanpath', self.buildid,
+            ]
+        self.assertEqual(BinaryPackageBuildState.SBUILD, self.getState())
+        self.assertEqual(expected_command, self.buildmanager.commands[-1])
+        self.assertNotEqual(
+            self.buildmanager.iterate, self.buildmanager.iterators[-1])
+        self.assertTrue(self.slave.wasCalled('buildFail'))
+        self.assertEqual([], self.slave.addWaitingFile.calls)
+
+        # Control returns to the DebianBuildManager in the UMOUNT state.
+        self.buildmanager.iterateReap(self.getState(), 0)
+        expected_command = [
+            'umountpath', 'umount-chroot', self.buildid
+            ]
+        self.assertEqual(BinaryPackageBuildState.UMOUNT, self.getState())
+        self.assertEqual(expected_command, self.buildmanager.commands[-1])
+        self.assertEqual(
+            self.buildmanager.iterate, self.buildmanager.iterators[-1])
+        self.assertTrue(self.slave.wasCalled('buildFail'))