Change Builder Names, MkIII

master/skia_master_scripts/factory.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.
 
 
 """Utility class to build the Skia master BuildFactory's.
 
 Based on gclient_factory.py and adds Skia-specific steps."""
 
 
@@ skipping 14 matching lines @@
 # and thus remove the need for the whole target_platform parameter.
 # For now, these must match the target_platform values used in
 # third_party/chromium_buildbot/scripts/master/factory/gclient_factory.py ,
 # because we pass these values into GClientFactory.__init__() .
 TARGET_PLATFORM_LINUX = 'linux'
 TARGET_PLATFORM_MAC = 'mac'
 TARGET_PLATFORM_WIN32 = 'win32'
 
 CONFIG_DEBUG = 'Debug'
 CONFIG_RELEASE = 'Release'
-CONFIG_BENCH = 'Bench'
 CONFIGURATIONS = [CONFIG_DEBUG, CONFIG_RELEASE]
 
 
 class SkiaFactory(BuildFactory):
   """Encapsulates data and methods common to the Skia master.cfg files."""
 
   def __init__(self, other_subdirs=None, do_upload_results=False,
                do_patch_step=False, build_subdir='trunk',
                target_platform=None, configuration=CONFIG_DEBUG,
                default_timeout=8*60*60,
@@ skipping 66 matching lines @@
     self._do_upload_results = do_upload_results
     self._do_upload_bench_results = do_upload_results and \
         perf_output_basedir != None
     self._do_patch_step = do_patch_step
 
     if not environment_variables:
       self._env_vars = {}
     else:
       self._env_vars = dict(environment_variables)
     gyp_defines = self._env_vars.get('GYP_DEFINES', '')
-    self._env_vars['GYP_DEFINES'] = gyp_defines + \
-        ' skia_warnings_as_errors=%d' % int(compile_warnings_as_errors)
+    if gyp_defines:
+      self._env_vars['GYP_DEFINES'] = gyp_defines + \
+          ' skia_warnings_as_errors=%d' % int(compile_warnings_as_errors)
+    else:

[rmistry, 2013/04/29 21:02:58]

Instead of the else you could remove line 123 and do:
self._env_vars['GYP_DEFINES'] += ' skia_warnings_as_errors=%d' %
int(compile_warnings_as_errors)
since the same line is repeated in both blocks.

[borenet, 2013/04/30 11:43:51]

The problem is that GYP_DEFINES may not be in self._env_vars.  So using +=
unconditionally will fail if it is None.

[rmistry, 2013/04/30 11:59:24]

self._env_vars['GYP_DEFINES'] = self._env_vars.get('GYP_DEFINES', '') + ...

Either way this is minor you can make the change later, lets get this CL in.

+      self._env_vars['GYP_DEFINES'] = \
+          ' skia_warnings_as_errors=%d' % int(compile_warnings_as_errors)
 
     # Get an implementation of SkiaCommands as appropriate for
     # this target_platform.
     workdir = self.TargetPathJoin('build', build_subdir)
     self._skia_cmd_obj = skia_commands.SkiaCommands(
         target_platform=target_platform, factory=self,
         configuration=configuration, workdir=workdir,
         target_arch=None, default_timeout=default_timeout,
         environment_variables=self._env_vars)
 
@@ skipping 370 matching lines @@
     self.RunBench()
     self.BenchPictures()
     self.PostBench()
     self.CheckForRegressions()
     self.BenchGraphs()
     if self._do_upload_bench_results:
       self.UploadBenchResults()
       self.UploadBenchResultsToAppEngine()
       self.UploadBenchGraphs()
 
-  def Build(self, clobber=None):
+  def Build(self, role=None, clobber=None):
     """Build and return the complete BuildFactory.
 
+    role: string; the intended role of this builder. The role affects which
+        steps are run. Known values are given in the utils module.
     clobber: boolean indicating whether we should clean before building
     """
-    self.CommonSteps(clobber)
-    self.NonPerfSteps()
-    self.PerfSteps()
+    if not role:
+      # If no role is provide, just run everything.
+      self.CommonSteps(clobber)
+      self.NonPerfSteps()
+      self.PerfSteps()
+    elif role == utils.BUILDER_ROLE_COMPILE:
+      # Compile-only builder.
+      self.UpdateSteps()
+      self.Compile(clobber=clobber, build_in_one_step=False)
+    else:
+      self.CommonSteps(clobber)
+      if role == utils.BUILDER_ROLE_TEST:
+        # Test-running builder.
+        self.NonPerfSteps()
+        if self._configuration == CONFIG_DEBUG:
+          # Debug-mode testers run all steps, but release-mode testers don't.
+          self.PerfSteps()
+      elif role == utils.BUILDER_ROLE_PERF:
+        # Perf-only builder.
+        if not self._perf_output_basedir:
+          raise ValueError(
+              'BuildPerfOnly requires perf_output_basedir to be defined.')
+        if self._configuration != CONFIG_RELEASE:
+          raise ValueError('BuildPerfOnly should run in %s configuration.' %
+                           CONFIG_RELEASE)
+        self.PerfSteps()
     self.Validate()
     return self
-
-  def BuildCompileOnly(self, clobber=None):
-    """Build and return the complete BuildFactory, with only the compile step.
-    Does not build in one step; the assumption is that if we're only performing
-    the compile, we want as much information as possible about *which* compile
-    steps are passing and failing.
-
-    clobber: boolean indicating whether we should clean before building
-    """
-    self.UpdateSteps()
-    self.Compile(clobber=clobber, build_in_one_step=False)
-    self.Validate()
-    return self
-
-  def BuildNoPerf(self, clobber=None):
-    """Build and return the complete BuildFactory, without the benchmarking
-    steps.
-
-    clobber: boolean indicating whether we should clean before building
-    """
-    self.CommonSteps(clobber)
-    self.NonPerfSteps()
-    self.Validate()
-    return self
-
-  def BuildPerfOnly(self, clobber=None):
-    """Build and return the complete BuildFactory, with only the benchmarking
-    steps.
-
-    clobber: boolean indicating whether we should clean before building
-    """
-    if not self._perf_output_basedir:
-      raise ValueError(
-          'BuildPerfOnly requires perf_output_basedir to be defined.')
-    if self._configuration != CONFIG_RELEASE:
-      raise ValueError('BuildPerfOnly should run in %s configuration.' %
-                       CONFIG_RELEASE)
-    self.CommonSteps(clobber)
-    self.PerfSteps()
-    self.Validate()
-    return self