Merge lp:~wgrant/launchpad-buildd/cut-down-config into lp:launchpad-buildd

[William Grant]

Drop pointless binary paths from the config file.

Drop pointless binary paths from the config file. sharepath is all we need, since the paths under there really aren't user-configurable.

[Colin Watson (cjwatson)]

review: Needs Fixing

[lp:~wgrant/launchpad-buildd/cut-down-config]

145. By William Grant on 2015-05-12

Rewrite upgrade-config for 127 to preserve comments etc.

[Colin Watson (cjwatson)]

Thanks, the output looks neater now. Sorry for the extra fiddly work!

review: Approve

[lp:~wgrant/launchpad-buildd/cut-down-config]

146. By William Grant on 2015-05-12

Drop unused import.

=== modified file 'debian/changelog'
--- debian/changelog	2015-05-11 14:37:01 +0000
+++ debian/changelog	2015-05-12 09:58:23 +0000
@@ -2,6 +2,7 @@
 
   [ William Grant ]
   * Refactor lpbuildd.binarypackage tests to be readable.
+  * Drop duplicated paths from the config file.
 
   [ Colin Watson ]
   * Apply more reasonable log handling, as well as logrotate.

=== modified file 'debian/upgrade-config'
--- debian/upgrade-config	2014-08-13 00:41:22 +0000
+++ debian/upgrade-config	2015-05-12 09:58:23 +0000
@@ -5,6 +5,7 @@
 
 """Upgrade a launchpad-buildd configuration file."""
 
+import os
 import re
 import sys
 import subprocess
@@ -167,6 +168,38 @@
     in_file.close()
     out_file.close()
 
+def upgrade_to_127():
+    print "Upgrading %s to version 127" % conf_file
+    os.rename(conf_file, conf_file + "-prev127~")
+
+    in_file = open(conf_file + "-prev127~", "r")
+    out_file = open(conf_file, "w")
+    obsolete_prefixes = [
+        '[allmanagers]', '[debianmanager]', '[sourcepackagerecipemanager]',
+        '[livefilesystemmanager]', 'preppath ', 'unpackpath ', 'cleanpath ',
+        'mountpath ', 'umountpath ', 'processscanpath ', 'updatepath ',
+        'sourcespath ', 'sbuildpath ', 'buildrecipepath ', 'generatepath ',
+        'buildlivefspath ',
+        ]
+    wrote_blank = False
+    for line in in_file:
+        # Remove obsolete paths and sections.
+        if any(line.startswith(p) for p in obsolete_prefixes):
+            continue
+        # Squash any sequences of blank lines into a single one.
+        if not line.strip():
+            if wrote_blank:
+                continue
+            wrote_blank = True
+        else:
+            wrote_blank = False
+        out_file.write(line)
+        # Add single new sharepath to the end of the slave section.
+        if line.startswith("ntphost "):
+            out_file.write("sharepath = /usr/share/launchpad-buildd\n")
+    in_file.close()
+    out_file.close()
+
 if __name__ == "__main__":
     old_version = re.sub(r'[~-].*', '', old_version)
     if version_compare(old_version, "12") < 0:
@@ -191,3 +224,5 @@
         upgrade_to_120()
     if version_compare(old_version, "126") < 0:
         upgrade_to_126()
+    if version_compare(old_version, "127") < 0:
+        upgrade_to_127()

=== modified file 'lpbuildd/binarypackage.py'
--- lpbuildd/binarypackage.py	2014-08-06 07:54:20 +0000
+++ lpbuildd/binarypackage.py	2015-05-12 09:58:23 +0000
@@ -2,6 +2,7 @@
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 
+import os
 import re
 
 from lpbuildd.debian import DebianBuildManager, DebianBuildState
@@ -41,7 +42,7 @@
 
     def __init__(self, slave, buildid, **kwargs):
         DebianBuildManager.__init__(self, slave, buildid, **kwargs)
-        self._sbuildpath = slave._config.get("binarypackagemanager", "sbuildpath")
+        self._sbuildpath = os.path.join(self._slavebin, "sbuild-package")
         self._sbuildargs = slave._config.get("binarypackagemanager",
                                              "sbuildargs").split(" ")
 

=== modified file 'lpbuildd/debian.py'
--- lpbuildd/debian.py	2014-05-04 21:35:37 +0000
+++ lpbuildd/debian.py	2015-05-12 09:58:23 +0000
@@ -33,9 +33,10 @@
 
     def __init__(self, slave, buildid, **kwargs):
         BuildManager.__init__(self, slave, buildid, **kwargs)
-        self._updatepath = slave._config.get("debianmanager", "updatepath")
-        self._sourcespath = slave._config.get("debianmanager", "sourcespath")
-        self._cachepath = slave._config.get("slave","filecache")
+        self._updatepath = os.path.join(self._slavebin, "update-debian-chroot")
+        self._sourcespath = os.path.join(
+            self._slavebin, "override-sources-list")
+        self._cachepath = slave._config.get("slave", "filecache")
         self._state = DebianBuildState.INIT
         slave.emptyLog()
         self.alreadyfailed = False

=== modified file 'lpbuildd/livefs.py'
--- lpbuildd/livefs.py	2014-06-27 12:19:31 +0000
+++ lpbuildd/livefs.py	2015-05-12 09:58:23 +0000
@@ -29,8 +29,7 @@
 
     def __init__(self, slave, buildid, **kwargs):
         DebianBuildManager.__init__(self, slave, buildid, **kwargs)
-        self.build_livefs_path = slave._config.get(
-            "livefilesystemmanager", "buildlivefspath")
+        self.build_livefs_path = os.path.join(self._slavebin, "buildlivefs")
 
     def initiate(self, files, chroot, extra_args):
         """Initiate a build with a given set of files and chroot."""

=== modified file 'lpbuildd/slave.py'
--- lpbuildd/slave.py	2014-05-10 17:40:29 +0000
+++ lpbuildd/slave.py	2015-05-12 09:58:23 +0000
@@ -108,12 +108,14 @@
         if reactor is None:
             reactor = default_reactor
         self._reactor = reactor
-        self._preppath = slave._config.get("allmanagers", "preppath")
-        self._unpackpath = slave._config.get("allmanagers", "unpackpath")
-        self._cleanpath = slave._config.get("allmanagers", "cleanpath")
-        self._mountpath = slave._config.get("allmanagers", "mountpath")
-        self._umountpath = slave._config.get("allmanagers", "umountpath")
-        self._scanpath = slave._config.get("allmanagers", "processscanpath")
+        self._sharepath = slave._config.get("slave", "sharepath")
+        self._slavebin = os.path.join(self._sharepath, "slavebin")
+        self._preppath = os.path.join(self._slavebin, "slave-prep")
+        self._unpackpath = os.path.join(self._slavebin, "unpack-chroot")
+        self._cleanpath = os.path.join(self._slavebin, "remove-build")
+        self._mountpath = os.path.join(self._slavebin, "mount-chroot")
+        self._umountpath = os.path.join(self._slavebin, "umount-chroot")
+        self._scanpath = os.path.join(self._slavebin, "scan-for-processes")
         self._subprocess = None
         self._reaped_states = set()
         self.is_archive_private = False

=== modified file 'lpbuildd/sourcepackagerecipe.py'
--- lpbuildd/sourcepackagerecipe.py	2013-10-03 10:39:23 +0000
+++ lpbuildd/sourcepackagerecipe.py	2015-05-12 09:58:23 +0000
@@ -60,8 +60,7 @@
         :param buildid: The id of the build (a str).
         """
         DebianBuildManager.__init__(self, slave, buildid)
-        self.build_recipe_path = slave._config.get(
-            "sourcepackagerecipemanager", "buildrecipepath")
+        self.build_recipe_path = os.path.join(self._slavebin, "buildrecipe")
 
     def initiate(self, files, chroot, extra_args):
         """Initiate a build with a given set of files and chroot.

=== modified file 'lpbuildd/tests/buildd-slave-test.conf'
--- lpbuildd/tests/buildd-slave-test.conf	2013-07-24 10:42:05 +0000
+++ lpbuildd/tests/buildd-slave-test.conf	2015-05-12 09:58:23 +0000
@@ -5,23 +5,7 @@
 filecache = /var/tmp/buildd/filecache
 bindhost = localhost
 bindport = 8221
-
-[allmanagers]
-unpackpath = /var/tmp/buildd/slavebin/unpack-chroot
-cleanpath = /var/tmp/buildd/slavebin/remove-build
-mountpath = /var/tmp/buildd/slavebin/mount-chroot
-umountpath = /var/tmp/buildd/slavebin/umount-chroot
-preppath = /var/tmp/buildd/slavebin/slave-prep
-processscanpath = /var/tmp/buildd/slavebin/scan-for-processes
-
-[debianmanager]
-sbuildpath = /var/tmp/buildd/slavebin/sbuild-package
-sbuildargs = -dautobuild --nolog --batch
-updatepath = /var/tmp/buildd/slavebin/update-debian-chroot
-sourcespath = /usr/share/launchpad-buildd/slavebin/override-sources-list
+sharepath = /var/tmp/buildd
 
 [binarypackagemanager]
-sbuildpath = /var/tmp/buildd/slavebin/sbuild-package
 sbuildargs = -dautobuild --nolog --batch
-updatepath = /var/tmp/buildd/slavebin/update-debian-chroot
-sourcespath = /usr/share/launchpad-buildd/slavebin/override-sources-list

=== modified file 'lpbuildd/tests/test_binarypackage.py'
--- lpbuildd/tests/test_binarypackage.py	2015-04-17 19:16:17 +0000
+++ lpbuildd/tests/test_binarypackage.py	2015-05-12 09:58:23 +0000
@@ -94,9 +94,9 @@
         self.assertState(
             BinaryPackageBuildState.SBUILD,
             [
-            'sbuildpath', 'sbuild-package', self.buildid, 'i386', 'warty',
-            'sbuildargs', '--archive=ubuntu', '--dist=warty',
-            '--architecture=i386', '--comp=main', 'foo_1.dsc',
+            'sharepath/slavebin/sbuild-package', 'sbuild-package',
+            self.buildid, 'i386', 'warty', 'sbuildargs', '--archive=ubuntu',
+            '--dist=warty', '--architecture=i386', '--comp=main', 'foo_1.dsc',
             ], True)
         self.assertFalse(self.slave.wasCalled('chrootFail'))
 
@@ -115,13 +115,15 @@
         self.buildmanager.iterate(exit_code)
         self.assertState(
             BinaryPackageBuildState.SBUILD,
-            ['processscanpath', 'scan-for-processes', self.buildid], False)
+            ['sharepath/slavebin/scan-for-processes', 'scan-for-processes',
+             self.buildid], False)
 
     def assertUnmountsSanely(self):
         self.buildmanager.iterateReap(self.getState(), 0)
         self.assertState(
             BinaryPackageBuildState.UMOUNT,
-            ['umountpath', 'umount-chroot', self.buildid], True)
+            ['sharepath/slavebin/umount-chroot', 'umount-chroot',
+             self.buildid], True)
 
     def test_iterate(self):
         # The build manager iterates a normal build from start to finish.
@@ -153,7 +155,8 @@
         self.buildmanager.abort()
         self.assertState(
             BinaryPackageBuildState.SBUILD,
-            ['processscanpath', 'scan-for-processes', self.buildid], False)
+            ['sharepath/slavebin/scan-for-processes', 'scan-for-processes',
+             self.buildid], False)
         self.assertFalse(self.slave.wasCalled('buildFail'))
 
         # If reaping completes successfully, the build manager returns
@@ -171,7 +174,8 @@
         self.buildmanager.abort()
         self.assertState(
             BinaryPackageBuildState.SBUILD,
-            ['processscanpath', 'scan-for-processes', self.buildid], False)
+            ['sharepath/slavebin/scan-for-processes', 'scan-for-processes',
+             self.buildid], False)
         self.assertFalse(self.slave.wasCalled('builderFail'))
         reap_subprocess = self.buildmanager._subprocess
 
@@ -194,7 +198,8 @@
         self.buildmanager.iterate(128 + 9)  # SIGKILL
         self.assertState(
             BinaryPackageBuildState.UMOUNT,
-            ['umountpath', 'umount-chroot', self.buildid], True)
+            ['sharepath/slavebin/umount-chroot', 'umount-chroot',
+             self.buildid], True)
 
     def test_abort_between_subprocesses(self):
         # If a build is aborted between subprocesses, the build manager
@@ -207,12 +212,14 @@
         self.buildmanager.abort()
         self.assertState(
             BinaryPackageBuildState.INIT,
-            ['processscanpath', 'scan-for-processes', self.buildid], False)
+            ['sharepath/slavebin/scan-for-processes', 'scan-for-processes',
+             self.buildid], False)
 
         self.buildmanager.iterate(0)
         self.assertState(
             BinaryPackageBuildState.CLEANUP,
-            ['cleanpath', 'remove-build', self.buildid], True)
+            ['sharepath/slavebin/remove-build', 'remove-build', self.buildid],
+            True)
         self.assertFalse(self.slave.wasCalled('builderFail'))
 
     def test_missing_changes(self):

=== modified file 'lpbuildd/tests/test_livefs.py'
--- lpbuildd/tests/test_livefs.py	2014-07-24 11:44:04 +0000
+++ lpbuildd/tests/test_livefs.py	2015-05-12 09:58:23 +0000
@@ -72,8 +72,9 @@
         self.assertEqual(
             LiveFilesystemBuildState.BUILD_LIVEFS, self.getState())
         expected_command = [
-            "buildlivefspath", "buildlivefs", "--build-id", self.buildid,
-            "--arch", "i386", "--project", "ubuntu", "--series", "saucy",
+            "sharepath/slavebin/buildlivefs", "buildlivefs", "--build-id",
+            self.buildid, "--arch", "i386", "--project", "ubuntu",
+            "--series", "saucy",
             ]
         self.assertEqual(expected_command, self.buildmanager.commands[-1])
         self.assertEqual(
@@ -98,7 +99,8 @@
         # After building the package, reap processes.
         self.buildmanager.iterate(0)
         expected_command = [
-            "processscanpath", "scan-for-processes", self.buildid,
+            "sharepath/slavebin/scan-for-processes", "scan-for-processes",
+            self.buildid,
             ]
         self.assertEqual(
             LiveFilesystemBuildState.BUILD_LIVEFS, self.getState())
@@ -111,7 +113,8 @@
 
         # Control returns to the DebianBuildManager in the UMOUNT state.
         self.buildmanager.iterateReap(self.getState(), 0)
-        expected_command = ["umountpath", "umount-chroot", self.buildid]
+        expected_command = [
+            "sharepath/slavebin/umount-chroot", "umount-chroot", self.buildid]
         self.assertEqual(LiveFilesystemBuildState.UMOUNT, self.getState())
         self.assertEqual(expected_command, self.buildmanager.commands[-1])
         self.assertEqual(

=== modified file 'lpbuildd/tests/test_sourcepackagerecipe.py'
--- lpbuildd/tests/test_sourcepackagerecipe.py	2013-10-03 10:39:23 +0000
+++ lpbuildd/tests/test_sourcepackagerecipe.py	2015-05-12 09:58:23 +0000
@@ -84,7 +84,7 @@
         self.assertEqual(
             SourcePackageRecipeBuildState.BUILD_RECIPE, self.getState())
         expected_command = [
-            'buildrecipepath', 'buildrecipe', self.buildid,
+            'sharepath/slavebin/buildrecipe', 'buildrecipe', self.buildid,
             'Steve\u1234'.encode('utf-8'), '[e-mail]',
             'maverick', 'maverick', 'universe', 'puppies',
             ]
@@ -118,7 +118,8 @@
         # After building the package, reap processes.
         self.buildmanager.iterate(0)
         expected_command = [
-            'processscanpath', 'scan-for-processes', self.buildid,
+            'sharepath/slavebin/scan-for-processes', 'scan-for-processes',
+            self.buildid,
             ]
         self.assertEqual(
             SourcePackageRecipeBuildState.BUILD_RECIPE, self.getState())
@@ -133,7 +134,7 @@
         # Control returns to the DebianBuildManager in the UMOUNT state.
         self.buildmanager.iterateReap(self.getState(), 0)
         expected_command = [
-            'umountpath', 'umount-chroot', self.buildid
+            'sharepath/slavebin/umount-chroot', 'umount-chroot', self.buildid
             ]
         self.assertEqual(SourcePackageRecipeBuildState.UMOUNT, self.getState())
         self.assertEqual(expected_command, self.buildmanager.commands[-1])
@@ -156,7 +157,8 @@
         # The buildmanager calls depFail correctly and reaps processes.
         self.buildmanager.iterate(RETCODE_FAILURE_INSTALL_BUILD_DEPS)
         expected_command = [
-            'processscanpath', 'scan-for-processes', self.buildid
+            'sharepath/slavebin/scan-for-processes', 'scan-for-processes',
+            self.buildid,
             ]
         self.assertEqual(
             SourcePackageRecipeBuildState.BUILD_RECIPE, self.getState())
@@ -170,7 +172,7 @@
         # Control returns to the DebianBuildManager in the UMOUNT state.
         self.buildmanager.iterateReap(self.getState(), 0)
         expected_command = [
-            'umountpath', 'umount-chroot', self.buildid
+            'sharepath/slavebin/umount-chroot', 'umount-chroot', self.buildid,
             ]
         self.assertEqual(SourcePackageRecipeBuildState.UMOUNT, self.getState())
         self.assertEqual(expected_command, self.buildmanager.commands[-1])
@@ -191,7 +193,8 @@
         # The buildmanager calls buildFail correctly and reaps processes.
         self.buildmanager.iterate(RETCODE_FAILURE_INSTALL_BUILD_DEPS)
         expected_command = [
-            'processscanpath', 'scan-for-processes', self.buildid
+            'sharepath/slavebin/scan-for-processes', 'scan-for-processes',
+            self.buildid,
             ]
         self.assertEqual(
             SourcePackageRecipeBuildState.BUILD_RECIPE, self.getState())
@@ -204,7 +207,7 @@
         # Control returns to the DebianBuildManager in the UMOUNT state.
         self.buildmanager.iterateReap(self.getState(), 0)
         expected_command = [
-            'umountpath', 'umount-chroot', self.buildid
+            'sharepath/slavebin/umount-chroot', 'umount-chroot', self.buildid,
             ]
         self.assertEqual(SourcePackageRecipeBuildState.UMOUNT, self.getState())
         self.assertEqual(expected_command, self.buildmanager.commands[-1])

=== modified file 'lpbuildd/tests/test_translationtemplatesbuildmanager.py'
--- lpbuildd/tests/test_translationtemplatesbuildmanager.py	2013-09-26 09:03:14 +0000
+++ lpbuildd/tests/test_translationtemplatesbuildmanager.py	2015-05-12 09:58:23 +0000
@@ -82,7 +82,9 @@
         self.assertEqual(
             TranslationTemplatesBuildState.GENERATE, self.getState())
         expected_command = [
-            'generatepath', 'generatepath', self.buildid, url, 'resultarchive'
+            'sharepath/slavebin/generate-translation-templates',
+            'sharepath/slavebin/generate-translation-templates',
+            self.buildid, url, 'resultarchive'
             ]
         self.assertEqual(expected_command, self.buildmanager.commands[-1])
         self.assertEqual(
@@ -101,7 +103,8 @@
         # After generating templates, reap processes.
         self.buildmanager.iterate(0)
         expected_command = [
-            'processscanpath', 'scan-for-processes', self.buildid
+            'sharepath/slavebin/scan-for-processes', 'scan-for-processes',
+            self.buildid,
             ]
         self.assertEqual(
             TranslationTemplatesBuildState.GENERATE, self.getState())
@@ -115,7 +118,7 @@
         # The control returns to the DebianBuildManager in the UMOUNT state.
         self.buildmanager.iterateReap(self.getState(), 0)
         expected_command = [
-            'umountpath', 'umount-chroot', self.buildid
+            'sharepath/slavebin/umount-chroot', 'umount-chroot', self.buildid,
             ]
         self.assertEqual(
             TranslationTemplatesBuildState.UMOUNT, self.getState())
@@ -139,7 +142,7 @@
         self.assertEqual(
             TranslationTemplatesBuildState.UMOUNT, self.getState())
         expected_command = [
-            'umountpath', 'umount-chroot', self.buildid
+            'sharepath/slavebin/umount-chroot', 'umount-chroot', self.buildid,
             ]
         self.assertEqual(expected_command, self.buildmanager.commands[-1])
         self.assertEqual(
@@ -159,7 +162,8 @@
         # The buildmanager fails and reaps processes.
         self.buildmanager.iterate(-1)
         expected_command = [
-            'processscanpath', 'scan-for-processes', self.buildid
+            'sharepath/slavebin/scan-for-processes', 'scan-for-processes',
+            self.buildid,
             ]
         self.assertEqual(
             TranslationTemplatesBuildState.GENERATE, self.getState())
@@ -173,7 +177,7 @@
         self.assertEqual(
             TranslationTemplatesBuildState.UMOUNT, self.getState())
         expected_command = [
-            'umountpath', 'umount-chroot', self.buildid
+            'sharepath/slavebin/umount-chroot', 'umount-chroot', self.buildid
             ]
         self.assertEqual(expected_command, self.buildmanager.commands[-1])
         self.assertEqual(

=== modified file 'lpbuildd/translationtemplates.py'
--- lpbuildd/translationtemplates.py	2013-07-25 17:26:10 +0000
+++ lpbuildd/translationtemplates.py	2015-05-12 09:58:23 +0000
@@ -25,8 +25,8 @@
 
     def __init__(self, slave, buildid):
         super(TranslationTemplatesBuildManager, self).__init__(slave, buildid)
-        self._generatepath = slave._config.get(
-            "translationtemplatesmanager", "generatepath")
+        self._generatepath = os.path.join(
+            self._slavebin, "generate-translation-templates")
         self._resultname = slave._config.get(
             "translationtemplatesmanager", "resultarchive")
 

=== modified file 'template-buildd-slave.conf'
--- template-buildd-slave.conf	2014-08-06 08:01:35 +0000
+++ template-buildd-slave.conf	2015-05-12 09:58:23 +0000
@@ -8,29 +8,10 @@
 bindhost = @BINDHOST@
 bindport = @BINDPORT@
 ntphost = ntp.buildd
-
-[allmanagers]
-preppath = /usr/share/launchpad-buildd/slavebin/slave-prep
-unpackpath = /usr/share/launchpad-buildd/slavebin/unpack-chroot
-cleanpath = /usr/share/launchpad-buildd/slavebin/remove-build
-mountpath = /usr/share/launchpad-buildd/slavebin/mount-chroot
-umountpath = /usr/share/launchpad-buildd/slavebin/umount-chroot
-processscanpath = /usr/share/launchpad-buildd/slavebin/scan-for-processes
-
-[debianmanager]
-updatepath = /usr/share/launchpad-buildd/slavebin/update-debian-chroot
-sourcespath = /usr/share/launchpad-buildd/slavebin/override-sources-list
+sharepath = /usr/share/launchpad-buildd
 
 [binarypackagemanager]
-sbuildpath = /usr/share/launchpad-buildd/slavebin/sbuild-package
 sbuildargs = --nolog --batch
 
-[sourcepackagerecipemanager]
-buildrecipepath = /usr/share/launchpad-buildd/slavebin/buildrecipe
-
 [translationtemplatesmanager]
-generatepath = /usr/share/launchpad-buildd/slavebin/generate-translation-templates
 resultarchive = translation-templates.tar.gz
-
-[livefilesystemmanager]
-buildlivefspath = /usr/share/launchpad-buildd/slavebin/buildlivefs