add build.buildid, sl.buildslaveid, and st.stepid

This required a lot of refactoring of old tests.  It also required
changing the automatic generation of source step names to generate
proper identifiers.

master/buildbot/buildslave/base.py

@@ -316,10 +316,8 @@ def updateSlave(self):
             return defer.succeed(None)
 
     def updateSlaveStatus(self, buildStarted=None, buildFinished=None):
-        if buildStarted:
-            self.slave_status.buildStarted(buildStarted)
-        if buildFinished:
-            self.slave_status.buildFinished(buildFinished)
+        # TODO
+        pass
 
     @defer.inlineCallbacks
     def attached(self, conn):

master/buildbot/process/build.py

@@ -86,6 +86,8 @@ def __init__(self, requests):
         self.progress = None
         self.currentStep = None
         self.slaveEnvironment = {}
+        self.buildid = None
+        self.number = None
 
         self.terminate = False
 
@@ -109,9 +111,6 @@ def setLocks(self, lockList):
     def setSlaveEnvironment(self, env):
         self.slaveEnvironment = env
 
-    def setBuilDBId(self, dbid):
-        self._dbid = dbid
-
     def getSourceStamp(self, codebase=''):
         for source in self.sources:
             if source.codebase == codebase:
@@ -121,11 +120,6 @@ def getSourceStamp(self, codebase=''):
     def getAllSourceStamps(self):
         return list(self.sources)
 
-    def getBuilDBId(self):
-        # TODO:
-        # Maybe is needed to implement a function in the DATA API to retreive the id
-        return self._dbid
-
     def allChanges(self):
         for s in self.sources:
             for c in s.changes:
@@ -198,7 +192,7 @@ def setupProperties(self):
 
         # now set some properties of our own, corresponding to the
         # build itself
-        props.setProperty("buildnumber", self.build_status.number, "Build")
+        props.setProperty("buildnumber", self.number, "Build")
 
         if self.sources and len(self.sources) == 1:
             # old interface for backwards compatibility
@@ -230,10 +224,12 @@ def setupSlaveBuilder(self, slavebuilder):
         self.slavename = slavebuilder.slave.slavename
         self.build_status.setSlavename(self.slavename)
 
+    @defer.inlineCallbacks
     def startBuild(self, build_status, expectations, slavebuilder):
         """This method sets up the build, then starts it by invoking the
         first Step. It returns a Deferred which will fire when the build
         finishes. This Deferred is guaranteed to never errback."""
+        slave = slavebuilder.slave
 
         # we are taking responsibility for watching the connection to the
         # remote. This responsibility was held by the Builder until our
@@ -242,10 +238,19 @@ def startBuild(self, build_status, expectations, slavebuilder):
 
         log.msg("%s.startBuild" % self)
         self.build_status = build_status
+        # TODO: this will go away when build collapsing is implemented; until
+        # then we just assign the bulid to the first buildrequest
+        brid = self.requests[0].id
+        self.buildid, self.number = \
+            yield self.master.data.updates.newBuild(
+                builderid=(yield self.builder.getBuilderId()),
+                buildrequestid=brid,
+                buildslaveid=slave.buildslaveid)
+
         # now that we have a build_status, we can set properties
         self.setupProperties()
         self.setupSlaveBuilder(slavebuilder)
-        slavebuilder.slave.updateSlaveStatus(buildStarted=build_status)
+        slave.updateSlaveStatus(buildStarted=self)
 
         # then narrow SlaveLocks down to the right slave
         self.locks = [(l.getLock(self.slavebuilder.slave), a)
@@ -255,18 +260,7 @@ def startBuild(self, build_status, expectations, slavebuilder):
 
         metrics.MetricCountEvent.log('active_builds', 1)
 
-        d = self.deferred = defer.Deferred()
-
-        def _uncount_build(res):
-            metrics.MetricCountEvent.log('active_builds', -1)
-            return res
-        d.addBoth(_uncount_build)
-
-        def _release_slave(res, slave, bs):
-            self.slavebuilder.buildFinished()
-            slave.updateSlaveStatus(buildFinished=bs)
-            return res
-        d.addCallback(_release_slave, self.slavebuilder.slave, build_status)
+        self.deferred = defer.Deferred()
 
         try:
             self.setupBuild(expectations)  # create .steps
@@ -279,19 +273,27 @@ def _release_slave(res, slave, bs):
             # handler
             log.msg("Build.setupBuild failed")
             log.err(Failure())
-            self.builder.builder_status.addPointEvent(["setupBuild",
-                                                       "exception"])
             self.finished = True
             self.results = EXCEPTION
             self.deferred = None
-            d.callback(self)
-            return d
+            return
 
-        self.master.data.updates.setBuildStateStrings(self.getBuilDBId(), [u'starting'])
+        self.master.data.updates.setBuildStateStrings(self.buildid, [u'starting'])
         self.build_status.buildStarted(self)
+        yield self.acquireLocks()
+
+        try:
+            # start the sequence of steps and wait until it's finished
+            self.startNextStep()
+            yield self.deferred
+        finally:
+            metrics.MetricCountEvent.log('active_builds', -1)
+
+        yield self.master.db.builds.finishBuild(self.buildid, self.results)
 
-        self.acquireLocks().addCallback(self._startBuild_2)
-        return d
+        # mark the build as finished
+        self.slavebuilder.buildFinished()
+        slave.updateSlaveStatus(buildFinished=self)
 
     @staticmethod
     def canStartWithSlavebuilder(lockList, slavebuilder):
@@ -320,9 +322,6 @@ def acquireLocks(self, res=None):
             lock.claim(self, access)
         return defer.succeed(None)
 
-    def _startBuild_2(self, res):
-        self.startNextStep()
-
     def setupBuild(self, expectations):
         # create the actual BuildSteps. If there are any name collisions, we
         # add a count to the loser until it is unique.
@@ -509,7 +508,6 @@ def stopBuild(self, reason="<no reason given>"):
         if self.finished:
             return
         # TODO: include 'reason' in this point event
-        self.builder.builder_status.addPointEvent(['interrupt'])
         self.stopped = True
         if self.currentStep:
             self.currentStep.interrupt(reason)

master/buildbot/process/builder.py

@@ -307,10 +307,6 @@ def canStartBuild(self, slavebuilder, breq):
 
     @defer.inlineCallbacks
     def _startBuildFor(self, slavebuilder, buildrequests):
-        # get some ID's for use later
-        buildslaveid = slavebuilder.slave.buildslaveid
-        builderid = yield self.getBuilderId()
-
         # Build a stack of cleanup functions so that, at any point, we can
         # abort this operation and unwind the commitments made so far.
         cleanups = []
@@ -327,8 +323,6 @@ def run_cleanups():
         # status based on any other cleanup
         cleanups.append(lambda: self.updateBigStatus())
 
-        builderid = yield self.getBuilderId()
-
         build = self.config.factory.newBuild(buildrequests)
         build.setBuilder(self)
         log.msg("starting build %s using slave %s" % (build, slavebuilder))
@@ -402,24 +396,6 @@ def run_cleanups():
         # create the BuildStatus object that goes with the Build
         bs = self.builder_status.newBuild()
 
-        # record the build in the db, but only for the last buildrequest
-        # (NOTE: this is a behavior change that will cause unexpected results
-        #  in the nine branch!)
-        # (NOTE: the build number the db assigns may not be the same that the
-        #  builder status assigns!)
-        try:
-            bids = []
-            req = build.requests[-1]
-            bid, number = yield self.master.data.updates.newBuild(builderid=builderid,
-                                                                  buildrequestid=req.id, buildslaveid=buildslaveid)
-            bids.append(bid)
-            build.setBuilDBId(bid)
-        except:
-            log.err(failure.Failure(), 'while adding rows to build table:')
-            run_cleanups()
-            defer.returnValue(False)
-            return
-
         # IMPORTANT: no yielding is allowed from here to the startBuild call!
 
         # it's possible that we lost the slave remote between the ping above
@@ -432,9 +408,6 @@ def run_cleanups():
             defer.returnValue(False)
             return
 
-        # let status know
-        self.master.status.build_started(req.id, self.name, bs)
-
         # start the build. This will first set up the steps, then tell the
         # BuildStatus that it has started, which will announce it to the world
         # (through our BuilderStatus object, which is its parent).  Finally it
@@ -445,7 +418,7 @@ def run_cleanups():
         # http://trac.buildbot.net/ticket/2428).
         d = defer.maybeDeferred(build.startBuild,
                                 bs, self.expectations, slavebuilder)
-        d.addCallback(self.buildFinished, slavebuilder, bids)
+        d.addCallback(lambda _: self.buildFinished(build, slavebuilder))
         # this shouldn't happen. if it does, the slave will be wedged
         d.addErrback(log.err, 'from a running build; this is a '
                      'serious error - please file a bug at http://buildbot.net')
@@ -463,7 +436,7 @@ def setupProperties(self, props):
                                   self.config.properties[propertyname],
                                   "Builder")
 
-    def buildFinished(self, build, sb, bids):
+    def buildFinished(self, build, sb):
         """This is called when the Build has finished (either success or
         failure). Any exceptions during the build are reported with
         results=FAILURE, not with an errback."""
@@ -474,15 +447,6 @@ def buildFinished(self, build, sb, bids):
 
         results = build.build_status.getResults()
 
-        # mark the builds as finished, although since nothing ever reads this
-        # table, it's not too important that it complete successfully
-        # TODO: The www service use this table
-        #       So, I think it's important that it complete successfully.
-        #       tardyp, djmitche do you confirm ?
-        d = self.master.data.updates.finishBuild(bids[0], results)
-        d.addCallback(lambda _: self.master.data.updates.setBuildStateStrings(bids[0], [u'finished']))
-        d.addErrback(log.err, 'while marking builds as finished (ignored)')
-
         self.building.remove(build)
         if results == RETRY:
             d = self._resubmit_buildreqs(build)

master/buildbot/process/buildstep.py

@@ -287,6 +287,8 @@ def setStepStatus(self, step_status):
 
     def setupProgress(self):
         if self.useProgress:
+            # XXX this uses self.name, but the name may change when the
+            # step is started..
             sp = progress.StepProgress(self.name, self.progressMetrics)
             self.progress = sp
             self.step_status.setProgress(sp)
@@ -305,6 +307,15 @@ def setStateStrings(self, strings):
     @defer.inlineCallbacks
     def startStep(self, remote):
         self.remote = remote
+
+        # create and start the step, noting that the name may be altered to
+        # ensure uniqueness
+        # XXX self.number != self.step_status.number..
+        self.stepid, self.number, self.name = yield self.master.data.updates.newStep(
+            buildid=self.build.buildid,
+            name=util.ascii2unicode(self.name))
+        yield self.master.data.updates.startStep(self.stepid)
+
         # convert all locks into their real form
         self.locks = [(self.build.builder.botmaster.getLockByID(access.lockid), access)
                       for access in self.locks]

master/buildbot/process/remotecommand.py

@@ -13,7 +13,6 @@
 #
 # Copyright Buildbot Team Members
 
-from buildbot import interfaces
 from buildbot import util
 from buildbot.process import metrics
 from buildbot.status.results import FAILURE

master/buildbot/steps/source/base.py

@@ -104,7 +104,7 @@ def __init__(self, workdir=None, mode='update', alwaysUseLatest=False,
 
         self.codebase = codebase
         if self.codebase:
-            self.name = ' '.join((self.name, self.codebase))
+            self.name = '-'.join((self.name, self.codebase))
 
         self.alwaysUseLatest = alwaysUseLatest
 

master/buildbot/test/fake/fakebuild.py

@@ -41,6 +41,8 @@ def __init__(self, props=None):
         self.build_status = FakeBuildStatus()
         self.builder = mock.Mock(name='build.builder')
         self.path_module = posixpath
+        self.buildid = 92
+        self.number = 13
 
         self.sources = {}
         if props is None:

master/buildbot/test/fake/fakedata.py

@@ -252,7 +252,12 @@ def newStep(self, buildid, name):
                               validation.IntValidator())
         validation.verifyType(self.testcase, 'name', name,
                               validation.IdentifierValidator(50))
-        return defer.succeed((10, 1))
+        return defer.succeed((10, 1, name))
+
+    def startStep(self, stepid):
+        validation.verifyType(self.testcase, 'stepid', stepid,
+                              validation.IntValidator())
+        return defer.succeed(None)
 
     def setStepStateStrings(self, stepid, state_strings):
         validation.verifyType(self.testcase, 'stepid', stepid,
@@ -320,10 +325,6 @@ def buildslaveDisconnected(self, buildslaveid, masterid):
             buildslaveid=buildslaveid,
             masterid=masterid)
 
-    def __getattr__(self, name):
-        import traceback
-        traceback.print_stack()
-
 
 class FakeDataConnector(object):
     # FakeDataConnector delegates to the real DataConnector so it can get all

master/buildbot/test/fake/slave.py

@@ -27,6 +27,10 @@ def __init__(self, master):
         self.master = master
         self.conn = fakeprotocol.FakeConnection(master, self)
         self.properties = properties.Properties()
+        self.buildslaveid = 383
+
+    def updateSlaveStatus(self, buildStarted=None, buildFinished=None):
+        pass
 
     def acquireLocks(self):
         pass

master/buildbot/test/integration/test_custom_buildstep.py

@@ -146,8 +146,8 @@ def do_test_step(self):
         bfd = defer.Deferred()
         old_buildFinished = self.builder.buildFinished
 
-        def buildFinished(build, sb, bids):
-            old_buildFinished(build, sb, bids)
+        def buildFinished(*args):
+            old_buildFinished(*args)
             bfd.callback(None)
         self.builder.buildFinished = buildFinished
 

master/buildbot/test/unit/test_data_steps.py

@@ -170,12 +170,25 @@ def test_signature_newStep(self):
         def newStep(self, buildid, name):
             pass
 
+    def test_signature_startStep(self):
+        @self.assertArgSpecMatches(
+            self.master.data.updates.startStep,  # fake
+            self.rtype.startStep)  # real
+        def newStep(self, stepid):
+            pass
+
     def test_newStep(self):
         self.do_test_callthrough('addStep', self.rtype.newStep,
                                  buildid=10, name=u'name',
                                  exp_kwargs=dict(buildid=10, name=u'name',
                                                  state_strings=['pending']))
 
+    @defer.inlineCallbacks
+    def test_fake_newStep(self):
+        self.assertEqual(
+            len((yield self.master.data.updates.newStep(10, u'ten'))),
+            3)
+
     def test_startStep(self):
         self.do_test_callthrough('startStep', self.rtype.startStep,
                                  stepid=10,