Revert "Reland "Remove Python 2 support for PRESUBMIT.py""

This reverts commit 119cff3d2f.

Reason for revert: some builders still call py2 presubmit
http://go/bbid/8781466817788761185

We need to clean up recipe first before relanding this

Original change's description:
> Reland "Remove Python 2 support for PRESUBMIT.py"
>
> This reverts commit d4c6cbeb61.
>
> Reason for revert: Python 2 invocations of the presubmit system
> should be gone now.
>
> Original change's description:
> > Revert "Remove Python 2 support for PRESUBMIT.py"
> >
> > This reverts commit 8454fc2458.
> >
> > Reason for revert: post submit hooks failing
> > https://crbug.com/1422416
> >
> > Original change's description:
> > > Remove Python 2 support for PRESUBMIT.py
> > >
> > > The presubmit system still supports invoking PRESUBMIT.py files using
> > > Python 2. This has recently been turned off on the bots so this change
> > > removes support more completely.
> > >
> > > There are still some python3 parameters being passed around - it seemed
> > > better to do the simplest possible removal now, with a follow-up change
> > > to remove more support code after this has sat for a while.
> > >
> > > Tests run from PRESUBMIT.py files could still be run using Python 2, but
> > > those should also have been addressed already. Removing support for that
> > > will be done in a subsequent change.
> > >
> > > Bug: 1207012
> > > Change-Id: Id244d547a04438f83734dba269c3cc180c148b37
> > > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4315183
> > > Reviewed-by: Aravind Vasudevan <aravindvasudev@google.com>
> > > Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
> >
> > Bug: 1207012
> > Bug: 1422416
> > Change-Id: Iaf3102e63ec3c698d0258fac5746dbd92c30edbb
> > No-Presubmit: true
> > No-Tree-Checks: true
> > No-Try: true
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4317176
> > Auto-Submit: Josip Sokcevic <sokcevic@chromium.org>
> > Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
> > Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
>
> Bug: 1207012
> Bug: 1422416
> Change-Id: I9521095989c708188fca0251fa13881e086b1395
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4516015
> Reviewed-by: Aravind Vasudevan <aravindvasudev@google.com>
> Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
> Reviewed-by: Josip Sokcevic <sokcevic@chromium.org>

Bug: 1207012
Bug: 1422416
Change-Id: Ifa775ca19f8c7b3c166eb89a255c25c20aaf4a98
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4521578
Commit-Queue: Josip Sokcevic <sokcevic@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>

git_cl.py

@@ -1432,11 +1432,21 @@ class Changelist(object):
             ' was not specified. To enable ResultDB, please run the command'
             ' again with the --realm argument to specify the LUCI realm.')
 
-    return self._RunPresubmit(args,
-                              description,
-                              use_python3=True,
-                              resultdb=resultdb,
-                              realm=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)
 
   def _RunPresubmit(self,
                     args,

presubmit_support.py

@@ -309,6 +309,33 @@ def prompt_should_continue(prompt_string):
   response = sys.stdin.readline().strip().lower()
   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):
@@ -1479,9 +1506,14 @@ 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 verbose:
-      sys.stdout.write('Running %s\n' % filename)
-    results.extend(executer.ExecPresubmitScript(presubmit_script, filename))
+    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 not results:
     return 0
@@ -1755,18 +1787,29 @@ def DoPresubmitChecks(change,
     executer = PresubmitExecuter(change, committing, verbose, gerrit_obj,
                                  dry_run, thread_pool, parallel, use_python3,
                                  no_diffs)
+    skipped_count = 0;
     if default_presubmit:
       if verbose:
         sys.stdout.write('Running default presubmit script.\n')
       fake_path = os.path.join(change.RepositoryRoot(), 'PRESUBMIT.py')
-      results += executer.ExecPresubmitScript(default_presubmit, fake_path)
+      if _ShouldRunPresubmit(default_presubmit, use_python3):
+        results += executer.ExecPresubmitScript(default_presubmit, fake_path)
+      else:
+        skipped_count += 1
     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 verbose:
-        sys.stdout.write('Running %s\n' % filename)
-      results += executer.ExecPresubmitScript(presubmit_script, filename)
+      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
 
     results += thread_pool.RunAsync()
 
@@ -1837,6 +1880,7 @@ def DoPresubmitChecks(change,
             for warning in messages.get('Warnings', [])
         ],
         'more_cc': executer.more_cc,
+        'skipped_presubmits': skipped_count,
       }
 
       gclient_utils.FileWrite(

tests/git_cl_test.py

@@ -3706,6 +3706,25 @@ class ChangelistTest(unittest.TestCase):
         '--json_output', '/tmp/fake-temp2',
         '--description_file', '/tmp/fake-temp1',
     ])
+    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',
+        '--commit',
+        '--may_prompt',
+        '--parallel',
+        '--all_files',
+        '--no_diffs',
+        '--json_output', '/tmp/fake-temp4',
+        '--description_file', '/tmp/fake-temp3',
+    ])
     gclient_utils.FileWrite.assert_any_call(
         '/tmp/fake-temp1', 'description')
     metrics.collector.add_repeated('sub_commands', {

tests/presubmit_unittest.py

@@ -664,6 +664,11 @@ class PresubmitUnittest(PresubmitTestsBase):
             change=change, gerrit_obj=None, verbose=False))
     expected = (r'Running Python ' + str(sys.version_info.major) +
                 r' post upload checks \.\.\.\n')
+    if sys.version_info[0] == 2:
+      expected += ('Running .*PRESUBMIT.py under Python 2. Add USE_PYTHON3 = '
+                   'True to prevent this.\n')
+      expected += ('Running .*PRESUBMIT.py under Python 2. Add USE_PYTHON3 = '
+                   'True to prevent this.\n')
     self.assertRegexpMatches(sys.stdout.getvalue(), expected)
 
   def testDoPostUploadExecuterWarning(self):
@@ -695,12 +700,17 @@ class PresubmitUnittest(PresubmitTestsBase):
         presubmit.DoPostUploadExecuter(
             change=change, gerrit_obj=None, verbose=False))
 
+    extra = ''
+    if sys.version_info[0] == 2:
+      extra = ('Running .*PRESUBMIT.py under Python 2. Add USE_PYTHON3 = True '
+               'to prevent this.\n')
     expected = ('Running Python ' + str(sys.version_info.major) + ' '
                 'post upload checks \.\.\.\n'
+                '%s'
                 '\n'
                 '\*\* Post Upload Hook Messages \*\*\n'
                 '!!\n'
-                '\n')
+                '\n' % extra)
     self.assertRegexpMatches(sys.stdout.getvalue(), expected)
 
   def testDoPresubmitChecksNoWarningsOrErrors(self):
@@ -796,6 +806,7 @@ def CheckChangeOnCommit(input_api, output_api):
           }
         ],
         'more_cc': ['me@example.com'],
+        'skipped_presubmits': 0,
     }
 
     fake_result_json = json.dumps(fake_result, sort_keys=True)