Use magic __new__ for step factories.

master/buildbot/config.py

@@ -552,7 +552,7 @@ def check_lock(l):
             if not hasattr(b.factory, 'steps'):
                 continue
             for s in b.factory.steps:
-                for l in s[1].get('locks', []):
+                for l in s[2].get('locks', []):
                     check_lock(l)
 
 

master/buildbot/process/build.py

@@ -297,13 +297,14 @@ def setupBuild(self, expectations):
         stepnames = {}
         sps = []
 
-        for factory, args in self.stepFactories:
-            args = args.copy()
+        for factory, args, kwargs in self.stepFactories:
+            args = tuple(args)
+            kwargs = kwargs.copy()
             try:
-                step = factory(**args)
+                step = factory(*args, **kwargs)
             except:
-                log.msg("error while creating step, factory=%s, args=%s"
-                        % (factory, args))
+                log.msg("error while creating step, factory=%s, args=%s, kwargs=%s"
+                        % (factory, args, kwargs))
                 raise
 
             step.setBuild(self)

master/buildbot/process/buildstep.py

@@ -381,7 +381,7 @@ def _start(self):
     def __repr__(self):
         return "<RemoteShellCommand '%s'>" % repr(self.command)
 
-class BuildStep(properties.PropertiesMixin):
+class BuildStep(object, properties.PropertiesMixin):
 
     haltOnFailure = False
     flunkOnWarnings = False
@@ -424,7 +424,6 @@ class BuildStep(properties.PropertiesMixin):
     progress = None
 
     def __init__(self, **kwargs):
-        self.factory = (self.__class__, dict(kwargs))
         for p in self.__class__.parms:
             if kwargs.has_key(p):
                 setattr(self, p, kwargs[p])
@@ -438,6 +437,12 @@ def __init__(self, **kwargs):
         self._acquiringLock = None
         self.stopped = False
 
+    def __new__(klass, *args, **kwargs):
+        self = object.__new__(klass)
+        klass.__init__(self, *args, **kwargs)
+        self.factory = (klass, args, kwargs)
+        return self
+
     def describe(self, done=False):
         return [self.name]
 
@@ -451,7 +456,8 @@ def setDefaultWorkdir(self, workdir):
         pass
 
     def addFactoryArguments(self, **kwargs):
-        self.factory[1].update(kwargs)
+        # this is here for backwards compatability
+        pass
 
     def getStepFactory(self):
         return self.factory

master/buildbot/test/unit/test_config.py

@@ -706,7 +706,7 @@ def bldr(name):
             b = mock.Mock()
             b.name = name
             b.locks = []
-            b.factory.steps = [ ('cls', dict(locks=[])) ]
+            b.factory.steps = [ ('cls', (), dict(locks=[])) ]
             return b
 
         def lock(name):
@@ -721,7 +721,7 @@ def lock(name):
             if dup_builder_lock:
                 b2.locks.append(lock(builder_lock))
         if step_lock:
-            s1, s2 = b1.factory.steps[0][1], b2.factory.steps[0][1]
+            s1, s2 = b1.factory.steps[0][2], b2.factory.steps[0][2]
             s1['locks'].append(lock(step_lock))
             if dup_step_lock:
                 s2['locks'].append(lock(step_lock))

master/buildbot/test/unit/test_process_build.py

@@ -102,7 +102,7 @@ def testRunSuccessfulBuild(self):
         step = Mock()
         step.return_value = step
         step.startStep.return_value = SUCCESS
-        b.setStepFactories([(step, {})])
+        b.setStepFactories([(step, (), {})])
 
         slavebuilder = Mock()
 
@@ -117,7 +117,7 @@ def testStopBuild(self):
 
         step = Mock()
         step.return_value = step
-        b.setStepFactories([(step, {})])
+        b.setStepFactories([(step, (), {})])
 
         slavebuilder = Mock()
 
@@ -148,8 +148,8 @@ def testAlwaysRunStepStopBuild(self):
         step2.return_value = step2
         step2.alwaysRun = True
         b.setStepFactories([
-            (step1, {}),
-            (step2, {}),
+            (step1, (), {}),
+            (step2, (), {}),
             ])
 
         slavebuilder = Mock()
@@ -196,7 +196,7 @@ def claim(owner, access):
         step = Mock()
         step.return_value = step
         step.startStep.return_value = SUCCESS
-        b.setStepFactories([(step, {})])
+        b.setStepFactories([(step, (), {})])
 
         b.startBuild(FakeBuildStatus(), None, slavebuilder)
 
@@ -225,7 +225,7 @@ def claim(owner, access):
         step = Mock()
         step.return_value = step
         step.startStep.return_value = SUCCESS
-        b.setStepFactories([(step, {})])
+        b.setStepFactories([(step, (), {})])
 
         real_lock.claim(Mock(), l.access('counting'))
 
@@ -252,7 +252,7 @@ def testStopBuildWaitingForLocks(self):
         step.return_value = step
         step.startStep.return_value = SUCCESS
         step.alwaysRun = False
-        b.setStepFactories([(step, {})])
+        b.setStepFactories([(step, (), {})])
 
         real_lock.claim(Mock(), l.access('counting'))
 
@@ -318,7 +318,7 @@ def testStopBuildWaitingForStepLocks(self):
         step = LoggingBuildStep(locks=[lock_access])
         def factory(*args):
             return step
-        b.setStepFactories([(factory, {})])
+        b.setStepFactories([(factory, (), {})])
 
         real_lock.claim(Mock(), l.access('counting'))
 

master/buildbot/test/util/steps.py

@@ -73,8 +73,8 @@ def setupStep(self, step, slave_version={'*':"99.99"}, slave_env={}):
         """
         # yes, Virginia, "factory" refers both to the tuple and its first
         # element TODO: fix that up
-        factory, args = step.getStepFactory()
-        step = self.step = factory(**args)
+        factory, args, kwargs = step.getStepFactory()
+        step = self.step = factory(*args, **kwargs)
 
         # step.build