Merge pull request #7302 from p12tic/builderid-prop

Implement builderid build property

master/buildbot/process/build.py

@@ -205,8 +205,13 @@ def getWorkerName(self):
         return self.workername
 
     @staticmethod
-    def setupPropertiesKnownBeforeBuildStarts(props, requests, builder,
-                                              workerforbuilder=None):
+    @defer.inlineCallbacks
+    def setup_properties_known_before_build_starts(
+        props,
+        requests,
+        builder,
+        workerforbuilder=None
+    ):
         # Note that this function does not setup the 'builddir' worker property
         # It's not possible to know it until before the actual worker has
         # attached.
@@ -225,7 +230,7 @@ def setupPropertiesKnownBeforeBuildStarts(props, requests, builder,
             props.updateFromProperties(rq.properties)
 
         # get builder properties
-        builder.setupProperties(props)
+        yield builder.setup_properties(props)
 
         # get worker properties
         # navigate our way back to the L{buildbot.worker.Worker}

master/buildbot/process/builder.py

@@ -304,6 +304,17 @@ def detached(self, worker):
     def getAvailableWorkers(self):
         return [wfb for wfb in self.workers if wfb.isAvailable()]
 
+    @defer.inlineCallbacks
+    def _setup_props_if_needed(self, props, workerforbuilder, buildrequest):
+        # don't unnecessarily setup properties for build
+        if props is not None:
+            return props
+        props = Properties()
+        yield Build.setup_properties_known_before_build_starts(
+            props, [buildrequest], self, workerforbuilder
+        )
+        return props
+
     @defer.inlineCallbacks
     def canStartBuild(self, workerforbuilder, buildrequest):
         can_start = True
@@ -314,28 +325,19 @@ def canStartBuild(self, workerforbuilder, buildrequest):
         worker = workerforbuilder.worker
         props = None
 
-        # don't unnecessarily setup properties for build
-        def setupPropsIfNeeded(props):
-            if props is not None:
-                return props
-            props = Properties()
-            Build.setupPropertiesKnownBeforeBuildStarts(props, [buildrequest],
-                                                        self, workerforbuilder)
-            return props
-
         if worker.builds_may_be_incompatible:
             # Check if the latent worker is actually compatible with the build.
             # The instance type of the worker may depend on the properties of
             # the build that substantiated it.
-            props = setupPropsIfNeeded(props)
+            props = yield self._setup_props_if_needed(props, workerforbuilder, buildrequest)
             can_start = yield worker.isCompatibleWithBuild(props)
             if not can_start:
                 return False
 
         if IRenderable.providedBy(locks):
             # collect properties that would be set for a build if we
             # started it now and render locks using it
-            props = setupPropsIfNeeded(props)
+            props = yield self._setup_props_if_needed(props, workerforbuilder, buildrequest)
             locks = yield props.render(locks)
 
         locks = yield self.botmaster.getLockFromLockAccesses(locks, self.config_version)
@@ -360,8 +362,9 @@ def _startBuildFor(self, workerforbuilder, buildrequests):
         # give the properties a reference back to this build
         props.build = build
 
-        Build.setupPropertiesKnownBeforeBuildStarts(
-            props, build.requests, build.builder, workerforbuilder)
+        yield Build.setup_properties_known_before_build_starts(
+            props, build.requests, build.builder, workerforbuilder
+        )
 
         log.msg(f"starting build {build} using worker {workerforbuilder}")
 
@@ -394,8 +397,12 @@ def _startBuildFor(self, workerforbuilder, buildrequests):
 
         return True
 
-    def setupProperties(self, props):
+    @defer.inlineCallbacks
+    def setup_properties(self, props):
+        builderid = yield self.getBuilderId()
+
         props.setProperty("buildername", self.name, "Builder")
+        props.setProperty("builderid", builderid, "Builder")
         if self.config.properties:
             for propertyname in self.config.properties:
                 props.setProperty(propertyname,

master/buildbot/reporters/generators/buildrequest.py

@@ -54,7 +54,7 @@ def partial_build_dict(self, master, buildrequest):
         buildrequest = yield BuildRequest.fromBrdict(master, brdict)
         builder = yield master.botmaster.getBuilderById(brdict['builderid'])
 
-        Build.setupPropertiesKnownBeforeBuildStarts(props, [buildrequest], builder)
+        yield Build.setup_properties_known_before_build_starts(props, [buildrequest], builder)
         Build.setupBuildProperties(props, [buildrequest])
 
         bdict['properties'] = props.asDict()

master/buildbot/test/integration/test_custom_buildstep.py

@@ -61,6 +61,14 @@ def log_consumer(self):
         raise RuntimeError('fail')
 
 
+class SucceedingCustomStep(buildstep.BuildStep):
+
+    flunkOnFailure = True
+
+    def run(self):
+        return defer.succeed(results.SUCCESS)
+
+
 class FailingCustomStep(buildstep.BuildStep):
 
     flunkOnFailure = True
@@ -142,3 +150,29 @@ def test_Latin1ProducingCustomBuildStep(self):
         yield self.assertLogs(1, {
             'xx': 'o\N{CENT SIGN}\n',
         })
+
+    @defer.inlineCallbacks
+    def test_all_properties(self):
+        builder_id = yield self.create_config_for_step(SucceedingCustomStep())
+
+        yield self.do_test_build(builder_id)
+
+        properties = yield self.master.data.get(("builds", 1, "properties"))
+
+        self.assertIn("builddir", properties)
+        properties.pop("builddir")
+
+        self.assertEqual(
+            properties,
+            {
+                "buildername": ("builder", "Builder"),
+                "builderid": (1, "Builder"),
+                "workername": ("worker1", "Worker"),
+                "buildnumber": (1, "Build"),
+                "branch": (None, "Build"),
+                "revision": (None, "Build"),
+                "repository": ("", "Build"),
+                "codebase": ("", "Build"),
+                "project": ("", "Build"),
+            }
+        )

master/buildbot/test/unit/process/test_build.py

@@ -115,8 +115,8 @@ def __init__(self, master):
     def getBuilderId(self):
         return defer.succeed(self.builderid)
 
-    def setupProperties(self, props):
-        pass
+    def setup_properties(self, props):
+        return defer.succeed(None)
 
     def buildFinished(self, build, workerforbuilder):
         pass

master/buildbot/test/unit/process/test_builder.py

@@ -350,7 +350,7 @@ def rendered_locks(props):
                 'buildbot.process.build.Build._canAcquireLocks',
                 mock.Mock(return_value=False)):
             with mock.patch(
-                    'buildbot.process.build.Build.setupPropertiesKnownBeforeBuildStarts',
+                    'buildbot.process.build.Build.setup_properties_known_before_build_starts',
                     mock.Mock()):
                 startable = yield self.bldr.canStartBuild(wfb, 100)
                 self.assertEqual(startable, False)
@@ -365,7 +365,7 @@ def test_canStartBuild_with_incompatible_latent_worker(self):
         wfb.worker = FakeLatentWorker(is_compatible_with_build=False)
 
         with mock.patch(
-                'buildbot.process.build.Build.setupPropertiesKnownBeforeBuildStarts',
+                'buildbot.process.build.Build.setup_properties_known_before_build_starts',
                 mock.Mock()):
             startable = yield self.bldr.canStartBuild(wfb, 100)
         self.assertFalse(startable)
@@ -392,7 +392,7 @@ def locks_renderer(props):
                 'buildbot.process.build.Build._canAcquireLocks',
                 mock.Mock(return_value=False)):
             with mock.patch(
-                    'buildbot.process.build.Build.setupPropertiesKnownBeforeBuildStarts',
+                    'buildbot.process.build.Build.setup_properties_known_before_build_starts',
                     mock.Mock()):
                 startable = yield self.bldr.canStartBuild(wfb, 100)
                 self.assertEqual(startable, False)
@@ -471,7 +471,7 @@ def test_defaultProperties(self):
 
         yield self.makeBuilder(defaultProperties={'bar': 'onoes', 'cuckoo': 42})
 
-        self.bldr.setupProperties(props)
+        yield self.bldr.setup_properties(props)
 
         self.assertEquals(props.getProperty('bar'), 'bleh')
         self.assertEquals(props.getProperty('cuckoo'), 42)

master/buildbot/test/unit/reporters/test_bitbucketserver.py

@@ -196,11 +196,14 @@ def setupReporter(self, token=None, **kwargs):
         self.master = fakemaster.make_master(self, wantData=True, wantDb=True,
                                              wantMq=True)
 
+        def setup_properties(props):
+            props.setProperty("buildername", "Builder0", "Builder")
+            return defer.succeed(None)
+
         builder = Mock(spec=Builder)
         builder.master = self.master
         builder.name = "Builder0"
-        builder.setupProperties = lambda props: props.setProperty(
-            "buildername", "Builder0", "Builder")
+        builder.setup_properties = setup_properties
         self.master.botmaster.getBuilderById = Mock(return_value=builder)
 
         http_headers = {} if token is None else {'Authorization': 'Bearer tokentoken'}

master/buildbot/test/unit/reporters/test_gitlab.py

@@ -55,11 +55,14 @@ def setUp(self):
         self.sp = GitLabStatusPush(Interpolate('XXYYZZ'))
         yield self.sp.setServiceParent(self.master)
 
+        def setup_properties(props):
+            props.setProperty("buildername", "Builder0", "Builder")
+            return defer.succeed(None)
+
         builder = mock.Mock(spec=Builder)
         builder.master = self.master
         builder.name = "Builder0"
-        builder.setupProperties = lambda props: props.setProperty(
-            "buildername", "Builder0", "Builder")
+        builder.setup_properties = setup_properties
         self.master.botmaster.getBuilderById = mock.Mock(return_value=builder)
 
     def tearDown(self):

master/docs/manual/configuration/properties.rst

@@ -66,6 +66,11 @@ The following build properties are set when the build is started, and are availa
     This is a string that indicates which :class:`Builder` the build was a part of.
     The combination of buildername and buildnumber uniquely identify a build.
 
+.. index:: single: Properties; builderid
+
+``builderid``
+    This is a number that indicates which :class:`Builder` the build was a part of.
+
 .. index:: single: Properties; buildnumber
 
 ``buildnumber``

newsfragments/builderid-property.feature

@@ -0,0 +1 @@
+Builds now have ``builderid`` property.