diff --git a/git_cl.py b/git_cl.py index 7222404dd..d59ea135b 100755 --- a/git_cl.py +++ b/git_cl.py @@ -1409,8 +1409,11 @@ class Changelist(object): with gclient_utils.temporary_file() as description_file: gclient_utils.FileWrite(description_file, description) args.extend(['--description_file', description_file]) - p = subprocess2.Popen(['vpython', PRESUBMIT_SUPPORT] + args) - p.wait() + p_py2 = subprocess2.Popen(['vpython', PRESUBMIT_SUPPORT] + args) + p_py3 = subprocess2.Popen(['vpython3', PRESUBMIT_SUPPORT] + args + + ['--use-python3']) + p_py2.wait() + p_py3.wait() def _GetDescriptionForUpload(self, options, git_diff_args, files): # Get description message for upload. diff --git a/presubmit_support.py b/presubmit_support.py index d74f57f98..84d5711fc 100755 --- a/presubmit_support.py +++ b/presubmit_support.py @@ -302,6 +302,27 @@ def prompt_should_continue(prompt_string): return response in ('y', 'yes') +def _ShouldRunPresubmit(script_text, use_python3): + """Try to figure out whether these presubmit checks should be run under + python2 or python3. We need to do this without actually trying to + compile the text, since the text might compile in one but not the + other. + + Args: + script_text: The text of the presubmit script. + use_python3: if true, will use python3 instead of python2 by default + if USE_PYTHON3 is not specified. + + Return: + A boolean if presubmit should be executed + """ + m = re.search('^USE_PYTHON3 = (True|False)$', script_text, flags=re.MULTILINE) + if m: + use_python3 = m.group(1) == 'True' + + return ((sys.version_info.major == 2) and not use_python3) or \ + ((sys.version_info.major == 3) and use_python3) + # Top level object so multiprocessing can pickle # Public access through OutputApi object. class _PresubmitResult(object): @@ -1352,8 +1373,16 @@ def ListRelevantPresubmitFiles(files, root): class GetPostUploadExecuter(object): - @staticmethod - def ExecPresubmitScript(script_text, presubmit_path, gerrit_obj, change): + def __init__(self, use_python3): + """ + Args: + use_python3: if true, will use python3 instead of python2 by default + if USE_PYTHON3 is not specified. + """ + self.use_python3 = use_python3 + + def ExecPresubmitScript(self, script_text, presubmit_path, gerrit_obj, + change): """Executes PostUploadHook() from a single presubmit script. Args: @@ -1365,6 +1394,9 @@ class GetPostUploadExecuter(object): Return: A list of results objects. """ + if not _ShouldRunPresubmit(script_text, self.use_python3): + return {} + context = {} try: exec(compile(script_text, 'PRESUBMIT.py', 'exec', dont_inherit=True), @@ -1394,22 +1426,24 @@ def _MergeMasters(masters1, masters2): return result -def DoPostUploadExecuter(change, - gerrit_obj, - verbose): +def DoPostUploadExecuter(change, gerrit_obj, verbose, use_python3=False): """Execute the post upload hook. Args: change: The Change object. gerrit_obj: The GerritAccessor object. verbose: Prints debug info. + use_python3: if true, default to using Python3 for presubmit checks + rather than Python2. """ + python_version = 'Python %s' % sys.version_info.major + sys.stdout.write('Running %s post upload checks ...\n' % python_version) presubmit_files = ListRelevantPresubmitFiles( change.LocalPaths(), change.RepositoryRoot()) if not presubmit_files and verbose: sys.stdout.write('Warning, no PRESUBMIT.py found.\n') results = [] - executer = GetPostUploadExecuter() + executer = GetPostUploadExecuter(use_python3) # The root presubmit file should be executed after the ones in subdirectories. # i.e. the specific post upload hooks should run before the general ones. # Thus, reverse the order provided by ListRelevantPresubmitFiles. @@ -1474,6 +1508,8 @@ class PresubmitExecuter(object): Return: A list of result objects, empty if no problems. """ + if not _ShouldRunPresubmit(script_text, self.use_python3): + return [] # Change to the presubmit file's directory to support local imports. main_path = os.getcwd() @@ -1488,20 +1524,6 @@ class PresubmitExecuter(object): output_api = OutputApi(self.committing) context = {} - # Try to figure out whether these presubmit checks should be run under - # python2 or python3. We need to do this without actually trying to - # compile the text, since the text might compile in one but not the - # other. - m = re.search('^USE_PYTHON3 = (True|False)$', script_text, - flags=re.MULTILINE) - if m: - use_python3 = m.group(1) == 'True' - else: - use_python3 = self.use_python3 - if (((sys.version_info.major == 2) and use_python3) or - ((sys.version_info.major == 3) and not use_python3)): - return [] - try: exec(compile(script_text, 'PRESUBMIT.py', 'exec', dont_inherit=True), context) @@ -1948,10 +1970,8 @@ def main(argv=None): try: if options.post_upload: - return DoPostUploadExecuter( - change, - gerrit_obj, - options.verbose) + return DoPostUploadExecuter(change, gerrit_obj, options.verbose, + options.use_python3) with canned_check_filter(options.skip_canned): return DoPresubmitChecks( change, diff --git a/tests/git_cl_test.py b/tests/git_cl_test.py index 7be4ebc3c..a50473554 100755 --- a/tests/git_cl_test.py +++ b/tests/git_cl_test.py @@ -3204,20 +3204,58 @@ class ChangelistTest(unittest.TestCase): cl = git_cl.Changelist() cl.RunPostUploadHook(2, 'upstream', 'description') - subprocess2.Popen.assert_called_once_with([ - 'vpython', 'PRESUBMIT_SUPPORT', - '--root', 'root', - '--upstream', 'upstream', - '--verbose', '--verbose', - '--gerrit_url', 'https://chromium-review.googlesource.com', - '--gerrit_project', 'project', - '--gerrit_branch', 'refs/heads/main', - '--author', 'author', - '--issue', '123456', - '--patchset', '7', + subprocess2.Popen.assert_any_call([ + 'vpython', + 'PRESUBMIT_SUPPORT', + '--root', + 'root', + '--upstream', + 'upstream', + '--verbose', + '--verbose', + '--gerrit_url', + 'https://chromium-review.googlesource.com', + '--gerrit_project', + 'project', + '--gerrit_branch', + 'refs/heads/main', + '--author', + 'author', + '--issue', + '123456', + '--patchset', + '7', '--post_upload', - '--description_file', '/tmp/fake-temp1', + '--description_file', + '/tmp/fake-temp1', ]) + subprocess2.Popen.assert_called_with([ + 'vpython3', + 'PRESUBMIT_SUPPORT', + '--root', + 'root', + '--upstream', + 'upstream', + '--verbose', + '--verbose', + '--gerrit_url', + 'https://chromium-review.googlesource.com', + '--gerrit_project', + 'project', + '--gerrit_branch', + 'refs/heads/main', + '--author', + 'author', + '--issue', + '123456', + '--patchset', + '7', + '--post_upload', + '--description_file', + '/tmp/fake-temp1', + '--use-python3', + ]) + gclient_utils.FileWrite.assert_called_once_with( '/tmp/fake-temp1', 'description') diff --git a/tests/presubmit_unittest.py b/tests/presubmit_unittest.py index ede038309..1789edf62 100755 --- a/tests/presubmit_unittest.py +++ b/tests/presubmit_unittest.py @@ -645,7 +645,9 @@ class PresubmitUnittest(PresubmitTestsBase): 0, presubmit.DoPostUploadExecuter( change=change, gerrit_obj=None, verbose=False)) - self.assertEqual('', sys.stdout.getvalue()) + self.assertEqual( + 'Running Python ' + str(sys.version_info.major) + ' ' + 'post upload checks ...\n', sys.stdout.getvalue()) def testDoPostUploadExecuterWarning(self): path = os.path.join(self.fake_root_dir, 'PRESUBMIT.py') @@ -658,11 +660,12 @@ class PresubmitUnittest(PresubmitTestsBase): presubmit.DoPostUploadExecuter( change=change, gerrit_obj=None, verbose=False)) self.assertEqual( + 'Running Python ' + str(sys.version_info.major) + ' ' + 'post upload checks ...\n' '\n' '** Post Upload Hook Messages **\n' '??\n' - '\n', - sys.stdout.getvalue()) + '\n', sys.stdout.getvalue()) def testDoPostUploadExecuterWarning(self): path = os.path.join(self.fake_root_dir, 'PRESUBMIT.py') @@ -675,11 +678,12 @@ class PresubmitUnittest(PresubmitTestsBase): presubmit.DoPostUploadExecuter( change=change, gerrit_obj=None, verbose=False)) self.assertEqual( + 'Running Python ' + str(sys.version_info.major) + ' ' + 'post upload checks ...\n' '\n' '** Post Upload Hook Messages **\n' '!!\n' - '\n', - sys.stdout.getvalue()) + '\n', sys.stdout.getvalue()) def testDoPresubmitChecksNoWarningsOrErrors(self): haspresubmit_path = os.path.join(