Add use_python3 to codereview.settings to set default for PRESUBMITs.

This CL adds a new field to the codereview.settings file used by
git_cl for project-wide defaults. If `USE_PYTHON3` is set to True,
then we will run the PRESUBMIT checks under Python3 by default
instead of Python2, unless the PRESUBMIT.py file contains
`USE_PYTHON3 = False` on a line by itself

(as opposed to now, where we'll use Python2 by default unless the
file contains `USE_PYTHON3 = True`).

This will allow us have Python3 be the default for new files
and to eventually eliminate any uses of `USE_PYTHON3` from the
individual presubmit files. Of course, you will have to go in and
explicitly add `USE_PYTHON3 = False` to any Py2-only files prior
to enabling this.

Bug: 1207012
Change-Id: Id8ec45b43940e5bcffeee196398c711c541e733e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2917747
Reviewed-by: Nico Weber <thakis@chromium.org>
Reviewed-by: Josip Sokcevic <sokcevic@google.com>
Reviewed-by: Edward Lesmes <ehmaldonado@chromium.org>
Commit-Queue: Dirk Pranke <dpranke@google.com>

git_cl.py

@@ -773,6 +773,9 @@ 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:
@@ -1179,6 +1182,9 @@ 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'
 
@@ -1343,7 +1349,8 @@ class Changelist(object):
         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
         if resultdb and realm:
@@ -2953,6 +2960,7 @@ 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']])

presubmit_support.py

@@ -1537,7 +1537,7 @@ def DoPostUploadExecuter(change,
 
 class PresubmitExecuter(object):
   def __init__(self, change, committing, verbose, gerrit_obj, dry_run=None,
-               thread_pool=None, parallel=False):
+               thread_pool=None, parallel=False, use_python3=False):
     """
     Args:
       change: The Change object.
@@ -1546,6 +1546,8 @@ 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.
     """
     self.change = change
     self.committing = committing
@@ -1555,6 +1557,7 @@ class PresubmitExecuter(object):
     self.more_cc = []
     self.thread_pool = thread_pool
     self.parallel = parallel
+    self.use_python3 = use_python3
 
   def ExecPresubmitScript(self, script_text, presubmit_path):
     """Executes a single presubmit script.
@@ -1585,8 +1588,12 @@ class PresubmitExecuter(object):
     # 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$', script_text, flags=re.MULTILINE)
-    use_python3 = m is not None
+    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 []
@@ -1715,7 +1722,8 @@ def DoPresubmitChecks(change,
                       gerrit_obj,
                       dry_run=None,
                       parallel=False,
-                      json_output=None):
+                      json_output=None,
+                      use_python3=False):
   """Runs all presubmit checks that apply to the files in the change.
 
   This finds all PRESUBMIT.py files in directories enclosing the files in the
@@ -1736,7 +1744,8 @@ 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.
   Return:
     1 if presubmit checks failed or 0 otherwise.
   """
@@ -1761,7 +1770,7 @@ def DoPresubmitChecks(change,
     results = []
     thread_pool = ThreadPool()
     executer = PresubmitExecuter(change, committing, verbose, gerrit_obj,
-                                 dry_run, thread_pool, parallel)
+                                 dry_run, thread_pool, parallel, use_python3)
     if default_presubmit:
       if verbose:
         sys.stdout.write('Running default presubmit script.\n')
@@ -2012,6 +2021,8 @@ def main(argv=None):
                       help='List of files to be marked as modified when '
                       'executing presubmit or post-upload hooks. fnmatch '
                       'wildcards can also be used.')
+  parser.add_argument('--use-python3', action='store_true',
+                      help='Use python3 for presubmit checks by default')
   options = parser.parse_args(argv)
 
   log_level = logging.ERROR
@@ -2044,7 +2055,8 @@ def main(argv=None):
           gerrit_obj,
           options.dry_run,
           options.parallel,
-          options.json_output)
+          options.json_output,
+          options.use_python3)
   except PresubmitFailure as e:
     print(e, file=sys.stderr)
     print('Maybe your depot_tools is out of date?', file=sys.stderr)

tests/git_cl_test.py

@@ -71,8 +71,9 @@ class ChangelistMock(object):
   # instance that's being set.
   desc = ''
 
-  def __init__(self, gerrit_change=None, **kwargs):
+  def __init__(self, gerrit_change=None, use_python3=False, **kwargs):
     self._gerrit_change = gerrit_change
+    self._use_python3 = use_python3
 
   def GetIssue(self):
     return 1
@@ -90,6 +91,8 @@ 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):
@@ -763,6 +766,8 @@ 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))
@@ -3031,6 +3036,7 @@ 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()