From 1d00cb3e8c41a62542b7c06219e2cc1d8d4a7369 Mon Sep 17 00:00:00 2001 From: Srinu P Date: Mon, 6 May 2013 11:42:11 +0530 Subject: [PATCH 1/4] Fix for the bug #2222. Git source checkout steps support reference repositories --- master/buildbot/steps/source/git.py | 17 ++++++++----- .../test/unit/test_steps_source_git.py | 25 +++++++++++++++++++ master/docs/manual/cfg-buildsteps.rst | 5 ++++ master/docs/relnotes/index.rst | 2 ++ 4 files changed, 43 insertions(+), 6 deletions(-) diff --git a/master/buildbot/steps/source/git.py b/master/buildbot/steps/source/git.py index 6a53f49ac9e..54b6550c064 100644 --- a/master/buildbot/steps/source/git.py +++ b/master/buildbot/steps/source/git.py @@ -57,12 +57,11 @@ def isTrueOrIsExactlyZero(v): class Git(Source): """ Class for Git with all the smarts """ name='git' - renderables = [ "repourl"] + renderables = [ "repourl", "reference"] - def __init__(self, repourl=None, branch='HEAD', mode='incremental', - method=None, submodules=False, shallow=False, progress=False, - retryFetch=False, clobberOnFailure=False, getDescription=False, - config=None, **kwargs): + def __init__(self, repourl=None, branch='HEAD', mode='incremental', method=None, + reference=None, submodules=False, shallow=False, progress=False, retryFetch=False, + clobberOnFailure=False, getDescription=False, config=None, **kwargs): """ @type repourl: string @param repourl: the URL which points at the git repository @@ -83,10 +82,14 @@ def __init__(self, repourl=None, branch='HEAD', mode='incremental', @param method: Full builds can be done is different ways. This parameter specifies which method to use. + @type reference: string + @param reference: If available use a reference repo. + Uses `--reference` in git command. Refer `git clone --help` @type progress: boolean @param progress: Pass the --progress option when fetching. This can solve long fetches getting killed due to lack of output, but requires Git 1.7.2+. + @type shallow: boolean @param shallow: Use a shallow or clone, if possible @@ -106,6 +109,7 @@ def __init__(self, repourl=None, branch='HEAD', mode='incremental', self.method = method self.prog = progress self.repourl = repourl + self.reference = reference self.retryFetch = retryFetch self.submodules = submodules self.shallow = shallow @@ -381,9 +385,10 @@ def _fullClone(self, shallowClone=False): args += ['--branch', self.branch] if shallowClone: args += ['--depth', '1'] + if self.reference: + args += ['--reference', self.reference] command = ['clone'] + args + [self.repourl, '.'] - #Fix references if self.prog: command.append('--progress') diff --git a/master/buildbot/test/unit/test_steps_source_git.py b/master/buildbot/test/unit/test_steps_source_git.py index 4f60813dd78..d68c8cc6be1 100644 --- a/master/buildbot/test/unit/test_steps_source_git.py +++ b/master/buildbot/test/unit/test_steps_source_git.py @@ -290,6 +290,31 @@ def test_mode_full_clean_no_existing_repo(self): self.expectOutcome(result=SUCCESS, status_text=["update"]) return self.runStep() + def test_mode_full_clean_no_existing_repo_with_reference(self): + self.setupStep( + git.Git(repourl='http://github.com/buildbot/buildbot.git', + mode='full', method='clean', reference='path/to/reference/repo')) + self.expectCommands( + ExpectShell(workdir='wkdir', + command=['git', '--version']) + + 0, + + Expect('stat', dict(file='wkdir/.git', + logEnviron=True)) + + 1, + ExpectShell(workdir='wkdir', + command=['git', 'clone', '--reference', 'path/to/reference/repo', + 'http://github.com/buildbot/buildbot.git', '.']) + + 0, + ExpectShell(workdir='wkdir', + command=['git', 'rev-parse', 'HEAD']) + + ExpectShell.log('stdio', + stdout='f6ad368298bd941e934a41f3babc827b2aa95a1d') + + 0, + ) + self.expectOutcome(result=SUCCESS, status_text=["update"]) + return self.runStep() + def test_mode_full_clean_no_existing_repo_branch(self): self.setupStep( git.Git(repourl='http://github.com/buildbot/buildbot.git', diff --git a/master/docs/manual/cfg-buildsteps.rst b/master/docs/manual/cfg-buildsteps.rst index 4dbd1dd33ec..b5f78aa4429 100644 --- a/master/docs/manual/cfg-buildsteps.rst +++ b/master/docs/manual/cfg-buildsteps.rst @@ -348,6 +348,11 @@ The Git step takes the following arguments: (optional): instructs git to attempt shallow clones (``--depth 1``). This option can be used only in full builds with clobber method. +``reference`` + (optional): use the specified string as a path to a reference + repository on the local machine. Git will try to grab objects from + this path first instead of the main repository, if they exist. + ``progress`` (optional): passes the (``--progress``) flag to (:command:`git fetch`). This solves issues of long fetches being killed due to diff --git a/master/docs/relnotes/index.rst b/master/docs/relnotes/index.rst index aa947112f44..8fd94fe6aa8 100644 --- a/master/docs/relnotes/index.rst +++ b/master/docs/relnotes/index.rst @@ -18,6 +18,8 @@ Features * Builder configurations can now include a ``description``, which will appear in the web UI to help humans figure out what the builder does. +* Git source checkout step now supports reference repositories. + Deprecations, Removals, and Non-Compatible Changes ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ From fd40f435f181b5aabaa9e7ce57d8b69aa275f284 Mon Sep 17 00:00:00 2001 From: "Dustin J. Mitchell" Date: Tue, 7 May 2013 16:58:55 -0400 Subject: [PATCH 2/4] raise a better exception if the Mercurial revision doesn't have the right shape --- master/buildbot/clients/tryclient.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/master/buildbot/clients/tryclient.py b/master/buildbot/clients/tryclient.py index cfc7e46f788..a7c7aedf461 100644 --- a/master/buildbot/clients/tryclient.py +++ b/master/buildbot/clients/tryclient.py @@ -193,7 +193,9 @@ def getBaseRevision(self): def parseStatus(self, output): m = re.search(r'^(\w+)', output) - self.baserev = m.group(0) + if not m: + raise RuntimeError("Revision %r is not in the right format" % (output,)) + self.baserev = m.group(0) def getPatch(self, res): d = self.dovc(["diff", "-r", self.baserev]) From 6c991812b8428591f7753ff12d5bb770a8e85ff0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Fleschenberg?= Date: Wed, 8 May 2013 00:49:51 +0200 Subject: [PATCH 3/4] Try client: properly call ssh when no username was supplied --- master/buildbot/clients/tryclient.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/master/buildbot/clients/tryclient.py b/master/buildbot/clients/tryclient.py index a7c7aedf461..8e5b5b90b5d 100644 --- a/master/buildbot/clients/tryclient.py +++ b/master/buildbot/clients/tryclient.py @@ -609,8 +609,12 @@ def deliverJob(self): tryuser = self.getopt("username") trydir = self.getopt("jobdir") buildbotbin = self.getopt("buildbotbin") - argv = ["ssh", "-l", tryuser, tryhost, - buildbotbin, "tryserver", "--jobdir", trydir] + if tryuser: + argv = ["ssh", "-l", tryuser, tryhost, + buildbotbin, "tryserver", "--jobdir", trydir] + else: + argv = ["ssh", tryhost, + buildbotbin, "tryserver", "--jobdir", trydir] pp = RemoteTryPP(self.jobfile) reactor.spawnProcess(pp, argv[0], argv, os.environ) d = pp.d From 099b3f179451bac102fea824dd6c953013df1f63 Mon Sep 17 00:00:00 2001 From: Jared Grubb Date: Sun, 5 May 2013 17:48:01 -0700 Subject: [PATCH 4/4] BuildSlave: add some unit tests for 'attached' --- master/buildbot/buildslave/__init__.py | 10 +- master/buildbot/test/fake/botmaster.py | 8 + master/buildbot/test/fake/fakemaster.py | 3 + master/buildbot/test/unit/test_buildslave.py | 146 ++++++++++++++++++- 4 files changed, 161 insertions(+), 6 deletions(-) diff --git a/master/buildbot/buildslave/__init__.py b/master/buildbot/buildslave/__init__.py index 6d81a590e28..7b9d2bf0c44 100644 --- a/master/buildbot/buildslave/__init__.py +++ b/master/buildbot/buildslave/__init__.py @@ -306,7 +306,7 @@ def _missing_timer_fired(self): if not self.parent: return - buildmaster = self.botmaster.master + buildmaster = self.master status = buildmaster.getStatus() text = "The Buildbot working for '%s'\n" % status.getTitle() text += ("has noticed that the buildslave named %s went away\n" % @@ -445,7 +445,7 @@ def _accept_slave(res): log.msg("bot attached") self.messageReceivedFromSlave() self.stopMissingTimer() - self.botmaster.master.status.slaveConnected(self.slavename) + self.master.status.slaveConnected(self.slavename) return self.updateSlave() d.addCallback(_accept_slave) @@ -469,7 +469,7 @@ def detached(self, mind): self.slave_status.removeGracefulWatcher(self._gracefulChanged) self.slave_status.setConnected(False) log.msg("BuildSlave.detached(%s)" % self.slavename) - self.botmaster.master.status.slaveDisconnected(self.slavename) + self.master.status.slaveDisconnected(self.slavename) self.stopKeepaliveTimer() self.releaseLocks() @@ -616,7 +616,7 @@ def canStartBuild(self): def _mail_missing_message(self, subject, text): # first, see if we have a MailNotifier we can use. This gives us a # fromaddr and a relayhost. - buildmaster = self.botmaster.master + buildmaster = self.master for st in buildmaster.status: if isinstance(st, MailNotifier): break @@ -869,7 +869,7 @@ def _substantiation_failed(self, failure): if not self.parent or not self.notify_on_missing: return - buildmaster = self.botmaster.master + buildmaster = self.master status = buildmaster.getStatus() text = "The Buildbot working for '%s'\n" % status.getTitle() text += ("has noticed that the latent buildslave named %s \n" % diff --git a/master/buildbot/test/fake/botmaster.py b/master/buildbot/test/fake/botmaster.py index f04a2a793ef..3a2d39de6d0 100644 --- a/master/buildbot/test/fake/botmaster.py +++ b/master/buildbot/test/fake/botmaster.py @@ -21,6 +21,8 @@ def __init__(self, master): self.setName("fake-botmaster") self.master = master self.locks = {} + self.builders = {} + self.buildsStartedForSlaves = [] def getLockByID(self, lockid): if not lockid in self.locks: @@ -33,3 +35,9 @@ def getLockByID(self, lockid): def getLockFromLockAccess(self, access): return self.getLockByID(access.lockid) + + def getBuildersForSlave(self, slavename): + return self.builders.get(slavename, []) + + def maybeStartBuildsForSlave(self, slavename): + self.buildsStartedForSlaves.append(slavename) \ No newline at end of file diff --git a/master/buildbot/test/fake/fakemaster.py b/master/buildbot/test/fake/fakemaster.py index 0187bedc04f..c5aaa8a4b88 100644 --- a/master/buildbot/test/fake/fakemaster.py +++ b/master/buildbot/test/fake/fakemaster.py @@ -49,6 +49,9 @@ class FakeStatus(object): def builderAdded(self, name, basedir, category=None, description=None): return FakeBuilderStatus() + def slaveConnected(self, name): + pass + class FakeBuilderStatus(object): diff --git a/master/buildbot/test/unit/test_buildslave.py b/master/buildbot/test/unit/test_buildslave.py index 92bc1e6c90f..3ec01639d9a 100644 --- a/master/buildbot/test/unit/test_buildslave.py +++ b/master/buildbot/test/unit/test_buildslave.py @@ -15,7 +15,7 @@ import mock from twisted.trial import unittest -from twisted.internet import defer +from twisted.internet import defer, task, reactor from buildbot import buildslave, config, locks from buildbot.test.fake import fakemaster, pbmanager from buildbot.test.fake.botmaster import FakeBotMaster @@ -25,6 +25,20 @@ class TestAbstractBuildSlave(unittest.TestCase): class ConcreteBuildSlave(buildslave.AbstractBuildSlave): pass + def setUp(self): + self.master = fakemaster.make_master(wantDb=True, testcase=self) + self.botmaster = FakeBotMaster(self.master) + + self.clock = task.Clock() + self.patch(reactor, 'callLater', self.clock.callLater) + self.patch(reactor, 'seconds', self.clock.seconds) + + def createBuildslave(self, name='bot', password='pass', **kwargs): + slave = self.ConcreteBuildSlave(name, password, **kwargs) + slave.master = self.master + slave.botmaster = self.botmaster + return slave + def test_constructor_minimal(self): bs = self.ConcreteBuildSlave('bot', 'pass') self.assertEqual(bs.slavename, 'bot') @@ -225,3 +239,133 @@ def test_setServiceParent_slaveLocks(self): lock = locks.SlaveLock('lock') bs = self.ConcreteBuildSlave('bot', 'pass', locks = [lock.access("counting")]) bs.setServiceParent(botmaster) + + @defer.inlineCallbacks + def test_startService_getSlaveInfo_empty(self): + slave = self.createBuildslave() + yield slave.startService() + + self.assertEqual(slave.slave_status.getAdmin(), None) + self.assertEqual(slave.slave_status.getHost(), None) + self.assertEqual(slave.slave_status.getAccessURI(), None) + self.assertEqual(slave.slave_status.getVersion(), None) + + def createRemoteBot(self): + class Bot(): + def __init__(self): + self.commands = [] + self.response = { + 'getSlaveInfo': mock.Mock(return_value=defer.succeed({})) + } + + def callRemote(self, command, *args): + self.commands.append((command,) + args) + response = self.response.get(command) + if response: + return response(*args) + return defer.succeed(None) + + return Bot() + + @defer.inlineCallbacks + def test_attached_checkRemoteCalls(self): + slave = self.createBuildslave() + yield slave.startService() + + bot = self.createRemoteBot() + yield slave.attached(bot) + + self.assertEqual(True, slave.slave_status.isConnected()) + self.assertEqual(5, len(bot.commands)) + self.assertEqual(bot.commands[0], ('print', 'attached')) + self.assertEqual(bot.commands[1], ('getSlaveInfo',)) + self.assertEqual(bot.commands[2], ('getVersion',)) + self.assertEqual(bot.commands[3], ('getCommands',)) + self.assertEqual(bot.commands[4], ('setBuilderList',[])) + + @defer.inlineCallbacks + def test_attached_callRemote_print_raises(self): + slave = self.createBuildslave() + yield slave.startService() + + bot = self.createRemoteBot() + bot.response['print'] = mock.Mock(return_value=defer.fail(ValueError())) + yield slave.attached(bot) + + # just check that things still go on + self.assertEqual(True, slave.slave_status.isConnected()) + self.assertEqual(5, len(bot.commands)) + + @defer.inlineCallbacks + def test_attached_callRemote_getSlaveInfo(self): + slave = self.createBuildslave() + yield slave.startService() + + ENVIRON = {} + + bot = self.createRemoteBot() + bot.response['getSlaveInfo'] = mock.Mock(return_value=defer.succeed({ + 'admin': 'TheAdmin', + 'host': 'TheHost', + 'access_uri': 'TheURI', + 'environ': ENVIRON, + 'basedir': 'TheBaseDir', + 'system': 'TheSlaveSystem' + })) + yield slave.attached(bot) + + # check that things were all good + self.assertEqual(True, slave.slave_status.isConnected()) + self.assertEqual(5, len(bot.commands)) + + # check the values get set right + self.assertEqual(slave.slave_status.getAdmin(), "TheAdmin") + self.assertEqual(slave.slave_status.getHost(), "TheHost") + self.assertEqual(slave.slave_status.getAccessURI(), "TheURI") + self.assertEqual(slave.slave_environ, ENVIRON) + self.assertEqual(slave.slave_basedir, 'TheBaseDir') + self.assertEqual(slave.slave_system, 'TheSlaveSystem') + + @defer.inlineCallbacks + def test_attached_callRemote_getVersion(self): + slave = self.createBuildslave() + yield slave.startService() + + bot = self.createRemoteBot() + bot.response['getVersion'] = mock.Mock(return_value=defer.succeed("TheVersion")) + yield slave.attached(bot) + + # check that things were all good + self.assertEqual(True, slave.slave_status.isConnected()) + self.assertEqual(5, len(bot.commands)) + + # check the values get set right + self.assertEqual(slave.slave_status.getVersion(), "TheVersion") + + @defer.inlineCallbacks + def test_attached_callRemote_getCommands(self): + slave = self.createBuildslave() + yield slave.startService() + + COMMANDS = ['a','b'] + + bot = self.createRemoteBot() + bot.response['getCommands'] = mock.Mock(return_value=defer.succeed(COMMANDS)) + yield slave.attached(bot) + + # check that things were all good + self.assertEqual(True, slave.slave_status.isConnected()) + self.assertEqual(5, len(bot.commands)) + + # check the values get set right + self.assertEqual(slave.slave_commands, COMMANDS) + + @defer.inlineCallbacks + def test_attached_callsMaybeStartBuildsForSlave(self): + slave = self.createBuildslave() + yield slave.startService() + + bot = self.createRemoteBot() + yield slave.attached(bot) + + self.assertEqual(self.botmaster.buildsStartedForSlaves, ["bot"])