PRESUBMIT: check if DEPS file is an existing file rather than path.

Bug: 382703190
Change-Id: I6fca194462fe20a4bd4dcd63a531970d91360c06
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6081832
Reviewed-by: Andrew Grieve <[email protected]>
Commit-Queue: Andrew Grieve <[email protected]>
Auto-Submit: Joanna Wang <[email protected]>
Reviewed-by: Josip Sokcevic <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1394373}

PRESUBMIT.py

@@ -3453,7 +3453,7 @@ def _FindAllDepsFilesForSubpath(input_api, subpath):
     ret = []
     while subpath:
         cur = input_api.os_path.join(input_api.change.RepositoryRoot(), subpath, 'DEPS')
-        if input_api.os_path.exists(cur):
+        if input_api.os_path.isfile(cur):
             ret.append(cur)
         subpath = input_api.os_path.dirname(subpath)
     return ret

PRESUBMIT_test.py

@@ -509,8 +509,13 @@ def mock(*args, **kwargs):
 
     def testApprovedAdditionalDep(self):
         self.input_api.InitFiles([
-            MockAffectedFile('pdf/DEPS', ['include_rules=["+v8/123"]']),
+            MockAffectedFile('pdf/DEPS',
+                             ['include_rules=["+v8/123", "+foo/bar"]']),
             MockAffectedFile('v8/DEPS', ['new_usages_require_review=True']),
+            # Check that we ignore "DEPS" directories. Note there are real cases
+            # of directories named "deps/" and, especially for case-insensitive file
+            # systems we should prevent these from being considered.
+            MockAffectedFile('foo/bar/DEPS/boofar', ['boofar file contents']),
         ])
 
         # mark the additional dep as approved.

PRESUBMIT_test_mocks.py

@@ -73,7 +73,20 @@ def __init__(self):
         self.fnmatch = fnmatch
         self.json = json
         self.re = re
-        self.os_path = os.path
+
+        # We want os_path.exists() and os_path.isfile() to work for files
+        # that are both in the filesystem and mock files we have added
+        # via InitFiles().
+        # By setting os_path to a copy of os.path rather than directly we
+        # can not only have os_path.exists() be a combined output for fake
+        # files and real files in the filesystem.
+        import importlib.util
+        SPEC_OS_PATH = importlib.util.find_spec('os.path')
+        os_path1 = importlib.util.module_from_spec(SPEC_OS_PATH)
+        SPEC_OS_PATH.loader.exec_module(os_path1)
+        sys.modules['os_path1'] = os_path1
+        self.os_path = os_path1
+
         self.platform = sys.platform
         self.python_executable = sys.executable
         self.python3_executable = sys.executable
@@ -107,13 +120,22 @@ def mock_exists(path):
             if not os.path.isabs(path):
                 path = os.path.join(self.presubmit_local_path, path)
             path = os.path.normpath(path)
-            return path in files_that_exist
+            return path in files_that_exist or any(
+                f.startswith(path)
+                for f in files_that_exist) or os.path.exists(path)
+
+        def mock_isfile(path):
+            if not os.path.isabs(path):
+                path = os.path.join(self.presubmit_local_path, path)
+            path = os.path.normpath(path)
+            return path in files_that_exist or os.path.isfile(path)
 
         def mock_glob(pattern, *args, **kwargs):
-          return fnmatch.filter(files_that_exist, pattern)
+            return fnmatch.filter(files_that_exist, pattern)
 
         # Do not stub these in the constructor to not break existing tests.
         self.os_path.exists = mock_exists
+        self.os_path.isfile = mock_isfile
         self.glob = mock_glob
 
     def AffectedFiles(self, file_filter=None, include_deletes=True):