Index: owners.py |
diff --git a/owners.py b/owners.py |
index 30646ffa24dd203754a08abf5bba6d60f0f938cf..d830f6793579a4f966a553dfa032431955b9cca7 100644 |
--- a/owners.py |
+++ b/owners.py |
@@ -18,6 +18,7 @@ line := directive |
| comment |
directive := "set noparent" |
+ | "file:" glob |
| email_address |
| "*" |
@@ -45,6 +46,10 @@ If "per-file glob=set noparent" is used, then global directives are ignored |
for the glob, and only the "per-file" owners are used for files matching that |
glob. |
+If the "file:" directive is used, the referred to OWNERS file will be parsed and |
+considered when determining the valid set of OWNERS. If the filename starts with |
+"//" it is considered to be an absolute path, otherwise a relative path. |
Dirk Pranke
2015/04/15 19:42:37
"absolute path" might be confusing. I might call t
Peter Beverloo
2015/04/15 20:04:31
Ah yes, thanks. I'll adopt your comment with s/top
Peter Beverloo
2015/04/16 09:54:05
Done.
|
+ |
Examples for all of these combinations can be found in tests/owners_unittest.py. |
""" |
@@ -52,6 +57,9 @@ import collections |
import random |
import re |
+# Name of the file in a directory containing the OWNERS contents. |
+OWNERS_FILE = 'OWNERS' |
+ |
# If this is present by itself on a line, this means that everyone can review. |
EVERYONE = '*' |
@@ -118,6 +126,9 @@ class Database(object): |
# (This is implicitly true for the root directory). |
self.stop_looking = set(['']) |
+ # Set of files which have already been read. |
+ self.read_files = set() |
+ |
def reviewers_for(self, files, author): |
"""Returns a suggested set of reviewers that will cover the files. |
@@ -189,15 +200,21 @@ class Database(object): |
for f in files: |
dirpath = self.os_path.dirname(f) |
while not dirpath in self.owners_for: |
- self._read_owners_in_dir(dirpath) |
+ self._read_owners_in_dir(dirpath, OWNERS_FILE) |
if self._stop_looking(dirpath): |
break |
dirpath = self.os_path.dirname(dirpath) |
- def _read_owners_in_dir(self, dirpath): |
- owners_path = self.os_path.join(self.root, dirpath, 'OWNERS') |
+ def _read_owners_in_dir(self, dirpath, owners_file): |
+ owners_path = self.os_path.join(self.root, dirpath, owners_file) |
if not self.os_path.exists(owners_path): |
return |
+ |
+ if owners_path in self.read_files: |
+ return |
Dirk Pranke
2015/04/15 19:42:37
Since read_files is a set, calling add() multiple
Peter Beverloo
2015/04/15 20:04:31
I may be misunderstanding you, but both the set an
Dirk Pranke
2015/04/15 20:52:03
Oh, I was misreading the code, and thought the fun
|
+ |
+ self.read_files.add(owners_path) |
+ |
comment = [] |
in_comment = False |
lineno = 0 |
@@ -244,6 +261,21 @@ class Database(object): |
line_type, owners_path, lineno, comment): |
if directive == 'set noparent': |
self.stop_looking.add(path) |
+ elif directive.startswith('file:'): |
+ dirpath, owners_file = self._resolve_include(directive[5:], owners_path) |
+ if not dirpath: |
+ raise SyntaxErrorInOwnersFile(owners_path, lineno, |
+ ('%s does not refer to an existing file.' % directive[5:])) |
+ |
+ self._read_owners_in_dir(dirpath, owners_file) |
+ for key in self.owned_by: |
+ if not dirpath in self.owned_by[key]: |
+ continue |
+ self.owned_by[key].add(path) |
+ |
+ if dirpath in self.owners_for: |
+ self.owners_for.setdefault(path, set()).update(self.owners_for[dirpath]) |
+ |
elif self.email_regexp.match(directive) or directive == EVERYONE: |
self.comments.setdefault(directive, {}) |
self.comments[directive][path] = comment |
@@ -251,9 +283,25 @@ class Database(object): |
self.owners_for.setdefault(path, set()).add(directive) |
else: |
raise SyntaxErrorInOwnersFile(owners_path, lineno, |
- ('%s is not a "set" directive, "*", ' |
+ ('%s is not a "set" directive, include, "*", ' |
'or an email address: "%s"' % (line_type, directive))) |
+ def _resolve_include(self, include_path, base_path): |
+ if include_path.startswith('//'): |
+ dirpath = self.os_path.dirname(include_path[2:]) |
+ else: |
+ assert base_path.startswith(self.root) |
+ base_path = self.os_path.dirname(base_path[len(self.root):]) |
+ dirpath = self.os_path.join(base_path, self.os_path.dirname(include_path)) |
+ |
+ owners_file = self.os_path.basename(include_path) |
+ |
+ owners_path = self.os_path.join(self.root, dirpath, owners_file) |
+ if not self.os_path.exists(owners_path): |
+ return None, None |
+ |
+ return dirpath, owners_path |
Dirk Pranke
2015/04/15 19:42:37
see comment above; it's not clear to me that retur
Peter Beverloo
2015/04/15 20:04:31
I named them to be consistent with the arguments o
Dirk Pranke
2015/04/15 20:52:03
I'm not fully sure I understood your comments, but
Peter Beverloo
2015/04/15 20:57:09
Yes, it does. Thanks! I'll apply the changes tomor
Peter Beverloo
2015/04/16 09:54:06
Done.
|
+ |
def _covering_set_of_owners_for(self, files, author): |
dirs_remaining = set(self._enclosing_dir_with_owners(f) for f in files) |
all_possible_owners = self.all_possible_owners(dirs_remaining, author) |