Change builder names

master/skia_master_scripts/utils.py

Index: master/skia_master_scripts/utils.py
===================================================================
--- master/skia_master_scripts/utils.py	(revision 8570)
+++ master/skia_master_scripts/utils.py	(working copy)
@@ -34,8 +34,9 @@
 import config_private
 
 
+BUILDER_NAME_SEP = '-'

[borenet, 2013/04/09 16:31:46]

Changed from the '_' which we used before to separate parts of builder names. 
IMO things joined with underscores group before things joined with dashes, for
example:
a_b-c_d-e

Groups to:
(a_b), (c_d), (e)

[epoger, 2013/04/09 17:38:36]

Agreed.

 CATEGORY_BUILD = ' Build'
-TRYBOT_NAME_SUFFIX = '_Trybot'
+TRYBOT_NAME_SUFFIX = 'Trybot'
 TRY_SCHEDULER_SVN = 'skia_try_svn'
 TRY_SCHEDULER_RIETVELD = 'skia_try_rietveld'
 TRY_SCHEDULERS = [TRY_SCHEDULER_SVN, TRY_SCHEDULER_RIETVELD]
@@ -367,42 +368,51 @@
       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(os, version, arch_width, hardware, role, extra_config=None,
+                    is_trybot=False):
+  """ Construct a builder name from an OS, version, hardware, configuration, and
+  builder role. """
+  name_parts = [os, version, arch_width, hardware]
+  if extra_config:
+    if BUILDER_NAME_SEP in extra_config:
+      raise ValueError('%s is not legal in %s' % (BUILDER_NAME_SEP,
+                                                  extra_config))

[borenet, 2013/04/09 16:31:46]

I already removed this duplicate check.

+    name_parts.append(extra_config)
+  name_parts.append(role)
+  if is_trybot:
+    name_parts.append(TRYBOT_NAME_SUFFIX)
+  for part in name_parts:
+    if BUILDER_NAME_SEP in part:
+      raise ValueError('%s is not legal in %s' % (BUILDER_NAME_SEP, part))
+  return BUILDER_NAME_SEP.join(name_parts)
 
 
-def MakeCompileBuilderName(builder_base_name, release=False):
+def MakeCompileBuilderName(release=False, **kwargs):
   if release:
-    compile_name = 'Compile_Release'
+    compile_name = 'ReleaseCompile'
   else:
-    compile_name = 'Compile_Debug'
-  return MakeBuilderName(builder_base_name, compile_name)
+    compile_name = 'DebugCompile'
+  return MakeBuilderName(role=compile_name, **kwargs)
 
 
-def MakeDebugBuilderName(builder_base_name):
-  return MakeBuilderName(builder_base_name, skia_factory.CONFIG_DEBUG)
+def MakeDebugBuilderName(**kwargs):
+  return MakeBuilderName(role=skia_factory.CONFIG_DEBUG, **kwargs)
 
 
-def MakeReleaseBuilderName(builder_base_name):
-  return MakeBuilderName(builder_base_name, skia_factory.CONFIG_RELEASE)
+def MakeReleaseBuilderName(**kwargs):
+  return MakeBuilderName(role=skia_factory.CONFIG_RELEASE, **kwargs)
 
 
-def MakeBenchBuilderName(builder_base_name):
-  return MakeBuilderName(builder_base_name, skia_factory.CONFIG_BENCH)
+def MakeBenchBuilderName(**kwargs):
+  return MakeBuilderName(role=skia_factory.CONFIG_BENCH, **kwargs)
 
 
-def MakeSchedulerName(builder_base_name):
-  return MakeBuilderName(builder_base_name, 'Scheduler')
+def MakeSchedulerName(**kwargs):
+  return MakeBuilderName(role='Scheduler', **kwargs)
 
 
-def _MakeBuilderSet(helper, builder_base_name, gm_image_subdir,
+def _MakeBuilderSet(helper, os, version, arch_width, hardware,
+                    gm_image_subdir, extra_config=None,

[borenet, 2013/04/09 16:31:46]

Passing these extra parameters is annoying, particularly since they only
contribute to the builder name.  Here are some alternatives I thought of:
- Call this function in master_builders_cfg and pass in the created name,
similar to how we did it before
- Collect these parameters into an object or dictionary

                     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,
@@ -419,21 +429,30 @@
   if not extra_branches:
     extra_branches = []
   subdirs_to_checkout = set(extra_branches)
+  is_trybot = try_schedulers is not None
   if gm_image_subdir:
     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
   else:
-    scheduler_name = MakeSchedulerName(builder_base_name)
+    scheduler_name = MakeSchedulerName(os=os,
+                                       version=version,
+                                       arch_width=arch_width,
+                                       hardware=hardware,
+                                       extra_config=extra_config)
     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)
+    compile_debug_builder_name = MakeCompileBuilderName(
+        os=os,
+        version=version,
+        arch_width=arch_width,
+        hardware=hardware,
+        extra_config=extra_config,
+        is_trybot=is_trybot,
+        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,
@@ -443,13 +462,19 @@
         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=compile_bot_warnings_as_errors,
         **kwargs
         ).BuildCompileOnly())
-    compile_release_builder_name = MakeCompileBuilderName(builder_base_name,
-                                                          release=True)
+    compile_release_builder_name = MakeCompileBuilderName(
+        os=os,
+        version=version,
+        arch_width=arch_width,
+        hardware=hardware,
+        extra_config=extra_config,
+        is_trybot=is_trybot,
+        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,
@@ -459,14 +484,19 @@
         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=compile_bot_warnings_as_errors,
         **kwargs
         ).BuildCompileOnly())
 
   if do_debug:
-    debug_builder_name = MakeDebugBuilderName(builder_base_name)
+    debug_builder_name = MakeDebugBuilderName(os=os,
+                                              version=version,
+                                              arch_width=arch_width,
+                                              hardware=hardware,
+                                              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 +504,19 @@
         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 = MakeReleaseBuilderName(os=os,
+                                                  version=version,
+                                                  arch_width=arch_width,
+                                                  hardware=hardware,
+                                                  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 +524,19 @@
         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 = MakeBenchBuilderName(os=os,
+                                             version=version,
+                                             arch_width=arch_width,
+                                             hardware=hardware,
+                                             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 +544,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        
@@ -527,18 +567,19 @@
 
   builder_factory_scheduler = [
     # The Percommit housekeeper
-    ('Skia_PerCommit_House_Keeping',
+    ('PerCommit-Housekeeping',

[borenet, 2013/04/09 16:31:46]

I'd really like to make the housekeepers conform to the same convention.  I
guess the name would be something like
Linux-Ubuntu12-VM-PerCommit_Housekeeping...

      housekeeping_percommit_factory.HouseKeepingPerCommitFactory,
      'skia_rel'),
     # The Periodic housekeeper
-    ('Skia_Periodic_House_Keeping',
+    ('Periodic-Housekeeping',
      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: