Review: Approve Thanks, the output looks neater now. Sorry for the extra fiddly work!
Diff comments: > === modified file 'debian/changelog' > --- debian/changelog 2015-04-17 20:49:23 +0000 > +++ debian/changelog 2015-05-12 01:58:50 +0000 > @@ -1,6 +1,7 @@ > launchpad-buildd (127) UNRELEASED; urgency=low > > * Refactor lpbuildd.binarypackage tests to be readable. > + * Drop duplicated paths from the config file. > > -- William Grant <[email protected]> Sat, 18 Apr 2015 06:41:52 +1000 > > > === modified file 'debian/upgrade-config' > --- debian/upgrade-config 2014-08-13 00:41:22 +0000 > +++ debian/upgrade-config 2015-05-12 01:58:50 +0000 > @@ -5,10 +5,13 @@ > > """Upgrade a launchpad-buildd configuration file.""" > > +import os > import re > import sys > import subprocess > > +from ConfigParser import SafeConfigParser > + You can drop this again now. > import apt_pkg > > > @@ -167,6 +170,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 +226,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 01:58:50 +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 01:58:50 +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 01:58:50 +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 01:58:50 +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 01:58:50 +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 01:58:50 +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 01:58:50 +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 01:58:50 +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 01:58:50 > +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'), '[email protected]', > '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 > 01:58:50 +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 01:58:50 +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 01:58:50 +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 > -- https://code.launchpad.net/~wgrant/launchpad-buildd/cut-down-config/+merge/258734 Your team Launchpad code reviewers is subscribed to branch lp:launchpad-buildd. _______________________________________________ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : [email protected] Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
