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

Side by Side Diff: PRESUBMIT.py

Issue 2792853002: Revert of Don't require DEPS OWNERS when moving lines around in a DEPS file. (Closed)
Patch Set: Created 3 years, 8 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 _ExtractAddRulesFromParsedDeps(parsed_deps): 1052 def _FilesToCheckForIncomingDeps(re, changed_lines):
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):
1103 """Helper method for _CheckAddedDepsHaveTargetApprovals. Returns 1053 """Helper method for _CheckAddedDepsHaveTargetApprovals. Returns
1104 a set of DEPS entries that we should look up. 1054 a set of DEPS entries that we should look up.
1105 1055
1106 For a directory (rather than a specific filename) we fake a path to 1056 For a directory (rather than a specific filename) we fake a path to
1107 a specific filename by adding /DEPS. This is chosen as a file that 1057 a specific filename by adding /DEPS. This is chosen as a file that
1108 will seldom or never be subject to per-file include_rules. 1058 will seldom or never be subject to per-file include_rules.
1109 """ 1059 """
1110 # We ignore deps entries on auto-generated directories. 1060 # We ignore deps entries on auto-generated directories.
1111 AUTO_GENERATED_DIRS = ['grit', 'jni'] 1061 AUTO_GENERATED_DIRS = ['grit', 'jni']
1112 1062
1113 old_deps = _ExtractAddRulesFromParsedDeps(_ParseDeps(old_contents)) 1063 # This pattern grabs the path without basename in the first
1114 new_deps = _ExtractAddRulesFromParsedDeps(_ParseDeps(new_contents)) 1064 # parentheses, and the basename (if present) in the second. It
1115 1065 # relies on the simple heuristic that if there is a basename it will
1116 added_deps = new_deps.difference(old_deps) 1066 # be a header file ending in ".h".
1117 1067 pattern = re.compile(
1068 r"""['"]\+([^'"]+?)(/[a-zA-Z0-9_]+\.h)?['"].*""")
1118 results = set() 1069 results = set()
1119 for added_dep in added_deps: 1070 for changed_line in changed_lines:
1120 if added_dep.split('/')[0] in AUTO_GENERATED_DIRS: 1071 m = pattern.match(changed_line)
1121 continue 1072 if m:
1122 # Assume that a rule that ends in .h is a rule for a specific file. 1073 path = m.group(1)
1123 if added_dep.endswith('.h'): 1074 if path.split('/')[0] not in AUTO_GENERATED_DIRS:
1124 results.add(added_dep) 1075 if m.group(2):
1125 else: 1076 results.add('%s%s' % (path, m.group(2)))
1126 results.add(os_path.join(added_dep, 'DEPS')) 1077 else:
1078 results.add('%s/DEPS' % path)
1127 return results 1079 return results
1128 1080
1129 1081
1130 def _CheckAddedDepsHaveTargetApprovals(input_api, output_api): 1082 def _CheckAddedDepsHaveTargetApprovals(input_api, output_api):
1131 """When a dependency prefixed with + is added to a DEPS file, we 1083 """When a dependency prefixed with + is added to a DEPS file, we
1132 want to make sure that the change is reviewed by an OWNER of the 1084 want to make sure that the change is reviewed by an OWNER of the
1133 target file or directory, to avoid layering violations from being 1085 target file or directory, to avoid layering violations from being
1134 introduced. This check verifies that this happens. 1086 introduced. This check verifies that this happens.
1135 """ 1087 """
1136 virtual_depended_on_files = set() 1088 changed_lines = set()
1137 1089
1138 file_filter = lambda f: not input_api.re.match( 1090 file_filter = lambda f: not input_api.re.match(
1139 r"^third_party[\\\/]WebKit[\\\/].*", f.LocalPath()) 1091 r"^third_party[\\\/]WebKit[\\\/].*", f.LocalPath())
1140 for f in input_api.AffectedFiles(include_deletes=False, 1092 for f in input_api.AffectedFiles(include_deletes=False,
1141 file_filter=file_filter): 1093 file_filter=file_filter):
1142 filename = input_api.os_path.basename(f.LocalPath()) 1094 filename = input_api.os_path.basename(f.LocalPath())
1143 if filename == 'DEPS': 1095 if filename == 'DEPS':
1144 virtual_depended_on_files.update(_CalculateAddedDeps( 1096 changed_lines |= set(line.strip()
1145 input_api.os_path, 1097 for line_num, line
1146 '\n'.join(f.OldContents()), 1098 in f.ChangedContents())
1147 '\n'.join(f.NewContents()))) 1099 if not changed_lines:
1100 return []
1148 1101
1102 virtual_depended_on_files = _FilesToCheckForIncomingDeps(input_api.re,
1103 changed_lines)
1149 if not virtual_depended_on_files: 1104 if not virtual_depended_on_files:
1150 return [] 1105 return []
1151 1106
1152 if input_api.is_committing: 1107 if input_api.is_committing:
1153 if input_api.tbr: 1108 if input_api.tbr:
1154 return [output_api.PresubmitNotifyResult( 1109 return [output_api.PresubmitNotifyResult(
1155 '--tbr was specified, skipping OWNERS check for DEPS additions')] 1110 '--tbr was specified, skipping OWNERS check for DEPS additions')]
1156 if input_api.dry_run: 1111 if input_api.dry_run:
1157 return [output_api.PresubmitNotifyResult( 1112 return [output_api.PresubmitNotifyResult(
1158 'This is a dry run, skipping OWNERS check for DEPS additions')] 1113 'This is a dry run, skipping OWNERS check for DEPS additions')]
(...skipping 1258 matching lines...) Expand 10 before | Expand all | Expand 10 after
2417 output_api, 2372 output_api,
2418 json_url='http://chromium-status.appspot.com/current?format=json')) 2373 json_url='http://chromium-status.appspot.com/current?format=json'))
2419 2374
2420 results.extend( 2375 results.extend(
2421 input_api.canned_checks.CheckPatchFormatted(input_api, output_api)) 2376 input_api.canned_checks.CheckPatchFormatted(input_api, output_api))
2422 results.extend(input_api.canned_checks.CheckChangeHasBugField( 2377 results.extend(input_api.canned_checks.CheckChangeHasBugField(
2423 input_api, output_api)) 2378 input_api, output_api))
2424 results.extend(input_api.canned_checks.CheckChangeHasDescription( 2379 results.extend(input_api.canned_checks.CheckChangeHasDescription(
2425 input_api, output_api)) 2380 input_api, output_api))
2426 return results 2381 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