| 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 |