Change Builder Names, MkII

master/skia_master_scripts/utils.py

Index: master/skia_master_scripts/utils.py
===================================================================
--- master/skia_master_scripts/utils.py	(revision 8722)
+++ master/skia_master_scripts/utils.py	(working copy)
@@ -34,8 +34,25 @@
 import config_private
 
 
-CATEGORY_BUILD = ' Build'
-TRYBOT_NAME_SUFFIX = '_Trybot'
+BUILDER_NAME_SEP = '-'
+
+# Patterns for creating builder names, based on the role of the builder.
+# TODO(borenet): Extract these into a separate file (JSON?) so that they can be
+# read by other users.
+BUILDER_ROLE_COMPILE = 'Build'
+BUILDER_ROLE_PERF = 'Perf'
+BUILDER_ROLE_TEST = 'Test'
+BUILDER_ROLE_HOUSEKEEPER = 'Housekeeper'

[epoger, 2013/04/24 18:56:10]

ideas for shorter name:  House, Tasks, Other

+BUILDER_NAME_DEFAULT_ATTRS = ['os', 'model', 'gpu', 'configuration',
+                              'arch_width']
+BUILDER_NAME_SCHEMA = {
+  BUILDER_ROLE_COMPILE: ['os', 'compiler', 'configuration', 'target_arch'],
+  BUILDER_ROLE_TEST: BUILDER_NAME_DEFAULT_ATTRS,
+  BUILDER_ROLE_PERF: BUILDER_NAME_DEFAULT_ATTRS,
+  BUILDER_ROLE_HOUSEKEEPER: ['frequency'],
+}
+
+TRYBOT_NAME_SUFFIX = 'Trybot'
 TRY_SCHEDULER_SVN = 'skia_try_svn'
 TRY_SCHEDULER_RIETVELD = 'skia_try_rietveld'
 TRY_SCHEDULERS = [TRY_SCHEDULER_SVN, TRY_SCHEDULER_RIETVELD]
@@ -215,17 +232,11 @@
 class SkiaHelper(master_config.Helper):
 
   def Builder(self, name, factory, gatekeeper=None, scheduler=None,
-              builddir=None, auto_reboot=False, notify_on_missing=False,
-              override_category=None):
-    if override_category:
-      old_category = self._defaults.get('category')
-      self._defaults['category'] = override_category
+              builddir=None, auto_reboot=False, notify_on_missing=False):
     super(SkiaHelper, self).Builder(name=name, factory=factory,
                                     gatekeeper=gatekeeper, scheduler=scheduler,
                                     builddir=builddir, auto_reboot=auto_reboot,
                                     notify_on_missing=notify_on_missing)
-    if override_category:
-      self._defaults['category'] = old_category
 
   def AnyBranchScheduler(self, name, branches, treeStableTimer=60,
                          categories=None):
@@ -367,48 +378,32 @@
       skia_all_subdirs = all_subdirs
 
 
-def MakeBuilderName(builder_base_name, config):
-  """ Inserts config into builder_base_name at '%s', or if builder_base_name
-  does not contain '%s', appends config to the end of builder_base_name,
-  separated by an underscore. """
-  try:
-    return builder_base_name % config
-  except TypeError:
-    # If builder_base_name does not contain '%s'
-    return '%s_%s' % (builder_base_name, config)
+def MakeBuilderName(role, extra_config=None, is_trybot=False, **kwargs):
+  schema = BUILDER_NAME_SCHEMA.get(role)

[epoger, 2013/04/24 18:56:10]

I like how you did this!  The Python force is strong with this one.

+  if not schema:
+    raise ValueError('%s is not a recognized role.' % role)
+  for k, v in kwargs.iteritems():
+    if BUILDER_NAME_SEP in v:
+      raise ValueError('%s not allowed in %s.' % (v, BUILDER_NAME_SEP))
+    if not k in schema:
+      raise ValueError('Schema does not contain "%s": %s' %(k, schema))
+  if extra_config and BUILDER_NAME_SEP in extra_config:
+    raise ValueError('%s not allowed in %s.' % (extra_config,
+                                                BUILDER_NAME_SEP))
+  name_parts = [role]
+  name_parts.extend([kwargs[attribute] for attribute in schema])
+  if extra_config:
+    name_parts.append(extra_config)
+  if is_trybot:
+    name_parts.append(TRYBOT_NAME_SUFFIX)
+  return BUILDER_NAME_SEP.join(name_parts)
 
 
-def MakeCompileBuilderName(builder_base_name, release=False):
-  if release:
-    compile_name = 'Compile_Release'
-  else:
-    compile_name = 'Compile_Debug'
-  return MakeBuilderName(builder_base_name, compile_name)
-
-
-def MakeDebugBuilderName(builder_base_name):
-  return MakeBuilderName(builder_base_name, skia_factory.CONFIG_DEBUG)
-
-
-def MakeReleaseBuilderName(builder_base_name):
-  return MakeBuilderName(builder_base_name, skia_factory.CONFIG_RELEASE)
-
-
-def MakeBenchBuilderName(builder_base_name):
-  return MakeBuilderName(builder_base_name, skia_factory.CONFIG_BENCH)
-
-
-def MakeSchedulerName(builder_base_name):
-  return MakeBuilderName(builder_base_name, 'Scheduler')
-
-
-def _MakeBuilderSet(helper, builder_base_name, gm_image_subdir,
-                    perf_output_basedir=None, extra_branches=None,
-                    factory_type=None, do_compile=True, do_debug=True,
-                    do_release=True, do_bench=True, try_schedulers=None,
-                    compile_bot_warnings_as_errors=True,
-                    **kwargs):
-  """ Creates a trio of builders for a given platform:
+def _MakeBuilderSet(helper, os, model, gpu, arch_width, gm_image_subdir,
+                    extra_config=None, perf_output_basedir=None,
+                    extra_branches=None, factory_type=None, do_debug=True,
+                    do_release=True, do_bench=True, is_trybot=False, **kwargs):
+  """ Creates a set of builders for a given platform:
   1. Debug mode builder which runs all steps
   2. Release mode builder which runs all steps EXCEPT benchmarks
   3. Release mode builder which runs ONLY benchmarks.
@@ -423,50 +418,30 @@
     gm_image_branch = 'gm-expected/%s' % gm_image_subdir
     subdirs_to_checkout.add(gm_image_branch)
 
-  if try_schedulers:
-    scheduler_name = '|'.join(try_schedulers)
-    builder_base_name = builder_base_name + TRYBOT_NAME_SUFFIX
+  if is_trybot:
+    scheduler_name = TRY_SCHEDULERS_STR
   else:
-    scheduler_name = MakeSchedulerName(builder_base_name)
+    scheduler_name = MakeBuilderName(role=BUILDER_ROLE_TEST,
+                                     os=os,
+                                     model=model,
+                                     gpu=gpu,
+                                     arch_width=arch_width,
+                                     configuration='Scheduler',
+                                     extra_config=extra_config,
+                                     is_trybot=False)
     branches = list(subdirs_to_checkout.union(SKIA_PRIMARY_SUBDIRS))
     helper.AnyBranchScheduler(scheduler_name, branches=branches)
 
-  if do_compile:
-    compile_debug_builder_name = MakeCompileBuilderName(builder_base_name,
-                                                        release=False)
-    B(compile_debug_builder_name, 'f_%s' % compile_debug_builder_name,
-        # Do not add gatekeeper for trybots.
-        gatekeeper='GateKeeper' if try_schedulers is None else None,
-        scheduler=scheduler_name, override_category=CATEGORY_BUILD)
-    F('f_%s' % compile_debug_builder_name, factory_type(
-        builder_name=compile_debug_builder_name,
-        other_subdirs=subdirs_to_checkout,
+  if do_debug:
+    debug_builder_name = MakeBuilderName(
+        role=BUILDER_ROLE_TEST,
+        os=os,
+        model=model,
+        gpu=gpu,
         configuration=skia_factory.CONFIG_DEBUG,
-        gm_image_subdir=gm_image_subdir,
-        do_patch_step=(try_schedulers is not None),
-        perf_output_basedir=None,
-        compile_warnings_as_errors=compile_bot_warnings_as_errors,
-        **kwargs
-        ).BuildCompileOnly())
-    compile_release_builder_name = MakeCompileBuilderName(builder_base_name,
-                                                          release=True)
-    B(compile_release_builder_name, 'f_%s' % compile_release_builder_name,
-        # Do not add gatekeeper for trybots.
-        gatekeeper='GateKeeper' if try_schedulers is None else None,
-        scheduler=scheduler_name, override_category=CATEGORY_BUILD)
-    F('f_%s' % compile_release_builder_name, factory_type(
-        builder_name=compile_release_builder_name,
-        other_subdirs=subdirs_to_checkout,
-        configuration=skia_factory.CONFIG_RELEASE,
-        gm_image_subdir=gm_image_subdir,
-        do_patch_step=(try_schedulers is not None),
-        perf_output_basedir=None,
-        compile_warnings_as_errors=compile_bot_warnings_as_errors,
-        **kwargs
-        ).BuildCompileOnly())
-
-  if do_debug:
-    debug_builder_name = MakeDebugBuilderName(builder_base_name)
+        arch_width=arch_width,
+        extra_config=extra_config,
+        is_trybot=is_trybot)
     B(debug_builder_name, 'f_%s' % debug_builder_name,
         scheduler=scheduler_name)
     F('f_%s' % debug_builder_name, factory_type(
@@ -474,14 +449,22 @@
         other_subdirs=subdirs_to_checkout,
         configuration=skia_factory.CONFIG_DEBUG,
         gm_image_subdir=gm_image_subdir,
-        do_patch_step=(try_schedulers is not None),
+        do_patch_step=is_trybot,
         perf_output_basedir=None,
         compile_warnings_as_errors=False,
         **kwargs
         ).Build())
 
   if do_release:
-    no_perf_builder_name = MakeReleaseBuilderName(builder_base_name)
+    no_perf_builder_name = MakeBuilderName(
+        role=BUILDER_ROLE_TEST,
+        os=os,
+        model=model,
+        gpu=gpu,
+        configuration=skia_factory.CONFIG_RELEASE,
+        arch_width=arch_width,
+        extra_config=extra_config,
+        is_trybot=is_trybot)
     B(no_perf_builder_name, 'f_%s' % no_perf_builder_name,
         scheduler=scheduler_name)
     F('f_%s' % no_perf_builder_name,  factory_type(
@@ -489,14 +472,22 @@
         other_subdirs=subdirs_to_checkout,
         configuration=skia_factory.CONFIG_RELEASE,
         gm_image_subdir=gm_image_subdir,
-        do_patch_step=(try_schedulers is not None),
+        do_patch_step=is_trybot,
         perf_output_basedir=None,
         compile_warnings_as_errors=False,
         **kwargs
         ).BuildNoPerf())
 
   if do_bench:
-    perf_builder_name = MakeBenchBuilderName(builder_base_name)
+    perf_builder_name = MakeBuilderName(
+        role=BUILDER_ROLE_PERF,
+        os=os,
+        model=model,
+        gpu=gpu,
+        configuration=skia_factory.CONFIG_RELEASE,
+        arch_width=arch_width,
+        extra_config=extra_config,
+        is_trybot=is_trybot)
     B(perf_builder_name, 'f_%s' % perf_builder_name,
         scheduler=scheduler_name)
     F('f_%s' % perf_builder_name, factory_type(
@@ -504,7 +495,7 @@
         other_subdirs=subdirs_to_checkout,
         configuration=skia_factory.CONFIG_RELEASE,
         gm_image_subdir=gm_image_subdir,
-        do_patch_step=(try_schedulers is not None),
+        do_patch_step=is_trybot,
         perf_output_basedir=perf_output_basedir,
         compile_warnings_as_errors=False,
         **kwargs        
@@ -512,9 +503,9 @@
 
 
 def _MakeBuilderAndMaybeTrybotSet(do_trybots=True, **kwargs):
-  _MakeBuilderSet(try_schedulers=None, **kwargs)
+  _MakeBuilderSet(**kwargs)
   if do_trybots:
-    _MakeBuilderSet(try_schedulers=TRY_SCHEDULERS, **kwargs)
+    _MakeBuilderSet(is_trybot=True, **kwargs)
 
 
 def MakeBuilderSet(**kwargs):
@@ -527,18 +518,19 @@
 
   builder_factory_scheduler = [
     # The Percommit housekeeper
-    ('Skia_PerCommit_House_Keeping',
+    (MakeBuilderName(role='Housekeeper', frequency='PerCommit'),
      housekeeping_percommit_factory.HouseKeepingPerCommitFactory,
      'skia_rel'),
     # The Periodic housekeeper
-    ('Skia_Periodic_House_Keeping',
+    (MakeBuilderName(role='Housekeeper', frequency='Nightly'),
      housekeeping_periodic_factory.HouseKeepingPeriodicFactory,
      'skia_periodic'),
   ]
   if do_trybots:
     # Add the corresponding trybot builders to the above list.
     builder_factory_scheduler.extend([
-        (builder + TRYBOT_NAME_SUFFIX, factory, TRY_SCHEDULERS_STR)
+        (builder + BUILDER_NAME_SEP + TRYBOT_NAME_SUFFIX, factory,
+         TRY_SCHEDULERS_STR)
         for (builder, factory, _scheduler) in builder_factory_scheduler])
 
   for (builder_name, factory, scheduler) in builder_factory_scheduler:
@@ -552,12 +544,18 @@
       ).Build())
 
 
-def MakeAndroidBuilderSet(extra_branches=None, **kwargs):
+def _DeviceToModelName(device):
+  return device.replace('_', ' ').title().replace(' ', '')

[rmistry, 2013/04/25 11:36:45]

Lets add a short comment here, something like:
# 'Nexus_s' is converted to 'NexusS'

+
+
+def MakeAndroidBuilderSet(device, extra_branches=None, **kwargs):
   if not extra_branches:
     extra_branches = []
   extra_branches.append('android')
   _MakeBuilderAndMaybeTrybotSet(factory_type=android_factory.AndroidFactory,
                                 extra_branches=extra_branches,
+                                device=device,
+                                model=_DeviceToModelName(device),
                                 **kwargs)
 
 
@@ -574,6 +572,36 @@
   _MakeBuilderAndMaybeTrybotSet(factory_type=nacl_factory.NaClFactory, **kwargs)
 
 
+def _MakeCompileBuilder(helper, scheduler, os, compiler, configuration,
+                        target_arch, factory_type, is_trybot,
+                        extra_config=None, **kwargs):
+  builder_name = MakeBuilderName(role=BUILDER_ROLE_COMPILE,
+                                 os=os,
+                                 compiler=compiler,
+                                 configuration=configuration,
+                                 target_arch=target_arch,
+                                 extra_config=extra_config,
+                                 is_trybot=is_trybot)
+  helper.Builder(builder_name, 'f_%s' % builder_name,
+                 # Do not add gatekeeper for trybots.
+                 gatekeeper='GateKeeper' if is_trybot else None,
+                 scheduler=scheduler)
+  helper.Factory('f_%s' % builder_name, factory_type(
+      builder_name=builder_name,
+      do_patch_step=is_trybot,
+      configuration=configuration,
+      **kwargs
+      ).BuildCompileOnly())
+  return builder_name
+
+
+def MakeCompileBuilderSet(scheduler, do_trybots=True, **kwargs):
+  if do_trybots:
+    _MakeCompileBuilder(scheduler=scheduler, is_trybot=True, **kwargs)
+  _MakeCompileBuilder(scheduler=TRY_SCHEDULERS_STR, is_trybot=False, **kwargs)
+
+
+
 def CanMergeBuildRequests(req1, req2):
   """ Determine whether or not two BuildRequests can be merged. Note that the
   call to buildbot.sourcestamp.SourceStamp.canBeMergedWith() is conspicuously