Browse Source

Extract searching relevat files in utils function

This change introduces lib/ directory which will be used for classes
that are considered libraries and used by multiple executables.

Change-Id: I3da126778bf5ffdb28d7a3c2b966c85de08ba717
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4289679
Reviewed-by: Joanna Wang <jojwang@chromium.org>
Commit-Queue: Josip Sokcevic <sokcevic@chromium.org>
Josip Sokcevic 2 years ago
parent
commit
512fd3bd85
7 changed files with 231 additions and 217 deletions
  1. 1 1
      git_cl.py
  2. 0 0
      lib/__init__.py
  3. 95 0
      lib/utils.py
  4. 19 75
      presubmit_support.py
  5. 30 112
      tests/presubmit_unittest.py
  6. 86 4
      tests/utils_test.py
  7. 0 25
      utils.py

+ 1 - 1
git_cl.py

@@ -55,6 +55,7 @@ import subprocess2
 import swift_format
 import swift_format
 import watchlists
 import watchlists
 
 
+from lib import utils
 from third_party import six
 from third_party import six
 from six.moves import urllib
 from six.moves import urllib
 
 
@@ -6131,7 +6132,6 @@ def CMDlol(parser, args):
 
 
 
 
 def CMDversion(parser, args):
 def CMDversion(parser, args):
-  import utils
   print(utils.depot_tools_version())
   print(utils.depot_tools_version())
 
 
 
 

+ 0 - 0
lib/__init__.py


+ 95 - 0
lib/utils.py

@@ -0,0 +1,95 @@
+# Copyright 2022 The Chromium Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+""" A collection of commonly used functions across depot_tools.
+"""
+
+import logging
+import os
+import re
+import subprocess
+
+
+def depot_tools_version():
+  depot_tools_root = os.path.dirname(os.path.abspath(__file__))
+  try:
+    commit_hash = subprocess.check_output(['git', 'rev-parse', 'HEAD'],
+                                          cwd=depot_tools_root).decode(
+                                              'utf-8', 'ignore')
+    return 'git-%s' % commit_hash
+  except Exception:
+    pass
+
+  # git check failed, let's check last modification of frequently checked file
+  try:
+    mtime = os.path.getmtime(
+        os.path.join(depot_tools_root, 'infra', 'config', 'recipes.cfg'))
+    return 'recipes.cfg-%d' % (mtime)
+  except Exception:
+    return 'unknown'
+
+
+def normpath(path):
+  '''Version of os.path.normpath that also changes backward slashes to
+  forward slashes when not running on Windows.
+  '''
+  # This is safe to always do because the Windows version of os.path.normpath
+  # will replace forward slashes with backward slashes.
+  path = path.replace(os.sep, '/')
+  return os.path.normpath(path)
+
+
+def ListRelevantFilesInSourceCheckout(files, root, match_re, exclude_re):
+  """Finds all files that apply to a given set of source files, e.g. PRESUBMIT.
+
+  If inherit-review-settings-ok is present right under root, looks for matches
+  in directories enclosing root.
+
+  Args:
+    files: An iterable container containing file paths.
+    root: Path where to stop searching.
+    match_re: regex to match filename
+    exclude_re: regex to exclude filename
+
+  Return:
+    List of absolute paths of the existing PRESUBMIT.py scripts.
+  """
+  files = [normpath(os.path.join(root, f)) for f in files]
+
+  # List all the individual directories containing files.
+  directories = {os.path.dirname(f) for f in files}
+
+  # Ignore root if inherit-review-settings-ok is present.
+  if os.path.isfile(os.path.join(root, 'inherit-review-settings-ok')):
+    root = None
+
+  # Collect all unique directories that may contain PRESUBMIT.py.
+  candidates = set()
+  for directory in directories:
+    while True:
+      if directory in candidates:
+        break
+      candidates.add(directory)
+      if directory == root:
+        break
+      parent_dir = os.path.dirname(directory)
+      if parent_dir == directory:
+        # We hit the system root directory.
+        break
+      directory = parent_dir
+
+  # Look for PRESUBMIT.py in all candidate directories.
+  results = []
+  for directory in sorted(list(candidates)):
+    try:
+      for f in os.listdir(directory):
+        p = os.path.join(directory, f)
+        if os.path.isfile(p) and re.match(match_re,
+                                          f) and not re.match(exclude_re, f):
+          results.append(p)
+    except OSError:
+      pass
+
+  logging.debug('Presubmit files: %s', ','.join(results))
+  return results

+ 19 - 75
presubmit_support.py

@@ -50,6 +50,7 @@ import presubmit_canned_checks
 import rdb_wrapper
 import rdb_wrapper
 import scm
 import scm
 import subprocess2 as subprocess  # Exposed through the API.
 import subprocess2 as subprocess  # Exposed through the API.
+from lib import utils
 
 
 if sys.version_info.major == 2:
 if sys.version_info.major == 2:
   # TODO(1009814): Expose urllib2 only through urllib_request and urllib_error
   # TODO(1009814): Expose urllib2 only through urllib_request and urllib_error
@@ -69,6 +70,9 @@ _ASKED_FOR_FEEDBACK = False
 # are coming from.
 # are coming from.
 _SHOW_CALLSTACKS = False
 _SHOW_CALLSTACKS = False
 
 
+_PRESUBMIT_FILE_REGEX = r'PRESUBMIT.*\.py$'
+_PRESUBMIT_FILE_EXCLUDE = r'PRESUBMIT_test'
+
 
 
 def time_time():
 def time_time():
   # Use this so that it can be mocked in tests without interfering with python
   # Use this so that it can be mocked in tests without interfering with python
@@ -285,16 +289,6 @@ class ThreadPool(object):
     return self._messages
     return self._messages
 
 
 
 
-def normpath(path):
-  '''Version of os.path.normpath that also changes backward slashes to
-  forward slashes when not running on Windows.
-  '''
-  # This is safe to always do because the Windows version of os.path.normpath
-  # will replace forward slashes with backward slashes.
-  path = path.replace(os.sep, '/')
-  return os.path.normpath(path)
-
-
 def _RightHandSideLinesImpl(affected_files):
 def _RightHandSideLinesImpl(affected_files):
   """Implements RightHandSideLines for InputApi and GclChange."""
   """Implements RightHandSideLines for InputApi and GclChange."""
   for af in affected_files:
   for af in affected_files:
@@ -729,15 +723,17 @@ class InputApi(object):
     script, or subdirectories thereof. Note that files are listed using the OS
     script, or subdirectories thereof. Note that files are listed using the OS
     path separator, so backslashes are used as separators on Windows.
     path separator, so backslashes are used as separators on Windows.
     """
     """
-    dir_with_slash = normpath(self.PresubmitLocalPath())
+    dir_with_slash = utils.normpath(self.PresubmitLocalPath())
     # normpath strips trailing path separators, so the trailing separator has to
     # normpath strips trailing path separators, so the trailing separator has to
     # be added after the normpath call.
     # be added after the normpath call.
     if len(dir_with_slash) > 0:
     if len(dir_with_slash) > 0:
       dir_with_slash += os.path.sep
       dir_with_slash += os.path.sep
 
 
-    return list(filter(
-        lambda x: normpath(x.AbsoluteLocalPath()).startswith(dir_with_slash),
-        self.change.AffectedFiles(include_deletes, file_filter)))
+    return list(
+        filter(
+            lambda x: utils.normpath(x.AbsoluteLocalPath()).startswith(
+                dir_with_slash),
+            self.change.AffectedFiles(include_deletes, file_filter)))
 
 
   def LocalPaths(self):
   def LocalPaths(self):
     """Returns local paths of input_api.AffectedFiles()."""
     """Returns local paths of input_api.AffectedFiles()."""
@@ -958,7 +954,7 @@ class _GitDiffCache(_DiffCache):
           current_diff.append(x)
           current_diff.append(x)
 
 
       self._diffs_by_file = dict(
       self._diffs_by_file = dict(
-        (normpath(path), ''.join(diff)) for path, diff in diffs.items())
+          (utils.normpath(path), ''.join(diff)) for path, diff in diffs.items())
 
 
     if path not in self._diffs_by_file:
     if path not in self._diffs_by_file:
       # SCM didn't have any diff on this file. It could be that the file was not
       # SCM didn't have any diff on this file. It could be that the file was not
@@ -997,7 +993,7 @@ class AffectedFile(object):
     because presubmit checks are run with CWD=PresubmitLocalPath() (which is
     because presubmit checks are run with CWD=PresubmitLocalPath() (which is
     often != client root).
     often != client root).
     """
     """
-    return normpath(self._path)
+    return utils.normpath(self._path)
 
 
   def AbsoluteLocalPath(self):
   def AbsoluteLocalPath(self):
     """Returns the absolute path of this file on the local disk.
     """Returns the absolute path of this file on the local disk.
@@ -1358,59 +1354,6 @@ class GitChange(Change):
         cwd=root).decode('utf-8', 'ignore').splitlines()
         cwd=root).decode('utf-8', 'ignore').splitlines()
 
 
 
 
-def ListRelevantPresubmitFiles(files, root):
-  """Finds all presubmit files that apply to a given set of source files.
-
-  If inherit-review-settings-ok is present right under root, looks for
-  PRESUBMIT.py in directories enclosing root.
-
-  Args:
-    files: An iterable container containing file paths.
-    root: Path where to stop searching.
-
-  Return:
-    List of absolute paths of the existing PRESUBMIT.py scripts.
-  """
-  files = [normpath(os.path.join(root, f)) for f in files]
-
-  # List all the individual directories containing files.
-  directories = {os.path.dirname(f) for f in files}
-
-  # Ignore root if inherit-review-settings-ok is present.
-  if os.path.isfile(os.path.join(root, 'inherit-review-settings-ok')):
-    root = None
-
-  # Collect all unique directories that may contain PRESUBMIT.py.
-  candidates = set()
-  for directory in directories:
-    while True:
-      if directory in candidates:
-        break
-      candidates.add(directory)
-      if directory == root:
-        break
-      parent_dir = os.path.dirname(directory)
-      if parent_dir == directory:
-        # We hit the system root directory.
-        break
-      directory = parent_dir
-
-  # Look for PRESUBMIT.py in all candidate directories.
-  results = []
-  for directory in sorted(list(candidates)):
-    try:
-      for f in os.listdir(directory):
-        p = os.path.join(directory, f)
-        if os.path.isfile(p) and re.match(
-            r'PRESUBMIT.*\.py$', f) and not f.startswith('PRESUBMIT_test'):
-          results.append(p)
-    except OSError:
-      pass
-
-  logging.debug('Presubmit files: %s', ','.join(results))
-  return results
-
-
 class GetPostUploadExecuter(object):
 class GetPostUploadExecuter(object):
   def __init__(self, change, gerrit_obj, use_python3):
   def __init__(self, change, gerrit_obj, use_python3):
     """
     """
@@ -1491,15 +1434,16 @@ def DoPostUploadExecuter(change, gerrit_obj, verbose, use_python3=False):
   """
   """
   python_version = 'Python %s' % sys.version_info.major
   python_version = 'Python %s' % sys.version_info.major
   sys.stdout.write('Running %s post upload checks ...\n' % python_version)
   sys.stdout.write('Running %s post upload checks ...\n' % python_version)
-  presubmit_files = ListRelevantPresubmitFiles(
-      change.LocalPaths(), change.RepositoryRoot())
+  presubmit_files = utils.ListRelevantFilesInSourceCheckout(
+      change.LocalPaths(), change.RepositoryRoot(), _PRESUBMIT_FILE_REGEX,
+      _PRESUBMIT_FILE_EXCLUDE)
   if not presubmit_files and verbose:
   if not presubmit_files and verbose:
     sys.stdout.write('Warning, no PRESUBMIT.py found.\n')
     sys.stdout.write('Warning, no PRESUBMIT.py found.\n')
   results = []
   results = []
   executer = GetPostUploadExecuter(change, gerrit_obj, use_python3)
   executer = GetPostUploadExecuter(change, gerrit_obj, use_python3)
   # The root presubmit file should be executed after the ones in subdirectories.
   # 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.
   # i.e. the specific post upload hooks should run before the general ones.
-  # Thus, reverse the order provided by ListRelevantPresubmitFiles.
+  # Thus, reverse the order.
   presubmit_files.reverse()
   presubmit_files.reverse()
 
 
   for filename in presubmit_files:
   for filename in presubmit_files:
@@ -1771,8 +1715,9 @@ def DoPresubmitChecks(change,
       sys.stdout.write('Running %s presubmit upload checks ...\n' %
       sys.stdout.write('Running %s presubmit upload checks ...\n' %
                        python_version)
                        python_version)
     start_time = time_time()
     start_time = time_time()
-    presubmit_files = ListRelevantPresubmitFiles(
-        change.AbsoluteLocalPaths(), change.RepositoryRoot())
+    presubmit_files = utils.ListRelevantFilesInSourceCheckout(
+        change.AbsoluteLocalPaths(), change.RepositoryRoot(),
+        _PRESUBMIT_FILE_REGEX, _PRESUBMIT_FILE_EXCLUDE)
     if not presubmit_files and verbose:
     if not presubmit_files and verbose:
       sys.stdout.write('Warning, no PRESUBMIT.py found.\n')
       sys.stdout.write('Warning, no PRESUBMIT.py found.\n')
     results = []
     results = []
@@ -2123,7 +2068,6 @@ def main(argv=None):
           options.use_python3,
           options.use_python3,
           options.no_diffs)
           options.no_diffs)
   except PresubmitFailure as e:
   except PresubmitFailure as e:
-    import utils
     print(e, file=sys.stderr)
     print(e, file=sys.stderr)
     print('Maybe your depot_tools is out of date?', file=sys.stderr)
     print('Maybe your depot_tools is out of date?', file=sys.stderr)
     print('depot_tools version: %s' % utils.depot_tools_version(),
     print('depot_tools version: %s' % utils.depot_tools_version(),

+ 30 - 112
tests/presubmit_unittest.py

@@ -47,6 +47,7 @@ import presubmit_support as presubmit
 import rdb_wrapper
 import rdb_wrapper
 import scm
 import scm
 import subprocess2 as subprocess
 import subprocess2 as subprocess
+from lib import utils
 
 
 
 
 # Shortcut.
 # Shortcut.
@@ -220,7 +221,6 @@ index fe3de7b..54ae6e1 100755
 class PresubmitUnittest(PresubmitTestsBase):
 class PresubmitUnittest(PresubmitTestsBase):
   """General presubmit_support.py tests (excluding InputApi and OutputApi)."""
   """General presubmit_support.py tests (excluding InputApi and OutputApi)."""
 
 
-  _INHERIT_SETTINGS = 'inherit-review-settings-ok'
   fake_root_dir = '/foo/bar'
   fake_root_dir = '/foo/bar'
 
 
   def testCannedCheckFilter(self):
   def testCannedCheckFilter(self):
@@ -231,78 +231,6 @@ class PresubmitUnittest(PresubmitTestsBase):
       self.assertEqual(canned.CheckOwners(None, None), [])
       self.assertEqual(canned.CheckOwners(None, None), [])
     self.assertEqual(canned.CheckOwners, orig)
     self.assertEqual(canned.CheckOwners, orig)
 
 
-  def testListRelevantPresubmitFiles(self):
-    files = [
-        'blat.cc',
-        os.path.join('foo', 'haspresubmit', 'yodle', 'smart.h'),
-        os.path.join('moo', 'mat', 'gat', 'yo.h'),
-        os.path.join('foo', 'luck.h'),
-    ]
-
-    known_files = [
-        os.path.join(self.fake_root_dir, 'PRESUBMIT.py'),
-        os.path.join(self.fake_root_dir, 'foo', 'haspresubmit', 'PRESUBMIT.py'),
-        os.path.join(
-            self.fake_root_dir, 'foo', 'haspresubmit', 'yodle', 'PRESUBMIT.py'),
-    ]
-    os.path.isfile.side_effect = lambda f: f in known_files
-
-    dirs_with_presubmit = [
-        self.fake_root_dir,
-        os.path.join(self.fake_root_dir, 'foo', 'haspresubmit'),
-        os.path.join(self.fake_root_dir, 'foo', 'haspresubmit', 'yodle'),
-    ]
-    os.listdir.side_effect = (
-        lambda d: ['PRESUBMIT.py'] if d in dirs_with_presubmit else [])
-
-    presubmit_files = presubmit.ListRelevantPresubmitFiles(
-        files, self.fake_root_dir)
-    self.assertEqual(presubmit_files, known_files)
-
-  def testListUserPresubmitFiles(self):
-    files = ['blat.cc',]
-
-    os.path.isfile.side_effect = lambda f: 'PRESUBMIT' in f
-    os.listdir.return_value = [
-        'PRESUBMIT.py', 'PRESUBMIT_test.py', 'PRESUBMIT-user.py']
-
-    presubmit_files = presubmit.ListRelevantPresubmitFiles(
-        files, self.fake_root_dir)
-    self.assertEqual(presubmit_files, [
-        os.path.join(self.fake_root_dir, 'PRESUBMIT.py'),
-        os.path.join(self.fake_root_dir, 'PRESUBMIT-user.py'),
-    ])
-
-  def testListRelevantPresubmitFilesInheritSettings(self):
-    sys_root_dir = self._OS_SEP
-    root_dir = os.path.join(sys_root_dir, 'foo', 'bar')
-    inherit_path = os.path.join(root_dir, self._INHERIT_SETTINGS)
-    files = [
-        'test.cc',
-        os.path.join('moo', 'test2.cc'),
-        os.path.join('zoo', 'test3.cc')
-    ]
-
-    known_files = [
-        inherit_path,
-        os.path.join(sys_root_dir, 'foo', 'PRESUBMIT.py'),
-        os.path.join(sys_root_dir, 'foo', 'bar', 'moo', 'PRESUBMIT.py'),
-    ]
-    os.path.isfile.side_effect = lambda f: f in known_files
-
-    dirs_with_presubmit = [
-        os.path.join(sys_root_dir, 'foo'),
-        os.path.join(sys_root_dir, 'foo', 'bar','moo'),
-    ]
-    os.listdir.side_effect = (
-        lambda d: ['PRESUBMIT.py'] if d in dirs_with_presubmit else [])
-
-    presubmit_files = presubmit.ListRelevantPresubmitFiles(files, root_dir)
-    self.assertEqual(presubmit_files, [
-        os.path.join(sys_root_dir, 'foo', 'PRESUBMIT.py'),
-        os.path.join(sys_root_dir, 'foo', 'bar', 'moo', 'PRESUBMIT.py')
-    ])
-
   def testTagLineRe(self):
   def testTagLineRe(self):
     m = presubmit.Change.TAG_LINE_RE.match(' BUG =1223, 1445  \t')
     m = presubmit.Change.TAG_LINE_RE.match(' BUG =1223, 1445  \t')
     self.assertIsNotNone(m)
     self.assertIsNotNone(m)
@@ -994,11 +922,11 @@ def CheckChangeOnCommit(input_api, output_api):
         presubmit.main(
         presubmit.main(
             ['--root', self.fake_root_dir, 'random_file.txt', '--post_upload']))
             ['--root', self.fake_root_dir, 'random_file.txt', '--post_upload']))
 
 
-  @mock.patch('presubmit_support.ListRelevantPresubmitFiles')
+  @mock.patch('lib.utils.ListRelevantFilesInSourceCheckout')
   def testMainUnversioned(self, *_mocks):
   def testMainUnversioned(self, *_mocks):
     gclient_utils.FileRead.return_value = ''
     gclient_utils.FileRead.return_value = ''
     scm.determine_scm.return_value = None
     scm.determine_scm.return_value = None
-    presubmit.ListRelevantPresubmitFiles.return_value = [
+    utils.ListRelevantFilesInSourceCheckout.return_value = [
         os.path.join(self.fake_root_dir, 'PRESUBMIT.py')
         os.path.join(self.fake_root_dir, 'PRESUBMIT.py')
     ]
     ]
 
 
@@ -1006,14 +934,14 @@ def CheckChangeOnCommit(input_api, output_api):
         0,
         0,
         presubmit.main(['--root', self.fake_root_dir, 'random_file.txt']))
         presubmit.main(['--root', self.fake_root_dir, 'random_file.txt']))
 
 
-  @mock.patch('presubmit_support.ListRelevantPresubmitFiles')
+  @mock.patch('lib.utils.ListRelevantFilesInSourceCheckout')
   def testMainUnversionedChecksFail(self, *_mocks):
   def testMainUnversionedChecksFail(self, *_mocks):
     gclient_utils.FileRead.return_value = (
     gclient_utils.FileRead.return_value = (
         'USE_PYTHON3 = ' + str(sys.version_info.major == 3) + '\n'
         'USE_PYTHON3 = ' + str(sys.version_info.major == 3) + '\n'
         'def CheckChangeOnUpload(input_api, output_api):\n'
         'def CheckChangeOnUpload(input_api, output_api):\n'
         '  return [output_api.PresubmitError("!!")]\n')
         '  return [output_api.PresubmitError("!!")]\n')
     scm.determine_scm.return_value = None
     scm.determine_scm.return_value = None
-    presubmit.ListRelevantPresubmitFiles.return_value = [
+    utils.ListRelevantFilesInSourceCheckout.return_value = [
         os.path.join(self.fake_root_dir, 'PRESUBMIT.py')
         os.path.join(self.fake_root_dir, 'PRESUBMIT.py')
     ]
     ]
 
 
@@ -1285,26 +1213,22 @@ class InputApiUnittest(PresubmitTestsBase):
     # Doesn't filter much
     # Doesn't filter much
     got_files = input_api.AffectedFiles()
     got_files = input_api.AffectedFiles()
     self.assertEqual(len(got_files), 7)
     self.assertEqual(len(got_files), 7)
-    self.assertEqual(got_files[0].LocalPath(), presubmit.normpath(files[0][1]))
-    self.assertEqual(got_files[1].LocalPath(), presubmit.normpath(files[1][1]))
-    self.assertEqual(got_files[2].LocalPath(), presubmit.normpath(files[2][1]))
-    self.assertEqual(got_files[3].LocalPath(), presubmit.normpath(files[3][1]))
-    self.assertEqual(got_files[4].LocalPath(), presubmit.normpath(files[4][1]))
-    self.assertEqual(got_files[5].LocalPath(), presubmit.normpath(files[5][1]))
-    self.assertEqual(got_files[6].LocalPath(), presubmit.normpath(files[6][1]))
+    self.assertEqual(got_files[0].LocalPath(), utils.normpath(files[0][1]))
+    self.assertEqual(got_files[1].LocalPath(), utils.normpath(files[1][1]))
+    self.assertEqual(got_files[2].LocalPath(), utils.normpath(files[2][1]))
+    self.assertEqual(got_files[3].LocalPath(), utils.normpath(files[3][1]))
+    self.assertEqual(got_files[4].LocalPath(), utils.normpath(files[4][1]))
+    self.assertEqual(got_files[5].LocalPath(), utils.normpath(files[5][1]))
+    self.assertEqual(got_files[6].LocalPath(), utils.normpath(files[6][1]))
     # Ignores weird because of check_list, third_party because of skip_list,
     # Ignores weird because of check_list, third_party because of skip_list,
     # binary isn't a text file and being deleted doesn't exist. The rest is
     # binary isn't a text file and being deleted doesn't exist. The rest is
     # outside foo/.
     # outside foo/.
     rhs_lines = list(input_api.RightHandSideLines(None))
     rhs_lines = list(input_api.RightHandSideLines(None))
     self.assertEqual(len(rhs_lines), 14)
     self.assertEqual(len(rhs_lines), 14)
-    self.assertEqual(rhs_lines[0][0].LocalPath(),
-                     presubmit.normpath(files[0][1]))
-    self.assertEqual(rhs_lines[3][0].LocalPath(),
-                     presubmit.normpath(files[0][1]))
-    self.assertEqual(rhs_lines[7][0].LocalPath(),
-                     presubmit.normpath(files[4][1]))
-    self.assertEqual(rhs_lines[13][0].LocalPath(),
-                     presubmit.normpath(files[4][1]))
+    self.assertEqual(rhs_lines[0][0].LocalPath(), utils.normpath(files[0][1]))
+    self.assertEqual(rhs_lines[3][0].LocalPath(), utils.normpath(files[0][1]))
+    self.assertEqual(rhs_lines[7][0].LocalPath(), utils.normpath(files[4][1]))
+    self.assertEqual(rhs_lines[13][0].LocalPath(), utils.normpath(files[4][1]))
 
 
   def testInputApiFilterSourceFile(self):
   def testInputApiFilterSourceFile(self):
     files = [
     files = [
@@ -1344,10 +1268,10 @@ class InputApiUnittest(PresubmitTestsBase):
     got_files = input_api.AffectedSourceFiles(None)
     got_files = input_api.AffectedSourceFiles(None)
     self.assertEqual(len(got_files), 4)
     self.assertEqual(len(got_files), 4)
     # blat.cc, another.h, WebKit.cpp, and blink.cc remain.
     # blat.cc, another.h, WebKit.cpp, and blink.cc remain.
-    self.assertEqual(got_files[0].LocalPath(), presubmit.normpath(files[0][1]))
-    self.assertEqual(got_files[1].LocalPath(), presubmit.normpath(files[4][1]))
-    self.assertEqual(got_files[2].LocalPath(), presubmit.normpath(files[5][1]))
-    self.assertEqual(got_files[3].LocalPath(), presubmit.normpath(files[7][1]))
+    self.assertEqual(got_files[0].LocalPath(), utils.normpath(files[0][1]))
+    self.assertEqual(got_files[1].LocalPath(), utils.normpath(files[4][1]))
+    self.assertEqual(got_files[2].LocalPath(), utils.normpath(files[5][1]))
+    self.assertEqual(got_files[3].LocalPath(), utils.normpath(files[7][1]))
 
 
   def testDefaultFilesToCheckFilesToSkipFilters(self):
   def testDefaultFilesToCheckFilesToSkipFilters(self):
     def f(x):
     def f(x):
@@ -1430,8 +1354,7 @@ class InputApiUnittest(PresubmitTestsBase):
     for item in files:
     for item in files:
       results = list(filter(input_api.FilterSourceFile, item[0]))
       results = list(filter(input_api.FilterSourceFile, item[0]))
       for i in range(len(results)):
       for i in range(len(results)):
-        self.assertEqual(results[i].LocalPath(),
-                          presubmit.normpath(item[1][i]))
+        self.assertEqual(results[i].LocalPath(), utils.normpath(item[1][i]))
       # Same number of expected results.
       # Same number of expected results.
       self.assertEqual(sorted([f.LocalPath().replace(os.sep, '/')
       self.assertEqual(sorted([f.LocalPath().replace(os.sep, '/')
                                 for f in results]),
                                 for f in results]),
@@ -1491,7 +1414,7 @@ class InputApiUnittest(PresubmitTestsBase):
     self.assertEqual(got_files[1].LocalPath(), 'eecaee')
     self.assertEqual(got_files[1].LocalPath(), 'eecaee')
 
 
   def testGetAbsoluteLocalPath(self):
   def testGetAbsoluteLocalPath(self):
-    normpath = presubmit.normpath
+    normpath = utils.normpath
     # Regression test for bug of presubmit stuff that relies on invoking
     # Regression test for bug of presubmit stuff that relies on invoking
     # SVN (e.g. to get mime type of file) not working unless gcl invoked
     # SVN (e.g. to get mime type of file) not working unless gcl invoked
     # from the client root (e.g. if you were at 'src' and did 'cd base' before
     # from the client root (e.g. if you were at 'src' and did 'cd base' before
@@ -1512,13 +1435,11 @@ class InputApiUnittest(PresubmitTestsBase):
     self.assertEqual(affected_files[0].LocalPath(), normpath('isdir'))
     self.assertEqual(affected_files[0].LocalPath(), normpath('isdir'))
     self.assertEqual(affected_files[1].LocalPath(), normpath('isdir/blat.cc'))
     self.assertEqual(affected_files[1].LocalPath(), normpath('isdir/blat.cc'))
     # Absolute paths should be prefixed
     # Absolute paths should be prefixed
-    self.assertEqual(
-        affected_files[0].AbsoluteLocalPath(),
-        presubmit.normpath(os.path.join(self.fake_root_dir, 'isdir')))
+    self.assertEqual(affected_files[0].AbsoluteLocalPath(),
+                     utils.normpath(os.path.join(self.fake_root_dir, 'isdir')))
     self.assertEqual(
     self.assertEqual(
         affected_files[1].AbsoluteLocalPath(),
         affected_files[1].AbsoluteLocalPath(),
-        presubmit.normpath(os.path.join(
-            self.fake_root_dir, 'isdir/blat.cc')))
+        utils.normpath(os.path.join(self.fake_root_dir, 'isdir/blat.cc')))
 
 
     # New helper functions need to work
     # New helper functions need to work
     paths_from_change = change.AbsoluteLocalPaths()
     paths_from_change = change.AbsoluteLocalPaths()
@@ -1530,17 +1451,14 @@ class InputApiUnittest(PresubmitTestsBase):
         is_committing=True, gerrit_obj=None, verbose=False)
         is_committing=True, gerrit_obj=None, verbose=False)
     paths_from_api = api.AbsoluteLocalPaths()
     paths_from_api = api.AbsoluteLocalPaths()
     self.assertEqual(len(paths_from_api), 1)
     self.assertEqual(len(paths_from_api), 1)
-    self.assertEqual(
-        paths_from_change[0],
-        presubmit.normpath(os.path.join(self.fake_root_dir, 'isdir')))
+    self.assertEqual(paths_from_change[0],
+                     utils.normpath(os.path.join(self.fake_root_dir, 'isdir')))
     self.assertEqual(
     self.assertEqual(
         paths_from_change[1],
         paths_from_change[1],
-        presubmit.normpath(os.path.join(self.fake_root_dir, 'isdir',
-                                        'blat.cc')))
+        utils.normpath(os.path.join(self.fake_root_dir, 'isdir', 'blat.cc')))
     self.assertEqual(
     self.assertEqual(
         paths_from_api[0],
         paths_from_api[0],
-        presubmit.normpath(os.path.join(self.fake_root_dir, 'isdir',
-                                        'blat.cc')))
+        utils.normpath(os.path.join(self.fake_root_dir, 'isdir', 'blat.cc')))
 
 
   def testDeprecated(self):
   def testDeprecated(self):
     change = presubmit.Change(
     change = presubmit.Change(
@@ -1665,7 +1583,7 @@ class AffectedFileUnittest(PresubmitTestsBase):
   def testAffectedFile(self):
   def testAffectedFile(self):
     gclient_utils.FileRead.return_value = 'whatever\ncookie'
     gclient_utils.FileRead.return_value = 'whatever\ncookie'
     af = presubmit.GitAffectedFile('foo/blat.cc', 'M', self.fake_root_dir, None)
     af = presubmit.GitAffectedFile('foo/blat.cc', 'M', self.fake_root_dir, None)
-    self.assertEqual(presubmit.normpath('foo/blat.cc'), af.LocalPath())
+    self.assertEqual(utils.normpath('foo/blat.cc'), af.LocalPath())
     self.assertEqual('M', af.Action())
     self.assertEqual('M', af.Action())
     self.assertEqual(['whatever', 'cookie'], af.NewContents())
     self.assertEqual(['whatever', 'cookie'], af.NewContents())
 
 

+ 86 - 4
tests/utils_test.py

@@ -17,13 +17,10 @@ DEPOT_TOOLS_ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
 sys.path.insert(0, DEPOT_TOOLS_ROOT)
 sys.path.insert(0, DEPOT_TOOLS_ROOT)
 
 
 from testing_support import coverage_utils
 from testing_support import coverage_utils
-import utils
+from lib import utils
 
 
 
 
 class GitCacheTest(unittest.TestCase):
 class GitCacheTest(unittest.TestCase):
-  def setUp(self):
-    pass
-
   @mock.patch('subprocess.check_output', lambda x, **kwargs: b'foo')
   @mock.patch('subprocess.check_output', lambda x, **kwargs: b'foo')
   def testVersionWithGit(self):
   def testVersionWithGit(self):
     version = utils.depot_tools_version()
     version = utils.depot_tools_version()
@@ -45,6 +42,91 @@ class GitCacheTest(unittest.TestCase):
     self.assertEqual(version, 'unknown')
     self.assertEqual(version, 'unknown')
 
 
 
 
+class ListRelevantFilesInSourceCheckoutTest(unittest.TestCase):
+  fake_root_dir = os.path.join(os.sep, 'foo', 'bar')
+  _INHERIT_SETTINGS = 'inherit-review-settings-ok'
+
+  def setUp(self):
+    mock.patch('os.listdir').start()
+    mock.patch('os.path.isfile').start()
+
+  def testListRelevantPresubmitFiles(self):
+    files = [
+        'blat.cc',
+        os.path.join('foo', 'haspresubmit', 'yodle', 'smart.h'),
+        os.path.join('moo', 'mat', 'gat', 'yo.h'),
+        os.path.join('foo', 'luck.h'),
+    ]
+
+    known_files = [
+        os.path.join(self.fake_root_dir, 'PRESUBMIT.py'),
+        os.path.join(self.fake_root_dir, 'foo', 'haspresubmit', 'PRESUBMIT.py'),
+        os.path.join(self.fake_root_dir, 'foo', 'haspresubmit', 'yodle',
+                     'PRESUBMIT.py'),
+    ]
+    os.path.isfile.side_effect = lambda f: f in known_files
+
+    dirs_with_presubmit = [
+        self.fake_root_dir,
+        os.path.join(self.fake_root_dir, 'foo', 'haspresubmit'),
+        os.path.join(self.fake_root_dir, 'foo', 'haspresubmit', 'yodle'),
+    ]
+    os.listdir.side_effect = (
+        lambda d: ['PRESUBMIT.py'] if d in dirs_with_presubmit else [])
+
+    presubmit_files = utils.ListRelevantFilesInSourceCheckout(
+        files, self.fake_root_dir, r'PRESUBMIT.*', r'PRESUBMIT_test')
+    self.assertEqual(presubmit_files, known_files)
+
+  def testListUserPresubmitFiles(self):
+    files = [
+        'blat.cc',
+    ]
+
+    os.path.isfile.side_effect = lambda f: 'PRESUBMIT' in f
+    os.listdir.return_value = [
+        'PRESUBMIT.py', 'PRESUBMIT_test.py', 'PRESUBMIT-user.py'
+    ]
+
+    presubmit_files = utils.ListRelevantFilesInSourceCheckout(
+        files, self.fake_root_dir, r'PRESUBMIT.*', r'PRESUBMIT_test')
+    self.assertEqual(presubmit_files, [
+        os.path.join(self.fake_root_dir, 'PRESUBMIT.py'),
+        os.path.join(self.fake_root_dir, 'PRESUBMIT-user.py'),
+    ])
+
+  def testListRelevantPresubmitFilesInheritSettings(self):
+    sys_root_dir = os.sep
+    root_dir = os.path.join(sys_root_dir, 'foo', 'bar')
+    inherit_path = os.path.join(root_dir, self._INHERIT_SETTINGS)
+    files = [
+        'test.cc',
+        os.path.join('moo', 'test2.cc'),
+        os.path.join('zoo', 'test3.cc')
+    ]
+
+    known_files = [
+        inherit_path,
+        os.path.join(sys_root_dir, 'foo', 'PRESUBMIT.py'),
+        os.path.join(sys_root_dir, 'foo', 'bar', 'moo', 'PRESUBMIT.py'),
+    ]
+    os.path.isfile.side_effect = lambda f: f in known_files
+
+    dirs_with_presubmit = [
+        os.path.join(sys_root_dir, 'foo'),
+        os.path.join(sys_root_dir, 'foo', 'bar', 'moo'),
+    ]
+    os.listdir.side_effect = (
+        lambda d: ['PRESUBMIT.py'] if d in dirs_with_presubmit else [])
+
+    presubmit_files = utils.ListRelevantFilesInSourceCheckout(
+        files, root_dir, r'PRESUBMIT.*', r'PRESUBMIT_test')
+    self.assertEqual(presubmit_files, [
+        os.path.join(sys_root_dir, 'foo', 'PRESUBMIT.py'),
+        os.path.join(sys_root_dir, 'foo', 'bar', 'moo', 'PRESUBMIT.py')
+    ])
+
+
 if __name__ == '__main__':
 if __name__ == '__main__':
   logging.basicConfig(
   logging.basicConfig(
       level=logging.DEBUG if '-v' in sys.argv else logging.ERROR)
       level=logging.DEBUG if '-v' in sys.argv else logging.ERROR)

+ 0 - 25
utils.py

@@ -1,25 +0,0 @@
-# Copyright 2022 The Chromium Authors. All rights reserved.
-# Use of this source code is governed by a BSD-style license that can be
-# found in the LICENSE file.
-
-import os
-import subprocess
-
-
-def depot_tools_version():
-  depot_tools_root = os.path.dirname(os.path.abspath(__file__))
-  try:
-    commit_hash = subprocess.check_output(['git', 'rev-parse', 'HEAD'],
-                                          cwd=depot_tools_root).decode(
-                                              'utf-8', 'ignore')
-    return 'git-%s' % commit_hash
-  except Exception:
-    pass
-
-  # git check failed, let's check last modification of frequently checked file
-  try:
-    mtime = os.path.getmtime(
-        os.path.join(depot_tools_root, 'infra', 'config', 'recipes.cfg'))
-    return 'recipes.cfg-%d' % (mtime)
-  except Exception:
-    return 'unknown'