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

Side by Side 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 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 _CalculateAddedDeps(os_path, old_contents, new_contents):
1053 """Helper method for _CheckAddedDepsHaveTargetApprovals. Returns 1072 """Helper method for _CheckAddedDepsHaveTargetApprovals. Returns
1054 a set of DEPS entries that we should look up. 1073 a set of DEPS entries that we should look up.
1055 1074
1056 For a directory (rather than a specific filename) we fake a path to 1075 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 1076 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. 1077 will seldom or never be subject to per-file include_rules.
1059 """ 1078 """
1060 # We ignore deps entries on auto-generated directories. 1079 # We ignore deps entries on auto-generated directories.
1061 AUTO_GENERATED_DIRS = ['grit', 'jni'] 1080 AUTO_GENERATED_DIRS = ['grit', 'jni']
1062 1081
1063 # This pattern grabs the path without basename in the first 1082 global_scope = {}
1064 # parentheses, and the basename (if present) in the second. It 1083 local_scope = {}
1065 # relies on the simple heuristic that if there is a basename it will 1084 exec old_contents in global_scope, local_scope
1066 # be a header file ending in ".h". 1085 old_deps = _ExtractAddRulesFromParsedDeps(local_scope)
1067 pattern = re.compile( 1086
1068 r"""['"]\+([^'"]+?)(/[a-zA-Z0-9_]+\.h)?['"].*""") 1087 global_scope = {}
1088 local_scope = {}
1089 exec new_contents in global_scope, local_scope
1090 new_deps = _ExtractAddRulesFromParsedDeps(local_scope)
1091
1092 added_deps = new_deps.difference(old_deps)
1093
1069 results = set() 1094 results = set()
1070 for changed_line in changed_lines: 1095 for added_dep in added_deps:
1071 m = pattern.match(changed_line) 1096 if added_dep.split('/')[0] in AUTO_GENERATED_DIRS:
1072 if m: 1097 continue
1073 path = m.group(1) 1098 # 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: 1099 if added_dep.endswith('.h'):
1075 if m.group(2): 1100 results.add(added_dep)
1076 results.add('%s%s' % (path, m.group(2))) 1101 else:
1077 else: 1102 results.add(os_path.join(added_dep, 'DEPS'))
1078 results.add('%s/DEPS' % path)
1079 return results 1103 return results
1080 1104
1081 1105
1082 def _CheckAddedDepsHaveTargetApprovals(input_api, output_api): 1106 def _CheckAddedDepsHaveTargetApprovals(input_api, output_api):
1083 """When a dependency prefixed with + is added to a DEPS file, we 1107 """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 1108 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 1109 target file or directory, to avoid layering violations from being
1086 introduced. This check verifies that this happens. 1110 introduced. This check verifies that this happens.
1087 """ 1111 """
1088 changed_lines = set() 1112 virtual_depended_on_files = set()
1089 1113
1090 file_filter = lambda f: not input_api.re.match( 1114 file_filter = lambda f: not input_api.re.match(
1091 r"^third_party[\\\/]WebKit[\\\/].*", f.LocalPath()) 1115 r"^third_party[\\\/]WebKit[\\\/].*", f.LocalPath())
1092 for f in input_api.AffectedFiles(include_deletes=False, 1116 for f in input_api.AffectedFiles(include_deletes=False,
1093 file_filter=file_filter): 1117 file_filter=file_filter):
1094 filename = input_api.os_path.basename(f.LocalPath()) 1118 filename = input_api.os_path.basename(f.LocalPath())
1095 if filename == 'DEPS': 1119 if filename == 'DEPS':
1096 changed_lines |= set(line.strip() 1120 virtual_depended_on_files.update(_CalculateAddedDeps(
1097 for line_num, line 1121 input_api.os_path,
1098 in f.ChangedContents()) 1122 '\n'.join(f.OldContents()),
1099 if not changed_lines: 1123 '\n'.join(f.NewContents())))
1100 return []
1101 1124
1102 virtual_depended_on_files = _FilesToCheckForIncomingDeps(input_api.re,
1103 changed_lines)
1104 if not virtual_depended_on_files: 1125 if not virtual_depended_on_files:
1105 return [] 1126 return []
1106 1127
1107 if input_api.is_committing: 1128 if input_api.is_committing:
1108 if input_api.tbr: 1129 if input_api.tbr:
1109 return [output_api.PresubmitNotifyResult( 1130 return [output_api.PresubmitNotifyResult(
1110 '--tbr was specified, skipping OWNERS check for DEPS additions')] 1131 '--tbr was specified, skipping OWNERS check for DEPS additions')]
1111 if input_api.dry_run: 1132 if input_api.dry_run:
1112 return [output_api.PresubmitNotifyResult( 1133 return [output_api.PresubmitNotifyResult(
1113 'This is a dry run, skipping OWNERS check for DEPS additions')] 1134 'This is a dry run, skipping OWNERS check for DEPS additions')]
(...skipping 1260 matching lines...) Expand 10 before | Expand all | Expand 10 after
2374 output_api, 2395 output_api,
2375 json_url='http://chromium-status.appspot.com/current?format=json')) 2396 json_url='http://chromium-status.appspot.com/current?format=json'))
2376 2397
2377 results.extend( 2398 results.extend(
2378 input_api.canned_checks.CheckPatchFormatted(input_api, output_api)) 2399 input_api.canned_checks.CheckPatchFormatted(input_api, output_api))
2379 results.extend(input_api.canned_checks.CheckChangeHasBugField( 2400 results.extend(input_api.canned_checks.CheckChangeHasBugField(
2380 input_api, output_api)) 2401 input_api, output_api))
2381 results.extend(input_api.canned_checks.CheckChangeHasDescription( 2402 results.extend(input_api.canned_checks.CheckChangeHasDescription(
2382 input_api, output_api)) 2403 input_api, output_api))
2383 return results 2404 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