Change builder names

master/skia_master_scripts/utils.py

 # Copyright (c) 2013 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 
 """Miscellaneous utilities needed by the Skia buildbot master."""
 
 
 import httplib2
 import re
@@ skipping 16 matching lines @@
 from skia_master_scripts import chromeos_factory
 from skia_master_scripts import factory as skia_factory
 from skia_master_scripts import housekeeping_percommit_factory, \
                                 housekeeping_periodic_factory
 from skia_master_scripts import ios_factory
 from skia_master_scripts import nacl_factory
 
 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]
 TRY_SCHEDULERS_STR = '|'.join(TRY_SCHEDULERS)
 
 
 def IsTrybot(builder_name):
   return builder_name.endswith(TRYBOT_NAME_SUFFIX)
 
 
@@ skipping 311 matching lines @@
     # been defined.
     # pylint: disable=W0601
     global skia_all_subdirs
     try:
       if skia_all_subdirs:
         raise Exception('skia_all_subdirs has already been defined!')
     except NameError:
       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,
                     compile_bot_warnings_as_errors=True,
                     **kwargs):
   """ Creates a trio 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.
   """
   B = helper.Builder
   F = helper.Factory
 
   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,
         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,
         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,
         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),
+        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(
         builder_name=debug_builder_name,
         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(
         builder_name=no_perf_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),
+        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(
         builder_name=perf_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),
+        do_patch_step=is_trybot,
         perf_output_basedir=perf_output_basedir,
         compile_warnings_as_errors=False,
         **kwargs        
         ).BuildPerfOnly())
 
 
 def _MakeBuilderAndMaybeTrybotSet(do_trybots=True, **kwargs):
   _MakeBuilderSet(try_schedulers=None, **kwargs)
   if do_trybots:
     _MakeBuilderSet(try_schedulers=TRY_SCHEDULERS, **kwargs)
 
 
 def MakeBuilderSet(**kwargs):
   _MakeBuilderAndMaybeTrybotSet(factory_type=skia_factory.SkiaFactory, **kwargs)
 
 
 def MakeHousekeeperBuilderSet(helper, do_trybots, do_upload_results):
   B = helper.Builder
   F = helper.Factory
 
   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:
     B(builder_name, 'f_%s' % builder_name, scheduler=scheduler)
     F('f_%s' % builder_name,
       factory(
         do_upload_results=do_upload_results,
         target_platform=skia_factory.TARGET_PLATFORM_LINUX,
         builder_name=builder_name,
         do_patch_step=(scheduler == TRY_SCHEDULERS_STR),
@@ skipping 62 matching lines @@
   # request is associated with a change but the revisions match (#5 above).
   if req1.source.changes and not req2.source.changes:
     return False
   if not req1.source.changes and req2.source.changes:
     return False
   if not (req1.source.changes and req2.source.changes):
     if req1.source.revision != req2.source.revision:
       return False
   
   return True