Merge branch 'master' into buildbot-0.8.0

* master:
  Adding retrying logic for sqlite
  Add comments about why we're doing unicode encoding
  Fix running .exes under windows when we don't have an abolute path
  obfuscate P4 passwords - fixes buildbot#846
  Fix error handling for _startCommand so that tracebacks get back to the

buildbot/db/dbspec.py

@@ -84,6 +84,62 @@ def disconnect(self, conn):
         tid = self.threadID()
         del self.connection_lastused[tid]
 
+class TimeoutError(Exception):
+    def __init__(self, msg):
+        Exception.__init__(self, msg)
+
+class RetryingCursor:
+    max_retry_time = 1800 # Half an hour
+    max_sleep_time = 1
+
+    def __init__(self, dbapi, cursor):
+        self.dbapi = dbapi
+        self.cursor = cursor
+
+    def sleep(self, s):
+        time.sleep(s)
+
+    def execute(self, *args, **kw):
+        start_time = util.now()
+        sleep_time = 0.1
+        while True:
+            try:
+                query_start_time = util.now()
+                result = self.cursor.execute(*args, **kw)
+                end_time = util.now()
+                if end_time - query_start_time > 2:
+                    log.msg("Long query (%is): %s" % ((end_time - query_start_time), str((args, kw))))
+                return result
+            except self.dbapi.OperationalError, e:
+                if e.args[0] == 'database is locked':
+                    # Retry
+                    log.msg("Retrying query %s" % str((args, kw)))
+                    now = util.now()
+                    if start_time + self.max_retry_time < now:
+                        raise TimeoutError("Exceeded timeout trying to do %s" % str((args, kw)))
+                    self.sleep(sleep_time)
+                    sleep_time = max(self.max_sleep_time, sleep_time * 2)
+                    continue
+                raise
+
+    def __getattr__(self, name):
+        return getattr(self.cursor, name)
+
+class RetryingConnection:
+    def __init__(self, dbapi, conn):
+        self.dbapi = dbapi
+        self.conn = conn
+
+    def cursor(self):
+        return RetryingCursor(self.dbapi, self.conn.cursor())
+
+    def __getattr__(self, name):
+        return getattr(self.conn, name)
+
+class RetryingConnectionPool(adbapi.ConnectionPool):
+    def connect(self):
+        return RetryingConnection(self.dbapi, adbapi.ConnectionPool.connect(self))
+
 class DBSpec(object):
     """
     A specification for the database type and other connection parameters.
@@ -195,6 +251,8 @@ def get_sync_connection(self):
             if arg in connkw:
                 del connkw[arg]
         conn = dbapi.connect(*self.connargs, **connkw)
+        if 'sqlite' in self.dbapiName:
+            conn = RetryingConnection(dbapi, conn)
         return conn
 
     def get_async_connection_pool(self):
@@ -221,7 +279,7 @@ def get_async_connection_pool(self):
         if self.dbapiName == 'MySQLdb':
             return ExpiringConnectionPool(self.dbapiName, *self.connargs, **connkw)
         else:
-            return adbapi.ConnectionPool(self.dbapiName, *self.connargs, **connkw)
+            return RetryingConnectionPool(self.dbapiName, *self.connargs, **connkw)
 
     def get_maxidle(self):
         default = None

buildbot/slave/commands/base.py

@@ -1,6 +1,6 @@
 # -*- test-case-name: buildbot.test.test_slavecommand -*-
 
-import os, signal, types, time, re
+import os, signal, types, time, re, traceback
 from stat import ST_CTIME, ST_MTIME, ST_SIZE
 from collections import deque
 
@@ -283,6 +283,17 @@ def __init__(self, builder, command,
 
         self.builder = builder
         self.command = Obfuscated.get_real(command)
+
+        # We need to take unicode commands and arguments and encode them using
+        # the appropriate encoding for the slave.  This is mostly platform
+        # specific, but can be overridden in the slave's buildbot.tac file.
+        #
+        # Encoding the command line here ensures that the called executables
+        # receive arguments as bytestrings encoded with an appropriate
+        # platform-specific encoding.  It also plays nicely with twisted's
+        # spawnProcess which checks that arguments are regular strings or
+        # unicode strings that can be encoded as ascii (which generates a
+        # warning).
         if isinstance(self.command, (tuple, list)):
             for i, a in enumerate(self.command):
                 if isinstance(a, unicode):
@@ -390,6 +401,9 @@ def start(self):
         except:
             log.msg("error in ShellCommand._startCommand")
             log.err()
+            self._addToBuffers('stderr', "error in ShellCommand._startCommand\n")
+            self._addToBuffers('stderr', traceback.format_exc())
+            self._sendBuffers()
             # pretend it was a shell error
             self.deferred.errback(AbandonChain(-1))
         return self.deferred
@@ -419,7 +433,13 @@ def _startCommand(self):
                 argv = ['/bin/sh', '-c', self.command]
             display = self.fake_command
         else:
-            if runtime.platformType  == 'win32' and not self.command[0].lower().endswith(".exe"):
+            # On windows, CreateProcess requires an absolute path to the executable.
+            # When we call spawnProcess below, we pass argv[0] as the executable.
+            # So, for .exe's that we have absolute paths to, we can call directly
+            # Otherwise, we should run under COMSPEC (usually cmd.exe) to
+            # handle path searching, etc.
+            if runtime.platformType == 'win32' and not \
+                    (self.command[0].lower().endswith(".exe") and os.path.isabs(self.command[0])):
                 argv = os.environ['COMSPEC'].split() # allow %COMSPEC% to have args
                 if '/c' not in argv: argv += ['/c']
                 argv += list(self.command)

buildbot/slave/commands/vcs.py

@@ -1695,7 +1695,7 @@ def parseGotRevision(self):
         if self.p4user:
             command.extend(['-u', self.p4user])
         if self.p4passwd:
-            command.extend(['-P', self.p4passwd])
+            command.extend(['-P', Obfuscated(self.p4passwd, "XXXXXXXX")])
         if self.p4client:
             command.extend(['-c', self.p4client])
         # add '-s submitted' for bug #626
@@ -1777,7 +1777,7 @@ def _doP4Sync(self, force):
         if self.p4user:
             command.extend(['-u', self.p4user])
         if self.p4passwd:
-            command.extend(['-P', self.p4passwd])
+            command.extend(['-P', Obfuscated(self.p4passwd, "XXXXXXXX")])
         if self.p4client:
             command.extend(['-c', self.p4client])
         command.extend(['sync'])
@@ -1820,7 +1820,7 @@ def doVCFull(self):
         if self.p4user:
             command.extend(['-u', self.p4user])
         if self.p4passwd:
-            command.extend(['-P', self.p4passwd])
+            command.extend(['-P', Obfuscated(self.p4passwd, "XXXXXXXX")])
         command.extend(['client', '-i'])
         log.msg(client_spec)
         c = ShellCommand(self.builder, command, self.builder.basedir,
@@ -1870,7 +1870,7 @@ def _doVC(self, force):
         if self.p4user:
             command.extend(['-u', self.p4user])
         if self.p4passwd:
-            command.extend(['-P', self.p4passwd])
+            command.extend(['-P', Obfuscated(self.p4passwd, "XXXXXXXX")])
         if self.p4client:
             command.extend(['-c', self.p4client])
         command.extend(['sync'])

buildbot/test/unit/test_slave_commands_base.py

@@ -162,6 +162,14 @@ def testBadCommand(self):
         def check(err):
             self.flushLoggedErrors()
             err.trap(AbandonChain)
+            stderr = []
+            # Here we're checking that the exception starting up the command
+            # actually gets propogated back to the master.
+            for u in b.updates:
+                if 'stderr' in u:
+                    stderr.append(u['stderr'])
+            stderr = "".join(stderr)
+            self.failUnless("TypeError" in stderr, stderr)
         d.addBoth(check)
         return d