Ignore +grit entries in DEPS files.
Sometimes the trailing slash doesn't exist on the DEPS on grit, but we were expecting it. BUG=None R=maruel@chromium.org Review URL: https://codereview.chromium.org/20770002 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@213989 0039d316-1c4b-4281-b951-d872f2087c98
This commit is contained in:
35
PRESUBMIT.py
35
PRESUBMIT.py
@ -727,6 +727,26 @@ def _CheckNoAbbreviationInPngFileName(input_api, output_api):
|
||||
return results
|
||||
|
||||
|
||||
def _DepsFilesToCheck(re, changed_lines):
|
||||
"""Helper method for _CheckAddedDepsHaveTargetApprovals. Returns
|
||||
a set of DEPS entries that we should look up."""
|
||||
results = set()
|
||||
|
||||
# This pattern grabs the path without basename in the first
|
||||
# parentheses, and the basename (if present) in the second. It
|
||||
# relies on the simple heuristic that if there is a basename it will
|
||||
# be a header file ending in ".h".
|
||||
pattern = re.compile(
|
||||
r"""['"]\+([^'"]+?)(/[a-zA-Z0-9_]+\.h)?['"].*""")
|
||||
for changed_line in changed_lines:
|
||||
m = pattern.match(changed_line)
|
||||
if m:
|
||||
path = m.group(1)
|
||||
if not (path.startswith('grit/') or path == 'grit'):
|
||||
results.add('%s/DEPS' % m.group(1))
|
||||
return results
|
||||
|
||||
|
||||
def _CheckAddedDepsHaveTargetApprovals(input_api, output_api):
|
||||
"""When a dependency prefixed with + is added to a DEPS file, we
|
||||
want to make sure that the change is reviewed by an OWNER of the
|
||||
@ -743,20 +763,7 @@ def _CheckAddedDepsHaveTargetApprovals(input_api, output_api):
|
||||
if not changed_lines:
|
||||
return []
|
||||
|
||||
virtual_depended_on_files = set()
|
||||
# This pattern grabs the path without basename in the first
|
||||
# parentheses, and the basename (if present) in the second. It
|
||||
# relies on the simple heuristic that if there is a basename it will
|
||||
# be a header file ending in ".h".
|
||||
pattern = input_api.re.compile(
|
||||
r"""['"]\+([^'"]+?)(/[a-zA-Z0-9_]+\.h)?['"].*""")
|
||||
for changed_line in changed_lines:
|
||||
m = pattern.match(changed_line)
|
||||
if m:
|
||||
path = m.group(1)
|
||||
if not path.startswith('grit/'):
|
||||
virtual_depended_on_files.add('%s/DEPS' % m.group(1))
|
||||
|
||||
virtual_depended_on_files = _DepsFilesToCheck(input_api.re, changed_lines)
|
||||
if not virtual_depended_on_files:
|
||||
return []
|
||||
|
||||
|
@ -369,5 +369,42 @@ class InvalidOSMacroNamesTest(unittest.TestCase):
|
||||
self.assertEqual(0, len(errors))
|
||||
|
||||
|
||||
class CheckAddedDepsHaveTetsApprovalsTest(unittest.TestCase):
|
||||
def testDepsFilesToCheck(self):
|
||||
changed_lines = [
|
||||
'"+breakpad",',
|
||||
'"+chrome/installer",',
|
||||
'"+chrome/plugin/chrome_content_plugin_client.h",',
|
||||
'"+chrome/utility/chrome_content_utility_client.h",',
|
||||
'"+chromeos/chromeos_paths.h",',
|
||||
'"+components/breakpad",',
|
||||
'"+components/nacl/common",',
|
||||
'"+content/public/browser/render_process_host.h",',
|
||||
'"+grit", # For generated headers',
|
||||
'"+grit/generated_resources.h",',
|
||||
'"+grit/",',
|
||||
'"+policy", # For generated headers and source',
|
||||
'"+sandbox",',
|
||||
'"+tools/memory_watcher",',
|
||||
'"+third_party/lss/linux_syscall_support.h",',
|
||||
]
|
||||
files_to_check = PRESUBMIT._DepsFilesToCheck(re, changed_lines)
|
||||
expected = set([
|
||||
'breakpad/DEPS',
|
||||
'chrome/installer/DEPS',
|
||||
'chrome/plugin/DEPS',
|
||||
'chrome/utility/DEPS',
|
||||
'chromeos/DEPS',
|
||||
'components/breakpad/DEPS',
|
||||
'components/nacl/common/DEPS',
|
||||
'content/public/browser/DEPS',
|
||||
'policy/DEPS',
|
||||
'sandbox/DEPS',
|
||||
'tools/memory_watcher/DEPS',
|
||||
'third_party/lss/DEPS',
|
||||
])
|
||||
self.assertEqual(expected, files_to_check);
|
||||
|
||||
|
||||
if __name__ == '__main__':
|
||||
unittest.main()
|
||||
|
Reference in New Issue
Block a user