Rework how bots get the version of Blink.

scripts/master/chromium_step.py

 # Copyright (c) 2012 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.
 
 """Subclasses of various slave command classes."""
 
 import copy
 import errno
 import json
 import logging
 import os
-import re
 import time
 
 from twisted.python import log
 
 from buildbot import interfaces, util
 from buildbot.process import buildstep
 from buildbot.process.properties import WithProperties
 from buildbot.status import builder
 from buildbot.steps import shell
 from buildbot.steps import source
@@ skipping 37 matching lines @@
 
 class GClient(source.Source):
   """Check out a source tree using gclient."""
 
   name = 'update'
 
   def __init__(self, svnurl=None, rm_timeout=None, gclient_spec=None, env=None,
                sudo_for_remove=False, gclient_deps=None, gclient_nohooks=False,
                no_gclient_branch=False, no_gclient_revision=False,
                gclient_transitive=False, primary_repo=None,
-               gclient_jobs=None, **kwargs):
+               gclient_jobs=None, blink_config=None, **kwargs):
+    # TODO: We shouldn't need to hard-code blink-specific info here. We
+    # should figure out how to generalize this to sub-repos somehow.

[iannucci, 2013/04/24 02:36:46]

This could conceivably be a {custom_var: value} dictionary. Then you could
represent:
  Deps version -> no entry
  HEAD version -> var: 'HEAD'
  LKGR version -> var: rev

This would require reworking how gclient_spec is passed around (it's passed
around as a raw string, which makes manipulating it at the point where we
actually run the gclient commands difficult). Alternately, it may be possible to
compute all this at the point where the current gclient_spec is stringified
(edit: Looks like not so much. This is because the gclient_spec belongs to the
_Factory_ and not to the Build. That would have to be altered (so that the
gclient spec was computed in the factory method instead of the factory
constructor, or allowing overrides to be passed to
BuildGclientSpec()/GetSpec())).

That said, it would be a larger change, and would be better to do as a follow-on
patch than in this CL.

     source.Source.__init__(self, **kwargs)
     if env:
       self.args['env'] = env.copy()
     self.args['rm_timeout'] = rm_timeout
     self.args['svnurl'] = svnurl
     self.args['sudo_for_remove'] = sudo_for_remove
     # linux doesn't handle spaces in command line args properly so remove them.
     # This doesn't matter for the format of the DEPS file.
     self.args['gclient_spec'] = gclient_spec.replace(' ', '')
     self.args['gclient_deps'] = gclient_deps
     self.args['gclient_nohooks'] = gclient_nohooks
     self.args['no_gclient_branch'] = no_gclient_branch
     self.args['no_gclient_revision'] = no_gclient_revision
     self.args['gclient_transitive'] = gclient_transitive
     self.args['primary_repo'] = primary_repo or ''
     self.args['gclient_jobs'] = gclient_jobs
+    self.args['blink_config'] = blink_config
 
   def computeSourceRevision(self, changes):
     """Finds the latest revision number from the changeset that have
     triggered the build.
 
     This is a hook method provided by the parent source.Source class and
     default implementation in source.Source returns None. Return value of this
     method is be used to set 'revsion' argument value for startVC() method."""
     if not changes:
       return None
     # Change revision numbers can be invalid, for a try job for instance.
     # TODO(maruel): Make this work for git hash.
     lastChange = max([change_to_revision(c) for c in changes])
     return lastChange
 
   def startVC(self, branch, revision, patch):
     warnings = []
     args = copy.copy(self.args)
-    wk_revision = None
-    # branch == 'trunk' means the change came from the blink poller, and the
-    # revision is a blink revision.
-    if branch == 'trunk':
-      wk_revision = revision
-    try:
-      # parent_wk_revision might be set, but empty.
-      if self.getProperty('parent_wk_revision'):
-        wk_revision = self.getProperty('parent_wk_revision')
-    except KeyError:
-      pass
-    nacl_revision = revision
-    try:
-      # parent_nacl_revision might be set, but empty.
-      if self.getProperty('parent_got_nacl_revision'):
-        nacl_revision = self.getProperty('parent_got_nacl_revision')
-    except KeyError:
-      pass
+
     try:
       # parent_cr_revision might be set, but empty.
       if self.getProperty('parent_cr_revision'):
         revision = 'src@' + self.getProperty('parent_cr_revision')
     except KeyError:
       pass
     self.setProperty('primary_repo', args['primary_repo'], 'Source')
     args['revision'] = revision
     args['branch'] = branch
+
     if args.get('gclient_spec'):
-      args['gclient_spec'] = args['gclient_spec'].replace(
-          '$$WK_REV$$', str(wk_revision or ''))
-      args['gclient_spec'] = args['gclient_spec'].replace(
-          '$$NACL_REV$$', str(nacl_revision or ''))
-      webkit_try_revision = None
-      if patch:
-        match = re.match(r'third_party/WebKit@(\w+)', patch[1])
-        if match:
-          webkit_try_revision = match.group(1)
-      if webkit_try_revision:
-        args['gclient_spec'] = args['gclient_spec'].replace(
-            '$$WK_TRY_REV$$', webkit_try_revision)
-      else:
-        args['gclient_spec'] = args['gclient_spec'].replace(
-            ',"webkit_revision":"$$WK_TRY_REV$$"', '')
+      self.adjustGclientSpecForBlink(branch, revision, args)
+      self.adjustGclientSpecForNaCl(branch, revision, patch, args)
+
     if patch:
       args['patch'] = patch
     elif args.get('patch') is None:
       del args['patch']
+
     cmd = buildstep.LoggedRemoteCommand('gclient', args)
     self.startCommand(cmd, warnings)
 
+  def adjustGclientSpecForBlink(self, branch, revision, args):
+    # If the bot in question is a dedicated bot for Blink changes (either
+    # on a waterfall, or a blink-specific trybot), we want to set a custom
+    # version of Blink, otherwise we leave the gclient spec alone.
+    if args['blink_config'] != 'blink':
+      return
+
+    # branch == 'trunk' means the change came from the blink poller, and the
+    # revision is a blink revision; otherwise, we use '', or HEAD.
+    wk_revision = ''
+    if branch == 'trunk':
+      wk_revision = revision
+
+    try:
+      # parent_wk_revision might be set, but empty.
+      if self.getProperty('parent_wk_revision'):
+        wk_revision = self.getProperty('parent_wk_revision')
+    except KeyError:
+      pass
+
+    # TODO: Make this be something less fragile.
+    args['gclient_spec'] = args['gclient_spec'].replace(
+        '"custom_vars":{"webkit_trunk"',
+        '"custom_vars":{"webkit_revision":"%s","webkit_trunk"' % wk_revision)

[iannucci, 2013/04/24 02:36:46]

Hm... this would break in mysterious ways if there were ever any custom_vars
which sorted before webkit_trunk...

+
+  def adjustGclientSpecForNaCl(self, branch, revision, patch, args):
+    nacl_revision = revision
+    try:
+      # parent_nacl_revision might be set, but empty.
+      if self.getProperty('parent_got_nacl_revision'):
+        nacl_revision = self.getProperty('parent_got_nacl_revision')
+    except KeyError:
+      pass
+    args['gclient_spec'] = args['gclient_spec'].replace(
+        '$$NACL_REV$$', str(nacl_revision or ''))
+
   def describe(self, done=False):
     """Tries to append the revision number to the description."""
     description = source.Source.describe(self, done)
     self.appendChromeRevision(description)
     self.appendWebKitRevision(description)
     self.appendNaClRevision(description)
     self.appendV8Revision(description)
     return description
 
   def appendChromeRevision(self, description):
@@ skipping 911 matching lines @@
   def evaluateCommand(self, cmd):
     observer_result = self.script_observer.annotate_status
     # Check if ProcessLogShellStep detected a failure or warning also.
     log_processor_result = ProcessLogShellStep.evaluateCommand(self, cmd)
     return BuilderStatus.combine(observer_result, log_processor_result)
 
   def commandComplete(self, cmd):
     self.script_observer.handleReturnCode(cmd.rc)
     self._removePreamble()
     return ProcessLogShellStep.commandComplete(self, cmd)