From 90b3c4ff661b247980cb73b62911f042b1e19572 Mon Sep 17 00:00:00 2001 From: Aneesh Agrawal Date: Fri, 28 Oct 2016 12:20:45 -0400 Subject: [PATCH] Ensure dynamic Buildbot steps have unique names Buildbot requires that each step in a build has a unique name, and uses this property to e.g. disambiguate log files. By default, each step's name reflect what kind of step it is, e.g. `git` or `test`. Add count suffixes to the end of the step name if there are multiple steps of the same type to disambiguate them. (Buildbot already does this internally for steps added statically in the `setupBuild` method). This requires checking all previously-added steps for name collisions, so add all the steps at one time to amortize this cost. --- buildbot/master/files/config/factories.py | 41 ++++++++++++++++++++--- 1 file changed, 36 insertions(+), 5 deletions(-) diff --git a/buildbot/master/files/config/factories.py b/buildbot/master/files/config/factories.py index bfa793d1..b637ea75 100644 --- a/buildbot/master/files/config/factories.py +++ b/buildbot/master/files/config/factories.py @@ -1,3 +1,4 @@ +import collections import copy import re @@ -85,17 +86,47 @@ def run(self): )) pkill_step = [self.make_pkill_step("servo")] - - for step in pkill_step + dynamic_steps: - self.add_step(step) + self.add_steps(pkill_step + dynamic_steps) defer.returnValue(result) - def add_step(self, step): + def add_steps(self, steps): + """\ + Adds new steps to this build, making sure to avoid name collisions + by adding counts to disambiguate multiple steps of the same type, + and respecting internal Buildbot invariants. + Semi-polyfill for addStepsAfterLastStep from Buildbot 9. + """ + + def step_type(step): + return step.name.split('__')[0] + + name_counts = collections.Counter() + + # Check for existing max step counts for each type of step + # in the existing steps on the build. + # Adding multiple steps at the same time makes it more efficient + # to check for collisions since this is amortized over all + # steps added together. + for step in self.build.steps: + name_counts[step_type(step)] += 1 + + # Add new steps, updating `name_counts` along the way + for step in steps: + existing_count = name_counts[step_type(step)] + if existing_count > 0: + # First step has count = 0 but no suffix, + # so second step will have `__1` as suffix, etc. + step.name += '__{}'.format(existing_count) + name_counts[step_type(step)] += 1 + self._add_step(self, step) + + def _add_step(self, step): """\ Adds a new step to this build, making sure to maintain internal Buildbot invariants. - Semi-polyfill for addStepsAfterLastStep from Buildbot 9. + Do not call this method directly, but go through add_steps + to prevent `name` collisions. """ step.setBuild(self.build) step.setBuildSlave(self.build.slavebuilder.slave)