launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad-buildd/clean-duplicate-files into lp:launchpad-buildd.

Commit message:
Cope with builds that return multiple files with identical content.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad-buildd/clean-duplicate-files/+merge/219102

Some livefs build types return multiple files with identical content, such as ubuntu-touch builds due to this code in livecd-rootfs:

                        # drop this following line once cdimage can handle -boot-*.img
                        cp "${PREFIX}.boot-i386+${subarch}.img" "${PREFIX}.bootimg-${subarch}"

This currently causes launchpad-buildd to crash, because it tries to remove the same entry in the filecache more than once.  Regardless of the sanity of such a build, we clearly shouldn't crash.  This fixes that.
-- 
https://code.launchpad.net/~cjwatson/launchpad-buildd/clean-duplicate-files/+merge/219102
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad-buildd/clean-duplicate-files into lp:launchpad-buildd.

=== modified file 'debian/changelog'
--- debian/changelog	2014-05-03 14:48:19 +0000
+++ debian/changelog	2014-05-10 17:44:38 +0000
@@ -3,6 +3,7 @@
   * Drop the status_dict XML-RPC method, now that the master uses the
     new-style dict-flavoured status method.
   * Don't add symlinks to the results of livefs builds (LP: #1247461).
+  * Cope with builds that return multiple files with identical content.
 
  -- Colin Watson <cjwatson@xxxxxxxxxx>  Fri, 25 Apr 2014 13:59:16 +0100
 

=== modified file 'lpbuildd/slave.py'
--- lpbuildd/slave.py	2014-04-25 13:01:00 +0000
+++ lpbuildd/slave.py	2014-05-10 17:44:38 +0000
@@ -429,8 +429,8 @@
         """Clean up pending files and reset the internal build state."""
         if self.builderstatus != BuilderStatus.WAITING:
             raise ValueError('Slave is not WAITING when asked to clean')
-        for f in self.waitingfiles:
-            os.remove(self.cachePath(self.waitingfiles[f]))
+        for f in set(self.waitingfiles.values()):
+            os.remove(self.cachePath(f))
         self.builderstatus = BuilderStatus.IDLE
         if self._log is not None:
             self._log.close()

=== modified file 'lpbuildd/tests/test_buildd_slave.py'
--- lpbuildd/tests/test_buildd_slave.py	2011-11-21 06:22:28 +0000
+++ lpbuildd/tests/test_buildd_slave.py	2014-05-10 17:44:38 +0000
@@ -18,12 +18,16 @@
 import difflib
 import os
 import shutil
+import tempfile
 import urllib2
 import unittest
 import xmlrpclib
 
 from lpbuildd.tests.harness import (
-    BuilddSlaveTestSetup, BuilddTestCase)
+    BuilddSlaveTestSetup,
+    BuilddTestCase,
+    MockBuildManager,
+    )
 
 
 def read_file(path):
@@ -183,6 +187,22 @@
         log_tail = self.slave.getLogTail()
         self.assertEqual(len(log_tail), 0)
 
+    def testCleanDuplicateFiles(self):
+        """The clean method copes with duplicate waiting files."""
+        workdir = tempfile.mkdtemp()
+        self.addCleanup(shutil.rmtree, workdir)
+        self.slave._log = None
+        self.slave.startBuild(MockBuildManager())
+        self.slave.buildComplete()
+        paths = [os.path.join(workdir, name) for name in ('a', 'b')]
+        for path in paths:
+            f = open(path, 'w')
+            f.write('data')
+            f.close()
+            self.slave.addWaitingFile(path)
+        self.slave.clean()
+        self.assertEqual([], os.listdir(self.slave._cachepath))
+
 
 class XMLRPCBuildDSlaveTests(unittest.TestCase):