Support for |change| argument to |GetPreferredTrySlaves()|.

trychange.py

Index: trychange.py
===================================================================
--- trychange.py	(revision 102730)
+++ trychange.py	(working copy)
@@ -101,6 +101,7 @@
     self.diff_against = items[1]
     self.options = options
     self.files = self.options.files
+    self.file_tuples = [('M', f) for f in self.files]
     self.options.files = None
     self.codereview_settings = None
     self.codereview_settings_file = 'codereview.settings'
@@ -110,6 +111,10 @@
     """Return the list of files in the diff."""
     return self.files
 
+  def GetFileTuples(self):
+    """Return the list of file (status, filename) tuples in the diff."""
+    return self.file_tuples
+
   def GetCodeReviewSetting(self, key):
     """Returns a value for the given key for this repository.
 
@@ -187,7 +192,21 @@
     logging.warning('Didn\'t find %s' % filename)
     return None
 
+  def SetFileTuples(self, file_tuples):

[M-A Ruel, 2011/09/26 17:48:34]

I'd rather have self.files and self.file_tuples be a @property and have it lazy
loaded as needed. wdyt?

[Alexei Svitkine (slow), 2011/09/26 18:14:06]

Like so?

+    excluded = ['!', '?', 'X', ' ', '~']
+    def Excluded(f):
+      if f[0][0] in excluded:
+        return True
+      for r in self.options.exclude:
+        if re.search(r, f[1]):
+          logging.info('Ignoring "%s"' % f[1])
+          return True
+      return False
 
+    self.file_tuples = [f for f in file_tuples if not Excluded(f)]
+    self.files = [f[1] for f in self.file_tuples]
+
+
 class SVN(SCM):
   """Gathers the options and diff for a subversion checkout."""
   def __init__(self, *args, **kwargs):
@@ -219,19 +238,7 @@
     if not self.files:
       previous_cwd = os.getcwd()
       os.chdir(self.checkout_root)
-
-      excluded = ['!', '?', 'X', ' ', '~']
-      def Excluded(f):
-        if f[0][0] in excluded:
-          return True
-        for r in self.options.exclude:
-          if re.search(r, f[1]):
-            logging.info('Ignoring "%s"' % f[1])
-            return True
-        return False
-
-      self.files = [f[1] for f in scm.SVN.CaptureStatus(self.checkout_root)
-                    if not Excluded(f)]
+      self.SetFileTuples(scm.SVN.CaptureStatus(self.checkout_root))
       os.chdir(previous_cwd)
     return scm.SVN.GenerateDiff(self.files, self.checkout_root, full_move=True,
                                 revision=self.diff_against)
@@ -257,17 +264,8 @@
 
   def GenerateDiff(self):
     if not self.files:
-      self.files = scm.GIT.GetDifferentFiles(self.checkout_root,
-                                             branch=self.diff_against)
-
-    def NotExcluded(f):
-      for r in self.options.exclude:
-        if re.search(r, f):
-          logging.info('Ignoring "%s"' % f)
-          return False
-      return True
-
-    self.files = filter(NotExcluded, self.files)
+      self.SetFileTuples(scm.GIT.CaptureStatus(self.checkout_root,
+                                               self.diff_against))
     return scm.GIT.GenerateDiff(self.checkout_root, files=self.files,
                                 full_move=True,
                                 branch=self.diff_against)
@@ -464,6 +462,7 @@
 
 
 def TryChange(argv,
+              change,
               file_list,
               swallow_exception,
               prog=None,
@@ -701,6 +700,19 @@
         diffs.extend(GetMungedDiff(path_diff, diff))
       options.diff = ''.join(diffs)
 
+    if options.name is None:

[M-A Ruel, 2011/09/26 17:48:34]

if not options.name:

Not sure why it was the other form before.

[Alexei Svitkine (slow), 2011/09/26 18:14:06]

Done.

+      if options.issue:
+        options.name = 'Issue %s' % options.issue
+      else:
+        options.name = 'Unnamed'
+        print('Note: use --name NAME to change the try job name.')
+
+	if not options.email:
+	  parser.error('Using an anonymous checkout. Please use --email or set '
+	               'the TRYBOT_RESULTS_EMAIL_ADDRESS environment variable.')
+	else:

[M-A Ruel, 2011/09/26 17:48:34]

actually, remove the else, since the parser.error() call quits anyway.

[Alexei Svitkine (slow), 2011/09/26 18:14:06]

Done.

+	  print('Results will be emailed to: ' + options.email)
+
     if not options.bot:
       # Get try slaves from PRESUBMIT.py files if not specified.
       # Even if the diff comes from options.url, use the local checkout for bot
@@ -708,7 +720,16 @@
       try:
         import presubmit_support
         root_presubmit = checkouts[0].ReadRootFile('PRESUBMIT.py')
+        if change is None:
+	      change = presubmit_support.Change(options.name,
+	                                        '',
+	                                        checkouts[0].checkout_root,
+	                                        checkouts[0].GetFileTuples(),
+	                                        options.issue,
+	                                        options.patchset,
+	                                        options.email)
         options.bot = presubmit_support.DoGetTrySlaves(
+            change,
             checkouts[0].GetFileNames(),
             checkouts[0].checkout_root,
             root_presubmit,
@@ -720,18 +741,6 @@
       # If no bot is specified, either the default pool will be selected or the
       # try server will refuse the job. Either case we don't need to interfere.
 
-    if options.name is None:
-      if options.issue:
-        options.name = 'Issue %s' % options.issue
-      else:
-        options.name = 'Unnamed'
-        print('Note: use --name NAME to change the try job name.')
-    if not options.email:
-      parser.error('Using an anonymous checkout. Please use --email or set '
-                   'the TRYBOT_RESULTS_EMAIL_ADDRESS environment variable.')
-    else:
-      print('Results will be emailed to: ' + options.email)
-
     # Prevent rietveld updates if we aren't running all the tests.
     if options.testfilter is not None:
       options.issue = None
@@ -767,4 +776,4 @@
 
 if __name__ == "__main__":
   fix_encoding.fix_encoding()
-  sys.exit(TryChange(None, [], False))
+  sys.exit(TryChange(None, None, [], False))