diff --git a/gcl.py b/gcl.py index b593ff1eb..0c8fbb61e 100755 --- a/gcl.py +++ b/gcl.py @@ -961,11 +961,20 @@ 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', diff --git a/git_cl.py b/git_cl.py index cb33488e4..9946999c6 100755 --- a/git_cl.py +++ b/git_cl.py @@ -488,8 +488,7 @@ or verify this branch is set up to track another (via the --track argument to self.SetPatchset(0) self.has_issue = False - def RunHook(self, committing, upstream_branch, may_prompt, verbose, author): - """Calls sys.exit() if the hook fails; returns a HookResults otherwise.""" + def GetChange(self, upstream_branch, author): root = RunCommand(['git', 'rev-parse', '--show-cdup']).strip() or '.' absroot = os.path.abspath(root) @@ -511,7 +510,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 - change = presubmit_support.GitChange( + return presubmit_support.GitChange( name, description, absroot, @@ -520,6 +519,10 @@ 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()) diff --git a/git_try.py b/git_try.py index 06aaa6eb5..27eafba37 100755 --- a/git_try.py +++ b/git_try.py @@ -13,6 +13,7 @@ from scm import GIT import subprocess2 import third_party.upload import trychange +import git_cl def GetRietveldIssueNumber(): @@ -53,8 +54,10 @@ 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, file_list=[], swallow_exception=False, + args, change, file_list=[], swallow_exception=False, prog='git try', extra_epilog='\n' 'git try will diff against your tracked branch and will ' diff --git a/presubmit_support.py b/presubmit_support.py index 5ac38ac3c..fa2261a7f 100755 --- a/presubmit_support.py +++ b/presubmit_support.py @@ -16,6 +16,7 @@ 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 @@ -875,7 +876,7 @@ def ListRelevantPresubmitFiles(files, root): class GetTrySlavesExecuter(object): @staticmethod - def ExecPresubmitScript(script_text, presubmit_path, project): + def ExecPresubmitScript(script_text, presubmit_path, project, change): """Executes GetPreferredTrySlaves() from a single presubmit script. Args: @@ -894,10 +895,14 @@ class GetTrySlavesExecuter(object): function_name = 'GetPreferredTrySlaves' if function_name in context: - try: - result = eval(function_name + '(' + repr(project) + ')', context) - except TypeError: - result = eval(function_name + '()', context) + get_preferred_try_slaves = context[function_name] + function_info = inspect.getargspec(get_preferred_try_slaves) + if len(function_info.args) == 1: + result = get_preferred_try_slaves(project) + elif len(function_info.args) == 2: + result = get_preferred_try_slaves(project, change) + else: + result = get_preferred_try_slaves() if not isinstance(result, types.ListType): raise PresubmitFailure( 'Presubmit functions must return a list, got a %s instead: %s' % @@ -913,7 +918,8 @@ class GetTrySlavesExecuter(object): return result -def DoGetTrySlaves(changed_files, +def DoGetTrySlaves(change, + changed_files, repository_root, default_presubmit, project, @@ -942,7 +948,7 @@ def DoGetTrySlaves(changed_files, 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) + default_presubmit, fake_path, project, change) for filename in presubmit_files: filename = os.path.abspath(filename) if verbose: @@ -950,7 +956,7 @@ def DoGetTrySlaves(changed_files, # Accept CRLF presubmit script. presubmit_script = gclient_utils.FileRead(filename, 'rU') results += executer.ExecPresubmitScript( - presubmit_script, filename, project) + presubmit_script, filename, project, change) slaves = list(set(results)) if slaves and verbose: diff --git a/tests/presubmit_unittest.py b/tests/presubmit_unittest.py index 466189e9c..055e8b6fc 100755 --- a/tests/presubmit_unittest.py +++ b/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', 'json', + 'fix_encoding', 'fnmatch', 'gclient_utils', 'glob', 'inspect', 'json', 'load_files', 'logging', 'marshal', 'normpath', 'optparse', 'os', 'owners', 'pickle', 'presubmit_canned_checks', 'random', 're', 'rietveld', 'scm', @@ -669,11 +669,18 @@ 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('', '', '')) - self.assertEqual( - [], executer.ExecPresubmitScript('def foo():\n return\n', '', '')) + self.assertEqual([], executer.ExecPresubmitScript('', '', '', change)) + self.assertEqual([], + executer.ExecPresubmitScript('def foo():\n return\n', '', '', change)) # bad results starts_with_space_result = [' starts_with_space'] @@ -682,7 +689,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, '', '') + self.presubmit_tryslave % result, '', '', change) # good results expected_result = ['1', '2', '3'] @@ -692,20 +699,31 @@ def CheckChangeOnCommit(input_api, output_api): self.assertEqual( result, executer.ExecPresubmitScript( - self.presubmit_tryslave % result, '', '')) + self.presubmit_tryslave % result, '', '', change)) 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')) + expected_result1, executer.ExecPresubmitScript(script, '', 'foo', + change)) self.assertEqual( - expected_result2, executer.ExecPresubmitScript(script, '', 'bar')) + expected_result2, executer.ExecPresubmitScript(script, '', 'bar', + change)) def testDoGetTrySlaves(self): join = presubmit.os.path.join @@ -730,13 +748,18 @@ 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([filename], self.fake_root_dir, + presubmit.DoGetTrySlaves(change, [filename], + self.fake_root_dir, None, None, False, output)) output = StringIO.StringIO() self.assertEqual(['win', 'linux'], - presubmit.DoGetTrySlaves([filename, filename_linux], + presubmit.DoGetTrySlaves(change, + [filename, filename_linux], self.fake_root_dir, None, None, False, output)) diff --git a/tests/trychange_unittest.py b/tests/trychange_unittest.py index 22c4e6a57..f02c1bf21 100755 --- a/tests/trychange_unittest.py +++ b/tests/trychange_unittest.py @@ -59,8 +59,8 @@ class SVNUnittest(TryChangeTestsBase): """trychange.SVN tests.""" def testMembersChanged(self): members = [ - 'AutomagicalSettings', 'GetCodeReviewSetting', 'ReadRootFile', - 'GenerateDiff', 'GetFileNames', + 'AutomagicalSettings', 'CaptureStatus', 'GetCodeReviewSetting', + 'ReadRootFile', 'GenerateDiff', 'GetFileNames', 'files', 'file_tuples', ] # 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', 'GetCodeReviewSetting', 'ReadRootFile', - 'GenerateDiff', 'GetFileNames', + 'AutomagicalSettings', 'CaptureStatus', 'GetCodeReviewSetting', + 'ReadRootFile', 'GenerateDiff', 'GetFileNames', 'files', 'file_tuples', ] # If this test fails, you should add the relevant test. self.compareMembers(trychange.GIT, members) diff --git a/trychange.py b/trychange.py index 9825b1e8b..e322f1794 100755 --- a/trychange.py +++ b/trychange.py @@ -100,7 +100,8 @@ class SCM(object): items.append(None) self.diff_against = items[1] self.options = options - self.files = self.options.files + 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' @@ -187,6 +188,38 @@ 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.""" @@ -210,29 +243,19 @@ 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) @@ -255,19 +278,10 @@ class GIT(SCM): "(via the --track argument to \"git checkout -b ...\"") logging.info("GIT(%s)" % self.checkout_root) - 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 + def CaptureStatus(self): + return scm.GIT.CaptureStatus(self.checkout_root, self.diff_against) - self.files = filter(NotExcluded, self.files) + def GenerateDiff(self): return scm.GIT.GenerateDiff(self.checkout_root, files=self.files, full_move=True, branch=self.diff_against) @@ -464,6 +478,7 @@ def GetMungedDiff(path_diff, diff): def TryChange(argv, + change, file_list, swallow_exception, prog=None, @@ -701,6 +716,18 @@ 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 @@ -708,7 +735,16 @@ 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, @@ -720,18 +756,6 @@ 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 @@ -767,4 +791,4 @@ def TryChange(argv, if __name__ == "__main__": fix_encoding.fix_encoding() - sys.exit(TryChange(None, [], False)) + sys.exit(TryChange(None, None, [], False))