Automatically add extra CQ trybots for predetermined paths

If the CL does not contain a "CQ_EXTRA_TRYBOTS=" section then it constructs one from the paths modified in your CL.

If you already have an existing "CQ_EXTRA_TRYBOTS=" section in your CL then it parses it, preserves your options, and only adds the builders and masters that were missing.

Also ensures no trybots are repeated (i.e. each master trybot combination only shows up once).

And I killed the hashtags feature in this CL, nobody was using it.

BUG=skia:4364

Review URL: https://codereview.chromium.org/1383743002

HASHTAGS

@@ -1,22 +0,0 @@
-# This file is used by the post upload hook in the PRESUBMIT file to
-# automatically change a CL's description based on the specified hashtags.
-# Please see skia:3586 for more details.
-#
-# The format of this file is:
-# hashtag_name,mapped_text
-#
-# Here are some examples:
-# * "projectxyz, BUG=skia:123" would convert "#projectxyz" into "BUG=skia:123".
-# * "notry, NOTRY=true" would convert "#notry" into "NOTRY=true".
-
-dummyproject,BUG=skia:2139,BUG=skia:2812
-notry,NOTRY=true
-nocommit,COMMIT=false
-
-floats,BUG=skia:3592
-neon,#n5,#n9
-n5,CQ_INCLUDE_TRYBOTS=client.skia.android:Test-Android-Nexus5-Adreno330-Arm7-Debug-Trybot
-n7,CQ_INCLUDE_TRYBOTS=client.skia.android:Test-Android-Nexus7-Tegra3-Arm7-Debug-Trybot
-n9,CQ_INCLUDE_TRYBOTS=client.skia.android:Test-Android-Nexus9-TegraK1-Arm64-Release-Trybot
-
-crskps,BUG=skia:3574,NOTRY=true

PRESUBMIT.py

@@ -9,6 +9,7 @@ See http://dev.chromium.org/developers/how-tos/depottools/presubmit-scripts
 for more details about the presubmit API built into gcl.
 """
 
+import collections
 import csv
 import fnmatch
 import os
@@ -22,9 +23,6 @@ REVERT_CL_SUBJECT_PREFIX = 'Revert '
 
 SKIA_TREE_STATUS_URL = 'http://skia-tree-status.appspot.com'
 
-CQ_KEYWORDS_THAT_NEED_APPENDING = ('CQ_INCLUDE_TRYBOTS', 'CQ_EXTRA_TRYBOTS',
-                                   'CQ_EXCLUDE_TRYBOTS', 'CQ_TRYBOTS')
-
 # Please add the complete email address here (and not just 'xyz@' or 'xyz').
 PUBLIC_API_OWNERS = (
     'reed@chromium.org',
@@ -39,6 +37,19 @@ AUTHORS_FILE_NAME = 'AUTHORS'
 
 DOCS_PREVIEW_URL = 'https://skia.org/?cl='
 
+# Path to CQ bots feature is described in skbug.com/4364
+PATH_PREFIX_TO_EXTRA_TRYBOTS = {
+    # pylint: disable=line-too-long
+    'cmake/': 'client.skia.compile:Build-Mac10.9-Clang-x86_64-Release-CMake-Trybot,Build-Ubuntu-GCC-x86_64-Release-CMake-Trybot',
+    # pylint: disable=line-too-long
+    'src/opts/': 'client.skia:Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-SKNX_NO_SIMD-Trybot',
+
+    # Below are examples to show what is possible with this feature.
+    # 'src/svg/': 'master1:abc;master2:def',
+    # 'src/svg/parser/': 'master3:ghi,jkl;master4:mno',
+    # 'src/image/SkImage_Base.h': 'master5:pqr,stu;master1:abc1;master2:def',
+}
+
 
 def _CheckChangeHasEol(input_api, output_api, source_file_filter=None):
   """Checks that files end with atleast one \n (LF)."""
@@ -339,6 +350,7 @@ def PostUploadHook(cl, change, output_api):
     work on them.
   * Adds 'NOPRESUBMIT=true' for non master branch changes since those don't
     run the presubmit checks.
+  * Adds extra trybots for the paths defined in PATH_TO_EXTRA_TRYBOTS.
   """
 
   results = []
@@ -408,34 +420,22 @@ def PostUploadHook(cl, change, output_api):
             output_api.PresubmitNotifyResult(
                 'Branch changes do not run the presubmit checks.'))
 
-    # Read and process the HASHTAGS file.
+    # Automatically set CQ_EXTRA_TRYBOTS if any of the changed files here begin
-    hashtags_fullpath = os.path.join(change._local_root, 'HASHTAGS')
+    # with the paths of interest.
-    with open(hashtags_fullpath, 'rb') as hashtags_csv:
+    cq_master_to_trybots = collections.defaultdict(set)
-      hashtags_reader = csv.reader(hashtags_csv, delimiter=',')
+    for affected_file in change.AffectedFiles():
-      for row in hashtags_reader:
+      affected_file_path = affected_file.LocalPath()
-        if not row or row[0].startswith('#'):
+      for path_prefix, extra_bots in PATH_PREFIX_TO_EXTRA_TRYBOTS.iteritems():
-          # Ignore empty lines and comments
+        if affected_file_path.startswith(path_prefix):
-          continue
+          results.append(
-        hashtag = row[0]
+              output_api.PresubmitNotifyResult(
-        # Search for the hashtag in the description.
+                  'Your CL modifies the path %s.\nAutomatically adding %s to '
-        if re.search('#%s' % hashtag, new_description, re.M | re.I):
+                  'the CL description.' % (affected_file_path, extra_bots)))
-          for mapped_text in row[1:]:
+          _MergeCQExtraTrybotsMaps(
-            # Special case handling for CQ_KEYWORDS_THAT_NEED_APPENDING.
+              cq_master_to_trybots, _GetCQExtraTrybotsMap(extra_bots))
-            appended_description = _HandleAppendingCQKeywords(
+    if cq_master_to_trybots:
-                hashtag, mapped_text, new_description, results, output_api)
+      new_description = _AddCQExtraTrybotsToDesc(
-            if appended_description:
+          cq_master_to_trybots, new_description)
-              new_description = appended_description
-              continue
-
-            # Add the mapped text if it does not already exist in the
-            # CL's description.
-            if not re.search(
-                r'^%s$' % mapped_text, new_description, re.M | re.I):
-              new_description += '\n%s' % mapped_text
-              results.append(
-                  output_api.PresubmitNotifyResult(
-                      'Found \'#%s\', automatically added \'%s\' to the CL\'s '
-                      'description' % (hashtag, mapped_text)))
 
     # If the description has changed update it.
     if new_description != original_description:
@@ -444,29 +444,49 @@ def PostUploadHook(cl, change, output_api):
     return results
 
 
-def _HandleAppendingCQKeywords(hashtag, keyword_and_value, description,
+def _AddCQExtraTrybotsToDesc(cq_master_to_trybots, description):
-                               results, output_api):
+  """Adds the specified master and trybots to the CQ_EXTRA_TRYBOTS keyword.
-  """Handles the CQ keywords that need appending if specified in hashtags."""
+
-  keyword = keyword_and_value.split('=')[0]
+  If the keyword already exists in the description then it appends to it only
-  if keyword in CQ_KEYWORDS_THAT_NEED_APPENDING:
+  if the specified values do not already exist.
-    # If the keyword is already in the description then append to it.
+  If the keyword does not exist then it creates a new section in the
-    match = re.search(
+  description.
-        r'^%s=(.*)$' % keyword, description, re.M | re.I)
+  """
-    if match:
+  match = re.search(r'^CQ_EXTRA_TRYBOTS=(.*)$', description, re.M | re.I)
-      old_values = match.group(1).split(';')
+  if match:
-      new_value = keyword_and_value.split('=')[1]
+    original_trybots_map = _GetCQExtraTrybotsMap(match.group(1))
-      if new_value in old_values:
+    _MergeCQExtraTrybotsMaps(cq_master_to_trybots, original_trybots_map)
-        # Do not need to do anything here.
+    new_description = description.replace(
-        return description
+        match.group(0), _GetCQExtraTrybotsStr(cq_master_to_trybots))
-      # Update the description with the new values.
+  else:
-      new_description = description.replace(
+    new_description = description + "\n%s" % (
-          match.group(0), "%s;%s" % (match.group(0), new_value))
+        _GetCQExtraTrybotsStr(cq_master_to_trybots))
-      results.append(
+  return new_description
-          output_api.PresubmitNotifyResult(
+
-          'Found \'#%s\', automatically appended \'%s\' to %s in '
+
-          'the CL\'s description' % (hashtag, new_value, keyword)))
+def _MergeCQExtraTrybotsMaps(dest_map, map_to_be_consumed):
-      return new_description
+  """Merges two maps of masters to trybots into one."""
-  return None
+  for master, trybots in map_to_be_consumed.iteritems():
+    dest_map[master].update(trybots)
+  return dest_map
+
+
+def _GetCQExtraTrybotsMap(cq_extra_trybots_str):
+  """Parses the CQ_EXTRA_TRYBOTS str and returns a map of masters to trybots."""
+  cq_master_to_trybots = collections.defaultdict(set)
+  for section in cq_extra_trybots_str.split(';'):
+    if section:
+      master, bots = section.split(':')
+      cq_master_to_trybots[master].update(bots.split(','))
+  return cq_master_to_trybots
+
+
+def _GetCQExtraTrybotsStr(cq_master_to_trybots):
+  """Constructs the CQ_EXTRA_TRYBOTS str from a map of masters to trybots."""
+  sections = []
+  for master, trybots in cq_master_to_trybots.iteritems():
+    sections.append('%s:%s' % (master, ','.join(trybots)))
+  return 'CQ_EXTRA_TRYBOTS=%s' % ';'.join(sections)
 
 
 def CheckChangeOnCommit(input_api, output_api):