diff --git a/master/buildbot/buildslave/libvirt.py b/master/buildbot/buildslave/libvirt.py index e4133147575..5776f10dbc6 100644 --- a/master/buildbot/buildslave/libvirt.py +++ b/master/buildbot/buildslave/libvirt.py @@ -14,6 +14,7 @@ # Portions Copyright Buildbot Team Members # Portions Copyright 2010 Isotoma Limited +from __future__ import absolute_import import os from twisted.internet import defer, utils, threads diff --git a/master/buildbot/clients/tryclient.py b/master/buildbot/clients/tryclient.py index 405b3060fd0..34620327747 100644 --- a/master/buildbot/clients/tryclient.py +++ b/master/buildbot/clients/tryclient.py @@ -867,13 +867,7 @@ def getAvailableBuilderNames(self): "unknown connecttype '%s', should be 'pb'" % self.connect) def _getBuilderNames(self, remote, output): - # Older schedulers won't support the properties argument, so only - # attempt to send them when necessary. - properties = self.config.get('properties', {}) - if properties: - d = remote.callRemote("getAvailableBuilderNames", properties) - else: - d = remote.callRemote("getAvailableBuilderNames") + d = remote.callRemote("getAvailableBuilderNames") d.addCallback(self._getBuilderNames2) return d diff --git a/master/buildbot/process/buildrequestdistributor.py b/master/buildbot/process/buildrequestdistributor.py index 50bac814f95..93c3b1e621a 100644 --- a/master/buildbot/process/buildrequestdistributor.py +++ b/master/buildbot/process/buildrequestdistributor.py @@ -462,8 +462,7 @@ def _sortBuilders(self, buildernames): builders = yield defer.maybeDeferred(lambda : sorter(self.master, builders)) except Exception: - log.msg("Exception prioritizing builders; order unspecified") - log.err(Failure()) + log.err(Failure(), "prioritizing builders; order unspecified") # and return the names rv = [ b.name for b in builders ] @@ -492,9 +491,9 @@ def _activityLoop(self): bldr_name = self._pending_builders.pop(0) self.pending_builders_lock.release() + # get the actual builder object + bldr = self.botmaster.builders.get(bldr_name) try: - # get the actual builder object - bldr = self.botmaster.builders.get(bldr_name) if bldr: yield self._maybeStartBuildsOnBuilder(bldr) except Exception: diff --git a/master/buildbot/process/buildstep.py b/master/buildbot/process/buildstep.py index c8503696c0e..5f1cb984d87 100644 --- a/master/buildbot/process/buildstep.py +++ b/master/buildbot/process/buildstep.py @@ -30,7 +30,6 @@ EXCEPTION, RETRY, worst_status from buildbot.process import metrics, properties from buildbot.util.eventual import eventually -from buildbot.interfaces import BuildSlaveTooOldError class BuildStepFailed(Exception): pass @@ -362,8 +361,7 @@ def __init__(self, workdir, command, env=None, usePTY="slave-config", logEnviron=True, collectStdout=False,collectStderr=False, interruptSignal=None, - initialStdin=None, decodeRC={0:SUCCESS}, - user=None): + initialStdin=None, decodeRC={0:SUCCESS}): self.command = command # stash .command, set it later self.fake_command = [w[2] if (isinstance(w, tuple) and len(w) == 3 and w[0] =='obfuscated') @@ -386,8 +384,6 @@ def __init__(self, workdir, command, env=None, } if interruptSignal is not None: args['interruptSignal'] = interruptSignal - if user is not None: - args['user'] = user RemoteCommand.__init__(self, "shell", args, collectStdout=collectStdout, collectStderr=collectStderr, decodeRC=decodeRC) @@ -399,10 +395,6 @@ def _start(self): # fixup themselves if self.step.slaveVersion("shell", "old") == "old": self.args['dir'] = self.args['workdir'] - if ('user' in self.args and - self.step.slaveVersionIsOlderThan("shell", "2.16")): - m = "slave does not support the 'user' parameter" - raise BuildSlaveTooOldError(m) what = "command '%s' in dir '%s'" % (self.fake_command, self.args['workdir']) log.msg(what) diff --git a/master/buildbot/process/factory.py b/master/buildbot/process/factory.py index ef2cd6ab0bd..5578a0332fb 100644 --- a/master/buildbot/process/factory.py +++ b/master/buildbot/process/factory.py @@ -13,23 +13,22 @@ # # Copyright Buildbot Team Members +import warnings +from twisted.python import deprecate, versions + from buildbot import interfaces, util from buildbot.process.build import Build +from buildbot.process.buildstep import BuildStep from buildbot.steps.source import CVS, SVN from buildbot.steps.shell import Configure, Compile, Test, PerlModuleTest -class ArgumentsInTheWrongPlace(Exception): - """When calling BuildFactory.addStep(stepinstance), addStep() only takes - one argument. You passed extra arguments to addStep(), which you probably - intended to pass to your BuildStep constructor instead. For example, you - should do:: - - f.addStep(ShellCommand(command=['echo','stuff'], haltOnFailure=True)) +# deprecated, use BuildFactory.addStep +@deprecate.deprecated(versions.Version("buildbot", 0, 8, 6)) +def s(steptype, **kwargs): + # convenience function for master.cfg files, to create step + # specification tuples + return interfaces.IBuildStepFactory(steptype(**kwargs)) - instead of:: - - f.addStep(ShellCommand(command=['echo','stuff']), haltOnFailure=True) - """ class BuildFactory(util.ComparableMixin): """ @@ -58,7 +57,13 @@ def newBuild(self, requests): b.setStepFactories(self.steps) return b - def addStep(self, step): + def addStep(self, step, **kwargs): + if kwargs or (type(step) == type(BuildStep) and issubclass(step, BuildStep)): + warnings.warn( + "Passing a BuildStep subclass to factory.addStep is " + "deprecated. Please pass a BuildStep instance instead.", + DeprecationWarning, stacklevel=2) + step = step(**kwargs) self.steps.append(interfaces.IBuildStepFactory(step)) def addSteps(self, steps): diff --git a/master/buildbot/schedulers/trysched.py b/master/buildbot/schedulers/trysched.py index f5630a60048..c6f8eb2c3f9 100644 --- a/master/buildbot/schedulers/trysched.py +++ b/master/buildbot/schedulers/trysched.py @@ -264,7 +264,7 @@ def perspective_try(self, branch, revision, patch, repository, project, from buildbot.status.client import makeRemote defer.returnValue(makeRemote(bss)) - def perspective_getAvailableBuilderNames(self, properties={}): + def perspective_getAvailableBuilderNames(self): # Return a list of builder names that are configured # for the try service # This is mostly intended for integrating try services diff --git a/master/buildbot/scripts/base.py b/master/buildbot/scripts/base.py index 4379d601876..2e4d67ad8a7 100644 --- a/master/buildbot/scripts/base.py +++ b/master/buildbot/scripts/base.py @@ -43,8 +43,8 @@ def getConfigFileFromTac(basedir): # execute the .tac file to see if its configfile location exists tacFile = os.path.join(basedir, 'buildbot.tac') if os.path.exists(tacFile): - # don't mess with the global namespace - tacGlobals = {} + # don't mess with the global namespace, but set __file__ for relocatable buildmasters + tacGlobals = {'__file__' : tacFile} execfile(tacFile, tacGlobals) return tacGlobals.get("configfile", "master.cfg") else: diff --git a/master/buildbot/status/status_gerrit.py b/master/buildbot/status/status_gerrit.py index 1010df5b76e..b4925ccf3a0 100644 --- a/master/buildbot/status/status_gerrit.py +++ b/master/buildbot/status/status_gerrit.py @@ -93,7 +93,7 @@ def buildStarted(self, builderName, build): def buildFinished(self, builderName, build, result): """Do the SSH gerrit verify command to the server.""" - message, verified, reviewed = self.reviewCB(builderName, build, result, self.reviewArg) + message, verified, reviewed = self.reviewCB(builderName, build, result, self.status, self.reviewArg) self.sendCodeReviews(build, message, verified, reviewed) def sendCodeReviews(self, build, message, verified=0, reviewed=0): diff --git a/master/buildbot/status/web/base.py b/master/buildbot/status/web/base.py index 8a268f24470..05ef9d6b07d 100644 --- a/master/buildbot/status/web/base.py +++ b/master/buildbot/status/web/base.py @@ -110,16 +110,16 @@ def build_get_class(b): return builder.Results[result] def path_to_root(request): - # /waterfall : ['waterfall'] -> '/' + # /waterfall : ['waterfall'] -> './' # /somewhere/lower : ['somewhere', 'lower'] -> '../' # /somewhere/indexy/ : ['somewhere', 'indexy', ''] -> '../../' - # / : [] -> '/' + # / : [] -> './' if request.prepath: segs = len(request.prepath) - 1 else: segs = 0 - root = "../" * segs - return root if len(root) > 0 else "/" + root = "../" * segs if segs else './' + return root def path_to_authfail(request): return path_to_root(request) + "authfail" diff --git a/master/buildbot/status/web/baseweb.py b/master/buildbot/status/web/baseweb.py index a94a1c09e2d..1328de4db99 100644 --- a/master/buildbot/status/web/baseweb.py +++ b/master/buildbot/status/web/baseweb.py @@ -46,8 +46,10 @@ from buildbot.status.web.users import UsersResource from buildbot.status.web.change_hook import ChangeHookResource from twisted.cred.portal import IRealm, Portal +from twisted.cred import strcred +from twisted.cred.checkers import ICredentialsChecker +from twisted.cred.credentials import IUsernamePassword from twisted.web import resource, guard -from twisted.cred.checkers import InMemoryUsernamePasswordDatabaseDontUse # this class contains the WebStatus class. Basic utilities are in base.py, # and specific pages are each in their own module. @@ -322,9 +324,25 @@ def __init__(self, http_port=None, distrib_port=None, allowForce=None, # check for correctness of HTTP auth parameters if change_hook_auth is not None: - if not isinstance(change_hook_auth, tuple) or len(change_hook_auth) != 2: - config.error("Invalid credentials for change_hook auth") - self.change_hook_auth = change_hook_auth + self.change_hook_auth = [] + for checker in change_hook_auth: + if isinstance(checker, str): + try: + checker = strcred.makeChecker(checker) + except Exception, error: + config.error("Invalid change_hook checker description: %s" % (error,)) + continue + elif not ICredentialsChecker.providedBy(checker): + config.error("change_hook checker doesn't provide ICredentialChecker: %r" % (checker,)) + continue + + if IUsernamePassword not in checker.credentialInterfaces: + config.error("change_hook checker doesn't support IUsernamePassword: %r" % (checker,)) + continue + + self.change_hook_auth.append(checker) + else: + self.change_hook_auth = None self.orderConsoleByTime = order_console_by_time @@ -359,7 +377,8 @@ def __init__(self, http_port=None, distrib_port=None, allowForce=None, self.change_hook_dialects = change_hook_dialects resource_obj = ChangeHookResource(dialects=self.change_hook_dialects) if self.change_hook_auth is not None: - resource_obj = self.setupProtectedResource(resource_obj) + resource_obj = self.setupProtectedResource( + resource_obj, self.change_hook_auth) self.putChild("change_hook", resource_obj) # Set default feeds @@ -370,7 +389,7 @@ def __init__(self, http_port=None, distrib_port=None, allowForce=None, self.jinja_loaders = jinja_loaders - def setupProtectedResource(self, resource_obj): + def setupProtectedResource(self, resource_obj, checkers): class SimpleRealm(object): """ A realm which gives out L{ChangeHookResource} instances for authenticated @@ -383,10 +402,7 @@ def requestAvatar(self, avatarId, mind, *interfaces): return (resource.IResource, resource_obj, lambda: None) raise NotImplementedError() - login, password = self.change_hook_auth - checker = InMemoryUsernamePasswordDatabaseDontUse() - checker.addUser(login, password) - portal = Portal(SimpleRealm(), [checker]) + portal = Portal(SimpleRealm(), checkers) credentialFactory = guard.BasicCredentialFactory('Protected area') wrapper = guard.HTTPAuthSessionWrapper(portal, [credentialFactory]) return wrapper diff --git a/master/buildbot/status/web/feeds.py b/master/buildbot/status/web/feeds.py index 1bbdcbda79e..f29a11cad94 100644 --- a/master/buildbot/status/web/feeds.py +++ b/master/buildbot/status/web/feeds.py @@ -66,7 +66,7 @@ def render(self, request): def rfc822_time(tstamp): res = time.strftime("%%s, %d %%s %Y %H:%M:%S GMT", tstamp) - res = res % (tstamp.tm_wday, tstamp.tm_mon) + res = res % (_abbr_day[tstamp.tm_wday], _abbr_mon[tstamp.tm_mon]) return res class FeedResource(XmlResource): diff --git a/master/buildbot/steps/source/p4.py b/master/buildbot/steps/source/p4.py index 6bbea089877..b6d47daf130 100644 --- a/master/buildbot/steps/source/p4.py +++ b/master/buildbot/steps/source/p4.py @@ -19,12 +19,11 @@ from twisted.python import log from twisted.internet import defer -from buildbot import interfaces +from buildbot import interfaces, config from buildbot.process import buildstep from buildbot.steps.source import Source from buildbot.interfaces import BuildSlaveTooOldError from buildbot.process.properties import Interpolate -from buildbot.config import ConfigErrors from types import StringType @@ -72,30 +71,26 @@ def __init__(self, mode='incremental', Source.__init__(self, **kwargs) - errors = [] if self.mode not in self.possible_modes: - errors.append("mode %s is not one of %s" % (self.mode, self.possible_modes)) + config.error("mode %s is not one of %s" % (self.mode, self.possible_modes)) if not p4viewspec and p4base is None: - errors.append("You must provide p4base or p4viewspec") + config.error("You must provide p4base or p4viewspec") if p4viewspec and (p4base or p4branch or p4extra_views): - errors.append("Either provide p4viewspec or p4base and p4branch (and optionally p4extra_views") + config.error("Either provide p4viewspec or p4base and p4branch (and optionally p4extra_views") if p4viewspec and type(p4viewspec) is StringType: - errors.append("p4viewspec must not be a string, and should be a sequence of 2 element sequences") + config.error("p4viewspec must not be a string, and should be a sequence of 2 element sequences") if not interfaces.IRenderable.providedBy(p4base) and p4base and p4base.endswith('/'): - errors.append('p4base should not end with a trailing / [p4base = %s]' % p4base) + config.error('p4base should not end with a trailing / [p4base = %s]' % p4base) if not interfaces.IRenderable.providedBy(p4branch) and p4branch and p4branch.endswith('/'): - errors.append('p4branch should not end with a trailing / [p4branch = %s]' % p4branch) + config.error('p4branch should not end with a trailing / [p4branch = %s]' % p4branch) if (p4branch or p4extra_views) and not p4base: - errors.append('If you specify either p4branch or p4extra_views you must also specify p4base') - - if errors: - raise ConfigErrors(errors) + config.error('If you specify either p4branch or p4extra_views you must also specify p4base') def startVC(self, branch, revision, patch): if debug_logging: @@ -103,7 +98,7 @@ def startVC(self, branch, revision, patch): self.revision = revision self.method = self._getMethod() - self.stdio_log = self.addLog("stdio") + self.stdio_log = self.addLogForRemoteCommands("stdio") d = self.checkP4() @@ -135,10 +130,7 @@ def full(self, _): yield self._dovccmd(['sync', '#none']) # Then remove directory. - # NOTE: Not using CompositeStepMixin's runRmdir() as it requires self.rc_log - # to be defined and ran into issues where setting that in _dovccmd would - # yield multiple logs named 'stdio' in the waterfall report.. - yield self._rmdir(self.workdir) + yield self.runRmdir(self.workdir) # Then we need to sync the client if self.revision: @@ -360,13 +352,3 @@ def computeSourceRevision(self, changes): return None lastChange = max([int(c.revision) for c in changes]) return lastChange - - @defer.inlineCallbacks - def _rmdir(self, dir): - cmd = buildstep.RemoteCommand('rmdir', - {'dir': dir, - 'logEnviron': self.logEnviron}) - cmd.useLog(self.stdio_log, False) - yield self.runCommand(cmd) - if cmd.rc != 0: - raise buildstep.BuildStepFailed() diff --git a/master/buildbot/steps/source/repo.py b/master/buildbot/steps/source/repo.py index 29dd41a5be1..805bca4265e 100644 --- a/master/buildbot/steps/source/repo.py +++ b/master/buildbot/steps/source/repo.py @@ -18,29 +18,82 @@ from twisted.internet import defer, reactor +from buildbot import util from buildbot.process import buildstep from buildbot.steps.source.base import Source +from buildbot.interfaces import IRenderable +from zope.interface import implements -def default_sync_all_branches(properties): - # in case of manifest_override, we have no other choice to download all branches - # each project can indeed point on arbitrary commit - return properties.getProperty("manifest_override_url","")!="" -def default_update_tarball(properties,sync_all_branch_done): - # dont create a too big tarball in case we synched all branches - if not sync_all_branch_done: - return 7*24.0*3600.0 - return None +class RepoDownloadsFromProperties(util.ComparableMixin, object): + implements(IRenderable) + parse_download_re = (re.compile(r"repo download ([^ ]+) ([0-9]+/[0-9]+)"), + re.compile(r"([^ ]+) ([0-9]+/[0-9]+)"), + re.compile(r"([^ ]+)/([0-9]+/[0-9]+)"), + ) + + compare_attrs = ('names',) + + def __init__(self, names): + self.names = names + + def getRenderingFor(self, props): + downloads = [] + for propName in self.names: + s = props.getProperty(propName) + if s is not None: + downloads.extend(self.parseDownloadProperty(s)) + return downloads + + def parseDownloadProperty(self, s): + """ + lets try to be nice in the format we want + can support several instances of "repo download proj number/patch" (direct copy paste from gerrit web site) + or several instances of "proj number/patch" (simpler version) + This feature allows integrator to build with several pending interdependant changes. + returns list of repo downloads sent to the buildslave + """ + if s is None: + return [] + ret = [] + for cur_re in self.parse_download_re: + res = cur_re.search(s) + while res: + ret.append("%s %s" % (res.group(1), res.group(2))) + s = s[:res.start(0)] + s[res.end(0):] + res = cur_re.search(s) + return ret + + +class RepoDownloadsFromChangeSource(util.ComparableMixin, object): + implements(IRenderable) + compare_attrs = ('codebase',) + + def __init__(self, codebase=None): + self.codebase = codebase + + def getRenderingFor(self, props): + downloads = [] + if self.codebase is None: + changes = props.getBuild().allChanges() + else: + changes = props.getBuild().getSourceStamp(self.codebase).changes + for change in changes: + if ("event.type" in change.properties and + change.properties["event.type"] == "patchset-created"): + downloads.append("%s %s/%s" % (change.properties["event.change.project"], + change.properties["event.change.number"], + change.properties["event.patchSet.number"])) + return downloads + class Repo(Source): """ Class for Repo with all the smarts """ - name='repo' - renderables = ["manifest_url","manifest_branch","manifest_file", "tarball", "jobs"] + name = 'repo' + renderables = ["manifestURL", "manifestFile", "tarball", "jobs", + "syncAllBranches", "updateTarballAge", "manifestOverrideUrl", + "repoDownloads"] - parse_download_re = (re.compile(r"repo download ([^ ]+) ([0-9]+/[0-9]+)"), - re.compile(r"([^ ]+) ([0-9]+/[0-9]+)"), - re.compile(r"([^ ]+)/([0-9]+/[0-9]+)"), - ) ref_not_found_re = re.compile(r"fatal: Couldn't find remote ref") cherry_pick_error_re = re.compile(r"|".join([r"Automatic cherry-pick failed", r"error: " @@ -48,107 +101,67 @@ class Repo(Source): r"possibly due to conflict resolution."])) re_change = re.compile(r".* refs/changes/\d\d/(\d+)/(\d+) -> FETCH_HEAD$") re_head = re.compile(r"^HEAD is now at ([0-9a-f]+)...") - mirror_sync_retry = 10 # number of retries, if we detect mirror desynchronization - mirror_sync_sleep = 60 # wait 1min between retries (thus default total retry time is 10min) + mirror_sync_retry = 10 # number of retries, if we detect mirror desynchronization + mirror_sync_sleep = 60 # wait 1min between retries (thus default total retry time is 10min) + def __init__(self, - manifest_url=None, - manifest_branch="master", - manifest_file="default.xml", + manifestURL=None, + manifestBranch="master", + manifestFile="default.xml", tarball=None, jobs=None, - sync_all_branches=default_sync_all_branches, - update_tarball=default_update_tarball, + syncAllBranches=False, + updateTarballAge=7*24.0*3600.0, + manifestOverrideUrl=None, + repoDownloads=None, **kwargs): """ - @type manifest_url: string - @param manifest_url: The URL which points at the repo manifests repository. + @type manifestURL: string + @param manifestURL: The URL which points at the repo manifests repository. - @type manifest_branch: string - @param manifest_branch: The manifest branch to check out by default. + @type manifestBranch: string + @param manifestBranch: The manifest branch to check out by default. - @type manifest_file: string - @param manifest_file: The manifest to use for sync. + @type manifestFile: string + @param manifestFile: The manifest to use for sync. - @type sync_all_branches: lambda properties: bool. - @param sync_all_branches: returns the boolean we must synchronize all branches. + @type syncAllBranches: bool. + @param syncAllBranches: true, then we must slowly synchronize all branches. - @type update_tarball: lambda (properties,bool) : float - @param update_tarball: function to determine the update tarball policy, - given properties, and boolean indicating whether - the last repo sync was on all branches + @type updateTarballAge: float + @param updateTarballAge: renderable to determine the update tarball policy, + given properties Returns: max age of tarball in seconds, or None, if we want to skip tarball update + @type manifestOverrideUrl: string + @param manifestOverrideUrl: optional http URL for overriding the manifest + usually coming from Property setup by a ForceScheduler + + @type repoDownloads: list of strings + @param repoDownloads: optional repo download to perform after the repo sync + """ - self.manifest_url = manifest_url - self.manifest_branch = manifest_branch - self.manifest_file = manifest_file + self.manifestURL = manifestURL + self.manifestBranch = manifestBranch + self.manifestFile = manifestFile self.tarball = tarball self.jobs = jobs - def copy_callable(param_name,f): - if not callable(f): - raise ValueError("%s must be callable,but is of type %s"%(param_name,type(f))) - setattr(self, param_name, f) - copy_callable("sync_all_branches",sync_all_branches) - copy_callable("update_tarball",update_tarball) + self.syncAllBranches = syncAllBranches + self.updateTarballAge = updateTarballAge + self.manifestOverrideUrl = manifestOverrideUrl + if repoDownloads is None: + repoDownloads = [] + self.repoDownloads = repoDownloads Source.__init__(self, **kwargs) - assert self.manifest_url is not None + assert self.manifestURL is not None def computeSourceRevision(self, changes): if not changes: return None return changes[-1].revision - def parseDownloadProperty(self, s): - """ - lets try to be nice in the format we want - can support several instances of "repo download proj number/patch" (direct copy paste from gerrit web site) - or several instances of "proj number/patch" (simpler version) - This feature allows integrator to build with several pending interdependant changes. - returns list of repo downloads sent to the buildslave - """ - if s is None: - return [] - ret = [] - for cur_re in self.parse_download_re: - res = cur_re.search(s) - while res: - ret.append("%s %s" % (res.group(1), res.group(2))) - s = s[:res.start(0)] + s[res.end(0):] - res = cur_re.search(s) - return ret - - def buildDownloadList(self): - """taken the changesource and forcebuild property, - build the repo download command to send to the slave - making this a defereable allow config to tweak this - in order to e.g. manage dependancies - """ - downloads = self.build.getProperty("repo_downloads", []) - - # download patches based on GerritChangeSource events - for change in self.build.allChanges(): - if (change.properties.has_key("event.type") and - change.properties["event.type"] == "patchset-created"): - downloads.append("%s %s/%s"% (change.properties["event.change.project"], - change.properties["event.change.number"], - change.properties["event.patchSet.number"])) - - # download patches based on web site forced build properties: - # "repo_d", "repo_d0", .., "repo_d9" - # "repo_download", "repo_download0", .., "repo_download9" - for propName in ["repo_d"] + ["repo_d%d" % i for i in xrange(0,10)] + \ - ["repo_download"] + ["repo_download%d" % i for i in xrange(0,10)]: - s = self.build.getProperty(propName) - if s is not None: - downloads.extend(self.parseDownloadProperty(s)) - - self.repo_downloads = downloads - if downloads: - self.setProperty("repo_downloads", downloads, "repo step") - return defer.succeed(None) - def filterManifestPatches(self): """ Patches to manifest projects are a bit special. @@ -159,25 +172,25 @@ def filterManifestPatches(self): """ manifest_unrelated_downloads = [] manifest_related_downloads = [] - for download in self.repo_downloads: + for download in self.repoDownloads: project, ch_ps = download.split(" ")[-2:] - if ( self.manifest_url.endswith("/"+project) or - self.manifest_url.endswith("/"+project+".git")): + if (self.manifestURL.endswith("/"+project) or + self.manifestURL.endswith("/"+project+".git")): ch, ps = map(int, ch_ps.split("/")) - branch = "refs/changes/%02d/%d/%d"%(ch%100, ch, ps) + branch = "refs/changes/%02d/%d/%d" % (ch % 100, ch, ps) manifest_related_downloads.append( - ["git", "fetch", self.manifest_url, branch]) + ["git", "fetch", self.manifestURL, branch]) manifest_related_downloads.append( ["git", "cherry-pick", "FETCH_HEAD"]) else: manifest_unrelated_downloads.append(download) - self.repo_downloads = manifest_unrelated_downloads - self.manifest_downloads = manifest_related_downloads + self.repoDownloads = manifest_unrelated_downloads + self.manifestDownloads = manifest_related_downloads def _repoCmd(self, command, abandonOnFailure=True, **kwargs): return self._Cmd(["repo"]+command, abandonOnFailure=abandonOnFailure, **kwargs) - def _Cmd(self, command, abandonOnFailure=True,workdir=None, **kwargs): + def _Cmd(self, command, abandonOnFailure=True, workdir=None, **kwargs): if workdir is None: workdir = self.workdir self.cmd = cmd = buildstep.RemoteShellCommand(workdir, command, @@ -188,18 +201,21 @@ def _Cmd(self, command, abandonOnFailure=True,workdir=None, **kwargs): self.logEnviron = False cmd.useLog(self.stdio_log, False) self.stdio_log.addHeader("Starting command: %s\n" % (" ".join(command), )) - self.step_status.setText(["%s"%(" ".join(command[:2]))]) + self.step_status.setText(["%s" % (" ".join(command[:2]))]) d = self.runCommand(cmd) + def evaluateCommand(cmd): if abandonOnFailure and cmd.didFail(): - self.step_status.setText(["repo failed at: %s"%(" ".join(command[:2]))]) + self.step_status.setText(["repo failed at: %s" % (" ".join(command[:2]))]) self.stdio_log.addStderr("Source step failed while running command %s\n" % cmd) raise buildstep.BuildStepFailed() return cmd.rc d.addCallback(lambda _: evaluateCommand(cmd)) return d + def repoDir(self): return self.build.pathmodule.join(self.workdir, ".repo") + def sourcedirIsUpdateable(self): return self.pathExists(self.repoDir()) @@ -209,35 +225,25 @@ def startVC(self, branch, revision, patch): @defer.inlineCallbacks def doStartVC(self): - self.manifest_override_url = self.build.getProperty("manifest_override_url") self.stdio_log = self.addLogForRemoteCommands("stdio") - # run our setup callbacks from the start, we'll use the results later - properties = self.build.getProperties() - self.will_sync_all_branches = self.sync_all_branches(properties) - if self.update_tarball is not None: - time_to_update = self.update_tarball(properties, - self.will_sync_all_branches) - self.tarball_updating_age = time_to_update - - yield self.buildDownloadList() - self.filterManifestPatches() - if self.repo_downloads: - self.stdio_log.addHeader("will download:\n" + "repo download "+ "\nrepo download ".join(self.repo_downloads) + "\n") + if self.repoDownloads: + self.stdio_log.addHeader("will download:\n" + "repo download " + "\nrepo download ".join(self.repoDownloads) + "\n") self.willRetryInCaseOfFailure = True d = self.doRepoSync() + def maybeRetry(why): # in case the tree was corrupted somehow because of previous build # we clobber one time, and retry everything if why.check(buildstep.BuildStepFailed) and self.willRetryInCaseOfFailure: - self.stdio_log.addStderr("got issue at first try:\n" +str(why)+ + self.stdio_log.addStderr("got issue at first try:\n" + str(why) + "\nRetry after clobber...") return self.doRepoSync(forceClobber=True) - return why # propagate to self.failed + return why # propagate to self.failed d.addErrback(maybeRetry) yield d yield self.maybeUpdateTarball() @@ -262,32 +268,32 @@ def doRepoSync(self, forceClobber=False): yield self.doClobberStart() yield self.doCleanup() yield self._repoCmd(['init', - '-u', self.manifest_url, - '-b', self.manifest_branch, - '-m', self.manifest_file]) + '-u', self.manifestURL, + '-b', self.manifestBranch, + '-m', self.manifestFile]) - if self.manifest_override_url: - self.stdio_log.addHeader("overriding manifest with %s\n" %(self.manifest_override_url)) + if self.manifestOverrideUrl: + self.stdio_log.addHeader("overriding manifest with %s\n" % (self.manifestOverrideUrl)) local_file = yield self.pathExists(self.build.pathmodule.join(self.workdir, - self.manifest_override_url)) + self.manifestOverrideUrl)) if local_file: - yield self._Cmd(["cp", "-f", self.manifest_override_url, "manifest_override.xml"]) + yield self._Cmd(["cp", "-f", self.manifestOverrideUrl, "manifest_override.xml"]) else: - yield self._Cmd(["wget", self.manifest_override_url, "-O", "manifest_override.xml"]) + yield self._Cmd(["wget", self.manifestOverrideUrl, "-O", "manifest_override.xml"]) yield self._Cmd(["ln", "-sf", "../manifest_override.xml", "manifest.xml"], - workdir=self.build.pathmodule.join(self.workdir,".repo")) + workdir=self.build.pathmodule.join(self.workdir, ".repo")) - for command in self.manifest_downloads: - yield self._Cmd(command, workdir=self.build.pathmodule.join(self.workdir,".repo","manifests")) + for command in self.manifestDownloads: + yield self._Cmd(command, workdir=self.build.pathmodule.join(self.workdir, ".repo", "manifests")) command = ['sync'] if self.jobs: - command.append('-j' + str(self.jobs)) - if not self.will_sync_all_branches: + command.append('-j' + str(self.jobs)) + if not self.syncAllBranches: command.append('-c') self.step_status.setText(["repo sync"]) self.stdio_log.addHeader("synching manifest %s from branch %s from %s\n" - % (self.manifest_file, self.manifest_branch, self.manifest_url)) + % (self.manifestFile, self.manifestBranch, self.manifestURL)) yield self._repoCmd(command) command = ['manifest', '-r', '-o', 'manifest-original.xml'] @@ -300,45 +306,45 @@ def _findErrorMessages(self, error_re): if not hasattr(self.cmd, logname): continue msg = getattr(self.cmd, logname) - if not (re.search(error_re,msg) is None): + if not (re.search(error_re, msg) is None): return True return False def _sleep(self, delay): d = defer.Deferred() - reactor.callLater(delay,d.callback,1) + reactor.callLater(delay, d.callback, 1) return d @defer.inlineCallbacks def doRepoDownloads(self): self.repo_downloaded = "" - for download in self.repo_downloads: + for download in self.repoDownloads: command = ['download'] + download.split(' ') self.stdio_log.addHeader("downloading changeset %s\n" % (download)) retry = self.mirror_sync_retry + 1 while retry > 0: - yield self._repoCmd(command, abandonOnFailure = False, + yield self._repoCmd(command, abandonOnFailure=False, collectStdout=True, collectStderr=True) if not self._findErrorMessages(self.ref_not_found_re): break - retry -=1 - self.stdio_log.addStderr("failed downloading changeset %s\n"% (download)) + retry -= 1 + self.stdio_log.addStderr("failed downloading changeset %s\n" % (download)) self.stdio_log.addHeader("wait one minute for mirror sync\n") yield self._sleep(self.mirror_sync_sleep) if retry == 0: - self.step_status.setText(["repo: change %s does not exist"%download]) - self.step_status.setText2(["repo: change %s does not exist"%download]) + self.step_status.setText(["repo: change %s does not exist" % download]) + self.step_status.setText2(["repo: change %s does not exist" % download]) raise buildstep.BuildStepFailed() if self.cmd.didFail() or self._findErrorMessages(self.cherry_pick_error_re): # cherry pick error! We create a diff with status current workdir # in stdout, which reveals the merge errors and exit - command = ['forall','-c' ,'git' ,'diff', 'HEAD'] - yield self._repoCmd(command, abandonOnFailure = False) - self.step_status.setText(["download failed: %s"%download]) + command = ['forall', '-c', 'git', 'diff', 'HEAD'] + yield self._repoCmd(command, abandonOnFailure=False) + self.step_status.setText(["download failed: %s" % download]) raise buildstep.BuildStepFailed() if hasattr(self.cmd, 'stderr'): @@ -355,6 +361,7 @@ def doRepoDownloads(self): match2.group(1)) self.setProperty("repo_downloaded", self.repo_downloaded, "Source") + def computeTarballOptions(self): # Keep in mind that the compression part of tarball generation # can be non negligible @@ -372,31 +379,30 @@ def computeTarballOptions(self): @defer.inlineCallbacks def maybeExtractTarball(self): if self.tarball: - tar = self.computeTarballOptions() + [ '-xvf', self.tarball ] + tar = self.computeTarballOptions() + ['-xvf', self.tarball] res = yield self._Cmd(tar, abandonOnFailure=False) - if res: # error with tarball.. erase repo dir and tarball + if res: # error with tarball.. erase repo dir and tarball yield self._Cmd(["rm", "-f", self.tarball], abandonOnFailure=False) yield self.runRmdir(self.repoDir(), abandonOnFailure=False) @defer.inlineCallbacks def maybeUpdateTarball(self): - if not self.tarball or self.tarball_updating_age is None: + if not self.tarball or self.updateTarballAge is None: return # tarball path is absolute, so we cannot use slave's stat command # stat -c%Y gives mtime in second since epoch res = yield self._Cmd(["stat", "-c%Y", self.tarball], collectStdout=True, abandonOnFailure=False) if not res: tarball_mtime = int(self.cmd.stdout) - yield self._Cmd(["stat", "-c%Y", "." ],collectStdout=True) + yield self._Cmd(["stat", "-c%Y", "."], collectStdout=True) now_mtime = int(self.cmd.stdout) age = now_mtime - tarball_mtime - if res or age > self.tarball_updating_age: - tar = self.computeTarballOptions() + [ '-cvf', self.tarball,".repo"] + if res or age > self.updateTarballAge: + tar = self.computeTarballOptions() + ['-cvf', self.tarball, ".repo"] res = yield self._Cmd(tar, abandonOnFailure=False) - if res: # error with tarball.. erase tarball, but dont fail + if res: # error with tarball.. erase tarball, but dont fail yield self._Cmd(["rm", "-f", self.tarball], abandonOnFailure=False) - # a simple shell script to gather all cleanup tweaks... # doing them one by one just complicate the stuff # and messup the stdio log @@ -411,10 +417,10 @@ def _getCleanupCommand(self): cd .repo/manifests rm -f .git/index.lock git fetch origin - git reset --hard remotes/origin/%(manifest_branch)s - git config branch.default.merge %(manifest_branch)s + git reset --hard remotes/origin/%(manifestBranch)s + git config branch.default.merge %(manifestBranch)s cd .. - ln -sf manifests/%(manifest_file)s manifest.xml + ln -sf manifests/%(manifestFile)s manifest.xml cd .. fi repo forall -c rm -f .git/index.lock @@ -422,6 +428,7 @@ def _getCleanupCommand(self): repo forall -c git reset --hard HEAD 2>/dev/null rm -f %(workdir)s/.repo/project.list """) % self.__dict__ + def doCleanup(self): command = self._getCleanupCommand() - return self._Cmd(["bash", "-c", command],abandonOnFailure=False) + return self._Cmd(["bash", "-c", command], abandonOnFailure=False) diff --git a/master/buildbot/test/fake/remotecommand.py b/master/buildbot/test/fake/remotecommand.py index a092b9e4bc0..9c583c9278f 100644 --- a/master/buildbot/test/fake/remotecommand.py +++ b/master/buildbot/test/fake/remotecommand.py @@ -81,15 +81,12 @@ def __init__(self, workdir, command, env=None, timeout=20*60, maxTime=None, logfiles={}, usePTY="slave-config", logEnviron=True, collectStdout=False, collectStderr=False, - interruptSignal=None, initialStdin=None, decodeRC={0:SUCCESS}, - user=None): + interruptSignal=None, initialStdin=None, decodeRC={0:SUCCESS}): args = dict(workdir=workdir, command=command, env=env or {}, want_stdout=want_stdout, want_stderr=want_stderr, initial_stdin=initialStdin, timeout=timeout, maxTime=maxTime, logfiles=logfiles, usePTY=usePTY, logEnviron=logEnviron) - if user is not None: - args['user'] = user FakeRemoteCommand.__init__(self, "shell", args, collectStdout=collectStdout, collectStderr=collectStderr, @@ -286,8 +283,7 @@ class ExpectShell(Expect): def __init__(self, workdir, command, env={}, want_stdout=1, want_stderr=1, initialStdin=None, timeout=20*60, maxTime=None, logfiles={}, - usePTY="slave-config", logEnviron=True, - user=None): + usePTY="slave-config", logEnviron=True): args = dict(workdir=workdir, command=command, env=env, want_stdout=want_stdout, want_stderr=want_stderr, initial_stdin=initialStdin, diff --git a/master/buildbot/test/interfaces/test_remotecommand.py b/master/buildbot/test/interfaces/test_remotecommand.py index fd1223189ff..86cdb818fea 100644 --- a/master/buildbot/test/interfaces/test_remotecommand.py +++ b/master/buildbot/test/interfaces/test_remotecommand.py @@ -44,7 +44,7 @@ def __init__(self, workdir, command, env=None, want_stdout=1, want_stderr=1, timeout=20*60, maxTime=None, logfiles={}, usePTY="slave-config", logEnviron=True, collectStdout=False, collectStderr=False, interruptSignal=None, initialStdin=None, - decodeRC={0:SUCCESS}, user=None): + decodeRC={0:SUCCESS}): pass def test_signature_run(self): diff --git a/master/buildbot/test/regressions/test_shell_command_properties.py b/master/buildbot/test/regressions/test_shell_command_properties.py index 38b5446a433..8c575a34d93 100644 --- a/master/buildbot/test/regressions/test_shell_command_properties.py +++ b/master/buildbot/test/regressions/test_shell_command_properties.py @@ -15,7 +15,7 @@ from twisted.trial import unittest -from buildbot.steps.shell import ShellCommand, SetProperty +from buildbot.steps.shell import ShellCommand, SetPropertyFromCommand from buildbot.process.properties import WithProperties, Properties from buildbot.process.factory import BuildFactory from buildbot.sourcestamp import SourceStamp @@ -76,7 +76,7 @@ def mergeReasons(self, others): class TestShellCommandProperties(unittest.TestCase): def testCommand(self): f = BuildFactory() - f.addStep(SetProperty(command=["echo", "value"], property="propname")) + f.addStep(SetPropertyFromCommand(command=["echo", "value"], property="propname")) f.addStep(ShellCommand(command=["echo", WithProperties("%(propname)s")])) ss = SourceStamp() @@ -94,7 +94,7 @@ def testCommand(self): class TestSetProperty(unittest.TestCase): def testGoodStep(self): f = BuildFactory() - f.addStep(SetProperty(command=["echo", "value"], property="propname")) + f.addStep(SetPropertyFromCommand(command=["echo", "value"], property="propname")) ss = SourceStamp() @@ -109,8 +109,8 @@ def testGoodStep(self): def testErrorBothSet(self): self.assertRaises(config.ConfigErrors, - SetProperty, command=["echo", "value"], property="propname", extract_fn=lambda x:{"propname": "hello"}) + SetPropertyFromCommand, command=["echo", "value"], property="propname", extract_fn=lambda x:{"propname": "hello"}) def testErrorNoneSet(self): self.assertRaises(config.ConfigErrors, - SetProperty, command=["echo", "value"]) + SetPropertyFromCommand, command=["echo", "value"]) diff --git a/master/buildbot/test/unit/test_changes_gerritchangesource.py b/master/buildbot/test/unit/test_changes_gerritchangesource.py index a0c5e509f3a..bcd8fe32512 100644 --- a/master/buildbot/test/unit/test_changes_gerritchangesource.py +++ b/master/buildbot/test/unit/test_changes_gerritchangesource.py @@ -20,7 +20,6 @@ class TestGerritChangeSource(changesource.ChangeSourceMixin, unittest.TestCase): - def setUp(self): return self.setUpChangeSource() @@ -40,6 +39,28 @@ def test_describe(self): # TODO: test the backoff algorithm + # this variable is reused in test_steps_source_repo + # to ensure correct integration between change source and repo step + expected_change = {'category': u'patchset-created', + 'files': ['unknown'], + 'repository': u'ssh://someuser@somehost:29418/pr', + 'author': u'Dustin ', + 'comments': u'fix 1234', + 'project': u'pr', + 'branch': u'br/4321', + 'revlink': u'http://buildbot.net', + 'properties': {u'event.change.owner.email': u'dustin@mozilla.com', + u'event.change.subject': u'fix 1234', + u'event.change.project': u'pr', + u'event.change.owner.name': u'Dustin', + u'event.change.number': u'4321', + u'event.change.url': u'http://buildbot.net', + u'event.change.branch': u'br', + u'event.type': u'patchset-created', + u'event.patchSet.revision': u'abcdef', + u'event.patchSet.number': u'12'}, + u'revision': u'abcdef'} + def test_lineReceived_patchset_created(self): s = self.newChangeSource('somehost', 'someuser') d = s.lineReceived(json.dumps(dict( @@ -52,20 +73,13 @@ def test_lineReceived_patchset_created(self): url="http://buildbot.net", subject="fix 1234" ), - patchSet=dict(revision="abcdef") + patchSet=dict(revision="abcdef", number="12") ))) def check(_): self.failUnlessEqual(len(self.changes_added), 1) c = self.changes_added[0] - self.assertEqual(c['author'], "Dustin ") - self.assertEqual(c['project'], "pr") - self.assertEqual(c['branch'], "br/4321") - self.assertEqual(c['revision'], "abcdef") - self.assertEqual(c['revlink'], "http://buildbot.net") - self.assertEqual(c['repository'], "ssh://someuser@somehost:29418/pr") - self.assertEqual(c['comments'], "fix 1234") - self.assertEqual(c['files'], [ 'unknown' ]) - self.assertEqual(c['properties']['event.change.subject'], 'fix 1234') + for k, v in c.items(): + self.assertEqual(self.expected_change[k], v) d.addCallback(check) return d diff --git a/master/buildbot/test/unit/test_clients_sendchange.py b/master/buildbot/test/unit/test_clients_sendchange.py index baeacc8e524..a403694d373 100644 --- a/master/buildbot/test/unit/test_clients_sendchange.py +++ b/master/buildbot/test/unit/test_clients_sendchange.py @@ -14,25 +14,55 @@ # Copyright Buildbot Team Members +import mock from twisted.trial import unittest -from twisted.internet import defer +from twisted.spread import pb +from twisted.internet import defer, reactor from buildbot.clients import sendchange -from buildbot.test.util import pbclient -class Sender(unittest.TestCase, pbclient.PBClientMixin): +class Sender(unittest.TestCase): def setUp(self): - self.setUpPBClient() + # patch out some PB components and make up some mocks + self.patch(pb, 'PBClientFactory', self._fake_PBClientFactory) + self.patch(reactor, 'connectTCP', self._fake_connectTCP) + + self.factory = mock.Mock(name='PBClientFactory') + self.factory.login = self._fake_login + self.factory.login_d = defer.Deferred() + + self.remote = mock.Mock(name='PB Remote') + self.remote.callRemote = self._fake_callRemote + self.remote.broker.transport.loseConnection = self._fake_loseConnection # results + self.creds = None + self.conn_host = self.conn_port = None + self.lostConnection = False self.added_changes = [] self.vc_used = None + def _fake_PBClientFactory(self): + return self.factory + + def _fake_login(self, creds): + self.creds = creds + return self.factory.login_d + + def _fake_connectTCP(self, host, port, factory): + self.conn_host = host + self.conn_port = port + self.assertIdentical(factory, self.factory) + self.factory.login_d.callback(self.remote) + def _fake_callRemote(self, method, change): self.assertEqual(method, 'addChange') self.added_changes.append(change) return defer.succeed(None) + def _fake_loseConnection(self): + self.lostConnection = True + def assertProcess(self, host, port, username, password, changes): self.assertEqual([host, port, username, password, changes], [ self.conn_host, self.conn_port, diff --git a/master/buildbot/test/unit/test_clients_tryclient.py b/master/buildbot/test/unit/test_clients_tryclient.py index 806fc834032..e4f61d3cd2e 100644 --- a/master/buildbot/test/unit/test_clients_tryclient.py +++ b/master/buildbot/test/unit/test_clients_tryclient.py @@ -15,16 +15,10 @@ from __future__ import with_statement -import sys -import mock - from twisted.trial import unittest -from twisted.internet import defer from buildbot.clients import tryclient from buildbot.util import json -from buildbot.scripts.runner import TryOptions -from buildbot.test.util import pbclient class createJobfile(unittest.TestCase): @@ -139,57 +133,3 @@ def test_createJobfile_v5(self): 'properties': properties, })) self.assertEqual(job, jobstr) - - -class TestGetAvailableNames(unittest.TestCase, pbclient.PBClientMixin): - - def setUp(self): - self.setUpPBClient() - - # The try client likes to print to stdout, so mute it. - stdout = mock.Mock() - stdout.write = lambda _: None - self.patch(sys, 'stdout', stdout) - - def test_getAvailableNames_properties(self): - """ - Test that properties are sent to the server when - getAvailableNames is called. - """ - - self.properties = {'foo': 'bar'} - - def callRemote(method, *args): - self.assertEqual(method, "getAvailableBuilderNames") - self.assertEqual(args, (self.properties,)) - return defer.succeed(['builder']) - self.remote.callRemote = callRemote - - config = TryOptions() - config['properties'] = self.properties - config['connect'] = 'pb' - config['master'] = 'localhost:1234' - try_client = tryclient.Try(config) - - d = try_client.getAvailableBuilderNames() - return d - - def test_getAvailableNames_no_properties(self): - """ - Test that properties are sent to the server when - getAvailableNames is called. - """ - - def callRemote(method, *args): - self.assertEqual(method, "getAvailableBuilderNames") - self.assertEqual(args, ()) - return defer.succeed(['builder']) - self.remote.callRemote = callRemote - - config = TryOptions() - config['connect'] = 'pb' - config['master'] = 'localhost:1234' - try_client = tryclient.Try(config) - - d = try_client.getAvailableBuilderNames() - return d diff --git a/master/buildbot/test/unit/test_process_build.py b/master/buildbot/test/unit/test_process_build.py index ad066f12ca1..14042b3a0d0 100644 --- a/master/buildbot/test/unit/test_process_build.py +++ b/master/buildbot/test/unit/test_process_build.py @@ -210,28 +210,28 @@ def testBuildcanStartWithSlavebuilder(self): # no locks, so both these pass (call twice to verify there's no state/memory) lock_list = [(real_lock, counting_access)] - self.assertIdentical(True, Build.canStartWithSlavebuilder(lock_list, slavebuilder1)) - self.assertIdentical(True, Build.canStartWithSlavebuilder(lock_list, slavebuilder1)) - self.assertIdentical(True, Build.canStartWithSlavebuilder(lock_list, slavebuilder2)) - self.assertIdentical(True, Build.canStartWithSlavebuilder(lock_list, slavebuilder2)) + self.assertTrue(Build.canStartWithSlavebuilder(lock_list, slavebuilder1)) + self.assertTrue(Build.canStartWithSlavebuilder(lock_list, slavebuilder1)) + self.assertTrue(Build.canStartWithSlavebuilder(lock_list, slavebuilder2)) + self.assertTrue(Build.canStartWithSlavebuilder(lock_list, slavebuilder2)) slave_lock_1 = real_lock.getLock(slavebuilder1.slave) slave_lock_2 = real_lock.getLock(slavebuilder2.slave) # then have slavebuilder2 claim its lock: slave_lock_2.claim(slavebuilder2, counting_access) - self.assertIdentical(True, Build.canStartWithSlavebuilder(lock_list, slavebuilder1)) - self.assertIdentical(True, Build.canStartWithSlavebuilder(lock_list, slavebuilder1)) - self.assertIdentical(False, Build.canStartWithSlavebuilder(lock_list, slavebuilder2)) - self.assertIdentical(False, Build.canStartWithSlavebuilder(lock_list, slavebuilder2)) + self.assertTrue(Build.canStartWithSlavebuilder(lock_list, slavebuilder1)) + self.assertTrue(Build.canStartWithSlavebuilder(lock_list, slavebuilder1)) + self.assertFalse(Build.canStartWithSlavebuilder(lock_list, slavebuilder2)) + self.assertFalse(Build.canStartWithSlavebuilder(lock_list, slavebuilder2)) slave_lock_2.release(slavebuilder2, counting_access) # then have slavebuilder1 claim its lock: slave_lock_1.claim(slavebuilder1, counting_access) - self.assertIdentical(False, Build.canStartWithSlavebuilder(lock_list, slavebuilder1)) - self.assertIdentical(False, Build.canStartWithSlavebuilder(lock_list, slavebuilder1)) - self.assertIdentical(True, Build.canStartWithSlavebuilder(lock_list, slavebuilder2)) - self.assertIdentical(True, Build.canStartWithSlavebuilder(lock_list, slavebuilder2)) + self.assertFalse(Build.canStartWithSlavebuilder(lock_list, slavebuilder1)) + self.assertFalse(Build.canStartWithSlavebuilder(lock_list, slavebuilder1)) + self.assertTrue(Build.canStartWithSlavebuilder(lock_list, slavebuilder2)) + self.assertTrue(Build.canStartWithSlavebuilder(lock_list, slavebuilder2)) slave_lock_1.release(slavebuilder1, counting_access) diff --git a/master/buildbot/test/unit/test_process_buildstep.py b/master/buildbot/test/unit/test_process_buildstep.py index d53edd33bf8..5e258b46e1f 100644 --- a/master/buildbot/test/unit/test_process_buildstep.py +++ b/master/buildbot/test/unit/test_process_buildstep.py @@ -142,7 +142,7 @@ def test_runCommand(self): bs = buildstep.BuildStep() bs.buildslave = slave.FakeSlave() bs.remote = 'dummy' - cmd = buildstep.RemoteShellCommand("build", ["echo", "hello"], user=None) + cmd = buildstep.RemoteShellCommand("build", ["echo", "hello"]) cmd.run = lambda self, remote : SUCCESS bs.runCommand(cmd) self.assertEqual(bs.cmd, cmd) @@ -279,36 +279,3 @@ def cb(_): self.assertEqual(len(self.flushLoggedErrors(ValueError)), 1) return d - -class RemoteShellCommandTests(object): - - def test_user_argument(self): - """ - Test that the 'user' parameter is correctly threaded through - RemoteShellCommand to the 'args' member of the RemoteCommand - parent command, if and only if it is passed in as a non-None - value. - """ - - rc = self.makeRemoteShellCommand("build", ["echo", "hello"]) - self.assertNotIn('user', rc.args) - - rc = self.makeRemoteShellCommand("build", ["echo", "hello"], user=None) - self.assertNotIn('user', rc.args) - - user = 'test' - rc = self.makeRemoteShellCommand("build", ["echo", "hello"], user=user) - self.assertIn('user', rc.args) - self.assertEqual(rc.args['user'], user) - - -class TestRealRemoteShellCommand(unittest.TestCase, RemoteShellCommandTests): - - def makeRemoteShellCommand(self, *args, **kwargs): - return buildstep.RemoteShellCommand(*args, **kwargs) - - -class TestFakeRemoteShellCommand(unittest.TestCase, RemoteShellCommandTests): - - def makeRemoteShellCommand(self, *args, **kwargs): - return remotecommand.FakeRemoteShellCommand(*args, **kwargs) diff --git a/master/buildbot/test/unit/test_process_factory.py b/master/buildbot/test/unit/test_process_factory.py index c60d7636da7..2724f9249cb 100644 --- a/master/buildbot/test/unit/test_process_factory.py +++ b/master/buildbot/test/unit/test_process_factory.py @@ -15,7 +15,7 @@ from twisted.trial import unittest -from buildbot.process.factory import BuildFactory +from buildbot.process.factory import BuildFactory, s from buildbot.process.buildstep import BuildStep, _BuildStepFactory class TestBuildFactory(unittest.TestCase): @@ -31,6 +31,41 @@ def test_addStep(self): factory.addStep(step) self.assertEqual(factory.steps, [_BuildStepFactory(BuildStep)]) + def test_addStep_deprecated_withArguments(self): + """ + Passing keyword arguments to L{BuildFactory.addStep} is deprecated, + but pass the arguments to the first argument, to construct a step. + """ + factory = BuildFactory() + factory.addStep(BuildStep, name='test') + self.assertEqual(factory.steps, [_BuildStepFactory(BuildStep, name='test')]) + warnings = self.flushWarnings([self.test_addStep_deprecated_withArguments]) + self.assertEqual(len(warnings), 1) + self.assertEqual(warnings[0]['category'], DeprecationWarning) + + def test_addStep_deprecated(self): + """ + Passing keyword arguments to L{BuildFactory.addStep} is deprecated, + but pass the arguments to the first argument, to construct a step. + """ + factory = BuildFactory() + factory.addStep(BuildStep) + self.assertEqual(factory.steps, [_BuildStepFactory(BuildStep)]) + warnings = self.flushWarnings([self.test_addStep_deprecated]) + self.assertEqual(len(warnings), 1) + self.assertEqual(warnings[0]['category'], DeprecationWarning) + + def test_s(self): + """ + L{s} is deprecated, but pass keyword arguments to the first argument, + to construct a step. + """ + stepFactory = s(BuildStep, name='test') + self.assertEqual(stepFactory, _BuildStepFactory(BuildStep, name='test')) + warnings = self.flushWarnings([self.test_s]) + self.assertEqual(len(warnings), 1) + self.assertEqual(warnings[0]['category'], DeprecationWarning) + def test_addStep_notAStep(self): factory = BuildFactory() # This fails because object isn't adaptable to IBuildStepFactory diff --git a/master/buildbot/test/unit/test_schedulers_trysched.py b/master/buildbot/test/unit/test_schedulers_trysched.py index e9e5cdd7534..1aaa5b34fc4 100644 --- a/master/buildbot/test/unit/test_schedulers_trysched.py +++ b/master/buildbot/test/unit/test_schedulers_trysched.py @@ -685,14 +685,6 @@ def test_getAvailableBuilderNames(self): def check(buildernames): self.assertEqual(buildernames, ['a', 'b']) d.addCallback(check) - - # Test that getAvailabelBuilderNames also accepts the properties - # argument. - d.addCallback( - lambda _: persp.perspective_getAvailableBuilderNames( - properties={'foo': 'bar'})) - d.addCallback(check) - return d diff --git a/master/buildbot/test/unit/test_status_web_base.py b/master/buildbot/test/unit/test_status_web_base.py index 6df2f32b61a..8c456c87bea 100644 --- a/master/buildbot/test/unit/test_status_web_base.py +++ b/master/buildbot/test/unit/test_status_web_base.py @@ -67,6 +67,11 @@ def do_test_getRequestCharset(self, hdr, exp): self.assertEqual(base.getRequestCharset(req), exp) + def fakeRequest(self, prepath): + r = mock.Mock() + r.prepath = prepath + return r + def test_getRequestCharset_empty(self): return self.do_test_getRequestCharset(None, 'utf-8') @@ -136,3 +141,15 @@ def test_abbreviate_age_3_weeks(self): def test_abbreviate_age_long_time(self): self.assertEqual(base.abbreviate_age((base.MONTH * 4 + base.WEEK)), "a long time ago") + + def test_path_to_root_from_root(self): + self.assertEqual(base.path_to_root(self.fakeRequest([])), + './') + + def test_path_to_root_from_one_level(self): + self.assertEqual(base.path_to_root(self.fakeRequest(['waterfall'])), + './') + + def test_path_to_root_from_two_level(self): + self.assertEqual(base.path_to_root(self.fakeRequest(['a', 'b'])), + '../') diff --git a/master/buildbot/test/unit/test_steps_source_repo.py b/master/buildbot/test/unit/test_steps_source_repo.py index c0e62655fb1..9caf32c4b31 100644 --- a/master/buildbot/test/unit/test_steps_source_repo.py +++ b/master/buildbot/test/unit/test_steps_source_repo.py @@ -18,51 +18,79 @@ from buildbot.status.results import SUCCESS, FAILURE from buildbot.test.util import sourcesteps from buildbot.test.fake.remotecommand import ExpectShell, Expect +from buildbot.process.properties import Properties +from .test_changes_gerritchangesource import TestGerritChangeSource +from buildbot.changes.changes import Change import os.path + class RepoURL(unittest.TestCase): # testcases taken from old_source/Repo test + + def oneTest(self, props, expected): + p = Properties() + p.update(props, "test") + r = repo.RepoDownloadsFromProperties(props.keys()) + self.assertEqual(r.getRenderingFor(p), expected) + def test_parse1(self): - r = repo.Repo(manifest_url="a") - self.assertEqual(r.parseDownloadProperty("repo download test/bla 564/12"),["test/bla 564/12"]) + self.oneTest( + {'a': "repo download test/bla 564/12"}, ["test/bla 564/12"]) + def test_parse2(self): - r = repo.Repo(manifest_url="a") - self.assertEqual(r.parseDownloadProperty("repo download test/bla 564/12 repo download test/bla 564/2"),["test/bla 564/12","test/bla 564/2"]) + self.oneTest( + {'a': + "repo download test/bla 564/12 repo download test/bla 564/2"}, + ["test/bla 564/12", "test/bla 564/2"]) + self.oneTest({'a': "repo download test/bla 564/12", 'b': "repo download test/bla 564/2"}, [ + "test/bla 564/12", "test/bla 564/2"]) + def test_parse3(self): - r = repo.Repo(manifest_url="a") - self.assertEqual(r.parseDownloadProperty("repo download test/bla 564/12 repo download test/bla 564/2 test/foo 5/1"),["test/bla 564/12","test/bla 564/2","test/foo 5/1"]) - self.assertEqual(r.parseDownloadProperty("repo download test/bla 564/12"),["test/bla 564/12"]) + self.oneTest({'a': "repo download test/bla 564/12 repo download test/bla 564/2 test/foo 5/1"}, [ + "test/bla 564/12", "test/bla 564/2", "test/foo 5/1"]) + self.oneTest( + {'a': "repo download test/bla 564/12"}, ["test/bla 564/12"]) + class TestRepo(sourcesteps.SourceStepMixin, unittest.TestCase): def setUp(self): self.shouldRetry = False - self.logEnviron=True + self.logEnviron = True return self.setUpSourceStep() def tearDown(self): return self.tearDownSourceStep() + def shouldLogEnviron(self): r = self.logEnviron - self.logEnviron=False + self.logEnviron = False return r + def ExpectShell(self, **kw): - if not kw.has_key('workdir'): - kw['workdir']='wkdir' - if not kw.has_key('logEnviron'): - kw['logEnviron']=self.shouldLogEnviron() + if 'workdir' not in kw: + kw['workdir'] = 'wkdir' + if 'logEnviron' not in kw: + kw['logEnviron'] = self.shouldLogEnviron() return ExpectShell(**kw) - def mySetupStep(self,**kwargs): + + def mySetupStep(self, **kwargs): + if "repoDownloads" not in kwargs: + kwargs.update( + dict(repoDownloads=repo.RepoDownloadsFromProperties(["repo_download", "repo_download2"]))) self.setupStep( - repo.Repo(manifest_url='git://myrepo.com/manifest.git', - manifest_branch = "mb", - manifest_file = "mf", **kwargs)) + repo.Repo(manifestURL='git://myrepo.com/manifest.git', + manifestBranch="mb", + manifestFile="mf", + **kwargs)) self.build.allChanges = lambda x=None: [] + def myRunStep(self, result=SUCCESS, status_text=["update"]): self.expectOutcome(result=result, status_text=status_text) d = self.runStep() + def printlogs(res): - text= self.step.stdio_log.getTextWithHeaders() + text = self.step.stdio_log.getTextWithHeaders() if "Failure instance" in text and not self.shouldRetry: print text return res @@ -76,37 +104,42 @@ def expectClobber(self): logEnviron=self.logEnviron)) + 1, Expect('rmdir', dict(dir='wkdir', - logEnviron=self.logEnviron)) + logEnviron=self.logEnviron)) + 0, Expect('mkdir', dict(dir='wkdir', logEnviron=self.logEnviron)) + 0, ) + def expectnoClobber(self): # stat return 0, so nothing self.expectCommands( Expect('stat', dict(file=os.path.join('wkdir', '.repo'), logEnviron=self.logEnviron)) + 0, - ) - def expectRepoSync(self, which_fail=-1, breakatfail=False,syncoptions=["-c"], override_commands=[]): + ) + + def expectRepoSync(self, which_fail=-1, breakatfail=False, syncoptions=["-c"], override_commands=[]): commands = [ - self.ExpectShell(command=['bash', '-c', self.step._getCleanupCommand()]) - , - self.ExpectShell(command=['repo', 'init', '-u','git://myrepo.com/manifest.git', + self.ExpectShell( + command=[ + 'bash', '-c', self.step._getCleanupCommand()]), + self.ExpectShell( + command=['repo', 'init', '-u', 'git://myrepo.com/manifest.git', '-b', 'mb', '-m', 'mf']) - ]+ override_commands +[ - self.ExpectShell(command=['repo', 'sync']+syncoptions) - , - self.ExpectShell(command=['repo', 'manifest', '-r', '-o', 'manifest-original.xml']) - ] + ] + override_commands + [ + self.ExpectShell(command=['repo', 'sync'] + syncoptions), + self.ExpectShell( + command=['repo', 'manifest', '-r', '-o', 'manifest-original.xml']) + ] for i in xrange(len(commands)): - self.expectCommands(commands[i]+(which_fail==i and 1 or 0)) - if which_fail==i and breakatfail: + self.expectCommands(commands[i] + (which_fail == i and 1 or 0)) + if which_fail == i and breakatfail: break + def test_basic(self): """basic first time repo sync""" - self.mySetupStep() + self.mySetupStep(repoDownloads=None) self.expectClobber() self.expectRepoSync() return self.myRunStep(status_text=["update"]) @@ -122,12 +155,12 @@ def test_jobs(self): """basic first time repo sync with jobs""" self.mySetupStep(jobs=2) self.expectClobber() - self.expectRepoSync(syncoptions=["-j2","-c"]) + self.expectRepoSync(syncoptions=["-j2", "-c"]) return self.myRunStep(status_text=["update"]) def test_sync_all_branches(self): """basic first time repo sync with all branches""" - self.mySetupStep(sync_all_branches=lambda _:True) + self.mySetupStep(syncAllBranches=True) self.expectClobber() self.expectRepoSync(syncoptions=[]) return self.myRunStep(status_text=["update"]) @@ -136,20 +169,21 @@ def test_manifest_override(self): """repo sync with manifest_override_url property set download via wget """ - self.mySetupStep() - self.build.setProperty("manifest_override_url", - "http://u.rl/test.manifest", "test") + self.mySetupStep(manifestOverrideUrl= + "http://u.rl/test.manifest", + syncAllBranches=True) self.expectClobber() override_commands = [ - Expect('stat', dict(file=os.path.join('wkdir', 'http://u.rl/test.manifest'), - logEnviron=False)), - self.ExpectShell(logEnviron=False,command=['wget', + Expect( + 'stat', dict(file=os.path.join('wkdir', 'http://u.rl/test.manifest'), + logEnviron=False)), + self.ExpectShell(logEnviron=False, command=['wget', 'http://u.rl/test.manifest', - '-O', 'manifest_override.xml']) - , - self.ExpectShell(logEnviron=False,workdir=os.path.join('wkdir', '.repo'), - command=['ln', '-sf', '../manifest_override.xml', - 'manifest.xml']) + '-O', 'manifest_override.xml']), + self.ExpectShell( + logEnviron=False, workdir=os.path.join('wkdir', '.repo'), + command=['ln', '-sf', '../manifest_override.xml', + 'manifest.xml']) ] self.expectRepoSync(which_fail=2, syncoptions=[], override_commands=override_commands) @@ -159,21 +193,23 @@ def test_manifest_override_local(self): """repo sync with manifest_override_url property set copied from local FS """ - self.mySetupStep() - self.build.setProperty("manifest_override_url", - "test.manifest", "test") + self.mySetupStep(manifestOverrideUrl= + "test.manifest", + syncAllBranches=True) self.expectClobber() override_commands = [ Expect('stat', dict(file=os.path.join('wkdir', 'test.manifest'), logEnviron=False)), self.ExpectShell(logEnviron=False, - command=['cp', '-f', 'test.manifest', 'manifest_override.xml']), + command=[ + 'cp', '-f', 'test.manifest', 'manifest_override.xml']), self.ExpectShell(logEnviron=False, workdir=os.path.join('wkdir', '.repo'), command=['ln', '-sf', '../manifest_override.xml', 'manifest.xml']) ] - self.expectRepoSync(syncoptions=[], override_commands=override_commands) + self.expectRepoSync( + syncoptions=[], override_commands=override_commands) return self.myRunStep(status_text=["update"]) def test_tarball(self): @@ -181,13 +217,15 @@ def test_tarball(self): """ self.mySetupStep(tarball="/tarball.tar") self.expectClobber() - self.expectCommands(self.ExpectShell(command = ['tar', '-xvf', '/tarball.tar'])+0) + self.expectCommands( + self.ExpectShell(command=['tar', '-xvf', '/tarball.tar']) + 0) self.expectRepoSync() - self.expectCommands(self.ExpectShell(command = ['stat', '-c%Y', '/tarball.tar']) - + Expect.log('stdio',stdout=str(10000)) + self.expectCommands(self.ExpectShell(command=['stat', '-c%Y', '/tarball.tar']) + + Expect.log('stdio', stdout=str(10000)) + 0) - self.expectCommands(self.ExpectShell(command = ['stat', '-c%Y', '.']) - + Expect.log('stdio',stdout=str(10000+7*24*3600)) + self.expectCommands(self.ExpectShell(command=['stat', '-c%Y', '.']) + + Expect.log( + 'stdio', stdout=str(10000 + 7 * 24 * 3600)) + 0) return self.myRunStep(status_text=["update"]) @@ -197,95 +235,113 @@ def test_create_tarball(self): self.mySetupStep(tarball="/tarball.tgz") self.expectClobber() self.expectCommands( - self.ExpectShell(command = ['tar', '-z', '-xvf', '/tarball.tgz'])+1, - self.ExpectShell(command = ['rm', '-f', '/tarball.tgz'])+1, + self.ExpectShell( + command=['tar', '-z', '-xvf', '/tarball.tgz']) + 1, + self.ExpectShell(command=['rm', '-f', '/tarball.tgz']) + 1, Expect('rmdir', dict(dir=os.path.join('wkdir', '.repo'), logEnviron=False)) + 1) self.expectRepoSync() - self.expectCommands(self.ExpectShell(command = ['stat', '-c%Y', '/tarball.tgz']) - + Expect.log('stdio',stderr="file not found!") + self.expectCommands(self.ExpectShell(command=['stat', '-c%Y', '/tarball.tgz']) + + Expect.log('stdio', stderr="file not found!") + 1, - self.ExpectShell(command = ['tar', '-z', - '-cvf', '/tarball.tgz', '.repo']) + self.ExpectShell(command=['tar', '-z', + '-cvf', '/tarball.tgz', '.repo']) + 0) return self.myRunStep(status_text=["update"]) - def do_test_update_tarball(self,suffix,option): + def do_test_update_tarball(self, suffix, option): """repo sync update the tarball cache at the end (tarball older than a week) """ - self.mySetupStep(tarball="/tarball."+suffix) + self.mySetupStep(tarball="/tarball." + suffix) self.expectClobber() - self.expectCommands(self.ExpectShell(command = ['tar']+option+['-xvf', '/tarball.'+suffix])+0) + self.expectCommands( + self.ExpectShell(command=['tar'] + option + ['-xvf', '/tarball.' + suffix]) + 0) self.expectRepoSync() - self.expectCommands(self.ExpectShell(command = ['stat', '-c%Y', '/tarball.'+suffix]) - + Expect.log('stdio',stdout=str(10000)) + self.expectCommands(self.ExpectShell(command=['stat', '-c%Y', '/tarball.' + suffix]) + + Expect.log('stdio', stdout=str(10000)) + 0, - self.ExpectShell(command = ['stat', '-c%Y', '.']) - + Expect.log('stdio',stdout=str(10001+7*24*3600)) + self.ExpectShell(command=['stat', '-c%Y', '.']) + + Expect.log( + 'stdio', stdout=str(10001 + 7 * 24 * 3600)) + 0, - self.ExpectShell(command = ['tar']+option+ - ['-cvf', '/tarball.'+suffix, '.repo']) + self.ExpectShell(command=['tar'] + option + + ['-cvf', '/tarball.' + suffix, '.repo']) + 0) return self.myRunStep(status_text=["update"]) def test_update_tarball(self): self.do_test_update_tarball("tar", []) + def test_update_tarball_gz(self): """tarball compression variants""" self.do_test_update_tarball("tar.gz", ["-z"]) + def test_update_tarball_tgz(self): self.do_test_update_tarball("tgz", ["-z"]) + def test_update_tarball_bzip(self): self.do_test_update_tarball("tar.bz2", ["-j"]) + def test_update_tarball_lzma(self): self.do_test_update_tarball("tar.lzma", ["--lzma"]) + def test_update_tarball_lzop(self): self.do_test_update_tarball("tar.lzop", ["--lzop"]) - def test_update_tarball_fail1(self,suffix="tar",option=[]): + def test_update_tarball_fail1(self, suffix="tar", option=[]): """tarball extract fail -> remove the tarball + remove .repo dir """ - self.mySetupStep(tarball="/tarball."+suffix) + self.mySetupStep(tarball="/tarball." + suffix) self.expectClobber() - self.expectCommands(self.ExpectShell(command = ['tar']+option+['-xvf', '/tarball.'+suffix])+1, - self.ExpectShell(command = ['rm', '-f', '/tarball.tar'])+0, - Expect('rmdir', dict(dir=os.path.join('wkdir', '.repo'), - logEnviron=False)) - + 0) + self.expectCommands( + self.ExpectShell( + command=[ + 'tar'] + option + ['-xvf', '/tarball.' + suffix]) + 1, + self.ExpectShell( + command=['rm', '-f', '/tarball.tar']) + 0, + Expect( + 'rmdir', dict(dir=os.path.join('wkdir', '.repo'), + logEnviron=False)) + + 0) self.expectRepoSync() - self.expectCommands(self.ExpectShell(command = ['stat', '-c%Y', '/tarball.'+suffix]) - + Expect.log('stdio',stdout=str(10000)) + self.expectCommands(self.ExpectShell(command=['stat', '-c%Y', '/tarball.' + suffix]) + + Expect.log('stdio', stdout=str(10000)) + 0, - self.ExpectShell(command = ['stat', '-c%Y', '.']) - + Expect.log('stdio',stdout=str(10001+7*24*3600)) + self.ExpectShell(command=['stat', '-c%Y', '.']) + + Expect.log( + 'stdio', stdout=str(10001 + 7 * 24 * 3600)) + 0, - self.ExpectShell(command = ['tar']+option+ - ['-cvf', '/tarball.'+suffix, '.repo']) + self.ExpectShell(command=['tar'] + option + + ['-cvf', '/tarball.' + suffix, '.repo']) + 0) return self.myRunStep(status_text=["update"]) - def test_update_tarball_fail2(self,suffix="tar",option=[]): + def test_update_tarball_fail2(self, suffix="tar", option=[]): """tarball update fail -> remove the tarball + continue repo download """ - self.mySetupStep(tarball="/tarball."+suffix) + self.mySetupStep(tarball="/tarball." + suffix) self.build.setProperty("repo_download", "repo download test/bla 564/12", "test") self.expectClobber() - self.expectCommands(self.ExpectShell(command = ['tar']+option+['-xvf', '/tarball.'+suffix])+0) + self.expectCommands( + self.ExpectShell(command=['tar'] + option + ['-xvf', '/tarball.' + suffix]) + 0) self.expectRepoSync() - self.expectCommands(self.ExpectShell(command = ['stat', '-c%Y', '/tarball.'+suffix]) - + Expect.log('stdio',stdout=str(10000)) + self.expectCommands(self.ExpectShell(command=['stat', '-c%Y', '/tarball.' + suffix]) + + Expect.log('stdio', stdout=str(10000)) + 0, - self.ExpectShell(command = ['stat', '-c%Y', '.']) - + Expect.log('stdio',stdout=str(10001+7*24*3600)) + self.ExpectShell(command=['stat', '-c%Y', '.']) + + Expect.log( + 'stdio', stdout=str(10001 + 7 * 24 * 3600)) + 0, - self.ExpectShell(command = ['tar']+option+ - ['-cvf', '/tarball.'+suffix, '.repo']) + self.ExpectShell(command=['tar'] + option + + ['-cvf', '/tarball.' + suffix, '.repo']) + 1, - self.ExpectShell(command = ['rm', '-f', '/tarball.tar'])+0, - self.ExpectShell(command = ['repo', 'download', 'test/bla', '564/12']) - +0) + self.ExpectShell( + command=['rm', '-f', '/tarball.tar']) + 0, + self.ExpectShell( + command=['repo', 'download', 'test/bla', '564/12']) + + 0) return self.myRunStep(status_text=["update"]) def test_repo_downloads(self): @@ -296,11 +352,14 @@ def test_repo_downloads(self): self.expectnoClobber() self.expectRepoSync() self.expectCommands( - self.ExpectShell(command = ['repo', 'download', 'test/bla', '564/12']) - +0 - + Expect.log('stdio', stderr="test/bla refs/changes/64/564/12 -> FETCH_HEAD\n") + self.ExpectShell( + command=['repo', 'download', 'test/bla', '564/12']) + + 0 + + Expect.log( + 'stdio', stderr="test/bla refs/changes/64/564/12 -> FETCH_HEAD\n") + Expect.log('stdio', stderr="HEAD is now at 0123456789abcdef...\n")) - self.expectProperty("repo_downloaded", "564/12 0123456789abcdef ", "Source") + self.expectProperty( + "repo_downloaded", "564/12 0123456789abcdef ", "Source") return self.myRunStep(status_text=["update"]) def test_repo_downloads2(self): @@ -313,10 +372,12 @@ def test_repo_downloads2(self): self.expectnoClobber() self.expectRepoSync() self.expectCommands( - self.ExpectShell(command = ['repo', 'download', 'test/bla', '564/12']) - +0, - self.ExpectShell(command = ['repo', 'download', 'test/bla2', '565/12']) - +0) + self.ExpectShell( + command=['repo', 'download', 'test/bla', '564/12']) + + 0, + self.ExpectShell( + command=['repo', 'download', 'test/bla2', '565/12']) + + 0) return self.myRunStep(status_text=["update"]) def test_repo_download_manifest(self): @@ -328,60 +389,80 @@ def test_repo_download_manifest(self): "repo download manifest 565/12", "test") self.expectnoClobber() self.expectCommands( - self.ExpectShell(command=['bash', '-c', self.step._getCleanupCommand()]) - +0, - self.ExpectShell(command=['repo', 'init', '-u','git://myrepo.com/manifest.git', + self.ExpectShell( + command=['bash', '-c', self.step._getCleanupCommand()]) + + 0, + self.ExpectShell( + command=['repo', 'init', '-u', 'git://myrepo.com/manifest.git', '-b', 'mb', '-m', 'mf']) - +0, - self.ExpectShell(workdir=os.path.join('wkdir', '.repo', 'manifests'), - command=[ 'git','fetch','git://myrepo.com/manifest.git', - 'refs/changes/65/565/12']) - +0, - self.ExpectShell(workdir=os.path.join('wkdir', '.repo', 'manifests'), - command=['git','cherry-pick','FETCH_HEAD']) - +0, + + 0, + self.ExpectShell( + workdir=os.path.join('wkdir', '.repo', 'manifests'), + command=[ + 'git', 'fetch', 'git://myrepo.com/manifest.git', + 'refs/changes/65/565/12']) + + 0, + self.ExpectShell( + workdir=os.path.join('wkdir', '.repo', 'manifests'), + command=['git', 'cherry-pick', 'FETCH_HEAD']) + + 0, self.ExpectShell(command=['repo', 'sync', '-c']) - +0, - self.ExpectShell(command=['repo', 'manifest', '-r', '-o', 'manifest-original.xml']) - +0) + + 0, + self.ExpectShell( + command=['repo', 'manifest', '-r', '-o', 'manifest-original.xml']) + + 0) self.expectCommands( - self.ExpectShell(command = ['repo', 'download', 'test/bla', '564/12']) - +0) + self.ExpectShell( + command=['repo', 'download', 'test/bla', '564/12']) + + 0) return self.myRunStep(status_text=["update"]) def test_repo_downloads_mirror_sync(self): """repo downloads, with mirror synchronization issues""" self.mySetupStep() - self.step.mirror_sync_sleep = 0.001 # we dont really want the test to wait... + self.step.mirror_sync_sleep = 0.001 # we dont really want the test to wait... self.build.setProperty("repo_download", "repo download test/bla 564/12", "test") self.expectnoClobber() self.expectRepoSync() self.expectCommands( - self.ExpectShell(command = ['repo', 'download', 'test/bla', '564/12']) - +1 + Expect.log("stdio",stderr="fatal: Couldn't find remote ref \n"), - self.ExpectShell(command = ['repo', 'download', 'test/bla', '564/12']) - +1 + Expect.log("stdio",stderr="fatal: Couldn't find remote ref \n"), - self.ExpectShell(command = ['repo', 'download', 'test/bla', '564/12']) - +0) + self.ExpectShell( + command=['repo', 'download', 'test/bla', '564/12']) + + 1 + + Expect.log( + "stdio", stderr="fatal: Couldn't find remote ref \n"), + self.ExpectShell( + command=['repo', 'download', 'test/bla', '564/12']) + + 1 + + Expect.log( + "stdio", stderr="fatal: Couldn't find remote ref \n"), + self.ExpectShell( + command=['repo', 'download', 'test/bla', '564/12']) + + 0) return self.myRunStep(status_text=["update"]) def test_repo_downloads_change_missing(self): """repo downloads, with no actual mirror synchronization issues (still retries 2 times)""" self.mySetupStep() - self.step.mirror_sync_sleep = 0.001 # we dont really want the test to wait... - self.step.mirror_sync_retry = 1 # on retry once + self.step.mirror_sync_sleep = 0.001 # we dont really want the test to wait... + self.step.mirror_sync_retry = 1 # on retry once self.build.setProperty("repo_download", "repo download test/bla 564/12", "test") self.expectnoClobber() self.expectRepoSync() self.expectCommands( - self.ExpectShell(command = ['repo', 'download', 'test/bla', '564/12']) - +1 + Expect.log("stdio",stderr="fatal: Couldn't find remote ref \n"), - self.ExpectShell(command = ['repo', 'download', 'test/bla', '564/12']) - +1 + Expect.log("stdio",stderr="fatal: Couldn't find remote ref \n"), - ) - return self.myRunStep(result=FAILURE,status_text=["repo: change test/bla 564/12 does not exist"]) + self.ExpectShell( + command=['repo', 'download', 'test/bla', '564/12']) + + 1 + + Expect.log( + "stdio", stderr="fatal: Couldn't find remote ref \n"), + self.ExpectShell( + command=['repo', 'download', 'test/bla', '564/12']) + + 1 + + Expect.log( + "stdio", stderr="fatal: Couldn't find remote ref \n"), + ) + return self.myRunStep(result=FAILURE, status_text=["repo: change test/bla 564/12 does not exist"]) def test_repo_downloads_fail1(self): """repo downloads, cherry-pick returns 1""" @@ -391,12 +472,15 @@ def test_repo_downloads_fail1(self): self.expectnoClobber() self.expectRepoSync() self.expectCommands( - self.ExpectShell(command = ['repo', 'download', 'test/bla', '564/12']) - +1 + Expect.log("stdio",stderr="patch \n"), - self.ExpectShell(command = ['repo', 'forall', '-c', 'git', 'diff', 'HEAD']) - +0 - ) + self.ExpectShell( + command=['repo', 'download', 'test/bla', '564/12']) + + 1 + Expect.log("stdio", stderr="patch \n"), + self.ExpectShell( + command=['repo', 'forall', '-c', 'git', 'diff', 'HEAD']) + + 0 + ) return self.myRunStep(result=FAILURE, status_text=["download failed: test/bla 564/12"]) + def test_repo_downloads_fail2(self): """repo downloads, cherry-pick returns 0 but error in stderr""" self.mySetupStep() @@ -405,19 +489,61 @@ def test_repo_downloads_fail2(self): self.expectnoClobber() self.expectRepoSync() self.expectCommands( - self.ExpectShell(command = ['repo', 'download', 'test/bla', '564/12']) - +0 + Expect.log("stdio",stderr="Automatic cherry-pick failed \n"), - self.ExpectShell(command = ['repo', 'forall', '-c', 'git', 'diff', 'HEAD']) - +0 - ) + self.ExpectShell( + command=['repo', 'download', 'test/bla', '564/12']) + + 0 + + Expect.log("stdio", stderr="Automatic cherry-pick failed \n"), + self.ExpectShell( + command=['repo', 'forall', '-c', 'git', 'diff', 'HEAD']) + + 0 + ) return self.myRunStep(result=FAILURE, status_text=["download failed: test/bla 564/12"]) + def test_repo_downloads_from_change_source(self): + """basic repo download from change source, and check that repo_downloaded is updated""" + self.mySetupStep(repoDownloads=repo.RepoDownloadsFromChangeSource()) + chdict = TestGerritChangeSource.expected_change + change = Change(None, None, None, properties=chdict['properties']) + self.build.allChanges = lambda x=None: [change] + self.expectnoClobber() + self.expectRepoSync() + self.expectCommands( + self.ExpectShell(command=['repo', 'download', 'pr', '4321/12']) + + 0 + + Expect.log( + 'stdio', stderr="test/bla refs/changes/64/564/12 -> FETCH_HEAD\n") + + Expect.log('stdio', stderr="HEAD is now at 0123456789abcdef...\n")) + self.expectProperty( + "repo_downloaded", "564/12 0123456789abcdef ", "Source") + return self.myRunStep(status_text=["update"]) + + def test_repo_downloads_from_change_source_codebase(self): + """basic repo download from change source, and check that repo_downloaded is updated""" + self.mySetupStep(repoDownloads=repo.RepoDownloadsFromChangeSource("mycodebase")) + chdict = TestGerritChangeSource.expected_change + change = Change(None, None, None, properties=chdict['properties']) + # getSourceStamp is faked by SourceStepMixin + ss = self.build.getSourceStamp("") + ss.changes = [change] + self.expectnoClobber() + self.expectRepoSync() + self.expectCommands( + self.ExpectShell(command=['repo', 'download', 'pr', '4321/12']) + + 0 + + Expect.log( + 'stdio', stderr="test/bla refs/changes/64/564/12 -> FETCH_HEAD\n") + + Expect.log('stdio', stderr="HEAD is now at 0123456789abcdef...\n")) + self.expectProperty( + "repo_downloaded", "564/12 0123456789abcdef ", "Source") + return self.myRunStep(status_text=["update"]) + def test_update_fail1(self): """ fail at cleanup: ignored""" self.mySetupStep() self.expectnoClobber() self.expectRepoSync(which_fail=0, breakatfail=False) return self.myRunStep(status_text=["update"]) + def test_update_fail2(self): """fail at repo init: clobber""" self.mySetupStep() @@ -447,6 +573,7 @@ def test_update_fail4(self): self.expectRepoSync() self.shouldRetry = True return self.myRunStep(status_text=["update"]) + def test_update_doublefail(self): """fail at repo manifest: clobber but still fail""" self.mySetupStep() @@ -456,6 +583,7 @@ def test_update_doublefail(self): self.expectRepoSync(which_fail=3, breakatfail=True) self.shouldRetry = True return self.myRunStep(result=FAILURE, status_text=["repo failed at: repo manifest"]) + def test_update_doublefail2(self): """fail at repo sync: clobber but still fail""" self.mySetupStep() @@ -465,6 +593,7 @@ def test_update_doublefail2(self): self.expectRepoSync(which_fail=2, breakatfail=True) self.shouldRetry = True return self.myRunStep(result=FAILURE, status_text=["repo failed at: repo sync"]) + def test_update_doublefail3(self): """fail at repo init: clobber but still fail""" self.mySetupStep() diff --git a/master/buildbot/test/util/pbclient.py b/master/buildbot/test/util/pbclient.py deleted file mode 100644 index 4ba9c52a559..00000000000 --- a/master/buildbot/test/util/pbclient.py +++ /dev/null @@ -1,57 +0,0 @@ -# This file is part of Buildbot. Buildbot is free software: you can -# redistribute it and/or modify it under the terms of the GNU General Public -# License as published by the Free Software Foundation, version 2. -# -# This program is distributed in the hope that it will be useful, but WITHOUT -# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS -# FOR A PARTICULAR PURPOSE. See the GNU General Public License for more -# details. -# -# You should have received a copy of the GNU General Public License along with -# this program; if not, write to the Free Software Foundation, Inc., 51 -# Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. -# -# Copyright Buildbot Team Members - -import mock -from twisted.internet import defer, reactor -from twisted.spread import pb - -class PBClientMixin(object): - - def setUpPBClient(self): - # patch out some PB components and make up some mocks - self.patch(pb, 'PBClientFactory', self._fake_PBClientFactory) - self.patch(reactor, 'connectTCP', self._fake_connectTCP) - - self.factory = mock.Mock(name='PBClientFactory') - self.factory.login = self._fake_login - self.factory.login_d = defer.Deferred() - - self.remote = mock.Mock(name='PB Remote') - self.remote.callRemote = self._fake_callRemote - self.remote.broker.transport.loseConnection = self._fake_loseConnection - - # results - self.creds = None - self.conn_host = self.conn_port = None - self.lostConnection = False - - def _fake_PBClientFactory(self): - return self.factory - - def _fake_login(self, creds): - self.creds = creds - return self.factory.login_d - - def _fake_connectTCP(self, host, port, factory): - self.conn_host = host - self.conn_port = port - self.assertIdentical(factory, self.factory) - self.factory.login_d.callback(self.remote) - - def _fake_callRemote(self, method, change): - raise NotImplementedError() - - def _fake_loseConnection(self): - self.lostConnection = True diff --git a/master/docs/manual/cfg-buildsteps.rst b/master/docs/manual/cfg-buildsteps.rst index 84b6ce64417..e2403871e88 100644 --- a/master/docs/manual/cfg-buildsteps.rst +++ b/master/docs/manual/cfg-buildsteps.rst @@ -780,10 +780,12 @@ you will receive a configuration error exception. ``local``. +.. index:: double: Gerrit integration; Repo Build Step + .. bb:step:: Repo Repo -+++++++++++++++++ +++++ .. py:class:: buildbot.steps.source.repo.Repo @@ -795,15 +797,15 @@ for new and old projects. The Repo step takes the following arguments: -``manifest_url`` +``manifestURL`` (required): the URL at which the Repo's manifests source repository is available. -``manifest_branch`` +``manifestBranch`` (optional, defaults to ``master``): the manifest repository branch on which repo will take its manifest. Corresponds to the ``-b`` argument to the :command:`repo init` command. -``manifest_file`` +``manifestFile`` (optional, defaults to ``default.xml``): the manifest filename. Corresponds to the ``-m`` argument to the :command:`repo init` command. @@ -813,40 +815,74 @@ The Repo step takes the following arguments: fast bootstrap. If not present the tarball will be created automatically after first sync. It is a copy of the ``.repo`` directory which contains all the Git objects. This feature helps - to minimize network usage on very big projects. + to minimize network usage on very big projects with lots of slaves. ``jobs`` (optional, defaults to ``None``): Number of projects to fetch simultaneously while syncing. Passed to repo sync subcommand with "-j". -``sync_all_branches`` - (optional, defaults to if "manifest_override" property exists? -> True else -> False): - callback to control the policy of repo sync -c +``syncAllBranches`` + (optional, defaults to ``False``): renderable boolean to control whether ``repo`` + syncs all branches. i.e. ``repo sync -c`` -``update_tarball`` - (optional, defaults to "one week if we did not sync all branches"): - callback to control the policy of updating of the tarball - given properties, and boolean indicating whether - the last repo sync was on all branches - Returns: max age of tarball in seconds, or -1, if we +``updateTarballAge`` + (optional, defaults to "one week"): + renderable to control the policy of updating of the tarball + given properties + Returns: max age of tarball in seconds, or None, if we want to skip tarball update The default value should be good trade off on size of the tarball, and update frequency compared to cost of tarball creation -This Source step integrates with :bb:chsrc:`GerritChangeSource`, and will -automatically use the :command:`repo download` command of repo to -download the additionnal changes introduced by a pending changeset. +``repoDownloads`` + (optional, defaults to None): + list of ``repo download`` commands to perform at the end of the Repo step + each string in the list will be prefixed ``repo download``, and run as is. + This means you can include parameter in the string. e.g: -.. index:: double: Gerrit integration; Repo Build Step + - ``["-c project 1234/4"]`` will cherry-pick patchset 4 of patch 1234 in project ``project`` + + - ``["-f project 1234/4"]`` will enforce fast-forward on patchset 4 of patch 1234 in project ``project`` + +.. py:class:: buildbot.steps.source.repo.RepoDownloadsFromProperties + +``RepoDownloadsFromProperties`` can be used as a renderable of the ``repoDownload`` parameter +it will look in passed properties for string with following possible format: + + - ``repo download project change_number/patchset_number``. + + - ``project change_number/patchset_number``. + + - ``project/change_number/patchset_number``. -Gerrit integration can be also triggered using forced build with following properties: -``repo_d``, ``repo_d[0-9]``, ``repo_download``, ``repo_download[0-9]`` -with values in format: ``project/change_number/patchset_number``. All of these properties will be translated into a :command:`repo download`. This feature allows integrators to build with several pending interdependent changes, which at the moment cannot be described properly in Gerrit, and can only be described by humans. +.. py:class:: buildbot.steps.source.repo.RepoDownloadsFromChangeSource + +``RepoDownloadsFromChangeSource`` can be used as a renderable of the ``repoDownload`` parameter + +This rendereable integrates with :bb:chsrc:`GerritChangeSource`, and will +automatically use the :command:`repo download` command of repo to +download the additionnal changes introduced by a pending changeset. + +.. note:: you can use the two above Rendereable in conjuction by using the class ``buildbot.process.properties.FlattenList`` + +for example:: + + from buildbot.steps.source.repo import Repo, RepoDownloadsFromChangeSource, + from buildbot.steps.source.repo import RepoDownloadsFromProperties + from buildbot.process.properties import FlattenList + + factory.addStep(Repo(manifestUrl='git://mygerrit.org/manifest.git', + repoDownloads=FlattenList([RepoDownloadsFromChangeSource(), + RepoDownloadsFromProperties("repo_downloads") + ] + ) + )) + .. _Step-Gerrit: Gerrit @@ -1720,12 +1756,6 @@ The :bb:step:`ShellCommand` arguments are: The default is to treat just 0 as successful. (``{0:SUCCESS}``) any exit code not present in the dictionary will be treated as ``FAILURE`` -``user`` - When this is not None, runs the command as the given user by wrapping the - command with 'sudo', which typically requires root permissions to run - (and as discussed in the :ref:`System Architecture ` - section, is generally not a good idea). - .. bb:step:: Configure Configure diff --git a/master/docs/manual/cfg-statustargets.rst b/master/docs/manual/cfg-statustargets.rst index e330f359754..a1a8bf180d0 100644 --- a/master/docs/manual/cfg-statustargets.rst +++ b/master/docs/manual/cfg-statustargets.rst @@ -744,13 +744,21 @@ useful in cases where you cannot expose the WebStatus for public consumption. Anyone who can access the web status can "fake" a request from GitHub, potentially causing the buildmaster to run arbitrary code. -To protect URL against unauthorized access you should use ``change_hook_auth`` option. :: +To protect URL against unauthorized access you should use ``change_hook_auth`` option :: c['status'].append(html.WebStatus(..., - change_hook_auth=('user', 'password'))) + change_hook_auth=["file:changehook.passwd"])) + +And create a file ``changehook.passwd`` + +.. code-block:: none + + user:password Then, create a GitHub service hook (see https://help.github.com/articles/post-receive-hooks) with a WebHook URL like ``http://user:password@builds.mycompany.com/bbot/change_hook/github``. +See the `documentation `_ for twisted cred for more option to pass to ``change_hook_auth``. + Note that not using ``change_hook_auth`` can expose you to security risks. BitBucket hook diff --git a/master/docs/manual/introduction.rst b/master/docs/manual/introduction.rst index 7acd52fb805..7052d5bb326 100644 --- a/master/docs/manual/introduction.rst +++ b/master/docs/manual/introduction.rst @@ -112,15 +112,6 @@ admin*, who do not need to be quite as involved. Generally slaves are run by anyone who has an interest in seeing the project work well on their favorite platform. -While not always the case, it is typically a good idea to run the -buildslaves with reduced privileges (e.g. not as the 'root' user). -This protects the system installed on the machine the buildslave -is running on from the commands that are running on behalf of the -buildmaster. There are some cases where running buildslaves with -admin privileges is appropriate, one example of which is doing so -on latent slaves that are reverted to a snapshot on each startup -and contain no sensitive information. - .. _BuildSlave-Connections: BuildSlave Connections diff --git a/master/docs/relnotes/0.8.8.rst b/master/docs/relnotes/0.8.8.rst index 646bd29a4da..a0a5b487252 100644 --- a/master/docs/relnotes/0.8.8.rst +++ b/master/docs/relnotes/0.8.8.rst @@ -6,8 +6,7 @@ Release Notes for Buildbot v0.8.8 Most simply need an additional bulleted list item, but more significant changes can be given a subsection of their own. -The following are the release notes for Buildbot v0.8.8. -This version was released XXX. +The following are the release notes for Buildbot |version|. * The ``MasterShellCommand`` step now correctly handles environment variables passed as list. * The master now poll the database for pending tasks when running buildbot in multi-master mode. @@ -54,28 +53,30 @@ Features only a single property and therefore allows commas to be included in the property name and value. -* The ``Git`` step has a new ``config`` option, which accepts a dict of git configuration options to pass to the low-level git commands. - See :bb:step:`Git` for details. - -* The ``TryScheduler`` now accepts an additional ``properties`` argument to its - ``getAvailableBuilderNames`` method, which 'buildbot try' uses to send the properties - it was passed (and are normally sent when starting a build). +* The ``Git`` step has a new ``config`` option, which accepts a dict of git configuration options to + pass to the low-level git commands. See :bb:step:`Git` for details. * In :bb:step:`ShellCommand` ShellCommand now validates its arguments during config and will identify any invalid arguments before a build is started. * The list of force schedulers in the web UI is now sorted by name. -* The :bb:step:`ShellCommand` step has a new parameter ``user``. - When this is set, the slave will use 'sudo' to run the command as the given user. - * OpenStack-based Latent Buildslave support was added. See :bb:pull:`666`. * Master-side support for P4 is available, and provides a great deal more flexibility than the old slave-side step. See :bb:pull:`596`. +* Master-side support for Repo is available. + The step parameters changed to camelCase. + ``repo_downloads``, and ``manifest_override_url`` properties are no longer hardcoded, but instead consult as default values via renderables. + Renderable are used in favor of callables for ``syncAllBranches`` and ``updateTarball``. + +* Builder configurations can now include a ``description``, which will appear in the web UI to help humans figure out what the builder does. + * GNUAutoconf and other pre-defined factories now work correctly (:bb:bug:`2402`) +* The pubDate in RSS feeds is now rendered correctly (:bb:bug:`2530`) + Deprecations, Removals, and Non-Compatible Changes ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ @@ -104,10 +105,14 @@ Deprecations, Removals, and Non-Compatible Changes * The EC2 and libvirt latent slaves have been moved to ``buildbot.buildslave.ec2`` and ``buildbot.buildslave.libirt`` respectively. +* Pre v0.8.7 versions of buildbot supported passing keyword arguments to ``buildbot.process.BuildFactory.addStep``, but this was dropped. + Support was added again, while still being deprecated, to ease transition. + Changes for Developers ~~~~~~~~~~~~~~~~~~~~~~ * Added an optional build start callback to ``buildbot.status.status_gerrit.GerritStatusPush`` + This release includes the fix for :bb:bug:`2536`. * An optional ``startCB`` callback to :bb:status:`GerritStatusPush` can be used to send a message back to the committer. @@ -137,6 +142,18 @@ Details For a more detailed description of the changes made in this version, see the git log itself: -.. code-block:: bash +.. code-block:: none git log v0.8.7..v0.8.8 + +Older Versions +-------------- + +Release notes for older versions of Buildbot are available in the :bb:src:`master/docs/relnotes/` directory of the source tree. +Newer versions are also available here: + +.. toctree:: + :maxdepth: 1 + + 0.8.7 + 0.8.6 diff --git a/slave/buildslave/commands/base.py b/slave/buildslave/commands/base.py index fa23d0355c3..a5907d679c7 100644 --- a/slave/buildslave/commands/base.py +++ b/slave/buildslave/commands/base.py @@ -32,7 +32,7 @@ # this used to be a CVS $-style "Revision" auto-updated keyword, but since I # moved to Darcs as the primary repository, this is updated manually each # time this file is changed. The last cvs_ver that was here was 1.51 . -command_version = "2.16" +command_version = "2.15" # version history: # >=1.17: commands are interruptable @@ -64,7 +64,6 @@ # >= 2.13: SlaveFileUploadCommand supports option 'keepstamp' # >= 2.14: RemoveDirectory can delete multiple directories # >= 2.15: 'interruptSignal' option is added to SlaveShellCommand -# >= 2.16: 'user' option is added to SlaveShellCommand class Command: implements(ISlaveCommand) diff --git a/slave/buildslave/commands/shell.py b/slave/buildslave/commands/shell.py index a4b74b09124..be85eb9d2e3 100644 --- a/slave/buildslave/commands/shell.py +++ b/slave/buildslave/commands/shell.py @@ -39,7 +39,6 @@ def start(self): logfiles=args.get('logfiles', {}), usePTY=args.get('usePTY', "slave-config"), logEnviron=args.get('logEnviron', True), - user=args.get('user'), ) if args.get('interruptSignal'): c.interruptSignal = args['interruptSignal'] diff --git a/slave/buildslave/runprocess.py b/slave/buildslave/runprocess.py index fadc98301bf..656150be3f1 100644 --- a/slave/buildslave/runprocess.py +++ b/slave/buildslave/runprocess.py @@ -237,7 +237,7 @@ def __init__(self, builder, command, timeout=None, maxTime=None, initialStdin=None, keepStdout=False, keepStderr=False, logEnviron=True, logfiles={}, usePTY="slave-config", - useProcGroup=True, user=None): + useProcGroup=True): """ @param keepStdout: if True, we keep a copy of all the stdout text @@ -372,11 +372,6 @@ def subst(match): follow=follow) self.logFileWatchers.append(w) - if user is not None and runtime.platformType != 'posix': - raise RuntimeError( - "Cannot use 'user' parameter on this platform") - self.user = user - def __repr__(self): return "<%s '%s'>" % (self.__class__.__name__, self.fake_command) @@ -446,11 +441,6 @@ def _startCommand(self): # Attempt to format this for use by a shell, although the process isn't perfect display = shell_quote(self.fake_command) - # If requested, wrap the call in 'sudo' to run the command as a - # different user. - if self.user is not None: - argv = ['sudo', '-u', self.user, '-H'] + argv - # $PWD usually indicates the current directory; spawnProcess may not # update this value, though, so we set it explicitly here. This causes # weird problems (bug #456) on msys, though.. diff --git a/slave/buildslave/test/fake/runprocess.py b/slave/buildslave/test/fake/runprocess.py index e35941da086..e74aed3816f 100644 --- a/slave/buildslave/test/fake/runprocess.py +++ b/slave/buildslave/test/fake/runprocess.py @@ -103,8 +103,7 @@ def __init__(self, builder, command, workdir, **kwargs): sendStdout=True, sendStderr=True, sendRC=True, timeout=None, maxTime=None, initialStdin=None, keepStdout=False, keepStderr=False, - logEnviron=True, logfiles={}, usePTY="slave-config", - user=None) + logEnviron=True, logfiles={}, usePTY="slave-config") if not self._expectations: raise AssertionError("unexpected instantiation: %s" % (kwargs,)) @@ -133,7 +132,6 @@ def __init__(self, builder, command, workdir, **kwargs): self._builder = builder self.stdout = '' self.stderr = '' - self.user = kwargs.get('user') def start(self): # figure out the stdio-related parameters diff --git a/slave/buildslave/test/unit/test_commands_shell.py b/slave/buildslave/test/unit/test_commands_shell.py index 0e1c43b17f2..e5dcac4a5fc 100644 --- a/slave/buildslave/test/unit/test_commands_shell.py +++ b/slave/buildslave/test/unit/test_commands_shell.py @@ -49,33 +49,4 @@ def check(_): d.addCallback(check) return d - def test_user_parameter(self): - """ - Test that the 'user' parameter is threaded through into the - underlying RunProcess object. - """ - - user = 'test' - self.make_command(shell.SlaveShellCommand, dict( - command=[ 'true' ], - workdir='workdir', - user=user, - )) - - self.patch_runprocess( - Expect([ 'true', ], self.basedir_workdir, user=user) - + { 'hdr' : 'headers' } + { 'rc' : 0 } - + 0, - ) - - d = self.run_command() - - # Verify that the 'user' parameter is correctly passed into the - # RunProcess object. - def check(_): - self.assertTrue(hasattr(self.cmd.command, 'user')) - self.assertEqual(self.cmd.command.user, user) - d.addCallback(check) - return d - # TODO: test all functionality that SlaveShellCommand adds atop RunProcess diff --git a/slave/buildslave/test/unit/test_runprocess.py b/slave/buildslave/test/unit/test_runprocess.py index 7000dfecf00..865b7dfe63d 100644 --- a/slave/buildslave/test/unit/test_runprocess.py +++ b/slave/buildslave/test/unit/test_runprocess.py @@ -412,29 +412,6 @@ def testEnvironInt(self): runprocess.RunProcess(b, stdoutCommand('hello'), self.basedir, environ={"BUILD_NUMBER":13})) - @compat.skipUnlessPlatformIs("posix") - def testUserParameter(self): - """ - Test that setting the 'user' parameter causes RunProcess to - wrap the command using 'sudo'. - """ - - user = 'test' - cmd = ['whatever'] - b = FakeSlaveBuilder(False, self.basedir) - s = runprocess.RunProcess(b, cmd, self.basedir, user=user) - - # Override the '_spawnProcess' method so that we can verify - # that the command is run using 'sudo', as we expect. - def _spawnProcess(*args, **kwargs): - executable = args[1] - args = args[2] - self.assertEqual(executable, 'sudo') - self.assertEqual(args, ['sudo', '-u', user, '-H'] + cmd) - s._spawnProcess = _spawnProcess - s.start() - return s.finished(None, 0) - class TestPOSIXKilling(BasedirMixin, unittest.TestCase):