Remove old DepotToolsOwners implementation

code-owners should have been enabled for most hosts that depot_tools
supports by now. Remove our own implementation and rely on code-owners.

Change-Id: Iaf0d3db65b2e5063b67d42b92188c4ec51d2cd9a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3783475Reviewed-by: Aravind Vasudevan <aravindvasudev@google.com>
Reviewed-by: Joanna Wang <jojwang@chromium.org>
Commit-Queue: Gavin Mak <gavinmak@google.com>

git_cl.py

@@ -1023,8 +1023,6 @@ class Changelist(object):
       remote, remote_branch = self.GetRemoteBranch()
       branch = GetTargetRef(remote, remote_branch, None)
       self._owners_client = owners_client.GetCodeOwnersClient(
-          root=settings.GetRoot(),
-          upstream=self.GetCommonAncestorWithUpstream(),
           host=self.GetGerritHost(),
           project=self.GetGerritProject(),
           branch=branch)

owners_client.py

@@ -2,15 +2,11 @@
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
-import itertools
 import os
 import random
-import threading
 
 import gerrit_util
 import git_common
-import owners as owners_db
-import scm
 
 
 class OwnersClient(object):
@@ -117,47 +113,6 @@ class OwnersClient(object):
 
     return selected
 
-
-class DepotToolsClient(OwnersClient):
-  """Implement OwnersClient using owners.py Database."""
-  def __init__(self, root, branch, fopen=open, os_path=os.path):
-    super(DepotToolsClient, self).__init__()
-
-    self._root = root
-    self._branch = branch
-    self._fopen = fopen
-    self._os_path = os_path
-    self._db = None
-    self._db_lock = threading.Lock()
-
-  def _ensure_db(self):
-    if self._db is not None:
-      return
-    self._db = owners_db.Database(self._root, self._fopen, self._os_path)
-    self._db.override_files = self._GetOriginalOwnersFiles()
-
-  def _GetOriginalOwnersFiles(self):
-    return {
-      f: scm.GIT.GetOldContents(self._root, f, self._branch).splitlines()
-      for _, f in scm.GIT.CaptureStatus(self._root, self._branch)
-      if os.path.basename(f) == 'OWNERS'
-    }
-
-  def ListOwners(self, path):
-    # all_possible_owners is not thread safe.
-    with self._db_lock:
-      self._ensure_db()
-      # all_possible_owners returns a dict {owner: [(path, distance)]}. We want
-      # to return a list of owners sorted by increasing distance.
-      distance_by_owner = self._db.all_possible_owners([path], None)
-      # We add a small random number to the distance, so that owners at the
-      # same distance are returned in random order to avoid overloading those
-      # who would appear first.
-      return sorted(
-          distance_by_owner,
-          key=lambda o: distance_by_owner[o][0][1] + random.random())
-
-
 class GerritClient(OwnersClient):
   """Implement OwnersClient using OWNERS REST API."""
   def __init__(self, host, project, branch):
@@ -196,11 +151,14 @@ class GerritClient(OwnersClient):
     return self._owners_cache[path]
 
 
-def GetCodeOwnersClient(root, upstream, host, project, branch):
+def GetCodeOwnersClient(host, project, branch):
   """Get a new OwnersClient.
 
-  Defaults to GerritClient, and falls back to DepotToolsClient if code-owners
-  plugin is not available."""
+  Uses GerritClient and raises an exception if code-owners plugin is not
+  available."""
   if gerrit_util.IsCodeOwnersEnabledOnHost(host):
     return GerritClient(host, project, branch)
-  return DepotToolsClient(root, upstream)
+  raise Exception(
+      'code-owners plugin is not enabled. Ask your host admin to enable it '
+      'on %s. Read more about code-owners at '
+      'https://gerrit.googlesource.com/plugins/code-owners.' % host)

presubmit_support.py

@@ -44,7 +44,6 @@ import gclient_paths  # Exposed through the API
 import gclient_utils
 import git_footers
 import gerrit_util
-import owners as owners_db
 import owners_client
 import owners_finder
 import presubmit_canned_checks
@@ -680,15 +679,11 @@ class InputApi(object):
     if self.gerrit and not 'PRESUBMIT_SKIP_NETWORK' in self.environ:
       try:
         self.owners_client = owners_client.GetCodeOwnersClient(
-            root=change.RepositoryRoot(),
-            upstream=change.UpstreamBranch(),
             host=self.gerrit.host,
             project=self.gerrit.project,
             branch=self.gerrit.branch)
       except Exception as e:
         print('Failed to set owners_client - %s' % str(e))
-    self.owners_db = owners_db.Database(
-        change.RepositoryRoot(), fopen=open, os_path=self.os_path)
     self.owners_finder = owners_finder.OwnersFinder
     self.verbose = verbose
     self.Command = CommandData

tests/owners_client_test.py

@@ -26,41 +26,6 @@ dave = 'dave@example.com'
 emily = 'emily@example.com'
 
 
-class DepotToolsClientTest(unittest.TestCase):
-  def setUp(self):
-    self.repo = filesystem_mock.MockFileSystem(files={
-        '/OWNERS': '\n'.join([
-            'per-file approved.cc=approver@example.com',
-            'per-file reviewed.h=reviewer@example.com',
-            'missing@example.com',
-        ]),
-        '/approved.cc': '',
-        '/reviewed.h': '',
-        '/bar/insufficient_reviewers.py': '',
-        '/bar/everyone/OWNERS': '*',
-        '/bar/everyone/foo.txt': '',
-    })
-    self.root = '/'
-    self.fopen = self.repo.open_for_reading
-    self.addCleanup(mock.patch.stopall)
-    self.client = owners_client.DepotToolsClient(
-        '/', 'branch', self.fopen, self.repo)
-
-  @mock.patch('scm.GIT.CaptureStatus')
-  @mock.patch('scm.GIT.GetOldContents')
-  def testListOwners(self, mockGetOldContents, mockCaptureStatus):
-    mockGetOldContents.side_effect = lambda r, f, _b: self.repo.files[r + f]
-    mockCaptureStatus.return_value = [
-        ('M', 'bar/everyone/foo.txt'),
-        ('M', 'OWNERS'),
-    ]
-
-    self.assertEqual(
-        ['*', 'missing@example.com'],
-        self.client.ListOwners('bar/everyone/foo.txt'))
-    mockCaptureStatus.assert_called_once_with('/', 'branch')
-
-
 class GerritClientTest(unittest.TestCase):
   def setUp(self):
     self.client = owners_client.GerritClient('host', 'project', 'branch')
@@ -295,17 +260,16 @@ class GetCodeOwnersClientTest(unittest.TestCase):
     mock.patch('gerrit_util.IsCodeOwnersEnabledOnHost').start()
     self.addCleanup(mock.patch.stopall)
 
-  def testGetCodeOwnersClient_GerritClient(self):
+  def testGetCodeOwnersClient_CodeOwnersEnabled(self):
     gerrit_util.IsCodeOwnersEnabledOnHost.return_value = True
     self.assertIsInstance(
-        owners_client.GetCodeOwnersClient('', '', 'host', 'project', 'branch'),
+        owners_client.GetCodeOwnersClient('host', 'project', 'branch'),
         owners_client.GerritClient)
 
-  def testGetCodeOwnersClient_DepotToolsClient(self):
+  def testGetCodeOwnersClient_CodeOwnersDisabled(self):
     gerrit_util.IsCodeOwnersEnabledOnHost.return_value = False
-    self.assertIsInstance(
-        owners_client.GetCodeOwnersClient('root', 'branch', '', '', ''),
-        owners_client.DepotToolsClient)
+    with self.assertRaises(Exception):
+      owners_client.GetCodeOwnersClient('', '', '')
 
 
 if __name__ == '__main__':

tests/presubmit_unittest.py

@@ -43,9 +43,7 @@ import gclient_utils
 import git_cl
 import git_common as git
 import json
-import owners
 import owners_client
-import owners_finder
 import presubmit_support as presubmit
 import rdb_wrapper
 import scm
@@ -2567,26 +2565,10 @@ the current line as well!
 
   def GetInputApiWithOWNERS(self, owners_content, code_owners_enabled=False):
     input_api = self.GetInputApiWithFiles({'OWNERS': ('M', owners_content)})
-
-    owners_file = StringIO(owners_content)
-    fopen = lambda *args: owners_file
-
-    input_api.owners_db = owners.Database('', fopen, os.path)
     input_api.gerrit.IsCodeOwnersEnabledOnRepo = lambda: code_owners_enabled
 
     return input_api
 
-  def testCheckOwnersFormatWorks(self):
-    input_api = self.GetInputApiWithOWNERS('\n'.join([
-        'set noparent',
-        'per-file lalala = lemur@chromium.org',
-    ]))
-    self.assertEqual(
-        [],
-        presubmit_canned_checks.CheckOwnersFormat(
-            input_api, presubmit.OutputApi)
-    )
-
   def testCheckOwnersFormatWorks_CodeOwners(self):
     # If code owners is enabled, we rely on it to check owners format instead of
     # depot tools.