diff --git a/git-try b/git-try index 3b5f31223..146cd43af 100755 --- a/git-try +++ b/git-try @@ -72,8 +72,9 @@ def GetSubRepWorkingDir(sub_rep_path): def GetMungedDiff(branch, prefix, sub_rep_path): """Get the diff we'll send to the try server. We munge paths to match svn. - We add the prefix that the try bot is expecting. If sub_rep_path is - provided, diff will be calculated in the sub repository.""" + We add the prefix that the try bot is expecting. If sub_rep_path is + provided, diff will be calculated in the sub repository. + We also return the list of files in this diff, without munged paths.""" # Make the following changes: # - Prepend "src/" (or some other prefix) to paths as svn is expecting # - In the case of added files, replace /dev/null with the path to the file @@ -96,7 +97,7 @@ def GetMungedDiff(branch, prefix, sub_rep_path): # Add the right prefix command.extend(['--src-prefix=' + sub_rep_path]) command.extend(['--dst-prefix=' + sub_rep_path]) - + command.extend([branch, 'HEAD']) # Run diff tree @@ -113,23 +114,28 @@ def GetMungedDiff(branch, prefix, sub_rep_path): # Add root prefix output = [] + file_set = set() for line in diff: if line.startswith('--- ') or line.startswith('+++ '): - line = line[0:4] + os.path.join(prefix,line[4:]) + filename = line[4:] + line = line[0:4] + os.path.join(prefix, filename) + file_set.add(filename.rstrip('\r\n')) output.append(line) munged_diff = ''.join(output) if len(munged_diff.strip()) == 0: raise Exception("Patch was empty, did you give the right remote branch?") - return munged_diff + return (munged_diff, list(file_set)) def OneRepositoryDiff(diff_file, patch_names, branch, prefix, sub_rep_path): """Computes a diff for one git repository at a given path against a given branch. Writes the diff into diff_file and appends a name to the - patch_names list.""" - - diff = GetMungedDiff(branch, prefix, sub_rep_path) + patch_names list. + + Returns the list of files in the diff.""" + + (diff, file_list) = GetMungedDiff(branch, prefix, sub_rep_path) # Write the diff out diff_file.write(diff) @@ -137,6 +143,7 @@ def OneRepositoryDiff(diff_file, patch_names, branch, prefix, sub_rep_path): # Add patch name to list of patches patch_name = GetPatchName(GetSubRepWorkingDir(sub_rep_path)) patch_names.extend([patch_name]) + return file_list def ValidEmail(email): @@ -150,11 +157,9 @@ def GetEmail(): return email -def TryChange(args): +def TryChange(args, file_list): """Put a patch on the try server.""" - root_dir = Backquote(['git', 'rev-parse', '--show-cdup']) - trychange.checkout_root = os.path.abspath(root_dir) - trychange.TryChange(args, None, False) + trychange.TryChange(args, file_list, False) if __name__ == '__main__': @@ -169,12 +174,12 @@ if __name__ == '__main__': parser.add_option("-r", "--revision", help="Specify the SVN base revision to use") parser.add_option("--root", default="src", metavar="PATH", - help="Specify the root prefix that is appended to paths " + help="Specify the root prefix that is prepended to paths " "in the patch") parser.add_option("--dry_run", action="store_true", help="Print the diff but don't send it to the try bots") - parser.add_option("--sub_rep", nargs=2, action="append", default=[], - metavar="PATH BRANCH", + parser.add_option("--sub_rep", nargs=2, action="append", default=[], + metavar="PATH BRANCH", help="Specify a path to a git sub-repository and a branch " "to diff with in order to simultanously try changes " "in multiple git repositories. Option may be " @@ -182,7 +187,7 @@ if __name__ == '__main__': parser.add_option("--webkit", metavar="BRANCH", help="Specify webkit branch. Syntactic sugar for " "--sub_rep third_party/WebKit/ ") - + (options, args) = parser.parse_args(sys.argv) if options.webkit: @@ -195,17 +200,18 @@ if __name__ == '__main__': # Dump all diffs into one diff file. diff_file = tempfile.NamedTemporaryFile() - + # Calculate diff for main git repository. - OneRepositoryDiff(diff_file, patch_names, branch, options.root, None) - + file_list = OneRepositoryDiff(diff_file, patch_names, branch, options.root, + None) + # Calculate diff for each extra git repository. for path_and_branch in options.sub_rep: - OneRepositoryDiff(diff_file, - patch_names, - path_and_branch[1], - options.root, - path_and_branch[0]) + file_list.extend(OneRepositoryDiff(diff_file, + patch_names, + path_and_branch[1], + options.root, + path_and_branch[0])) # Make diff file ready for reading. diff_file.flush() @@ -217,7 +223,7 @@ if __name__ == '__main__': '-u', user, '-e', email, '-n', '-'.join(patch_names), - '--diff', diff_file.name, + '--diff', diff_file.name, ] # Send to try server via HTTP if we can parse the config, otherwise @@ -257,4 +263,4 @@ if __name__ == '__main__': exit(0) print sendmsg - TryChange(args=args) + TryChange(args, file_list) diff --git a/presubmit_support.py b/presubmit_support.py index 4ca487ec5..b4ee31197 100755 --- a/presubmit_support.py +++ b/presubmit_support.py @@ -6,7 +6,7 @@ """Enables directory-specific presubmit checks to run at upload and/or commit. """ -__version__ = '1.3.2' +__version__ = '1.3.3' # TODO(joi) Add caching where appropriate/needed. The API is designed to allow # caching (between all different invocations of presubmit scripts for a given @@ -744,6 +744,78 @@ def ListRelevantPresubmitFiles(files, root): return filter(lambda x: os.path.isfile(x), entries) +class GetTrySlavesExecuter(object): + def ExecPresubmitScript(self, script_text): + """Executes GetPreferredTrySlaves() from a single presubmit script. + + Args: + script_text: The text of the presubmit script. + + Return: + A list of try slaves. + """ + context = {} + exec script_text in context + + function_name = 'GetPreferredTrySlaves' + if function_name in context: + result = eval(function_name + '()', context) + if not isinstance(result, types.ListType): + raise exceptions.RuntimeError( + 'Presubmit functions must return a list, got a %s instead: %s' % + (type(result), str(result))) + for item in result: + if not isinstance(item, basestring): + raise exceptions.RuntimeError('All try slaves names must be strings.') + if item != item.strip(): + raise exceptions.RuntimeError('Try slave names cannot start/end' + 'with whitespace') + else: + result = [] + return result + + +def DoGetTrySlaves(changed_files, + repository_root, + default_presubmit, + 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. + 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") + results += executer.ExecPresubmitScript(default_presubmit) + 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 = gcl.ReadFile(filename, 'rU') + results += executer.ExecPresubmitScript(presubmit_script) + + slaves = list(set(results)) + if slaves and verbose: + output_stream.write(', '.join(slaves)) + output_stream.write('\n') + return slaves + + class PresubmitExecuter(object): def __init__(self, change, committing): """ diff --git a/tests/presubmit_unittest.py b/tests/presubmit_unittest.py index c985b2c04..95e49ba6e 100755 --- a/tests/presubmit_unittest.py +++ b/tests/presubmit_unittest.py @@ -31,6 +31,10 @@ def CheckChangeOnUpload(input_api, output_api): else: return () """ + presubmit_tryslave = """ +def GetPreferredTrySlaves(): + return %s +""" def setUp(self): super_mox.SuperMoxTestBase.setUp(self) @@ -73,8 +77,8 @@ class PresubmitUnittest(PresubmitTestsBase): def testMembersChanged(self): self.mox.ReplayAll() members = [ - 'AffectedFile', 'Change', 'DoPresubmitChecks', 'GitChange', - 'GitAffectedFile', 'InputApi', + 'AffectedFile', 'Change', 'DoGetTrySlaves', 'DoPresubmitChecks', + 'GetTrySlavesExecuter', 'GitChange', 'GitAffectedFile', 'InputApi', 'ListRelevantPresubmitFiles', 'Main', 'NotImplementedException', 'OutputApi', 'ParseFiles', 'PresubmitExecuter', 'ScanSubDirs', 'SvnAffectedFile', 'SvnChange', @@ -484,6 +488,60 @@ def CheckChangeOnCommit(input_api, output_api): '** Presubmit Messages **\n' 'http://tracker.com/42\n\n')) + def testGetTrySlavesExecuter(self): + self.mox.ReplayAll() + + executer = presubmit.GetTrySlavesExecuter() + self.assertEqual([], executer.ExecPresubmitScript('')) + self.assertEqual([], executer.ExecPresubmitScript('def foo():\n return\n')) + + # bad results + starts_with_space_result = [' starts_with_space'] + not_list_result1 = "'foo'" + not_list_result2 = "('a', 'tuple')" + for result in starts_with_space_result, not_list_result1, not_list_result2: + self.assertRaises(exceptions.RuntimeError, + executer.ExecPresubmitScript, + self.presubmit_tryslave % result) + + # good results + expected_result = ['1', '2', '3'] + empty_result = [] + space_in_name_result = ['foo bar', '1\t2 3'] + for result in expected_result, empty_result, space_in_name_result: + self.assertEqual(result, + executer.ExecPresubmitScript(self.presubmit_tryslave % + str(result))) + + def testDoGetTrySlaves(self): + join = presubmit.os.path.join + filename = 'foo.cc' + filename_linux = join('linux_only', 'penguin.cc') + root_presubmit = join(self.fake_root_dir, 'PRESUBMIT.py') + linux_presubmit = join(self.fake_root_dir, 'linux_only', 'PRESUBMIT.py') + + presubmit.os.path.isfile(root_presubmit).AndReturn(True) + presubmit.gcl.ReadFile(root_presubmit, 'rU').AndReturn( + self.presubmit_tryslave % '["win"]') + + presubmit.os.path.isfile(root_presubmit).AndReturn(True) + presubmit.os.path.isfile(linux_presubmit).AndReturn(True) + presubmit.gcl.ReadFile(root_presubmit, 'rU').AndReturn( + self.presubmit_tryslave % '["win"]') + presubmit.gcl.ReadFile(linux_presubmit, 'rU').AndReturn( + self.presubmit_tryslave % '["linux"]') + self.mox.ReplayAll() + + output = StringIO.StringIO() + self.assertEqual(['win'], + presubmit.DoGetTrySlaves([filename], self.fake_root_dir, + None, False, output)) + output = StringIO.StringIO() + self.assertEqual(['win', 'linux'], + presubmit.DoGetTrySlaves([filename, filename_linux], + self.fake_root_dir, None, False, + output)) + def testMain(self): self.mox.StubOutWithMock(presubmit, 'DoPresubmitChecks') self.mox.StubOutWithMock(presubmit, 'ParseFiles') diff --git a/tests/trychange_unittest.py b/tests/trychange_unittest.py index 84344cd72..e59786720 100644 --- a/tests/trychange_unittest.py +++ b/tests/trychange_unittest.py @@ -5,11 +5,14 @@ """Unit tests for trychange.py.""" +import optparse import unittest # Local imports +import gcl import super_mox import trychange +import upload from super_mox import mox @@ -27,8 +30,8 @@ class TryChangeUnittest(TryChangeTestsBase): 'HELP_STRING', 'InvalidScript', 'NoTryServerAccess', 'PathDifference', 'RunCommand', 'SCM', 'SVN', 'TryChange', 'USAGE', 'datetime', 'gcl', 'gclient', 'gclient_scm', 'getpass', 'logging', - 'optparse', 'os', 'shutil', 'socket', 'sys', 'tempfile', 'traceback', - 'upload', 'urllib', 'subprocess', + 'optparse', 'os', 'presubmit_support', 'shutil', 'socket', 'subprocess', + 'sys', 'tempfile', 'traceback', 'upload', 'urllib', ] # If this test fails, you should add the relevant test. self.compareMembers(trychange, members) @@ -36,23 +39,62 @@ class TryChangeUnittest(TryChangeTestsBase): class SVNUnittest(TryChangeTestsBase): """trychange.SVN tests.""" + def setUp(self): + self.fake_root = '/fake_root' + self.expected_files = ['foo.txt', 'bar.txt'] + change_info = gcl.ChangeInfo('test_change', 0, 0, 'desc', + [('M', f) for f in self.expected_files], + self.fake_root) + self.svn = trychange.SVN(None) + self.svn.change_info = change_info + super_mox.SuperMoxTestBase.setUp(self) + def testMembersChanged(self): members = [ - 'GenerateDiff', 'ProcessOptions', 'options' + 'GenerateDiff', 'GetFileNames', 'GetLocalRoot', 'ProcessOptions', + 'options' ] # If this test fails, you should add the relevant test. self.compareMembers(trychange.SVN(None), members) + def testGetFileNames(self): + self.mox.ReplayAll() + self.assertEqual(self.svn.GetFileNames(), self.expected_files) + + def testGetLocalRoot(self): + self.mox.ReplayAll() + self.assertEqual(self.svn.GetLocalRoot(), self.fake_root) + class GITUnittest(TryChangeTestsBase): """trychange.GIT tests.""" + def setUp(self): + self.fake_root = '/fake_root' + self.expected_files = ['foo.txt', 'bar.txt'] + options = optparse.Values() + options.files = self.expected_files + self.git = trychange.GIT(options) + super_mox.SuperMoxTestBase.setUp(self) + def testMembersChanged(self): members = [ - 'GenerateDiff', 'GetEmail', 'GetPatchName', 'ProcessOptions', 'options' + 'GenerateDiff', 'GetEmail', 'GetFileNames', 'GetLocalRoot', + 'GetPatchName', 'ProcessOptions', 'options' ] # If this test fails, you should add the relevant test. self.compareMembers(trychange.GIT(None), members) + def testGetFileNames(self): + self.mox.ReplayAll() + self.assertEqual(self.git.GetFileNames(), self.expected_files) + + def testGetLocalRoot(self): + self.mox.StubOutWithMock(upload, 'RunShell') + upload.RunShell(['git', 'rev-parse', '--show-cdup']).AndReturn( + self.fake_root) + self.mox.ReplayAll() + self.assertEqual(self.git.GetLocalRoot(), self.fake_root) + if __name__ == '__main__': unittest.main() diff --git a/trychange.py b/trychange.py index c556fd39f..64a9099e6 100755 --- a/trychange.py +++ b/trychange.py @@ -24,6 +24,7 @@ import urllib import gcl import gclient import gclient_scm +import presubmit_support import upload __version__ = '1.1.1' @@ -195,6 +196,14 @@ class SVN(SCM): os.chdir(previous_cwd) return "".join(diff) + def GetFileNames(self): + """Return the list of files in the diff.""" + return self.change_info.GetFileNames() + + def GetLocalRoot(self): + """Return the path of the repository root.""" + return self.change_info.GetLocalRoot() + def ProcessOptions(self): if not self.options.diff: # Generate the diff with svn and write it to the submit queue path. The @@ -205,6 +214,8 @@ class SVN(SCM): prefix = PathDifference(source_root, gcl.GetRepositoryRoot()) adjusted_paths = [os.path.join(prefix, x) for x in self.options.files] self.options.diff = self.GenerateDiff(adjusted_paths, root=source_root) + self.change_info = gcl.LoadChangelistInfoForMultiple(self.options.name, + gcl.GetRepositoryRoot(), True, True) class GIT(SCM): @@ -225,6 +236,16 @@ class GIT(SCM): # TODO: check for errors here? return upload.RunShell(['git', 'config', 'user.email']).strip() + def GetFileNames(self): + """Return the list of files in the diff.""" + return self.options.files + + def GetLocalRoot(self): + """Return the path of the repository root.""" + # TODO: check for errors here? + root = upload.RunShell(['git', 'rev-parse', '--show-cdup']).strip() + return os.path.abspath(root) + def GetPatchName(self): """Construct a name for this patch.""" # TODO: perhaps include the hash of the current commit, to distinguish @@ -410,6 +431,12 @@ def TryChange(argv, file_list, swallow_exception, prog=None): + """ + Args: + argv: Arguments and options. + file_list: Default value to pass to --file. + swallow_exception: Whether we raise or swallow exceptions. + """ default_settings = GetTryServerSettings() transport_functions = { 'http': _SendChangeHTTP, 'svn': _SendChangeSVN } default_transport = transport_functions.get( @@ -542,9 +569,19 @@ def TryChange(argv, if not options.diff: raise + # Get try slaves from PRESUBMIT.py files if not specified. + if not options.bot: + root_presubmit = gcl.GetCachedFile('PRESUBMIT.py', use_root=True) + options.bot = presubmit_support.DoGetTrySlaves(options.scm.GetFileNames(), + options.scm.GetLocalRoot(), + root_presubmit, + False, + sys.stdout) + # Send the patch. patch_name = options.send_patch(options) - print 'Patch \'%s\' sent to try server.' % patch_name + print 'Patch \'%s\' sent to try server: %s' % (patch_name, + ', '.join(options.bot)) if patch_name == 'Unnamed': print "Note: use --name NAME to change the try's name." except (InvalidScript, NoTryServerAccess), e: