Configure path to OWNERS.status file via toplevel OWNERS

BUG=694222
R=dpranke@chromium.org

Change-Id: Ie841332129dd6fa8555d2e940c919b812fc9408d
Reviewed-on: https://chromium-review.googlesource.com/467250
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Commit-Queue: Jochen Eisinger <jochen@chromium.org>

git_cl.py

@@ -3244,7 +3244,6 @@ class ChangeDescription(object):
         reviewers.append(name)
     if add_owners_tbr:
       owners_db = owners.Database(change.RepositoryRoot(),
-                                  change.GetOwnersStatusFile(),
                                   fopen=file, os_path=os.path)
       all_reviewers = set(tbr_names + reviewers)
       missing_files = owners_db.files_not_covered_by(change.LocalPaths(),
@@ -3464,9 +3463,6 @@ def LoadCodereviewSettingsFromFile(fileobj):
   SetProperty('run-post-upload-hook', 'RUN_POST_UPLOAD_HOOK',
               unset_error_ok=True)
 
-  if 'OWNERS_STATUS_FILE' in keyvals:
-    SetProperty('owners-status-file', 'OWNERS_STATUS_FILE', unset_error_ok=True)
-
   if 'GERRIT_HOST' in keyvals:
     RunGit(['config', 'gerrit.host', keyvals['GERRIT_HOST']])
 
@@ -5517,7 +5513,6 @@ def CMDowners(parser, args):
       [f.LocalPath() for f in
           cl.GetChange(base_branch, None).AffectedFiles()],
       change.RepositoryRoot(),
-      change.GetOwnersStatusFile(),
       author, fopen=file, os_path=os.path,
       disable_color=options.no_color).run()
 

owners.py

@@ -100,16 +100,14 @@ class Database(object):
   of changed files, and see if a list of changed files is covered by a
   list of reviewers."""
 
-  def __init__(self, root, status_file, fopen, os_path):
+  def __init__(self, root, fopen, os_path):
     """Args:
       root: the path to the root of the Repository
-      status_file: the path relative to root to global status entries or None
       open: function callback to open a text file for reading
       os_path: module/object callback with fields for 'abspath', 'dirname',
           'exists', 'join', and 'relpath'
     """
     self.root = root
-    self.status_file = status_file
     self.fopen = fopen
     self.os_path = os_path
 
@@ -140,6 +138,9 @@ class Database(object):
     # being included from another file.
     self._included_files = {}
 
+    # File with global status lines for owners.
+    self._status_file = None
+
   def reviewers_for(self, files, author):
     """Returns a suggested set of reviewers that will cover the files.
 
@@ -233,6 +234,8 @@ class Database(object):
 
     self.read_files.add(owners_path)
 
+    is_toplevel = path == 'OWNERS'
+
     comment = []
     dirpath = self.os_path.dirname(path)
     in_comment = False
@@ -241,6 +244,11 @@ class Database(object):
       lineno += 1
       line = line.strip()
       if line.startswith('#'):
+        if is_toplevel:
+          m = re.match('#\s*OWNERS_STATUS\s+=\s+(.+)$', line)
+          if m:
+            self._status_file = m.group(1).strip()
+            continue
         if not in_comment:
           comment = []
         comment.append(line[1:].strip())
@@ -276,12 +284,15 @@ class Database(object):
                       ' '.join(comment))
 
   def _read_global_comments(self):
-    if not self.status_file:
-      return
+    if not self._status_file:
+      if not 'OWNERS' in self.read_files:
+        self._read_owners('OWNERS')
+      if not self._status_file:
+        return
 
-    owners_status_path = self.os_path.join(self.root, self.status_file)
+    owners_status_path = self.os_path.join(self.root, self._status_file)
     if not self.os_path.exists(owners_status_path):
-      raise IOError('Could not find global status file "%s"' % 
+      raise IOError('Could not find global status file "%s"' %
                     owners_status_path)
 
     if owners_status_path in self.read_files:

owners_finder.py

@@ -22,7 +22,7 @@ class OwnersFinder(object):
 
   indentation = 0
 
-  def __init__(self, files, local_root, owners_status_file, author,
+  def __init__(self, files, local_root, author,
                fopen, os_path,
                email_postfix='@chromium.org',
                disable_color=False):
@@ -34,8 +34,7 @@ class OwnersFinder(object):
       self.COLOR_GREY = ''
       self.COLOR_RESET = ''
 
-    self.db = owners_module.Database(local_root, owners_status_file, fopen,
-                                     os_path)
+    self.db = owners_module.Database(local_root, fopen, os_path)
     self.db.load_data_needed_for(files)
 
     self.os_path = os_path
@@ -52,8 +51,7 @@ class OwnersFinder(object):
     if len(filtered_files) != len(files):
       files = filtered_files
       # Reload the database.
-      self.db = owners_module.Database(local_root, owners_status_file, fopen,
-                                       os_path)
+      self.db = owners_module.Database(local_root, fopen, os_path)
       self.db.load_data_needed_for(files)
 
     self.all_possible_owners = self.db.all_possible_owners(files, None)

presubmit_support.py

@@ -422,7 +422,6 @@ class InputApi(object):
     # TODO(dpranke): figure out a list of all approved owners for a repo
     # in order to be able to handle wildcard OWNERS files?
     self.owners_db = owners.Database(change.RepositoryRoot(),
-                                     change.GetOwnersStatusFile(),
                                      fopen=file, os_path=self.os_path)
     self.verbose = verbose
     self.Command = CommandData
@@ -918,10 +917,6 @@ class Change(object):
         x for x in self.AffectedFiles(include_deletes=False)
         if x.IsTestableFile())
 
-  def GetOwnersStatusFile(self):
-    """Returns the name of the global OWNERS status file."""
-    return None
-
 
 class GitChange(Change):
   _AFFECTED_FILES = GitAffectedFile
@@ -933,19 +928,6 @@ class GitChange(Change):
     return subprocess.check_output(
         ['git', 'ls-files', '--', '.'], cwd=root).splitlines()
 
-  def GetOwnersStatusFile(self):
-    """Returns the name of the global OWNERS status file."""
-
-    try:
-      status_file = subprocess.check_output(
-          ['git', 'config', 'rietveld.owners-status-file'],
-          cwd=self.RepositoryRoot())
-      return status_file
-    except subprocess.CalledProcessError:
-      pass
-
-    return None
-
 
 def ListRelevantPresubmitFiles(files, root):
   """Finds all presubmit files that apply to a given set of source files.

tests/git_cl_test.py

@@ -684,25 +684,6 @@ class TestGitCl(TestCase):
     ]
     self.assertIsNone(git_cl.LoadCodereviewSettingsFromFile(codereview_file))
 
-  def test_LoadCodereviewSettingsFromFile_owners_status(self):
-    codereview_file = StringIO.StringIO('OWNERS_STATUS_FILE: status')
-    self.calls = [
-      ((['git', 'config', '--unset-all', 'rietveld.server'],), ''),
-      ((['git', 'config', '--unset-all', 'rietveld.cc'],), CERR1),
-      ((['git', 'config', '--unset-all', 'rietveld.private'],), CERR1),
-      ((['git', 'config', '--unset-all', 'rietveld.tree-status-url'],), CERR1),
-      ((['git', 'config', '--unset-all', 'rietveld.viewvc-url'],), CERR1),
-      ((['git', 'config', '--unset-all', 'rietveld.bug-prefix'],), CERR1),
-      ((['git', 'config', '--unset-all', 'rietveld.cpplint-regex'],), CERR1),
-      ((['git', 'config', '--unset-all', 'rietveld.cpplint-ignore-regex'],),
-        CERR1),
-      ((['git', 'config', '--unset-all', 'rietveld.project'],), CERR1),
-      ((['git', 'config', '--unset-all', 'rietveld.run-post-upload-hook'],),
-        CERR1),
-      ((['git', 'config', 'rietveld.owners-status-file', 'status'],), ''),
-    ]
-    self.assertIsNone(git_cl.LoadCodereviewSettingsFromFile(codereview_file))
-
   @classmethod
   def _is_gerrit_calls(cls, gerrit=False):
     return [((['git', 'config', 'rietveld.autoupdate'],), ''),

tests/owners_finder_test.py

@@ -41,7 +41,8 @@ def owners_file(*email_addresses, **kwargs):
 def test_repo():
   return filesystem_mock.MockFileSystem(files={
     '/DEPS': '',
-    '/OWNERS': owners_file(ken, peter, tom),
+    '/OWNERS': owners_file(ken, peter, tom,
+                           comment='OWNERS_STATUS = build/OWNERS.status'),
     '/build/OWNERS.status': '%s: bar' % jochen,
     '/base/vlog.h': '',
     '/chrome/OWNERS': owners_file(ben, brett),
@@ -73,8 +74,7 @@ class OutputInterceptedOwnersFinder(owners_finder.OwnersFinder):
   def __init__(self, files, local_root,
                fopen, os_path, disable_color=False):
     super(OutputInterceptedOwnersFinder, self).__init__(
-      files, local_root, os_path.join('build', 'OWNERS.status'), None,
-      fopen, os_path, disable_color=disable_color)
+      files, local_root, None, fopen, os_path, disable_color=disable_color)
     self.output = []
     self.indentation_stack = []
 

tests/owners_unittest.py

@@ -74,11 +74,12 @@ class _BaseTestCase(unittest.TestCase):
     self.root = '/'
     self.fopen = self.repo.open_for_reading
 
-  def db(self, root=None, fopen=None, os_path=None, status_file=None):
+  def db(self, root=None, fopen=None, os_path=None):
     root = root or self.root
     fopen = fopen or self.fopen
     os_path = os_path or self.repo
-    return owners.Database(root, status_file, fopen, os_path)
+    # pylint: disable=no-value-for-parameter
+    return owners.Database(root, fopen, os_path)
 
 
 class OwnersDatabaseTest(_BaseTestCase):
@@ -335,8 +336,9 @@ class OwnersDatabaseTest(_BaseTestCase):
     self.assert_syntax_error('file:foo/bar/baz\n')
 
   def test_non_existant_status_file(self):
-    db = self.db(status_file='does_not_exist')
-    self.files['/foo/OWNERS'] = brett
+    db = self.db()
+    self.files['/OWNERS'] = owners_file(brett,
+                                        comment='OWNERS_STATUS = nonexistant')
     self.files['/foo/DEPS'] = ''
     self.assertRaises(IOError, db.reviewers_for, ['foo/DEPS'], None)
 

tests/presubmit_unittest.py

@@ -120,8 +120,6 @@ index fe3de7b..54ae6e1 100755
         self._root = obj.fake_root_dir
       def RepositoryRoot(self):
         return self._root
-      def GetOwnersStatusFile(self):
-        return None
 
     self.mox.StubOutWithMock(presubmit, 'random')
     self.mox.StubOutWithMock(presubmit, 'warn')
@@ -513,7 +511,6 @@ class PresubmitUnittest(PresubmitTestsBase):
         0,
         0,
         None)
-    change.GetOwnersStatusFile = lambda: None
     executer = presubmit.PresubmitExecuter(change, False, None, False)
     self.failIf(executer.ExecPresubmitScript('', fake_presubmit))
     # No error if no on-upload entry point
@@ -1068,7 +1065,6 @@ class InputApiUnittest(PresubmitTestsBase):
         0,
         0,
         None)
-    change.GetOwnersStatusFile = lambda: None
     input_api = presubmit.InputApi(
         change,
         presubmit.os.path.join(self.fake_root_dir, 'foo', 'PRESUBMIT.py'),
@@ -1197,7 +1193,6 @@ class InputApiUnittest(PresubmitTestsBase):
 
     change = presubmit.GitChange(
         'mychange', '', self.fake_root_dir, files, 0, 0, None)
-    change.GetOwnersStatusFile = lambda: None
     input_api = presubmit.InputApi(
         change,
         presubmit.os.path.join(self.fake_root_dir, 'PRESUBMIT.py'),
@@ -1218,7 +1213,6 @@ class InputApiUnittest(PresubmitTestsBase):
 
     change = presubmit.GitChange(
         'mychange', '', self.fake_root_dir, files, 0, 0, None)
-    change.GetOwnersStatusFile = lambda: None
     input_api = presubmit.InputApi(
         change, './PRESUBMIT.py', False, None, False)
     # Sample usage of overiding the default white and black lists.
@@ -1542,7 +1536,7 @@ class ChangeUnittest(PresubmitTestsBase):
         'AbsoluteLocalPaths', 'AffectedFiles', 'AffectedTestableFiles',
         'AffectedTextFiles',
         'AllFiles', 'DescriptionText', 'FullDescriptionText',
-        'GetOwnersStatusFile', 'LocalPaths', 'Name', 'RepositoryRoot',
+        'LocalPaths', 'Name', 'RepositoryRoot',
         'RightHandSideLines', 'SetDescriptionText', 'TAG_LINE_RE',
         'author_email', 'issue', 'patchset', 'scm', 'tags',
     ]