Allow PRESUBMIT.py files to have old and new-style trybot specifications (as long as each file is c…

presubmit_support.py

 #!/usr/bin/env python
 # 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.
 
 """Enables directory-specific presubmit checks to run at upload and/or commit.
 """
 
 __version__ = '1.7.0'
 
@@ skipping 1037 matching lines @@
                                  ' format.')
 
         if botname != botname.strip():
           raise PresubmitFailure(
               'Try slave names cannot start/end with whitespace')
         if ',' in botname:
           raise PresubmitFailure(
               'Do not use \',\' separated builder or test names: %s' % botname)
     else:
       result = []
+
+    def valid_oldstyle(result):
+      return all(isinstance(i, basestring) for i in result)
+
+    def valid_newstyle(result):
+      return (all(isinstance(i, tuple) for i in result) and
+              all(len(i) == 2 for i in result) and
+              all(isinstance(i[0], basestring) for i in result) and
+              all(isinstance(i[1], set) for i in result)
+             )
+
+    # Ensure it's either all old-style or all new-style.
+    if not (valid_oldstyle(result) or valid_newstyle(result)):

[M-A Ruel, 2013/12/18 00:52:43]

optional nit: I think the following would be slightly more readable but I don't
mind:
if not valid_oldstyle(result) and not valid_newstyle(result):

+      raise PresubmitFailure(
+          'PRESUBMIT.py returned invalid trybot specification!')
+
     return result
 
 
 def DoGetTrySlaves(change,
                    changed_files,
                    repository_root,
                    default_presubmit,
                    project,
                    verbose,
                    output_stream):
   """Get the list of try servers from the presubmit scripts.
 
   Args:
     changed_files: List of modified files.
     repository_root: The repository root.
     default_presubmit: A default presubmit script to execute in any case.
     project: Optional name of a project used in selecting trybots.
     verbose: Prints debug info.
     output_stream: A stream to write debug output to.
 
   Return:
     List of try slaves
   """
   presubmit_files = ListRelevantPresubmitFiles(changed_files, repository_root)
   if not presubmit_files and verbose:
     output_stream.write("Warning, no presubmit.py found.\n")
   results = []
   executer = GetTrySlavesExecuter()
+
   if default_presubmit:
     if verbose:
       output_stream.write("Running default presubmit script.\n")
     fake_path = os.path.join(repository_root, 'PRESUBMIT.py')
-    results += executer.ExecPresubmitScript(
-        default_presubmit, fake_path, project, change)
+    results.extend(executer.ExecPresubmitScript(
+        default_presubmit, fake_path, project, change))
   for filename in presubmit_files:
     filename = os.path.abspath(filename)
     if verbose:
       output_stream.write("Running %s\n" % filename)
     # Accept CRLF presubmit script.
     presubmit_script = gclient_utils.FileRead(filename, 'rU')
-    results += executer.ExecPresubmitScript(
-        presubmit_script, filename, project, change)
+    results.extend(executer.ExecPresubmitScript(
+        presubmit_script, filename, project, change))
 
-  if all(isinstance(i, tuple) for i in results):
-    # New-style [('bot', set(['tests']))] format.
-    slave_dict = {}
-    for result in results:
-      slave_dict.setdefault(result[0], set()).update(result[1])
-    slaves = list(slave_dict.iteritems())
-  elif all(isinstance(i, basestring) for i in results):
-    # Old-style ['bot'] format.
-    slaves = list(set(results))
-  else:
-    raise ValueError('PRESUBMIT.py returned invalid trybot specification!')
+
+  slave_dict = {}
+  old_style = filter(lambda x: isinstance(x, basestring), results)
+  new_style = filter(lambda x: isinstance(x, tuple), results)
+
+  for result in new_style:
+    slave_dict.setdefault(result[0], set()).update(result[1])
+  slaves = list(slave_dict.iteritems())
+
+  slaves.extend(set(old_style))
 
   if slaves and verbose:
-    output_stream.write(', '.join(slaves))
+    output_stream.write(', '.join((str(x) for x in slaves)))

[M-A Ruel, 2013/12/18 00:52:43]

sorted(slaves) would make a more deterministic output.

     output_stream.write('\n')
   return slaves
 
 
 class PresubmitExecuter(object):
   def __init__(self, change, committing, rietveld_obj, verbose):
     """
     Args:
       change: The Change object.
       committing: True if 'gcl commit' is running, False if 'gcl upload' is.
@@ skipping 338 matching lines @@
   except PresubmitFailure, e:
     print >> sys.stderr, e
     print >> sys.stderr, 'Maybe your depot_tools is out of date?'
     print >> sys.stderr, 'If all fails, contact maruel@'
     return 2
 
 
 if __name__ == '__main__':
   fix_encoding.fix_encoding()
   sys.exit(Main(None))