[Buildbot-devel] Upgrade headache: when are start() and finished() called?

[Greg Ward]

Hi all --

I've been running a lightly patched trunk version between 0.7.9 and
0.7.10 for about a year now.  It's about time to upgrade, mainly
because we need doStepIf.  So I'm upgrading stepwise, first to 0.7.10,
then 0.7.11, and I'll probably stop (for now) at 0.7.12.  But I'm
getting burned by a custom step that seems to making now-invalid
assumptions about when start() and finished() are called.  Here's the
code that worked under 0.7.9-ish BuildBot:

class ObservedShellCommand(shell.ShellCommand):
    """
    A ShellCommand that hooks in ErrorObserver to watch for error messages and
    ensure that the build step fails when error messages are seen (unless, of
    course, it already failed for a different reason).
    """
    def start(self):
        shell.ShellCommand.start(self)
        self.__observer = ErrorObserver()
        self.addLogObserver("stdio", self.__observer)

    def finished(self, results):
        # Calling observer.logFinished() has to come *before* calling base class
        # finished(), because after base finished() it's too late for
        # ErrorObserver to write to the BuildStep's "errors" log
        self.__observer.logFinished()
        log.msg("ObservedShellCommand.finished: results=%r" % (results,))
        [...other stuff...]
        shell.ShellCommand.finished(self, results)

Note that my finished() assumes start() was called, i.e.
self.__observer was set.  That assumption was fine under 0.7.9, but
seems to no longer be valid.  In particular, if I use doStepIf to skip
one of these ObservedShellCommand steps, finished() blows up with an
AttributeError accessing self.__observer.  A quick peek in
buildbot/process/buildstep.py (BuildStep._startStep_2()) reveals why:
if the step is skipped, we don't call start() but we do call
finished().  (I'm looking at v0.7.11p3 -- I want to get everything
working there before upgrading to 0.7.12.)

OK, so the contract has changed.  Obviously I can workaround this with
try/except in my finished() method, but that seems kludgey.  My hunch
is that I should be initializing self.__observer somewhere other than
start(): somewhere that is guaranteed to be called once for every
actual build step that either runs or is skipped.  Is there such a
method?

Thanks --

Greg