Merge branch 'master' of github.com:djmitche/buildbot

CREDITS

@@ -68,6 +68,7 @@ Phil Thompson
 Philipp Frauenfelder
 Rene Rivera
 Riccardo Magliocchetti
+Robert Collins
 Rob Helmer
 Roch Gadsdon
 Roy Rapoport

buildbot/buildslave.py

@@ -321,7 +321,7 @@ def _disconnected(rref):
 
     def sendBuilderList(self):
         our_builders = self.botmaster.getBuildersForSlave(self.slavename)
-        blist = [(b.name, b.builddir) for b in our_builders]
+        blist = [(b.name, b.slavebuilddir) for b in our_builders]
         d = self.slave.callRemote("setBuilderList", blist)
         return d
 

buildbot/master.py

@@ -6,6 +6,7 @@
     import signal
 except ImportError:
     pass
+import string
 from cPickle import load
 import warnings
 
@@ -642,6 +643,8 @@ def loadConfig(self, f):
         slavenames = [s.slavename for s in slaves]
         buildernames = []
         dirnames = []
+        badchars_map = string.maketrans("\t !#$%&'()*+,./:;<=>?@[\\]^{|}~",
+                                        "______________________________")
         for b in builders:
             if type(b) is tuple:
                 raise ValueError("builder %s must be defined with a dict, "
@@ -657,6 +660,10 @@ def loadConfig(self, f):
                 raise ValueError("duplicate builder name %s"
                                  % b['name'])
             buildernames.append(b['name'])
+
+            # Fix the dictionnary with default values.
+            b.setdefault('builddir', b['name'].translate(badchars_map))
+            b.setdefault('slavebuilddir', b['builddir'])
             if b['builddir'] in dirnames:
                 raise ValueError("builder %s reuses builddir %s"
                                  % (b['name'], b['builddir']))
@@ -892,7 +899,7 @@ def loadConfig_Builders(self, newBuilderData):
         # everything in newList is either unchanged, changed, or new
         for name, data in newList.items():
             old = self.botmaster.builders.get(name)
-            basedir = data['builddir'] # used on both master and slave
+            basedir = data['builddir']
             #name, slave, builddir, factory = data
             if not old: # new
                 # category added after 0.6.2

buildbot/process/builder.py

@@ -370,7 +370,7 @@ def __init__(self, setup, builder_status):
         @type  setup: dict
         @param setup: builder setup data, as stored in
                       BuildmasterConfig['builders'].  Contains name,
-                      slavename(s), builddir, factory, locks.
+                      slavename(s), builddir, slavebuilddir, factory, locks.
         @type  builder_status: L{buildbot.status.builder.BuilderStatus}
         """
         self.name = setup['name']
@@ -380,6 +380,7 @@ def __init__(self, setup, builder_status):
         if setup.has_key('slavenames'):
             self.slavenames.extend(setup['slavenames'])
         self.builddir = setup['builddir']
+        self.slavebuilddir = setup['slavebuilddir']
         self.buildFactory = setup['factory']
         self.nextSlave = setup.get('nextSlave')
         if self.nextSlave is not None and not callable(self.nextSlave):
@@ -432,6 +433,9 @@ def compareToSetup(self, setup):
         if setup['builddir'] != self.builddir:
             diffs.append('builddir changed from %s to %s' \
                          % (self.builddir, setup['builddir']))
+        if setup['slavebuilddir'] != self.slavebuilddir:
+            diffs.append('slavebuilddir changed from %s to %s' \
+                         % (self.slavebuilddir, setup['slavebuilddir']))
         if setup['factory'] != self.buildFactory: # compare objects
             diffs.append('factory changed')
         oldlocks = [(lock.__class__, lock.name)
@@ -919,5 +923,3 @@ def unsubscribe(self, observer):
 
     def cancel(self):
         self.original_builder.cancelBuildRequest(self.original_request)
-
-

buildbot/process/factory.py

@@ -12,6 +12,19 @@ def s(steptype, **kwargs):
     # specification tuples
     return (steptype, kwargs)
 
+class ArgumentsInTheWrongPlace(Exception):
+    """When calling BuildFactory.addStep(stepinstance), addStep() only takes
+    one argument. You passed extra arguments to addStep(), which you probably
+    intended to pass to your BuildStep constructor instead. For example, you
+    should do::
+
+     f.addStep(ShellCommand(command=['echo','stuff'], haltOnFailure=True))
+
+    instead of::
+
+     f.addStep(ShellCommand(command=['echo','stuff']), haltOnFailure=True)
+    """
+
 class BuildFactory(util.ComparableMixin):
     """
     @cvar  buildClass: class to use when creating builds
@@ -42,6 +55,8 @@ def newBuild(self, request):
 
     def addStep(self, step_or_factory, **kwargs):
         if isinstance(step_or_factory, BuildStep):
+            if kwargs:
+                raise ArgumentsInTheWrongPlace()
             s = step_or_factory.getStepFactory()
         else:
             s = (step_or_factory, dict(kwargs))

buildbot/process/subunitlogobserver.py

@@ -0,0 +1,64 @@
+# -*- test-case-name: buildbot.test.test_buildstep -*-
+
+from unittest import TestResult
+
+from buildbot.steps.shell import ShellCommand
+from buildbot.process import buildstep
+
+
+class DiscardStream:
+    """A trivial thunk used to discard passthrough content."""
+
+    def write(self, bytes):
+        pass
+
+
+class SubunitLogObserver(buildstep.LogLineObserver, TestResult):
+    """Observe a log that may contain subunit output.
+
+    This class extends TestResult to receive the callbacks from the subunit
+    parser in the most direct fashion.
+    """
+
+    def __init__(self):
+        buildstep.LogLineObserver.__init__(self)
+        TestResult.__init__(self)
+        try:
+            from subunit import TestProtocolServer
+        except ImportError:
+            raise ImportError("subunit is not importable, but is required for "
+                "SubunitLogObserver support.")
+        self.protocol = TestProtocolServer(self, DiscardStream())
+
+    def outLineReceived(self, line):
+        """Process a received line."""
+        # Impedance mismatch: subunit wants lines, observers get lines-no\n
+        self.protocol.lineReceived(line + '\n')
+
+    def startTest(self, test):
+        TestResult.startTest(self, test)
+        self.step.setProgress('tests', self.testsRun)
+
+    def addError(self, test, err):
+        TestResult.addError(self, test, err)
+        self.issue()
+
+    def addFailure(self, test, err):
+        TestResult.addFailure(self, test, err)
+        self.issue()
+
+    def issue(self):
+        """An issue - failing, erroring etc test."""
+        self.step.setProgress('tests failed', len(self.failures) + len(self.errors))
+
+
+class SubunitShellCommand(ShellCommand):
+    """A ShellCommand that sniffs subunit output.
+
+    Ideally not needed, and thus here to be trivially deleted. See issue #615
+    """
+
+    def __init__(self, *args, **kwargs):
+        ShellCommand.__init__(self, *args, **kwargs)
+        self.addLogObserver('stdio', SubunitLogObserver())
+        self.progressMetrics = self.progressMetrics + ('tests', 'tests failed')

buildbot/status/mail.py

@@ -190,7 +190,7 @@ def __init__(self, fromaddr, mode="all", categories=None, builders=None,
                  subject="buildbot %(result)s in %(projectName)s on %(builder)s",
                  lookup=None, extraRecipients=[],
                  sendToInterestedUsers=True, customMesg=message,
-                 extraHeaders=None):
+                 extraHeaders=None, addPatch=True):
         """
         @type  fromaddr: string
         @param fromaddr: the email address to be used in the 'From' header.
@@ -233,12 +233,16 @@ def __init__(self, fromaddr, mode="all", categories=None, builders=None,
                            categories). Use either builders or categories,
                            but not both.
 
-        @type  addLogs: boolean.
+        @type  addLogs: boolean
         @param addLogs: if True, include all build logs as attachments to the
                         messages.  These can be quite large. This can also be
                         set to a list of log names, to send a subset of the
                         logs. Defaults to False.
 
+        @type  addPatch: boolean
+        @param addPatch: if True, include the patch when the source stamp
+                         includes one.
+
         @type  relayhost: string
         @param relayhost: the host to which the outbound SMTP connection
                           should be made. Defaults to 'localhost'
@@ -340,6 +344,7 @@ def __init__(self, fromaddr, mode="all", categories=None, builders=None,
         if extraHeaders:
             assert isinstance(extraHeaders, dict)
         self.extraHeaders = extraHeaders
+        self.addPatch = addPatch
         self.watched = []
         self.status = None
 
@@ -461,7 +466,7 @@ def buildMessage(self, name, build, results):
         assert type in ('plain', 'html'), "'%s' message type must be 'plain' or 'html'." % type
 
         haveAttachments = False
-        if attrs['patch'] or self.addLogs:
+        if (attrs['patch'] and self.addPatch) or self.addLogs:
             haveAttachments = True
             if not canDoAttachments:
                 twlog.msg("warning: I want to send mail with attachments, "
@@ -485,7 +490,7 @@ def buildMessage(self, name, build, results):
         m['From'] = self.fromaddr
         # m['To'] is added later
 
-        if attrs['patch']:
+        if attrs['patch'] and self.addPatch:
             a = MIMEText(attrs['patch'][1])
             a.add_header('Content-Disposition', "attachment",
                          filename="source patch")