Merge lp:~cjwatson/launchpad-buildd/missing-changes into lp:launchpad-buildd

[Colin Watson]

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

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.

[William Grant (wgrant)]

review: Approve (code)

[lp:~cjwatson/launchpad-buildd/missing-changes]

77. By Colin Watson on 2013-09-24

merge trunk

=== modified file 'debian/changelog'
--- debian/changelog	2013-09-18 12:20:31 +0000
+++ debian/changelog	2013-09-24 08:54:25 +0000
@@ -3,6 +3,8 @@
   [ Colin Watson ]
   * Remove obsolete BuilderStatus.ABORTED.
   * Remove obsolete BuildDSlave.fetchFile method, unused since October 2005.
+  * 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-24 08:54:25 +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-24 08:54:25 +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'))