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

Side by Side Diff: owners.py

Issue 1085993004: Implement support for file: includes in OWNERS files. (Closed) Base URL: https://chromium.googlesource.com/chromium/tools/depot_tools.git@master
Patch Set: minor cleanups Created 5 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 | testing_support/filesystem_mock.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 """A database of OWNERS files. 5 """A database of OWNERS files.
6 6
7 OWNERS files indicate who is allowed to approve changes in a specific directory 7 OWNERS files indicate who is allowed to approve changes in a specific directory
8 (or who is allowed to make changes without needing approval of another OWNER). 8 (or who is allowed to make changes without needing approval of another OWNER).
9 Note that all changes must still be reviewed by someone familiar with the code, 9 Note that all changes must still be reviewed by someone familiar with the code,
10 so you may need approval from both an OWNER and a reviewer in many cases. 10 so you may need approval from both an OWNER and a reviewer in many cases.
11 11
12 The syntax of the OWNERS file is, roughly: 12 The syntax of the OWNERS file is, roughly:
13 13
14 lines := (\s* line? \s* "\n")* 14 lines := (\s* line? \s* "\n")*
15 15
16 line := directive 16 line := directive
17 | "per-file" \s+ glob \s* "=" \s* directive 17 | "per-file" \s+ glob \s* "=" \s* directive
18 | comment 18 | comment
19 19
20 directive := "set noparent" 20 directive := "set noparent"
21 | "file:" glob
21 | email_address 22 | email_address
22 | "*" 23 | "*"
23 24
24 glob := [a-zA-Z0-9_-*?]+ 25 glob := [a-zA-Z0-9_-*?]+
25 26
26 comment := "#" [^"\n"]* 27 comment := "#" [^"\n"]*
27 28
28 Email addresses must follow the foo@bar.com short form (exact syntax given 29 Email addresses must follow the foo@bar.com short form (exact syntax given
29 in BASIC_EMAIL_REGEXP, below). Filename globs follow the simple unix 30 in BASIC_EMAIL_REGEXP, below). Filename globs follow the simple unix
30 shell conventions, and relative and absolute paths are not allowed (i.e., 31 shell conventions, and relative and absolute paths are not allowed (i.e.,
31 globs only refer to the files in the current directory). 32 globs only refer to the files in the current directory).
32 33
33 If a user's email is one of the email_addresses in the file, the user is 34 If a user's email is one of the email_addresses in the file, the user is
34 considered an "OWNER" for all files in the directory. 35 considered an "OWNER" for all files in the directory.
35 36
36 If the "per-file" directive is used, the line only applies to files in that 37 If the "per-file" directive is used, the line only applies to files in that
37 directory that match the filename glob specified. 38 directory that match the filename glob specified.
38 39
39 If the "set noparent" directive used, then only entries in this OWNERS file 40 If the "set noparent" directive used, then only entries in this OWNERS file
40 apply to files in this directory; if the "set noparent" directive is not 41 apply to files in this directory; if the "set noparent" directive is not
41 used, then entries in OWNERS files in enclosing (upper) directories also 42 used, then entries in OWNERS files in enclosing (upper) directories also
42 apply (up until a "set noparent is encountered"). 43 apply (up until a "set noparent is encountered").
43 44
44 If "per-file glob=set noparent" is used, then global directives are ignored 45 If "per-file glob=set noparent" is used, then global directives are ignored
45 for the glob, and only the "per-file" owners are used for files matching that 46 for the glob, and only the "per-file" owners are used for files matching that
46 glob. 47 glob.
47 48
49 If the "file:" directive is used, the referred to OWNERS file will be parsed and
50 considered when determining the valid set of OWNERS. If the filename starts with
51 "//" it is relative to the root of the repository, otherwise it is relative to
52 the current file
53
48 Examples for all of these combinations can be found in tests/owners_unittest.py. 54 Examples for all of these combinations can be found in tests/owners_unittest.py.
49 """ 55 """
50 56
51 import collections 57 import collections
52 import random 58 import random
53 import re 59 import re
54 60
55 61
56 # If this is present by itself on a line, this means that everyone can review. 62 # If this is present by itself on a line, this means that everyone can review.
57 EVERYONE = '*' 63 EVERYONE = '*'
(...skipping 53 matching lines...) Expand 10 before | Expand all | Expand 10 after
111 # Mapping of paths to authorized owners. 117 # Mapping of paths to authorized owners.
112 self.owners_for = {} 118 self.owners_for = {}
113 119
114 # Mapping reviewers to the preceding comment per file in the OWNERS files. 120 # Mapping reviewers to the preceding comment per file in the OWNERS files.
115 self.comments = {} 121 self.comments = {}
116 122
117 # Set of paths that stop us from looking above them for owners. 123 # Set of paths that stop us from looking above them for owners.
118 # (This is implicitly true for the root directory). 124 # (This is implicitly true for the root directory).
119 self.stop_looking = set(['']) 125 self.stop_looking = set([''])
120 126
127 # Set of files which have already been read.
128 self.read_files = set()
129
121 def reviewers_for(self, files, author): 130 def reviewers_for(self, files, author):
122 """Returns a suggested set of reviewers that will cover the files. 131 """Returns a suggested set of reviewers that will cover the files.
123 132
124 files is a sequence of paths relative to (and under) self.root. 133 files is a sequence of paths relative to (and under) self.root.
125 If author is nonempty, we ensure it is not included in the set returned 134 If author is nonempty, we ensure it is not included in the set returned
126 in order avoid suggesting the author as a reviewer for their own changes.""" 135 in order avoid suggesting the author as a reviewer for their own changes."""
127 self._check_paths(files) 136 self._check_paths(files)
128 self.load_data_needed_for(files) 137 self.load_data_needed_for(files)
129 suggested_owners = self._covering_set_of_owners_for(files, author) 138 suggested_owners = self._covering_set_of_owners_for(files, author)
130 if EVERYONE in suggested_owners: 139 if EVERYONE in suggested_owners:
(...skipping 51 matching lines...) Expand 10 before | Expand all | Expand 10 after
182 while not dirpath in self.owners_for: 191 while not dirpath in self.owners_for:
183 if self._stop_looking(dirpath): 192 if self._stop_looking(dirpath):
184 break 193 break
185 dirpath = self.os_path.dirname(dirpath) 194 dirpath = self.os_path.dirname(dirpath)
186 return dirpath 195 return dirpath
187 196
188 def load_data_needed_for(self, files): 197 def load_data_needed_for(self, files):
189 for f in files: 198 for f in files:
190 dirpath = self.os_path.dirname(f) 199 dirpath = self.os_path.dirname(f)
191 while not dirpath in self.owners_for: 200 while not dirpath in self.owners_for:
192 self._read_owners_in_dir(dirpath) 201 self._read_owners(self.os_path.join(dirpath, 'OWNERS'))
193 if self._stop_looking(dirpath): 202 if self._stop_looking(dirpath):
194 break 203 break
195 dirpath = self.os_path.dirname(dirpath) 204 dirpath = self.os_path.dirname(dirpath)
196 205
197 def _read_owners_in_dir(self, dirpath): 206 def _read_owners(self, path):
198 owners_path = self.os_path.join(self.root, dirpath, 'OWNERS') 207 owners_path = self.os_path.join(self.root, path)
199 if not self.os_path.exists(owners_path): 208 if not self.os_path.exists(owners_path):
200 return 209 return
210
211 if owners_path in self.read_files:
212 return
213
214 self.read_files.add(owners_path)
215
201 comment = [] 216 comment = []
217 dirpath = self.os_path.dirname(path)
202 in_comment = False 218 in_comment = False
203 lineno = 0 219 lineno = 0
204 for line in self.fopen(owners_path): 220 for line in self.fopen(owners_path):
205 lineno += 1 221 lineno += 1
206 line = line.strip() 222 line = line.strip()
207 if line.startswith('#'): 223 if line.startswith('#'):
208 if not in_comment: 224 if not in_comment:
209 comment = [] 225 comment = []
210 comment.append(line[1:].strip()) 226 comment.append(line[1:].strip())
211 in_comment = True 227 in_comment = True
(...skipping 25 matching lines...) Expand all
237 raise SyntaxErrorInOwnersFile(owners_path, lineno, 253 raise SyntaxErrorInOwnersFile(owners_path, lineno,
238 'unknown option: "%s"' % line[4:].strip()) 254 'unknown option: "%s"' % line[4:].strip())
239 255
240 self._add_entry(dirpath, line, 'line', owners_path, lineno, 256 self._add_entry(dirpath, line, 'line', owners_path, lineno,
241 ' '.join(comment)) 257 ' '.join(comment))
242 258
243 def _add_entry(self, path, directive, 259 def _add_entry(self, path, directive,
244 line_type, owners_path, lineno, comment): 260 line_type, owners_path, lineno, comment):
245 if directive == 'set noparent': 261 if directive == 'set noparent':
246 self.stop_looking.add(path) 262 self.stop_looking.add(path)
263 elif directive.startswith('file:'):
264 owners_file = self._resolve_include(directive[5:], owners_path)
265 if not owners_file:
266 raise SyntaxErrorInOwnersFile(owners_path, lineno,
267 ('%s does not refer to an existing file.' % directive[5:]))
268
269 self._read_owners(owners_file)
270
271 dirpath = self.os_path.dirname(owners_file)
272 for key in self.owned_by:
273 if not dirpath in self.owned_by[key]:
274 continue
275 self.owned_by[key].add(path)
276
277 if dirpath in self.owners_for:
278 self.owners_for.setdefault(path, set()).update(self.owners_for[dirpath])
279
247 elif self.email_regexp.match(directive) or directive == EVERYONE: 280 elif self.email_regexp.match(directive) or directive == EVERYONE:
248 self.comments.setdefault(directive, {}) 281 self.comments.setdefault(directive, {})
249 self.comments[directive][path] = comment 282 self.comments[directive][path] = comment
250 self.owned_by.setdefault(directive, set()).add(path) 283 self.owned_by.setdefault(directive, set()).add(path)
251 self.owners_for.setdefault(path, set()).add(directive) 284 self.owners_for.setdefault(path, set()).add(directive)
252 else: 285 else:
253 raise SyntaxErrorInOwnersFile(owners_path, lineno, 286 raise SyntaxErrorInOwnersFile(owners_path, lineno,
254 ('%s is not a "set" directive, "*", ' 287 ('%s is not a "set" directive, file include, "*", '
255 'or an email address: "%s"' % (line_type, directive))) 288 'or an email address: "%s"' % (line_type, directive)))
256 289
290 def _resolve_include(self, path, start):
291 if path.startswith('//'):
292 include_path = path[2:]
293 else:
294 assert start.startswith(self.root)
295 start = self.os_path.dirname(start[len(self.root):])
296 include_path = self.os_path.join(start, path)
297
298 owners_path = self.os_path.join(self.root, include_path)
299 if not self.os_path.exists(owners_path):
300 return None
301
302 return include_path
303
257 def _covering_set_of_owners_for(self, files, author): 304 def _covering_set_of_owners_for(self, files, author):
258 dirs_remaining = set(self._enclosing_dir_with_owners(f) for f in files) 305 dirs_remaining = set(self._enclosing_dir_with_owners(f) for f in files)
259 all_possible_owners = self.all_possible_owners(dirs_remaining, author) 306 all_possible_owners = self.all_possible_owners(dirs_remaining, author)
260 suggested_owners = set() 307 suggested_owners = set()
261 while dirs_remaining: 308 while dirs_remaining:
262 owner = self.lowest_cost_owner(all_possible_owners, dirs_remaining) 309 owner = self.lowest_cost_owner(all_possible_owners, dirs_remaining)
263 suggested_owners.add(owner) 310 suggested_owners.add(owner)
264 dirs_to_remove = set(el[0] for el in all_possible_owners[owner]) 311 dirs_to_remove = set(el[0] for el in all_possible_owners[owner])
265 dirs_remaining -= dirs_to_remove 312 dirs_remaining -= dirs_to_remove
266 return suggested_owners 313 return suggested_owners
(...skipping 44 matching lines...) Expand 10 before | Expand all | Expand 10 after
311 @staticmethod 358 @staticmethod
312 def lowest_cost_owner(all_possible_owners, dirs): 359 def lowest_cost_owner(all_possible_owners, dirs):
313 total_costs_by_owner = Database.total_costs_by_owner(all_possible_owners, 360 total_costs_by_owner = Database.total_costs_by_owner(all_possible_owners,
314 dirs) 361 dirs)
315 # Return the lowest cost owner. In the case of a tie, pick one randomly. 362 # Return the lowest cost owner. In the case of a tie, pick one randomly.
316 lowest_cost = min(total_costs_by_owner.itervalues()) 363 lowest_cost = min(total_costs_by_owner.itervalues())
317 lowest_cost_owners = filter( 364 lowest_cost_owners = filter(
318 lambda owner: total_costs_by_owner[owner] == lowest_cost, 365 lambda owner: total_costs_by_owner[owner] == lowest_cost,
319 total_costs_by_owner) 366 total_costs_by_owner)
320 return random.Random().choice(lowest_cost_owners) 367 return random.Random().choice(lowest_cost_owners)
OLDNEW
« no previous file with comments | « no previous file | testing_support/filesystem_mock.py » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698