OLD | NEW |
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. |
(...skipping 60 matching lines...) Expand 10 before | Expand all | Expand 10 after Loading... |
71 | 71 |
72 | 72 |
73 class SyntaxErrorInOwnersFile(Exception): | 73 class SyntaxErrorInOwnersFile(Exception): |
74 def __init__(self, path, lineno, msg): | 74 def __init__(self, path, lineno, msg): |
75 super(SyntaxErrorInOwnersFile, self).__init__((path, lineno, msg)) | 75 super(SyntaxErrorInOwnersFile, self).__init__((path, lineno, msg)) |
76 self.path = path | 76 self.path = path |
77 self.lineno = lineno | 77 self.lineno = lineno |
78 self.msg = msg | 78 self.msg = msg |
79 | 79 |
80 def __str__(self): | 80 def __str__(self): |
81 return "%s:%d syntax error: %s" % (self.path, self.lineno, self.msg) | 81 return '%s:%d syntax error: %s' % (self.path, self.lineno, self.msg) |
82 | 82 |
83 | 83 |
84 class Database(object): | 84 class Database(object): |
85 """A database of OWNERS files for a repository. | 85 """A database of OWNERS files for a repository. |
86 | 86 |
87 This class allows you to find a suggested set of reviewers for a list | 87 This class allows you to find a suggested set of reviewers for a list |
88 of changed files, and see if a list of changed files is covered by a | 88 of changed files, and see if a list of changed files is covered by a |
89 list of reviewers.""" | 89 list of reviewers.""" |
90 | 90 |
91 def __init__(self, root, fopen, os_path, glob): | 91 def __init__(self, root, fopen, os_path, glob): |
(...skipping 12 matching lines...) Expand all Loading... |
104 | 104 |
105 # Pick a default email regexp to use; callers can override as desired. | 105 # Pick a default email regexp to use; callers can override as desired. |
106 self.email_regexp = re.compile(BASIC_EMAIL_REGEXP) | 106 self.email_regexp = re.compile(BASIC_EMAIL_REGEXP) |
107 | 107 |
108 # Mapping of owners to the paths they own. | 108 # Mapping of owners to the paths they own. |
109 self.owned_by = {EVERYONE: set()} | 109 self.owned_by = {EVERYONE: set()} |
110 | 110 |
111 # Mapping of paths to authorized owners. | 111 # Mapping of paths to authorized owners. |
112 self.owners_for = {} | 112 self.owners_for = {} |
113 | 113 |
| 114 # Mapping reviewers to the preceding comment per file in the OWNERS files. |
| 115 self.comments = {} |
| 116 |
114 # Set of paths that stop us from looking above them for owners. | 117 # Set of paths that stop us from looking above them for owners. |
115 # (This is implicitly true for the root directory). | 118 # (This is implicitly true for the root directory). |
116 self.stop_looking = set(['']) | 119 self.stop_looking = set(['']) |
117 | 120 |
118 def reviewers_for(self, files, author): | 121 def reviewers_for(self, files, author): |
119 """Returns a suggested set of reviewers that will cover the files. | 122 """Returns a suggested set of reviewers that will cover the files. |
120 | 123 |
121 files is a sequence of paths relative to (and under) self.root. | 124 files is a sequence of paths relative to (and under) self.root. |
122 If author is nonempty, we ensure it is not included in the set returned | 125 If author is nonempty, we ensure it is not included in the set returned |
123 in order avoid suggesting the author as a reviewer for their own changes.""" | 126 in order avoid suggesting the author as a reviewer for their own changes.""" |
124 self._check_paths(files) | 127 self._check_paths(files) |
125 self._load_data_needed_for(files) | 128 self.load_data_needed_for(files) |
126 suggested_owners = self._covering_set_of_owners_for(files, author) | 129 suggested_owners = self._covering_set_of_owners_for(files, author) |
127 if EVERYONE in suggested_owners: | 130 if EVERYONE in suggested_owners: |
128 if len(suggested_owners) > 1: | 131 if len(suggested_owners) > 1: |
129 suggested_owners.remove(EVERYONE) | 132 suggested_owners.remove(EVERYONE) |
130 else: | 133 else: |
131 suggested_owners = set(['<anyone>']) | 134 suggested_owners = set(['<anyone>']) |
132 return suggested_owners | 135 return suggested_owners |
133 | 136 |
134 def files_not_covered_by(self, files, reviewers): | 137 def files_not_covered_by(self, files, reviewers): |
135 """Returns the files not owned by one of the reviewers. | 138 """Returns the files not owned by one of the reviewers. |
136 | 139 |
137 Args: | 140 Args: |
138 files is a sequence of paths relative to (and under) self.root. | 141 files is a sequence of paths relative to (and under) self.root. |
139 reviewers is a sequence of strings matching self.email_regexp. | 142 reviewers is a sequence of strings matching self.email_regexp. |
140 """ | 143 """ |
141 self._check_paths(files) | 144 self._check_paths(files) |
142 self._check_reviewers(reviewers) | 145 self._check_reviewers(reviewers) |
143 self._load_data_needed_for(files) | 146 self.load_data_needed_for(files) |
144 | 147 |
145 covered_objs = self._objs_covered_by(reviewers) | 148 covered_objs = self._objs_covered_by(reviewers) |
146 uncovered_files = [f for f in files | 149 uncovered_files = [f for f in files |
147 if not self._is_obj_covered_by(f, covered_objs)] | 150 if not self._is_obj_covered_by(f, covered_objs)] |
148 | 151 |
149 return set(uncovered_files) | 152 return set(uncovered_files) |
150 | 153 |
151 def _check_paths(self, files): | 154 def _check_paths(self, files): |
152 def _is_under(f, pfx): | 155 def _is_under(f, pfx): |
153 return self.os_path.abspath(self.os_path.join(pfx, f)).startswith(pfx) | 156 return self.os_path.abspath(self.os_path.join(pfx, f)).startswith(pfx) |
(...skipping 21 matching lines...) Expand all Loading... |
175 | 178 |
176 def _enclosing_dir_with_owners(self, objname): | 179 def _enclosing_dir_with_owners(self, objname): |
177 """Returns the innermost enclosing directory that has an OWNERS file.""" | 180 """Returns the innermost enclosing directory that has an OWNERS file.""" |
178 dirpath = objname | 181 dirpath = objname |
179 while not dirpath in self.owners_for: | 182 while not dirpath in self.owners_for: |
180 if self._stop_looking(dirpath): | 183 if self._stop_looking(dirpath): |
181 break | 184 break |
182 dirpath = self.os_path.dirname(dirpath) | 185 dirpath = self.os_path.dirname(dirpath) |
183 return dirpath | 186 return dirpath |
184 | 187 |
185 def _load_data_needed_for(self, files): | 188 def load_data_needed_for(self, files): |
186 for f in files: | 189 for f in files: |
187 dirpath = self.os_path.dirname(f) | 190 dirpath = self.os_path.dirname(f) |
188 while not dirpath in self.owners_for: | 191 while not dirpath in self.owners_for: |
189 self._read_owners_in_dir(dirpath) | 192 self._read_owners_in_dir(dirpath) |
190 if self._stop_looking(dirpath): | 193 if self._stop_looking(dirpath): |
191 break | 194 break |
192 dirpath = self.os_path.dirname(dirpath) | 195 dirpath = self.os_path.dirname(dirpath) |
193 | 196 |
194 def _read_owners_in_dir(self, dirpath): | 197 def _read_owners_in_dir(self, dirpath): |
195 owners_path = self.os_path.join(self.root, dirpath, 'OWNERS') | 198 owners_path = self.os_path.join(self.root, dirpath, 'OWNERS') |
196 if not self.os_path.exists(owners_path): | 199 if not self.os_path.exists(owners_path): |
197 return | 200 return |
198 | 201 comment = [] |
| 202 in_comment = False |
199 lineno = 0 | 203 lineno = 0 |
200 for line in self.fopen(owners_path): | 204 for line in self.fopen(owners_path): |
201 lineno += 1 | 205 lineno += 1 |
202 line = line.strip() | 206 line = line.strip() |
203 if line.startswith('#') or line == '': | 207 if line.startswith('#'): |
| 208 if not in_comment: |
| 209 comment = [] |
| 210 comment.append(line[1:].strip()) |
| 211 in_comment = True |
204 continue | 212 continue |
| 213 if line == '': |
| 214 continue |
| 215 in_comment = False |
| 216 |
205 if line == 'set noparent': | 217 if line == 'set noparent': |
206 self.stop_looking.add(dirpath) | 218 self.stop_looking.add(dirpath) |
207 continue | 219 continue |
208 | 220 |
209 m = re.match("per-file (.+)=(.+)", line) | 221 m = re.match('per-file (.+)=(.+)', line) |
210 if m: | 222 if m: |
211 glob_string = m.group(1).strip() | 223 glob_string = m.group(1).strip() |
212 directive = m.group(2).strip() | 224 directive = m.group(2).strip() |
213 full_glob_string = self.os_path.join(self.root, dirpath, glob_string) | 225 full_glob_string = self.os_path.join(self.root, dirpath, glob_string) |
214 if '/' in glob_string or '\\' in glob_string: | 226 if '/' in glob_string or '\\' in glob_string: |
215 raise SyntaxErrorInOwnersFile(owners_path, lineno, | 227 raise SyntaxErrorInOwnersFile(owners_path, lineno, |
216 'per-file globs cannot span directories or use escapes: "%s"' % | 228 'per-file globs cannot span directories or use escapes: "%s"' % |
217 line) | 229 line) |
218 baselines = self.glob(full_glob_string) | 230 baselines = self.glob(full_glob_string) |
219 for baseline in (self.os_path.relpath(b, self.root) for b in baselines): | 231 for baseline in (self.os_path.relpath(b, self.root) for b in baselines): |
220 self._add_entry(baseline, directive, "per-file line", | 232 self._add_entry(baseline, directive, 'per-file line', |
221 owners_path, lineno) | 233 owners_path, lineno, '\n'.join(comment)) |
222 continue | 234 continue |
223 | 235 |
224 if line.startswith('set '): | 236 if line.startswith('set '): |
225 raise SyntaxErrorInOwnersFile(owners_path, lineno, | 237 raise SyntaxErrorInOwnersFile(owners_path, lineno, |
226 'unknown option: "%s"' % line[4:].strip()) | 238 'unknown option: "%s"' % line[4:].strip()) |
227 | 239 |
228 self._add_entry(dirpath, line, "line", owners_path, lineno) | 240 self._add_entry(dirpath, line, 'line', owners_path, lineno, |
| 241 ' '.join(comment)) |
229 | 242 |
230 def _add_entry(self, path, directive, line_type, owners_path, lineno): | 243 def _add_entry(self, path, directive, |
231 if directive == "set noparent": | 244 line_type, owners_path, lineno, comment): |
| 245 if directive == 'set noparent': |
232 self.stop_looking.add(path) | 246 self.stop_looking.add(path) |
233 elif self.email_regexp.match(directive) or directive == EVERYONE: | 247 elif self.email_regexp.match(directive) or directive == EVERYONE: |
| 248 self.comments.setdefault(directive, {}) |
| 249 self.comments[directive][path] = comment |
234 self.owned_by.setdefault(directive, set()).add(path) | 250 self.owned_by.setdefault(directive, set()).add(path) |
235 self.owners_for.setdefault(path, set()).add(directive) | 251 self.owners_for.setdefault(path, set()).add(directive) |
236 else: | 252 else: |
237 raise SyntaxErrorInOwnersFile(owners_path, lineno, | 253 raise SyntaxErrorInOwnersFile(owners_path, lineno, |
238 ('%s is not a "set" directive, "*", ' | 254 ('%s is not a "set" directive, "*", ' |
239 'or an email address: "%s"' % (line_type, directive))) | 255 'or an email address: "%s"' % (line_type, directive))) |
240 | 256 |
241 def _covering_set_of_owners_for(self, files, author): | 257 def _covering_set_of_owners_for(self, files, author): |
242 dirs_remaining = set(self._enclosing_dir_with_owners(f) for f in files) | 258 dirs_remaining = set(self._enclosing_dir_with_owners(f) for f in files) |
243 all_possible_owners = self._all_possible_owners(dirs_remaining, author) | 259 all_possible_owners = self.all_possible_owners(dirs_remaining, author) |
244 suggested_owners = set() | 260 suggested_owners = set() |
245 while dirs_remaining: | 261 while dirs_remaining: |
246 owner = self.lowest_cost_owner(all_possible_owners, dirs_remaining) | 262 owner = self.lowest_cost_owner(all_possible_owners, dirs_remaining) |
247 suggested_owners.add(owner) | 263 suggested_owners.add(owner) |
248 dirs_to_remove = set(el[0] for el in all_possible_owners[owner]) | 264 dirs_to_remove = set(el[0] for el in all_possible_owners[owner]) |
249 dirs_remaining -= dirs_to_remove | 265 dirs_remaining -= dirs_to_remove |
250 return suggested_owners | 266 return suggested_owners |
251 | 267 |
252 def _all_possible_owners(self, dirs, author): | 268 def all_possible_owners(self, dirs, author): |
253 """Returns a list of (potential owner, distance-from-dir) tuples; a | 269 """Returns a list of (potential owner, distance-from-dir) tuples; a |
254 distance of 1 is the lowest/closest possible distance (which makes the | 270 distance of 1 is the lowest/closest possible distance (which makes the |
255 subsequent math easier).""" | 271 subsequent math easier).""" |
256 all_possible_owners = {} | 272 all_possible_owners = {} |
257 for current_dir in dirs: | 273 for current_dir in dirs: |
258 dirname = current_dir | 274 dirname = current_dir |
259 distance = 1 | 275 distance = 1 |
260 while True: | 276 while True: |
261 for owner in self.owners_for.get(dirname, []): | 277 for owner in self.owners_for.get(dirname, []): |
262 if author and owner == author: | 278 if author and owner == author: |
263 continue | 279 continue |
264 all_possible_owners.setdefault(owner, []) | 280 all_possible_owners.setdefault(owner, []) |
265 # If the same person is in multiple OWNERS files above a given | 281 # If the same person is in multiple OWNERS files above a given |
266 # directory, only count the closest one. | 282 # directory, only count the closest one. |
267 if not any(current_dir == el[0] for el in all_possible_owners[owner]): | 283 if not any(current_dir == el[0] for el in all_possible_owners[owner]): |
268 all_possible_owners[owner].append((current_dir, distance)) | 284 all_possible_owners[owner].append((current_dir, distance)) |
269 if self._stop_looking(dirname): | 285 if self._stop_looking(dirname): |
270 break | 286 break |
271 dirname = self.os_path.dirname(dirname) | 287 dirname = self.os_path.dirname(dirname) |
272 distance += 1 | 288 distance += 1 |
273 return all_possible_owners | 289 return all_possible_owners |
274 | 290 |
275 @staticmethod | 291 @staticmethod |
276 def lowest_cost_owner(all_possible_owners, dirs): | 292 def total_costs_by_owner(all_possible_owners, dirs): |
277 # We want to minimize both the number of reviewers and the distance | 293 # We want to minimize both the number of reviewers and the distance |
278 # from the files/dirs needing reviews. The "pow(X, 1.75)" below is | 294 # from the files/dirs needing reviews. The "pow(X, 1.75)" below is |
279 # an arbitrarily-selected scaling factor that seems to work well - it | 295 # an arbitrarily-selected scaling factor that seems to work well - it |
280 # will select one reviewer in the parent directory over three reviewers | 296 # will select one reviewer in the parent directory over three reviewers |
281 # in subdirs, but not one reviewer over just two. | 297 # in subdirs, but not one reviewer over just two. |
282 total_costs_by_owner = {} | 298 result = {} |
283 for owner in all_possible_owners: | 299 for owner in all_possible_owners: |
284 total_distance = 0 | 300 total_distance = 0 |
285 num_directories_owned = 0 | 301 num_directories_owned = 0 |
286 for dirname, distance in all_possible_owners[owner]: | 302 for dirname, distance in all_possible_owners[owner]: |
287 if dirname in dirs: | 303 if dirname in dirs: |
288 total_distance += distance | 304 total_distance += distance |
289 num_directories_owned += 1 | 305 num_directories_owned += 1 |
290 if num_directories_owned: | 306 if num_directories_owned: |
291 total_costs_by_owner[owner] = (total_distance / | 307 result[owner] = (total_distance / |
292 pow(num_directories_owned, 1.75)) | 308 pow(num_directories_owned, 1.75)) |
| 309 return result |
293 | 310 |
| 311 @staticmethod |
| 312 def lowest_cost_owner(all_possible_owners, dirs): |
| 313 total_costs_by_owner = Database.total_costs_by_owner(all_possible_owners, |
| 314 dirs) |
294 # Return the lowest cost owner. In the case of a tie, pick one randomly. | 315 # Return the lowest cost owner. In the case of a tie, pick one randomly. |
295 lowest_cost = min(total_costs_by_owner.itervalues()) | 316 lowest_cost = min(total_costs_by_owner.itervalues()) |
296 lowest_cost_owners = filter( | 317 lowest_cost_owners = filter( |
297 lambda owner: total_costs_by_owner[owner] == lowest_cost, | 318 lambda owner: total_costs_by_owner[owner] == lowest_cost, |
298 total_costs_by_owner) | 319 total_costs_by_owner) |
299 return random.Random().choice(lowest_cost_owners) | 320 return random.Random().choice(lowest_cost_owners) |
OLD | NEW |