diff --git a/PRESUBMIT.py b/PRESUBMIT.py index 9ce0829949b8fb..1a5ebe1459762e 100644 --- a/PRESUBMIT.py +++ b/PRESUBMIT.py @@ -3385,6 +3385,7 @@ def _CalculateAddedDeps(os_path, old_contents, new_contents): results.add(os_path.join(added_dep, 'DEPS')) return results + def CheckForNewDEPSDownloadFromGoogleStorageHooks(input_api, output_api): """Checks that there are no new download_from_google_storage hooks""" for f in input_api.AffectedFiles(include_deletes=False): @@ -3417,6 +3418,38 @@ def CheckForNewDEPSDownloadFromGoogleStorageHooks(input_api, output_api): return [] +def _readDeps(input_api): + deps_file = input_api.os_path.join(input_api.PresubmitLocalPath(), 'DEPS') + with open(deps_file) as f: + return f.read() + + +def CheckNoNewGitFilesAddedInDependencies(input_api, output_api): + deps = _ParseDeps(_readDeps(input_api)) + dependency_paths = set([]) + for path in deps['deps']: + # TODO(crbug.com/40738689): Remove src/ prefix + dependency_paths.add(path[4:]) # 4 == len('src/') + + errors = [] + for file in input_api.AffectedFiles(include_deletes=False): + path = file.LocalPath() + # We are checking path, and all paths below up to root. E.g. if path is + # a/b/c, we start with path == "a/b/c", followed by "a/b" and "a". + while path: + if path in dependency_paths: + errors.append(output_api.PresubmitError( + 'You cannot place files tracked by Git inside a ' + 'first-party DEPS dependency (deps).\n' + f'Dependency: {path}\n' + f'File: {file.LocalPath()}' + )) + last_dir_path = path.rfind('/') + path = None if last_dir_path == -1 else path[:last_dir_path] + + return errors + + def CheckEachPerfettoTestDataFileHasDepsEntry(input_api, output_api): test_data_filter = lambda f: input_api.FilterSourceFile( f, files_to_check=[r'^base/tracing/test/data_sha256/.*\.sha256']) diff --git a/PRESUBMIT_test.py b/PRESUBMIT_test.py index 4751ab14ed1929..104fce5bfc88ef 100755 --- a/PRESUBMIT_test.py +++ b/PRESUBMIT_test.py @@ -8,6 +8,7 @@ import subprocess import textwrap import unittest +from unittest import mock import PRESUBMIT @@ -5541,6 +5542,45 @@ def testJavaPath(self): self.assertTrue('chrome/foo/file4.java' in results[0].message), self.assertTrue('chrome/foo/file5.java' in results[0].message), +class CheckNoNewGitFilesAddedInDependenciesTest(unittest.TestCase): + @mock.patch('PRESUBMIT._readDeps') + def testNonNested(self, readDeps): + readDeps.return_value = '''deps = { + 'src/foo': 'bar', + 'src/components/foo/bar': 'bar', + }''' + + input_api = MockInputApi() + input_api.files = [ + MockFile('components/foo/file1.java', ['otherFunction']), + MockFile('components/foo/file2.java', ['hasSyncConsent']), + MockFile('chrome/foo/file3.java', ['canSyncFeatureStart']), + MockFile('chrome/foo/file4.java', ['isSyncFeatureEnabled']), + MockFile('chrome/foo/file5.java', ['isSyncFeatureActive']), + ] + results = PRESUBMIT.CheckNoNewGitFilesAddedInDependencies( + input_api, + MockOutputApi()) + + self.assertEqual(0, len(results)) + + @mock.patch('PRESUBMIT._readDeps') + def testCollision(self, readDeps): + readDeps.return_value = '''deps = {'src/foo': 'ignore_me'}''' + + input_api = MockInputApi() + input_api.files = [ + MockAffectedFile('fo', 'content'), # no conflict + MockAffectedFile('foo', 'content'), # conflict + MockAffectedFile('foo/bar', 'content'), # conflict + ] + results = PRESUBMIT.CheckNoNewGitFilesAddedInDependencies( + input_api, + MockOutputApi()) + + self.assertEqual(2, len(results)) + + if __name__ == '__main__': unittest.main()