Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(265)

Unified Diff: PRESUBMIT.py

Issue 2759593003: Don't require DEPS OWNERS when moving lines around in a DEPS file. (Closed)
Patch Set: Update comment. Created 3 years, 9 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | PRESUBMIT_test.py » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: PRESUBMIT.py
diff --git a/PRESUBMIT.py b/PRESUBMIT.py
index 699a62b5a807a0b10882fec52c3322b01e408e61..6ded7c2c767eaa2a20e11ae58956cd7692d957d1 100644
--- a/PRESUBMIT.py
+++ b/PRESUBMIT.py
@@ -1049,7 +1049,26 @@ def _CheckNoAbbreviationInPngFileName(input_api, output_api):
return results
-def _FilesToCheckForIncomingDeps(re, changed_lines):
+def _ExtractAddRulesFromParsedDeps(parsed_deps):
+ """Extract the rules that add dependencies from a parsed DEPS file.
+
+ Args:
+ parsed_deps: the locals dictionary from evaluating the DEPS file."""
+ add_rules = set()
+ add_rules.update([
+ rule[1:] for rule in parsed_deps.get('include_rules', [])
+ if rule.startswith('+') or rule.startswith('!')
+ ])
+ for specific_file, rules in parsed_deps.get('specific_include_rules',
+ {}).iteritems():
+ add_rules.update([
+ rule[1:] for rule in rules
+ if rule.startswith('+') or rule.startswith('!')
+ ])
+ return add_rules
+
+
+def _CalculateAddedDeps(os_path, old_contents, new_contents):
"""Helper method for _CheckAddedDepsHaveTargetApprovals. Returns
a set of DEPS entries that we should look up.
@@ -1060,22 +1079,27 @@ def _FilesToCheckForIncomingDeps(re, changed_lines):
# We ignore deps entries on auto-generated directories.
AUTO_GENERATED_DIRS = ['grit', 'jni']
- # 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)?['"].*""")
+ global_scope = {}
+ local_scope = {}
+ exec old_contents in global_scope, local_scope
+ old_deps = _ExtractAddRulesFromParsedDeps(local_scope)
+
+ global_scope = {}
+ local_scope = {}
+ exec new_contents in global_scope, local_scope
+ new_deps = _ExtractAddRulesFromParsedDeps(local_scope)
+
+ added_deps = new_deps.difference(old_deps)
+
results = set()
- for changed_line in changed_lines:
- m = pattern.match(changed_line)
- if m:
- path = m.group(1)
- if path.split('/')[0] not in AUTO_GENERATED_DIRS:
- if m.group(2):
- results.add('%s%s' % (path, m.group(2)))
- else:
- results.add('%s/DEPS' % path)
+ for added_dep in added_deps:
+ if added_dep.split('/')[0] in AUTO_GENERATED_DIRS:
+ continue
+ # Assume that a rule that ends in .h is a rule for a specific file.
+ if added_dep.endswith('.h'):
+ results.add(added_dep)
+ else:
+ results.add(os_path.join(added_dep, 'DEPS'))
return results
@@ -1085,7 +1109,7 @@ def _CheckAddedDepsHaveTargetApprovals(input_api, output_api):
target file or directory, to avoid layering violations from being
introduced. This check verifies that this happens.
"""
- changed_lines = set()
+ virtual_depended_on_files = set()
file_filter = lambda f: not input_api.re.match(
r"^third_party[\\\/]WebKit[\\\/].*", f.LocalPath())
@@ -1093,14 +1117,11 @@ def _CheckAddedDepsHaveTargetApprovals(input_api, output_api):
file_filter=file_filter):
filename = input_api.os_path.basename(f.LocalPath())
if filename == 'DEPS':
- changed_lines |= set(line.strip()
- for line_num, line
- in f.ChangedContents())
- if not changed_lines:
- return []
+ virtual_depended_on_files.update(_CalculateAddedDeps(
+ input_api.os_path,
+ '\n'.join(f.OldContents()),
+ '\n'.join(f.NewContents())))
- virtual_depended_on_files = _FilesToCheckForIncomingDeps(input_api.re,
- changed_lines)
if not virtual_depended_on_files:
return []
« no previous file with comments | « no previous file | PRESUBMIT_test.py » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698