diff --git a/git_cl.py b/git_cl.py index a51f8783c..b97ed255e 100755 --- a/git_cl.py +++ b/git_cl.py @@ -822,9 +822,6 @@ class Settings(object): def GetDefaultCCList(self): return self._GetConfig('rietveld.cc') - def GetUsePython3(self): - return self._GetConfig('rietveld.use-python3') - def GetSquashGerritUploads(self): """Returns True if uploads to Gerrit should be squashed by default.""" if self.squash_gerrit_uploads is None: @@ -1259,9 +1256,6 @@ class Changelist(object): server = _KNOWN_GERRIT_TO_SHORT_URLS.get(server, server) return '%s/%s' % (server, issue) - def GetUsePython3(self): - return settings.GetUsePython3() - def FetchDescription(self, pretty=False): assert self.GetIssue(), 'issue is required to query Gerrit' @@ -1432,42 +1426,27 @@ class Changelist(object): ' was not specified. To enable ResultDB, please run the command' ' again with the --realm argument to specify the LUCI realm.') - py3_results = self._RunPresubmit(args, - description, - use_python3=True, - resultdb=resultdb, - realm=realm) - if py3_results.get('skipped_presubmits', 1) == 0: - print('No more presubmits to run - skipping Python 2 presubmits.') - return py3_results - - py2_results = self._RunPresubmit(args, - description, - use_python3=False, - resultdb=resultdb, - realm=realm) - return self._MergePresubmitResults(py2_results, py3_results) + return self._RunPresubmit(args, + description, + resultdb=resultdb, + realm=realm) def _RunPresubmit(self, args, description, - use_python3, resultdb=None, realm=None): # type: (Sequence[str], str, bool, Optional[bool], Optional[str] # ) -> Mapping[str, Any] args = args[:] - vpython = 'vpython3' if use_python3 else 'vpython' with gclient_utils.temporary_file() as description_file: with gclient_utils.temporary_file() as json_output: gclient_utils.FileWrite(description_file, description) args.extend(['--json_output', json_output]) args.extend(['--description_file', description_file]) - if self.GetUsePython3(): - args.append('--use-python3') start = time_time() - cmd = [vpython, PRESUBMIT_SUPPORT] + args + cmd = ['vpython3', PRESUBMIT_SUPPORT] + args if resultdb and realm: cmd = ['rdb', 'stream', '-new', '-realm', realm, '--'] + cmd @@ -1486,34 +1465,14 @@ class Changelist(object): json_results = gclient_utils.FileRead(json_output) return json.loads(json_results) - def _MergePresubmitResults(self, py2_results, py3_results): - return { - 'more_cc': sorted(set(py2_results.get('more_cc', []) + - py3_results.get('more_cc', []))), - 'errors': ( - py2_results.get('errors', []) + py3_results.get('errors', [])), - 'notifications': ( - py2_results.get('notifications', []) + - py3_results.get('notifications', [])), - 'warnings': ( - py2_results.get('warnings', []) + py3_results.get('warnings', [])) - } - - def RunPostUploadHook(self, verbose, upstream, description, py3_only): + def RunPostUploadHook(self, verbose, upstream, description): args = self._GetCommonPresubmitArgs(verbose, upstream) args.append('--post_upload') with gclient_utils.temporary_file() as description_file: gclient_utils.FileWrite(description_file, description) args.extend(['--description_file', description_file]) - run_py2 = not py3_only and os.getenv('LUCI_OMIT_PYTHON2') != 'true' - if run_py2: - p_py2 = subprocess2.Popen(['vpython', PRESUBMIT_SUPPORT] + args) - p_py3 = subprocess2.Popen(['vpython3', PRESUBMIT_SUPPORT] + args + - ['--use-python3']) - if run_py2: - p_py2.wait() - p_py3.wait() + subprocess2.Popen(['vpython3', PRESUBMIT_SUPPORT] + args).wait() def _GetDescriptionForUpload(self, options, git_diff_args, files): # type: (optparse.Values, Sequence[str], Sequence[str] @@ -1839,8 +1798,7 @@ class Changelist(object): if settings.GetRunPostUploadHook(): self.RunPostUploadHook(options.verbose, new_upload.parent, - new_upload.change_desc.description, - options.no_python2_post_upload_hooks) + new_upload.change_desc.description) if new_upload.reviewers or new_upload.ccs: gerrit_util.AddReviewers(self.GetGerritHost(), @@ -1896,8 +1854,7 @@ class Changelist(object): # Run post upload hooks, if specified. if settings.GetRunPostUploadHook(): self.RunPostUploadHook(options.verbose, base_branch, - change_desc.description, - options.no_python2_post_upload_hooks) + change_desc.description) # Upload all dependencies if specified. if options.dependencies: @@ -3429,7 +3386,6 @@ def LoadCodereviewSettingsFromFile(fileobj): unset_error_ok=True) SetProperty( 'format-full-by-default', 'FORMAT_FULL_BY_DEFAULT', unset_error_ok=True) - SetProperty('use-python3', 'USE_PYTHON3', unset_error_ok=True) if 'GERRIT_HOST' in keyvals: RunGit(['config', 'gerrit.host', keyvals['GERRIT_HOST']]) @@ -4744,9 +4700,6 @@ def CMDupload(parser, args): action='store_true', dest='no_add_changeid', help='Do not add change-ids to messages.') - parser.add_option('--no-python2-post-upload-hooks', - action='store_true', - help='Only run post-upload hooks in Python 3.') parser.add_option('--cherry-pick-stacked', '--cp', dest='cherry_pick_stacked', diff --git a/presubmit_support.py b/presubmit_support.py index 2375d7635..0d19a1cbd 100755 --- a/presubmit_support.py +++ b/presubmit_support.py @@ -310,32 +310,6 @@ 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 - """ - if os.getenv('LUCI_OMIT_PYTHON2') == 'true': - # If LUCI omits python2, run all presubmits with python3, regardless of - # USE_PYTHON3 variable. - return True - - 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): @@ -1412,17 +1386,14 @@ def ListRelevantPresubmitFiles(files, root): class GetPostUploadExecuter(object): - def __init__(self, change, gerrit_obj, use_python3): + def __init__(self, change, gerrit_obj): """ Args: change: The Change object. gerrit_obj: provides basic Gerrit codereview functionality. - use_python3: if true, will use python3 instead of python2 by default - if USE_PYTHON3 is not specified. """ self.change = change self.gerrit = gerrit_obj - self.use_python3 = use_python3 def ExecPresubmitScript(self, script_text, presubmit_path): """Executes PostUploadHook() from a single presubmit script. @@ -1479,15 +1450,13 @@ def _MergeMasters(masters1, masters2): return result -def DoPostUploadExecuter(change, gerrit_obj, verbose, use_python3=False): +def DoPostUploadExecuter(change, gerrit_obj, verbose): """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) @@ -1496,7 +1465,7 @@ def DoPostUploadExecuter(change, gerrit_obj, verbose, use_python3=False): if not presubmit_files and verbose: sys.stdout.write('Warning, no PRESUBMIT.py found.\n') results = [] - executer = GetPostUploadExecuter(change, gerrit_obj, use_python3) + executer = GetPostUploadExecuter(change, gerrit_obj) # 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. @@ -1506,14 +1475,9 @@ def DoPostUploadExecuter(change, gerrit_obj, verbose, use_python3=False): filename = os.path.abspath(filename) # Accept CRLF presubmit script. presubmit_script = gclient_utils.FileRead(filename).replace('\r\n', '\n') - if _ShouldRunPresubmit(presubmit_script, use_python3): - if sys.version_info[0] == 2: - sys.stdout.write( - 'Running %s under Python 2. Add USE_PYTHON3 = True to prevent ' - 'this.\n' % filename) - elif verbose: - sys.stdout.write('Running %s\n' % filename) - results.extend(executer.ExecPresubmitScript(presubmit_script, filename)) + if verbose: + sys.stdout.write('Running %s\n' % filename) + results.extend(executer.ExecPresubmitScript(presubmit_script, filename)) if not results: return 0 @@ -1532,8 +1496,7 @@ def DoPostUploadExecuter(change, gerrit_obj, verbose, use_python3=False): class PresubmitExecuter(object): def __init__(self, change, committing, verbose, gerrit_obj, dry_run=None, - thread_pool=None, parallel=False, use_python3=False, - no_diffs=False): + thread_pool=None, parallel=False, no_diffs=False): """ Args: change: The Change object. @@ -1542,8 +1505,6 @@ class PresubmitExecuter(object): dry_run: if true, some Checks will be skipped. parallel: if true, all tests reported via input_api.RunTests for all PRESUBMIT files will be run in parallel. - use_python3: if true, will use python3 instead of python2 by default - if USE_PYTHON3 is not specified. no_diffs: if true, implies that --files or --all was specified so some checks can be skipped, and some errors will be messages. """ @@ -1555,7 +1516,6 @@ class PresubmitExecuter(object): self.more_cc = [] self.thread_pool = thread_pool self.parallel = parallel - self.use_python3 = use_python3 self.no_diffs = no_diffs def ExecPresubmitScript(self, script_text, presubmit_path): @@ -1728,7 +1688,6 @@ def DoPresubmitChecks(change, dry_run=None, parallel=False, json_output=None, - use_python3=False, no_diffs=False): """Runs all presubmit checks that apply to the files in the change. @@ -1750,8 +1709,6 @@ def DoPresubmitChecks(change, dry_run: if true, some Checks will be skipped. parallel: if true, all tests specified by input_api.RunTests in all PRESUBMIT files will be run in parallel. - use_python3: if true, default to using Python3 for presubmit checks - rather than Python2. no_diffs: if true, implies that --files or --all was specified so some checks can be skipped, and some errors will be messages. Return: @@ -1785,31 +1742,19 @@ def DoPresubmitChecks(change, os.remove(python2_usage_log_file) thread_pool = ThreadPool() executer = PresubmitExecuter(change, committing, verbose, gerrit_obj, - dry_run, thread_pool, parallel, use_python3, - no_diffs) - skipped_count = 0; + dry_run, thread_pool, parallel, no_diffs) if default_presubmit: if verbose: sys.stdout.write('Running default presubmit script.\n') fake_path = os.path.join(change.RepositoryRoot(), 'PRESUBMIT.py') - if _ShouldRunPresubmit(default_presubmit, use_python3): - results += executer.ExecPresubmitScript(default_presubmit, fake_path) - else: - skipped_count += 1 + results += executer.ExecPresubmitScript(default_presubmit, fake_path) for filename in presubmit_files: filename = os.path.abspath(filename) # Accept CRLF presubmit script. presubmit_script = gclient_utils.FileRead(filename).replace('\r\n', '\n') - if _ShouldRunPresubmit(presubmit_script, use_python3): - if sys.version_info[0] == 2: - sys.stdout.write( - 'Running %s under Python 2. Add USE_PYTHON3 = True to prevent ' - 'this.\n' % filename) - elif verbose: - sys.stdout.write('Running %s\n' % filename) - results += executer.ExecPresubmitScript(presubmit_script, filename) - else: - skipped_count += 1 + if verbose: + sys.stdout.write('Running %s\n' % filename) + results += executer.ExecPresubmitScript(presubmit_script, filename) results += thread_pool.RunAsync() @@ -1880,7 +1825,6 @@ def DoPresubmitChecks(change, for warning in messages.get('Warnings', []) ], 'more_cc': executer.more_cc, - 'skipped_presubmits': skipped_count, } gclient_utils.FileWrite( @@ -2079,8 +2023,6 @@ def main(argv=None): 'wildcards can also be used.') parser.add_argument('--source_controlled_only', action='store_true', help='Constrain \'files\' to those in source control.') - parser.add_argument('--use-python3', action='store_true', - help='Use python3 for presubmit checks by default') parser.add_argument('--no_diffs', action='store_true', help='Assume that all "modified" files have no diffs.') options = parser.parse_args(argv) @@ -2107,8 +2049,7 @@ def main(argv=None): try: if options.post_upload: - return DoPostUploadExecuter(change, gerrit_obj, options.verbose, - options.use_python3) + return DoPostUploadExecuter(change, gerrit_obj, options.verbose) with canned_check_filter(options.skip_canned): return DoPresubmitChecks( change, @@ -2120,7 +2061,6 @@ def main(argv=None): options.dry_run, options.parallel, options.json_output, - options.use_python3, options.no_diffs) except PresubmitFailure as e: import utils diff --git a/recipes/recipe_modules/presubmit/api.py b/recipes/recipe_modules/presubmit/api.py index f8ad37574..538e49415 100644 --- a/recipes/recipe_modules/presubmit/api.py +++ b/recipes/recipe_modules/presubmit/api.py @@ -30,7 +30,7 @@ class PresubmitApi(recipe_api.RecipeApi): name = kwargs.pop('name', 'presubmit') with self.m.depot_tools.on_path(): - cmd = ['vpython3', self.presubmit_support_path, '--use-python3'] + cmd = ['vpython3', self.presubmit_support_path] cmd.extend(args) cmd.extend(['--json_output', self.m.json.output()]) if self.m.resultdb.enabled: diff --git a/recipes/recipe_modules/presubmit/examples/full.expected/basic.json b/recipes/recipe_modules/presubmit/examples/full.expected/basic.json index cee7a1137..e19d71e14 100644 --- a/recipes/recipe_modules/presubmit/examples/full.expected/basic.json +++ b/recipes/recipe_modules/presubmit/examples/full.expected/basic.json @@ -3,7 +3,6 @@ "cmd": [ "vpython3", "RECIPE_REPO[depot_tools]/presubmit_support.py", - "--use-python3", "--json_output", "/path/to/tmp/json" ], diff --git a/tests/git_cl_test.py b/tests/git_cl_test.py index 28d3d9d45..8bd8db704 100755 --- a/tests/git_cl_test.py +++ b/tests/git_cl_test.py @@ -91,9 +91,6 @@ class ChangelistMock(object): def GetRemoteBranch(self): return ('origin', 'refs/remotes/origin/main') - def GetUsePython3(self): - return self._use_python3 - class GitMocks(object): def __init__(self, config=None, branchref=None): self.branchref = branchref or 'refs/heads/main' @@ -724,8 +721,6 @@ class TestGitCl(unittest.TestCase): CERR1), ((['git', 'config', '--unset-all', 'rietveld.format-full-by-default'],), CERR1), - ((['git', 'config', '--unset-all', 'rietveld.use-python3'],), - CERR1), ((['git', 'config', 'gerrit.host', 'true'],), ''), ] self.assertIsNone(git_cl.LoadCodereviewSettingsFromFile(codereview_file)) @@ -3642,7 +3637,6 @@ class ChangelistTest(unittest.TestCase): mock.patch('git_cl.Changelist.GetAuthor', return_value='author').start() mock.patch('git_cl.Changelist.GetIssue', return_value=123456).start() mock.patch('git_cl.Changelist.GetPatchset', return_value=7).start() - mock.patch('git_cl.Changelist.GetUsePython3', return_value=False).start() mock.patch( 'git_cl.Changelist.GetRemoteBranch', return_value=('origin', 'refs/remotes/origin/main')).start() @@ -3875,7 +3869,7 @@ class ChangelistTest(unittest.TestCase): def testRunPostUploadHook(self): cl = git_cl.Changelist() - cl.RunPostUploadHook(2, 'upstream', 'description', False) + cl.RunPostUploadHook(2, 'upstream', 'description') subprocess2.Popen.assert_called_with([ 'vpython3', @@ -3901,7 +3895,6 @@ class ChangelistTest(unittest.TestCase): '--post_upload', '--description_file', '/tmp/fake-temp1', - '--use-python3', ]) gclient_utils.FileWrite.assert_called_once_with( @@ -3909,7 +3902,7 @@ class ChangelistTest(unittest.TestCase): def testRunPostUploadHookPy3Only(self): cl = git_cl.Changelist() - cl.RunPostUploadHook(2, 'upstream', 'description', True) + cl.RunPostUploadHook(2, 'upstream', 'description') subprocess2.Popen.assert_called_once_with([ 'vpython3', @@ -3935,7 +3928,6 @@ class ChangelistTest(unittest.TestCase): '--post_upload', '--description_file', '/tmp/fake-temp1', - '--use-python3', ]) gclient_utils.FileWrite.assert_called_once_with('/tmp/fake-temp1', @@ -4224,7 +4216,7 @@ class ChangelistTest(unittest.TestCase): ccs=ccs, notify=False) mockRunPostHook.assert_called_once_with(True, 'parent-commit', - change_desc.description, True) + change_desc.description) class CMDTestCaseBase(unittest.TestCase): diff --git a/tests/presubmit_unittest.py b/tests/presubmit_unittest.py index 00a65bab8..7081f4fb2 100755 --- a/tests/presubmit_unittest.py +++ b/tests/presubmit_unittest.py @@ -642,8 +642,7 @@ class PresubmitUnittest(PresubmitTestsBase): change = self.ExampleChange() executer = presubmit.GetPostUploadExecuter(change, - presubmit.GerritAccessor(), - False) + presubmit.GerritAccessor()) executer.ExecPresubmitScript(self.presubmit_text, fake_presubmit) @@ -806,7 +805,6 @@ def CheckChangeOnCommit(input_api, output_api): } ], 'more_cc': ['me@example.com'], - 'skipped_presubmits': 0, } fake_result_json = json.dumps(fake_result, sort_keys=True)