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

Side by Side Diff: PRESUBMIT.py

Issue 2768063004: Don't require DEPS OWNERS when moving lines around in a DEPS file. (Closed)
Patch Set: Revert test DEPS change 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 unified diff | Download patch
« no previous file with comments | « no previous file | PRESUBMIT_test.py » ('j') | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
OLDNEW
1 # Copyright (c) 2012 The Chromium Authors. All rights reserved. 1 # Copyright (c) 2012 The Chromium Authors. All rights reserved.
2 # Use of this source code is governed by a BSD-style license that can be 2 # Use of this source code is governed by a BSD-style license that can be
3 # found in the LICENSE file. 3 # found in the LICENSE file.
4 4
5 """Top-level presubmit script for Chromium. 5 """Top-level presubmit script for Chromium.
6 6
7 See http://dev.chromium.org/developers/how-tos/depottools/presubmit-scripts 7 See http://dev.chromium.org/developers/how-tos/depottools/presubmit-scripts
8 for more details about the presubmit API built into depot_tools. 8 for more details about the presubmit API built into depot_tools.
9 """ 9 """
10 10
(...skipping 1031 matching lines...) Expand 10 before | Expand all | Expand 10 after
1042 1042
1043 results = [] 1043 results = []
1044 if errors: 1044 if errors:
1045 results.append(output_api.PresubmitError( 1045 results.append(output_api.PresubmitError(
1046 'The name of PNG files should not have abbreviations. \n' 1046 'The name of PNG files should not have abbreviations. \n'
1047 'Use _hover.png, _center.png, instead of _h.png, _c.png.\n' 1047 'Use _hover.png, _center.png, instead of _h.png, _c.png.\n'
1048 'Contact oshima@chromium.org if you have questions.', errors)) 1048 'Contact oshima@chromium.org if you have questions.', errors))
1049 return results 1049 return results
1050 1050
1051 1051
1052 def _FilesToCheckForIncomingDeps(re, changed_lines): 1052 def _ExtractAddRulesFromParsedDeps(parsed_deps):
1053 """Extract the rules that add dependencies from a parsed DEPS file.
1054
1055 Args:
1056 parsed_deps: the locals dictionary from evaluating the DEPS file."""
1057 add_rules = set()
1058 add_rules.update([
1059 rule[1:] for rule in parsed_deps.get('include_rules', [])
1060 if rule.startswith('+') or rule.startswith('!')
1061 ])
1062 for specific_file, rules in parsed_deps.get('specific_include_rules',
1063 {}).iteritems():
1064 add_rules.update([
1065 rule[1:] for rule in rules
1066 if rule.startswith('+') or rule.startswith('!')
1067 ])
1068 return add_rules
1069
1070
1071 def _ParseDeps(contents):
1072 """Simple helper for parsing DEPS files."""
1073 # Stubs for handling special syntax in the root DEPS file.
1074 def FromImpl(*_):
1075 pass # NOP function so "From" doesn't fail.
1076
1077 def FileImpl(_):
1078 pass # NOP function so "File" doesn't fail.
1079
1080 class _VarImpl:
1081
1082 def __init__(self, local_scope):
1083 self._local_scope = local_scope
1084
1085 def Lookup(self, var_name):
1086 """Implements the Var syntax."""
1087 try:
1088 return self._local_scope['vars'][var_name]
1089 except KeyError:
1090 raise Exception('Var is not defined: %s' % var_name)
1091
1092 local_scope = {}
1093 global_scope = {
1094 'File': FileImpl,
1095 'From': FromImpl,
1096 'Var': _VarImpl(local_scope).Lookup,
1097 }
1098 exec contents in global_scope, local_scope
1099 return local_scope
1100
1101
1102 def _CalculateAddedDeps(os_path, old_contents, new_contents):
1053 """Helper method for _CheckAddedDepsHaveTargetApprovals. Returns 1103 """Helper method for _CheckAddedDepsHaveTargetApprovals. Returns
1054 a set of DEPS entries that we should look up. 1104 a set of DEPS entries that we should look up.
1055 1105
1056 For a directory (rather than a specific filename) we fake a path to 1106 For a directory (rather than a specific filename) we fake a path to
1057 a specific filename by adding /DEPS. This is chosen as a file that 1107 a specific filename by adding /DEPS. This is chosen as a file that
1058 will seldom or never be subject to per-file include_rules. 1108 will seldom or never be subject to per-file include_rules.
1059 """ 1109 """
1060 # We ignore deps entries on auto-generated directories. 1110 # We ignore deps entries on auto-generated directories.
1061 AUTO_GENERATED_DIRS = ['grit', 'jni'] 1111 AUTO_GENERATED_DIRS = ['grit', 'jni']
1062 1112
1063 # This pattern grabs the path without basename in the first 1113 old_deps = _ExtractAddRulesFromParsedDeps(_ParseDeps(old_contents))
1064 # parentheses, and the basename (if present) in the second. It 1114 new_deps = _ExtractAddRulesFromParsedDeps(_ParseDeps(new_contents))
1065 # relies on the simple heuristic that if there is a basename it will 1115
1066 # be a header file ending in ".h". 1116 added_deps = new_deps.difference(old_deps)
1067 pattern = re.compile( 1117
1068 r"""['"]\+([^'"]+?)(/[a-zA-Z0-9_]+\.h)?['"].*""")
1069 results = set() 1118 results = set()
1070 for changed_line in changed_lines: 1119 for added_dep in added_deps:
1071 m = pattern.match(changed_line) 1120 if added_dep.split('/')[0] in AUTO_GENERATED_DIRS:
1072 if m: 1121 continue
1073 path = m.group(1) 1122 # Assume that a rule that ends in .h is a rule for a specific file.
1074 if path.split('/')[0] not in AUTO_GENERATED_DIRS: 1123 if added_dep.endswith('.h'):
1075 if m.group(2): 1124 results.add(added_dep)
1076 results.add('%s%s' % (path, m.group(2))) 1125 else:
1077 else: 1126 results.add(os_path.join(added_dep, 'DEPS'))
1078 results.add('%s/DEPS' % path)
1079 return results 1127 return results
1080 1128
1081 1129
1082 def _CheckAddedDepsHaveTargetApprovals(input_api, output_api): 1130 def _CheckAddedDepsHaveTargetApprovals(input_api, output_api):
1083 """When a dependency prefixed with + is added to a DEPS file, we 1131 """When a dependency prefixed with + is added to a DEPS file, we
1084 want to make sure that the change is reviewed by an OWNER of the 1132 want to make sure that the change is reviewed by an OWNER of the
1085 target file or directory, to avoid layering violations from being 1133 target file or directory, to avoid layering violations from being
1086 introduced. This check verifies that this happens. 1134 introduced. This check verifies that this happens.
1087 """ 1135 """
1088 changed_lines = set() 1136 virtual_depended_on_files = set()
1089 1137
1090 file_filter = lambda f: not input_api.re.match( 1138 file_filter = lambda f: not input_api.re.match(
1091 r"^third_party[\\\/]WebKit[\\\/].*", f.LocalPath()) 1139 r"^third_party[\\\/]WebKit[\\\/].*", f.LocalPath())
1092 for f in input_api.AffectedFiles(include_deletes=False, 1140 for f in input_api.AffectedFiles(include_deletes=False,
1093 file_filter=file_filter): 1141 file_filter=file_filter):
1094 filename = input_api.os_path.basename(f.LocalPath()) 1142 filename = input_api.os_path.basename(f.LocalPath())
1095 if filename == 'DEPS': 1143 if filename == 'DEPS':
1096 changed_lines |= set(line.strip() 1144 virtual_depended_on_files.update(_CalculateAddedDeps(
1097 for line_num, line 1145 input_api.os_path,
1098 in f.ChangedContents()) 1146 '\n'.join(f.OldContents()),
1099 if not changed_lines: 1147 '\n'.join(f.NewContents())))
1100 return []
1101 1148
1102 virtual_depended_on_files = _FilesToCheckForIncomingDeps(input_api.re,
1103 changed_lines)
1104 if not virtual_depended_on_files: 1149 if not virtual_depended_on_files:
1105 return [] 1150 return []
1106 1151
1107 if input_api.is_committing: 1152 if input_api.is_committing:
1108 if input_api.tbr: 1153 if input_api.tbr:
1109 return [output_api.PresubmitNotifyResult( 1154 return [output_api.PresubmitNotifyResult(
1110 '--tbr was specified, skipping OWNERS check for DEPS additions')] 1155 '--tbr was specified, skipping OWNERS check for DEPS additions')]
1111 if input_api.dry_run: 1156 if input_api.dry_run:
1112 return [output_api.PresubmitNotifyResult( 1157 return [output_api.PresubmitNotifyResult(
1113 'This is a dry run, skipping OWNERS check for DEPS additions')] 1158 'This is a dry run, skipping OWNERS check for DEPS additions')]
(...skipping 1258 matching lines...) Expand 10 before | Expand all | Expand 10 after
2372 output_api, 2417 output_api,
2373 json_url='http://chromium-status.appspot.com/current?format=json')) 2418 json_url='http://chromium-status.appspot.com/current?format=json'))
2374 2419
2375 results.extend( 2420 results.extend(
2376 input_api.canned_checks.CheckPatchFormatted(input_api, output_api)) 2421 input_api.canned_checks.CheckPatchFormatted(input_api, output_api))
2377 results.extend(input_api.canned_checks.CheckChangeHasBugField( 2422 results.extend(input_api.canned_checks.CheckChangeHasBugField(
2378 input_api, output_api)) 2423 input_api, output_api))
2379 results.extend(input_api.canned_checks.CheckChangeHasDescription( 2424 results.extend(input_api.canned_checks.CheckChangeHasDescription(
2380 input_api, output_api)) 2425 input_api, output_api))
2381 return results 2426 return results
OLDNEW
« 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