Revert r102783 "Support for |change| argument to |GetPreferredTrySlaves()|."

Cause an infinite recursion in some context.

TBR=asvitkine@chromium.org
BUG=
TEST=


Review URL: http://codereview.chromium.org/8036046

git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@102836 0039d316-1c4b-4281-b951-d872f2087c98

gcl.py

@@ -961,20 +961,11 @@ def TryChange(change_info, args, swallow_exception):
     if change_info.patchset:
       trychange_args.extend(["--patchset", str(change_info.patchset)])
     file_list = change_info.GetFileNames()
-    change = presubmit_support.SvnChange(change_info.name,
-                                         change_info.description,
-                                         change_info.GetLocalRoot(),
-                                         change_info.GetFiles(),
-                                         change_info.issue,
-                                         change_info.patchset,
-                                         None)
   else:
     file_list = []
-
   trychange_args.extend(args)
   return trychange.TryChange(
       trychange_args,
-      change=change,
       file_list=file_list,
       swallow_exception=swallow_exception,
       prog='gcl try',

git_cl.py

@@ -488,7 +488,8 @@ or verify this branch is set up to track another (via the --track argument to
       self.SetPatchset(0)
     self.has_issue = False
 
-  def GetChange(self, upstream_branch, author):
+  def RunHook(self, committing, upstream_branch, may_prompt, verbose, author):
+    """Calls sys.exit() if the hook fails; returns a HookResults otherwise."""
     root = RunCommand(['git', 'rev-parse', '--show-cdup']).strip() or '.'
     absroot = os.path.abspath(root)
 
@@ -510,7 +511,7 @@ or verify this branch is set up to track another (via the --track argument to
 
     if not author:
       author = RunGit(['config', 'user.email']).strip() or None
-    return presubmit_support.GitChange(
+    change = presubmit_support.GitChange(
         name,
         description,
         absroot,
@@ -519,10 +520,6 @@ or verify this branch is set up to track another (via the --track argument to
         patchset,
         author)
 
-  def RunHook(self, committing, upstream_branch, may_prompt, verbose, author):
-    """Calls sys.exit() if the hook fails; returns a HookResults otherwise."""
-    change = self.GetChange(upstream_branch, author)
-
     # Apply watchlists on upload.
     if not committing:
       watchlist = watchlists.Watchlists(change.RepositoryRoot())

git_try.py

@@ -13,7 +13,6 @@ from scm import GIT
 import subprocess2
 import third_party.upload
 import trychange
-import git_cl
 
 
 def GetRietveldIssueNumber():
@@ -54,10 +53,8 @@ if __name__ == '__main__':
   # Hack around a limitation in logging.
   logging.getLogger().handlers = []
   try:
-    cl = git_cl.Changelist()
-    change = cl.GetChange(cl.GetUpstreamBranch(), None)
     sys.exit(trychange.TryChange(
-        args, change, file_list=[], swallow_exception=False,
+        args, file_list=[], swallow_exception=False,
         prog='git try',
         extra_epilog='\n'
                      'git try will diff against your tracked branch and will '

presubmit_support.py

@@ -16,7 +16,6 @@ import cPickle  # Exposed through the API.
 import cStringIO  # Exposed through the API.
 import fnmatch
 import glob
-import inspect
 import logging
 import marshal  # Exposed through the API.
 import optparse
@@ -876,7 +875,7 @@ def ListRelevantPresubmitFiles(files, root):
 
 class GetTrySlavesExecuter(object):
   @staticmethod
-  def ExecPresubmitScript(script_text, presubmit_path, project, change):
+  def ExecPresubmitScript(script_text, presubmit_path, project):
     """Executes GetPreferredTrySlaves() from a single presubmit script.
 
     Args:
@@ -895,14 +894,10 @@ class GetTrySlavesExecuter(object):
 
     function_name = 'GetPreferredTrySlaves'
     if function_name in context:
-      get_preferred_try_slaves = context[function_name]
-      function_info = inspect.getargspec(get_preferred_try_slaves)
-      if len(function_info[0]) == 1:
-        result = get_preferred_try_slaves(project)
-      elif len(function_info[0]) == 2:
-        result = get_preferred_try_slaves(project, change)
-      else:
-        result = get_preferred_try_slaves()
+      try:
+        result = eval(function_name + '(' + repr(project) + ')', context)
+      except TypeError:
+        result = eval(function_name + '()', context)
       if not isinstance(result, types.ListType):
         raise PresubmitFailure(
             'Presubmit functions must return a list, got a %s instead: %s' %
@@ -918,8 +913,7 @@ class GetTrySlavesExecuter(object):
     return result
 
 
-def DoGetTrySlaves(change,
-                   changed_files,
+def DoGetTrySlaves(changed_files,
                    repository_root,
                    default_presubmit,
                    project,
@@ -948,7 +942,7 @@ def DoGetTrySlaves(change,
       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)
+        default_presubmit, fake_path, project)
   for filename in presubmit_files:
     filename = os.path.abspath(filename)
     if verbose:
@@ -956,7 +950,7 @@ def DoGetTrySlaves(change,
     # Accept CRLF presubmit script.
     presubmit_script = gclient_utils.FileRead(filename, 'rU')
     results += executer.ExecPresubmitScript(
-        presubmit_script, filename, project, change)
+        presubmit_script, filename, project)
 
   slaves = list(set(results))
   if slaves and verbose:

tests/presubmit_unittest.py

@@ -153,7 +153,7 @@ class PresubmitUnittest(PresubmitTestsBase):
       'OutputApi', 'ParseFiles', 'PresubmitFailure',
       'PresubmitExecuter', 'PresubmitOutput', 'ScanSubDirs',
       'SvnAffectedFile', 'SvnChange', 'cPickle', 'cStringIO',
-      'fix_encoding', 'fnmatch', 'gclient_utils', 'glob', 'inspect', 'json',
+      'fix_encoding', 'fnmatch', 'gclient_utils', 'glob', 'json',
       'load_files',
       'logging', 'marshal', 'normpath', 'optparse', 'os', 'owners', 'pickle',
       'presubmit_canned_checks', 'random', 're', 'rietveld', 'scm',
@@ -669,18 +669,11 @@ def CheckChangeOnCommit(input_api, output_api):
 
   def testGetTrySlavesExecuter(self):
     self.mox.ReplayAll()
-    change = presubmit.Change(
-        'foo',
-        'Blah Blah\n\nSTORY=http://tracker.com/42\nBUG=boo\n',
-        self.fake_root_dir,
-        None,
-        0,
-        0,
-        None)
+
     executer = presubmit.GetTrySlavesExecuter()
-    self.assertEqual([], executer.ExecPresubmitScript('', '', '', change))
-    self.assertEqual([],
-        executer.ExecPresubmitScript('def foo():\n  return\n', '', '', change))
+    self.assertEqual([], executer.ExecPresubmitScript('', '', ''))
+    self.assertEqual(
+        [], executer.ExecPresubmitScript('def foo():\n  return\n', '', ''))
 
     # bad results
     starts_with_space_result = ['  starts_with_space']
@@ -689,7 +682,7 @@ def CheckChangeOnCommit(input_api, output_api):
     for result in starts_with_space_result, not_list_result1, not_list_result2:
       self.assertRaises(presubmit.PresubmitFailure,
                         executer.ExecPresubmitScript,
-                        self.presubmit_tryslave % result, '', '', change)
+                        self.presubmit_tryslave % result, '', '')
 
     # good results
     expected_result = ['1', '2', '3']
@@ -699,31 +692,20 @@ def CheckChangeOnCommit(input_api, output_api):
       self.assertEqual(
           result,
           executer.ExecPresubmitScript(
-              self.presubmit_tryslave % result, '', '', change))
+              self.presubmit_tryslave % result, '', ''))
 
   def testGetTrySlavesExecuterWithProject(self):
     self.mox.ReplayAll()
 
-    change = presubmit.Change(
-        'foo',
-        'Blah Blah\n\nSTORY=http://tracker.com/42\nBUG=boo\n',
-        self.fake_root_dir,
-        None,
-        0,
-        0,
-        None)
-
     executer = presubmit.GetTrySlavesExecuter()
     expected_result1 = ['1', '2']
     expected_result2 = ['a', 'b', 'c']
     script = self.presubmit_tryslave_project % (
         repr('foo'), repr(expected_result1), repr(expected_result2)) 
     self.assertEqual(
-        expected_result1, executer.ExecPresubmitScript(script, '', 'foo',
-                                                       change))
+        expected_result1, executer.ExecPresubmitScript(script, '', 'foo'))
     self.assertEqual(
-        expected_result2, executer.ExecPresubmitScript(script, '', 'bar',
-                                                       change))
+        expected_result2, executer.ExecPresubmitScript(script, '', 'bar'))
 
   def testDoGetTrySlaves(self):
     join = presubmit.os.path.join
@@ -748,18 +730,13 @@ def CheckChangeOnCommit(input_api, output_api):
         self.presubmit_tryslave % '["linux"]')
     self.mox.ReplayAll()
 
-    change = presubmit.Change(
-        'mychange', '', self.fake_root_dir, [], 0, 0, None)
-
     output = StringIO.StringIO()
     self.assertEqual(['win'],
-                     presubmit.DoGetTrySlaves(change, [filename],
-                                              self.fake_root_dir,
+                     presubmit.DoGetTrySlaves([filename], self.fake_root_dir,
                                               None, None, False, output))
     output = StringIO.StringIO()
     self.assertEqual(['win', 'linux'],
-                     presubmit.DoGetTrySlaves(change,
-                                              [filename, filename_linux],
+                     presubmit.DoGetTrySlaves([filename, filename_linux],
                                               self.fake_root_dir, None, None,
                                               False, output))
 

tests/trychange_unittest.py

@@ -59,8 +59,8 @@ class SVNUnittest(TryChangeTestsBase):
   """trychange.SVN tests."""
   def testMembersChanged(self):
     members = [
-      'AutomagicalSettings', 'CaptureStatus', 'GetCodeReviewSetting',
-      'ReadRootFile', 'GenerateDiff', 'GetFileNames', 'files', 'file_tuples',
+      'AutomagicalSettings', 'GetCodeReviewSetting', 'ReadRootFile',
+      'GenerateDiff', 'GetFileNames',
     ]
     # If this test fails, you should add the relevant test.
     self.compareMembers(trychange.SVN, members)
@@ -83,8 +83,8 @@ class GITUnittest(TryChangeTestsBase):
   """trychange.GIT tests."""
   def testMembersChanged(self):
     members = [
-      'AutomagicalSettings', 'CaptureStatus', 'GetCodeReviewSetting',
-      'ReadRootFile', 'GenerateDiff', 'GetFileNames', 'files', 'file_tuples',
+      'AutomagicalSettings', 'GetCodeReviewSetting', 'ReadRootFile',
+      'GenerateDiff', 'GetFileNames',
     ]
     # If this test fails, you should add the relevant test.
     self.compareMembers(trychange.GIT, members)

trychange.py

@@ -100,8 +100,7 @@ class SCM(object):
     items.append(None)
     self.diff_against = items[1]
     self.options = options
-    self._files = self.options.files
-    self._file_tuples = [('M', f) for f in self.files]
+    self.files = self.options.files
     self.options.files = None
     self.codereview_settings = None
     self.codereview_settings_file = 'codereview.settings'
@@ -188,38 +187,6 @@ class SCM(object):
     logging.warning('Didn\'t find %s' % filename)
     return None
 
-  def _SetFileTuples(self, file_tuples):
-    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]
-
-  def CaptureStatus(self):
-    """Returns the 'svn status' emulated output as an array of (status, file)
-       tuples."""
-    raise NotImplementedError(
-        "abstract method -- subclass %s must override" % self.__class__)
-
-  @property
-  def files(self):
-    if not self._files:
-      self._SetFileTuples(self.CaptureStatus())
-    return self._files
-
-  @property
-  def file_tuples(self):
-    if not self._file_tuples:
-      self._SetFileTuples(self.CaptureStatus())
-    return self._file_tuples
-
 
 class SVN(SCM):
   """Gathers the options and diff for a subversion checkout."""
@@ -243,19 +210,29 @@ class SVN(SCM):
     logging.debug('%s:\n%s' % (filename, data))
     return data
 
-  def CaptureStatus(self):
-    previous_cwd = os.getcwd()
-    os.chdir(self.checkout_root)
-    result = scm.SVN.CaptureStatus(self.checkout_root)
-    os.chdir(previous_cwd)
-    return result
-
   def GenerateDiff(self):
     """Returns a string containing the diff for the given file list.
 
     The files in the list should either be absolute paths or relative to the
     given root.
     """
+    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)]
+      os.chdir(previous_cwd)
     return scm.SVN.GenerateDiff(self.files, self.checkout_root, full_move=True,
                                 revision=self.diff_against)
 
@@ -278,10 +255,19 @@ class GIT(SCM):
             "(via the --track argument to \"git checkout -b ...\"")
     logging.info("GIT(%s)" % self.checkout_root)
 
-  def CaptureStatus(self):
-    return scm.GIT.CaptureStatus(self.checkout_root, self.diff_against)
-
   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)
     return scm.GIT.GenerateDiff(self.checkout_root, files=self.files,
                                 full_move=True,
                                 branch=self.diff_against)
@@ -478,7 +464,6 @@ def GetMungedDiff(path_diff, diff):
 
 
 def TryChange(argv,
-              change,
               file_list,
               swallow_exception,
               prog=None,
@@ -716,18 +701,6 @@ def TryChange(argv,
         diffs.extend(GetMungedDiff(path_diff, diff))
       options.diff = ''.join(diffs)
 
-    if not options.name:
-      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.')
-    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
@@ -735,16 +708,7 @@ def TryChange(argv,
       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].file_tuples,
-                                            options.issue,
-                                            options.patchset,
-                                            options.email)
         options.bot = presubmit_support.DoGetTrySlaves(
-            change,
             checkouts[0].GetFileNames(),
             checkouts[0].checkout_root,
             root_presubmit,
@@ -756,6 +720,18 @@ def TryChange(argv,
       # 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
@@ -791,4 +767,4 @@ def TryChange(argv,
 
 if __name__ == "__main__":
   fix_encoding.fix_encoding()
-  sys.exit(TryChange(None, None, [], False))
+  sys.exit(TryChange(None, [], False))