buildbot: correct monotone source step

Markus Wanner <[email protected]> Fri, 17 Apr 2015 15:02:55 +0200

Newsgroups	gmane.comp.version-control.monotone.devel,gmane.comp.python.buildbot.devel
Message-ID	<[email protected]>

Hi,

on buildbot.monotone.ca, a buildbot master instance is running. As
recently brought up on IRC, I had issues with it since I switched to use
the newer server-side source step.

The attached patch fixes two issues in steps/source/mtn.py:

 a) in _sourcedirIsUpdatable, the inline function cont served as a
    callback to the deferred returned by _sourcedirIsUpdatable. However,
    it uses d.addCallback, where d is what's calling it. I.e. at the
    time of execution, d is already called. It seems strange to add
    another callback. Further, and probably the actual bug: It doesn't
    return a value. I think that's what led to the hang I originally
    discovered.

    I fixed it rewriting _sourcedirIsUpdatable as an inlined
    callback method, which first checks for the existence of both, the
    _MTN directory and the database db.mtn, and only decides *after*
    both checks. I added proper logging to simplify debugging in the
    future.

 b) startVC calls _checkDb, which in turn runs 'mtn db info' on the
    database db.mtn. However, if the database doesn't exist, yet, that
    command fails, writing an error message to stderr (not stdout).
    This aborts the entire step, rather than creating and filling the
    database.

    The fix in the patch is not optimal: I simply set abandonOnFailure
    to False. A better solution would be to check for the existence of
    the database db.mtn first (that's done in _sourcedirIsUpdatable,
    anyways) and only run 'mtn db info' if it is known to exist. (Or
    skip the existence test and properly parse stderr. However, that
    seems prone to error, IMO.)

The patch as attached applies to buildbot release 0.8.10. I'm happy to
test an improved variant of it, if necessary.

Regards

Markus Wanner

_______________________________________________
Monotone-devel mailing list
[email protected]
https://lists.nongnu.org/mailman/listinfo/monotone-devel

buildbot_mtn_fix.diff (text/x-patch, 1.9 KB)

*** a/buildbot/steps/source/mtn.py.orig	2015-04-16 22:27:34.336585869 +0200
--- b/buildbot/steps/source/mtn.py	2015-04-17 14:33:35.539813501 +0200
***************
*** 319,325 ****
          return d
  
      def _checkDb(self):
!         d = self._dovccmd(['mtn', 'db', 'info', '--db', self.database], collectStdout=True)
  
          def checkInfo(stdout):
              if stdout.find("does not exist") > 0:
--- 319,327 ----
          return d
  
      def _checkDb(self):
!         d = self._dovccmd(['mtn', 'db', 'info', '--db', self.database],
!                           collectStdout=True,
!                           abandonOnFailure=False)
  
          def checkInfo(stdout):
              if stdout.find("does not exist") > 0:
***************
*** 337,352 ****
          d.addCallback(checkInfo)
          return d
  
      def _sourcedirIsUpdatable(self):
!         d = self.pathExists(self.build.path_module.join(self.workdir, '_MTN'))
  
!         def cont(res):
!             if res:
!                 d.addCallback(lambda _: self.pathExists('db.mtn'))
!             else:
!                 return False
!         d.addCallback(cont)
!         return d
  
      def finish(self, res):
          d = defer.succeed(res)
--- 339,358 ----
          d.addCallback(checkInfo)
          return d
  
+     @defer.inlineCallbacks
      def _sourcedirIsUpdatable(self):
!         workdir_path = self.build.path_module.join(self.workdir, '_MTN')
!         workdir_exists = yield self.pathExists(workdir_path)
  
!         db_path = self.build.path_module.join(self.workdir, self.database)
!         db_exists = yield self.pathExists(db_path)
! 
!         if not db_exists:
!             log.msg("Database does not exist, fallback to a fresh clone")
!         if not workdir_exists:
!             log.msg("Workdir does not exist, fallback to a fresh clone")
! 
!         defer.returnValue(db_exists and workdir_exists)
  
      def finish(self, res):
          d = defer.succeed(res)

signature.asc (application/pgp-signature, 1.5 KB)