Merge branch 'defer-doStepIf' of git://github.com/tomprince/buildbot

* 'defer-doStepIf' of git://github.com/tomprince/buildbot:
  Be proactive in failing a step when an exception occurs in the startStep callback chain.
  Allow doStepIf to return a Deferred.

master/buildbot/process/buildstep.py

@@ -740,6 +740,7 @@ def startStep(self, remote):
 
         d = self.acquireLocks()
         d.addCallback(self._startStep_2)
+        d.addErrback(self.failed)
         return self.deferred
 
     def acquireLocks(self, res=None):
@@ -771,29 +772,33 @@ def _startStep_2(self, res):
         if self.progress:
             self.progress.start()
 
+        doStep = defer.succeed(True)
+        if isinstance(self.doStepIf, bool):
+            if not self.doStepIf:
+                doStep = defer.succeed(False)
+        else:
+            doStep = defer.maybeDeferred(self.doStepIf, self)
+        doStep.addCallback(self._startStep_3)
+        return doStep
+
+    def _startStep_3(self, doStep):
         try:
-            skip = None
-            if isinstance(self.doStepIf, bool):
-                if not self.doStepIf:
-                    skip = SKIPPED
-            elif not self.doStepIf(self):
-                skip = SKIPPED
-
-            if skip is None:
-                skip = self.start()
-
-            if skip == SKIPPED:
-                self.step_status.setText(self.describe(True) + ['skipped'])
-                self.step_status.setSkipped(True)
-                # this return value from self.start is a shortcut to finishing
-                # the step immediately; we skip calling finished() as
-                # subclasses may have overridden that an expect it to be called
-                # after start() (bug #837)
-                reactor.callLater(0, self._finishFinished, SKIPPED)
+            if doStep:
+                if self.start() == SKIPPED:
+                    doStep = false
         except:
             log.msg("BuildStep.startStep exception in .start")
             self.failed(Failure())
 
+        if not doStep:
+            self.step_status.setText(self.describe(True) + ['skipped'])
+            self.step_status.setSkipped(True)
+            # this return value from self.start is a shortcut to finishing
+            # the step immediately; we skip calling finished() as
+            # subclasses may have overridden that an expect it to be called
+            # after start() (bug #837)
+            reactor.callLater(0, self._finishFinished, SKIPPED)
+
     def start(self):
         """Begin the step. Override this method and add code to do local
         processing, fire off remote commands, etc.

master/docs/cfg-buildsteps.texinfo

@@ -84,10 +84,10 @@ with @code{haltOnFailure=True} has failed.
 @item doStepIf
 A step can be configured to only run under certain conditions.  To do this, set
 the step's @code{doStepIf} to a boolean value, or to a function that returns a
-boolean value.  If the value or function result is false, then the step will
-return SKIPPED without doing anything.  Oherwise, the step will be executed
-normally.  If you set @code{doStepIf} to a function, that function should
-accept one parameter, which will be the @code{Step} object itself.
+boolean value or Deferred.  If the value or function result is false, then the
+step will return SKIPPED without doing anything.  Oherwise, the step will be
+executed normally.  If you set @code{doStepIf} to a function, that function
+should accept one parameter, which will be the @code{Step} object itself.
 
 @item locks
 a list of Locks (instances of @code{buildbot.locks.SlaveLock} or