Skip to content

Commit

Permalink
Revert "[PRESUBMIT] Check if files are written to a dep dir"
Browse files Browse the repository at this point in the history
This reverts commit 367476a.

Reason for revert: Causing presubmit bot failures. See b/345023320, or ask battre@ for more information.

Original change's description:
> [PRESUBMIT] Check if files are written to a dep dir
>
> No files should be written to a directory that's used by CIPD or GCS, as
> defined in DEPS. Git already doesn't allow files to be written to a
> directory that's a gitlink.
>
> Bug: 343199633
> Change-Id: Ica929b7c6e5ca84082f24343869014e9eccfde58
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5578422
> Auto-Submit: Josip Sokcevic <[email protected]>
> Reviewed-by: Joanna Wang <[email protected]>
> Reviewed-by: Daniel Cheng <[email protected]>
> Commit-Queue: Josip Sokcevic <[email protected]>
> Cr-Commit-Position: refs/heads/main@{#1310264}

Bug: 343199633
Change-Id: Idab125b2a0e2a7c673927ba6be4627c7ecb3fbe6
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5598862
Owners-Override: Bret Sepulveda <[email protected]>
Bot-Commit: Rubber Stamper <[email protected]>
Commit-Queue: Bret Sepulveda <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1310492}
  • Loading branch information
bsep-chromium authored and Chromium LUCI CQ committed Jun 5, 2024
1 parent fdcba1e commit 0ca7bfb
Show file tree
Hide file tree
Showing 2 changed files with 0 additions and 73 deletions.
33 changes: 0 additions & 33 deletions PRESUBMIT.py
Original file line number Diff line number Diff line change
Expand Up @@ -3385,7 +3385,6 @@ 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):
Expand Down Expand Up @@ -3418,38 +3417,6 @@ 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'])
Expand Down
40 changes: 0 additions & 40 deletions PRESUBMIT_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
import subprocess
import textwrap
import unittest
from unittest import mock

import PRESUBMIT

Expand Down Expand Up @@ -5542,45 +5541,6 @@ 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()

0 comments on commit 0ca7bfb

Please sign in to comment.