presubmit: Skip owners checks if code-owners plugin is enabled.

If code-owners plugin is enable for the repo, skip owners check on
commit, and skip checking owners format, as that will be done by
the plugin.

Change-Id: I1663baef4f0f27b00423071343fe740f6da50ce7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2727131
Auto-Submit: Edward Lesmes <ehmaldonado@chromium.org>
Reviewed-by: Gavin Mak <gavinmak@google.com>
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>

gerrit_util.py

@@ -771,13 +771,20 @@ def SetCommitMessage(host, change, description, notify='ALL'):
         'in change %s' % change)
 
 
-def IsCodeOwnersEnabled(host):
+def IsCodeOwnersEnabledOnHost(host):
   """Check if the code-owners plugin is enabled for the host."""
   path = 'config/server/capabilities'
   capabilities = ReadHttpJsonResponse(CreateHttpConn(host, path))
   return 'code-owners-checkCodeOwner' in capabilities
 
 
+def IsCodeOwnersEnabledOnRepo(host, repo):
+  """Check if the code-owners plugin is enabled for the repo."""
+  path = '/projects/%s/code_owners.project_config' % repo
+  config = ReadHttpJsonResponse(CreateHttpConn(host, path))
+  return config['status'].get('disabled', False)
+
+
 def GetOwnersForFile(host, project, branch, path, limit=100,
                      resolve_all_users=True, o_params=('DETAILS',)):
   """Gets information about owners attached to a file."""

presubmit_canned_checks.py

@@ -1108,6 +1108,8 @@ def CheckOwnersDirMetadataExclusive(input_api, output_api):
 
 
 def CheckOwnersFormat(input_api, output_api):
+  if input_api.gerrit and input_api.gerrit.IsCodeOwnersEnabledOnRepo():
+    return []
   affected_files = set([
       f.LocalPath()
       for f in input_api.change.AffectedFiles()
@@ -1139,8 +1141,17 @@ def CheckOwners(
     if input_api.gerrit.IsOwnersOverrideApproved(input_api.change.issue):
       return []
 
-  # Ignore tbr if not allowed for this repo.
-  tbr = input_api.tbr and allow_tbr
+  code_owners_enabled = False
+  if input_api.gerrit and input_api.gerrit.IsCodeOwnersEnabledOnRepo():
+    code_owners_enabled = True
+
+  # Skip OWNERS check on commit if code-owners plugin is enabled, as owners
+  # enforcement is done by the plugin.
+  if input_api.is_committing and code_owners_enabled:
+    return []
+
+  # Ignore tbr if the code-owners plugin is enabled on this repo.
+  tbr = not code_owners_enabled and input_api.tbr
 
   affected_files = set([f.LocalPath() for f in
       input_api.change.AffectedFiles(file_filter=source_file_filter)])

presubmit_support.py

@@ -381,6 +381,7 @@ class GerritAccessor(object):
     self.project = project
     self.branch = branch
     self.cache = {}
+    self.code_owners_enabled = None
 
   def _FetchChangeDetail(self, issue):
     # Separate function to be easily mocked in tests.
@@ -466,6 +467,12 @@ class GerritAccessor(object):
   def UpdateDescription(self, description, issue):
     gerrit_util.SetCommitMessage(self.host, issue, description, notify='NONE')
 
+  def IsCodeOwnersEnabledOnRepo(self):
+    if self.code_owners_enabled is None:
+      self.code_owners_enabled = gerrit_util.IsCodeOwnersEnabledOnRepo(
+          self.host, self.repo)
+    return self.code_owners_enabled
+
 
 class OutputApi(object):
   """An instance of OutputApi gets passed to presubmit scripts so that they

tests/git_cl_test.py

@@ -1505,7 +1505,7 @@ class TestGitCl(unittest.TestCase):
     change_id = git_cl.GenerateGerritChangeId('line1\nline2\n')
     self.assertEqual(change_id, 'Ihashchange')
 
-  @mock.patch('gerrit_util.IsCodeOwnersEnabled')
+  @mock.patch('gerrit_util.IsCodeOwnersEnabledOnHost')
   @mock.patch('git_cl.Settings.GetBugPrefix')
   @mock.patch('git_cl.Changelist.FetchDescription')
   @mock.patch('git_cl.Changelist.GetBranch')
@@ -1517,7 +1517,7 @@ class TestGitCl(unittest.TestCase):
       self, mockBatchListOwners=None, mockGetRemoteBranch=None,
       mockGetGerritProject=None, mockGetGerritHost=None, mockGetBranch=None,
       mockFetchDescription=None, mockGetBugPrefix=None,
-      mockIsCodeOwnersEnabled=None,
+      mockIsCodeOwnersEnabledOnHost=None,
       initial_description='desc', bug=None, fixed=None, branch='branch',
       reviewers=None, tbrs=None, add_owners_to=None,
       expected_description='desc'):
@@ -1528,7 +1528,7 @@ class TestGitCl(unittest.TestCase):
       'b': ['b@example.com'],
       'c': ['c@example.com'],
     }
-    mockIsCodeOwnersEnabled.return_value = True
+    mockIsCodeOwnersEnabledOnHost.return_value = True
     mockGetBranch.return_value = branch
     mockGetBugPrefix.return_value = 'prefix'
     mockGetRemoteBranch.return_value = ('origin', 'refs/remotes/origin/main')
@@ -4126,7 +4126,8 @@ class CMDOwnersTestCase(CMDTestCaseBase):
     mock.patch(
         'owners_client.OwnersClient.BatchListOwners',
         return_value=self.owners_by_path).start()
-    mock.patch('gerrit_util.IsCodeOwnersEnabled', return_value=True).start()
+    mock.patch(
+        'gerrit_util.IsCodeOwnersEnabledOnHost', return_value=True).start()
     self.addCleanup(mock.patch.stopall)
 
   def testShowAllNoArgs(self):

tests/owners_client_test.py

@@ -319,7 +319,7 @@ class OwnersClientTest(unittest.TestCase):
 
 class GetCodeOwnersClientTest(unittest.TestCase):
   def setUp(self):
-    mock.patch('gerrit_util.IsCodeOwnersEnabled').start()
+    mock.patch('gerrit_util.IsCodeOwnersEnabledOnHost').start()
     self.addCleanup(mock.patch.stopall)
 
   def testGetCodeOwnersClient_GerritClient(self):
@@ -328,7 +328,7 @@ class GetCodeOwnersClientTest(unittest.TestCase):
     pass
 
   def testGetCodeOwnersClient_DepotToolsClient(self):
-    gerrit_util.IsCodeOwnersEnabled.return_value = False
+    gerrit_util.IsCodeOwnersEnabledOnHost.return_value = False
     self.assertIsInstance(
         owners_client.GetCodeOwnersClient('root', 'host', 'project', 'branch'),
         owners_client.DepotToolsClient)

tests/presubmit_unittest.py

@@ -2668,13 +2668,14 @@ the current line as well!
     self.assertEqual(1, len(results))
     self.assertIsInstance(results[0], presubmit.OutputApi.PresubmitError)
 
-  def GetInputApiWithOWNERS(self, owners_content):
+  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
 
@@ -2689,6 +2690,17 @@ the current line as well!
             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.
+    input_api = self.GetInputApiWithOWNERS(
+        'any content', code_owners_enabled=True)
+    self.assertEqual(
+        [],
+        presubmit_canned_checks.CheckOwnersFormat(
+            input_api, presubmit.OutputApi)
+    )
+
   def testCheckOwnersFormatFails(self):
     input_api = self.GetInputApiWithOWNERS('\n'.join([
         'set noparent',
@@ -2704,7 +2716,7 @@ the current line as well!
       self, tbr=False, issue='1', approvers=None, modified_files=None,
       owners_by_path=None, is_committing=True, response=None,
       expected_output='', manually_specified_reviewers=None, dry_run=None,
-      allow_tbr=True):
+      code_owners_enabled=False):
     # The set of people who lgtm'ed a change.
     approvers = approvers or set()
     manually_specified_reviewers = manually_specified_reviewers or []
@@ -2748,13 +2760,14 @@ the current line as well!
     input_api.tbr = tbr
     input_api.dry_run = dry_run
     input_api.gerrit._FetchChangeDetail = lambda _: response
+    input_api.gerrit.IsCodeOwnersEnabledOnRepo = lambda: code_owners_enabled
 
     input_api.owners_client = owners_client.DepotToolsClient('root', 'branch')
 
     with mock.patch('owners_client.DepotToolsClient.ListOwners',
                     side_effect=lambda f: owners_by_path.get(f, [])):
       results = presubmit_canned_checks.CheckOwners(
-          input_api, presubmit.OutputApi, allow_tbr=allow_tbr)
+          input_api, presubmit.OutputApi)
     for result in results:
       result.handle()
     if expected_output:
@@ -3067,12 +3080,11 @@ the current line as well!
   def testCannedCheckOwners_TBRIgnored(self):
     self.AssertOwnersWorks(
         tbr=True,
-        allow_tbr=False,
-        expected_output='Missing LGTM from someone other than '
-                        'john@example.com\n')
+        code_owners_enabled=True,
+        expected_output='')
     self.AssertOwnersWorks(
         tbr=True,
-        allow_tbr=False,
+        code_owners_enabled=True,
         is_committing=False,
         expected_output='')