Add upstream parameter when building an owners client.

code-owners plugins wants a git ref in Gerrit (e.g. refs/heads/master)
while Depot Tools wants a local git ref (e.g. refs/remotes/origin/main,
or a git hash).

Add an upstream parameters to be used by Depot Tools, separate from
a Gerrit ref expected by owners_client.

Change-Id: Ieed97a186e3140b3f82830efa189dbe3e4d8c806
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2730049
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
Commit-Queue: Josip Sokcevic <sokcevic@google.com>
Auto-Submit: Edward Lesmes <ehmaldonado@chromium.org>
Reviewed-by: Josip Sokcevic <sokcevic@google.com>

git_cl.py

@@ -993,6 +993,7 @@ class Changelist(object):
       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

@@ -176,12 +176,9 @@ class DepotToolsClient(OwnersClient):
     self._db.override_files = self._GetOriginalOwnersFiles()
 
   def _GetOriginalOwnersFiles(self):
-    remote, _ = scm.GIT.FetchUpstreamTuple(self._root)
-    branch = ''.join(scm.GIT.RefToRemoteRef(self._branch, remote))
-
     return {
-      f: scm.GIT.GetOldContents(self._root, f, branch).splitlines()
-      for _, f in scm.GIT.CaptureStatus(self._root, branch)
+      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'
     }
 
@@ -232,11 +229,11 @@ class GerritClient(OwnersClient):
     return self._owners_cache[path]
 
 
-def GetCodeOwnersClient(root, host, project, branch):
+def GetCodeOwnersClient(root, upstream, host, project, branch):
   """Get a new OwnersClient.
 
   Defaults to GerritClient, and falls back to DepotToolsClient if code-owners
   plugin is not available."""
   # TODO(crbug.com/1183447): Use code-owners plugin if available on host once
   # code-owners plugin issues have been fixed.
-  return DepotToolsClient(root, branch)
+  return DepotToolsClient(root, upstream)

tests/git_cl_test.py

@@ -1509,13 +1509,15 @@ class TestGitCl(unittest.TestCase):
   @mock.patch('git_cl.Settings.GetBugPrefix')
   @mock.patch('git_cl.Changelist.FetchDescription')
   @mock.patch('git_cl.Changelist.GetBranch')
+  @mock.patch('git_cl.Changelist.GetCommonAncestorWithUpstream')
   @mock.patch('git_cl.Changelist.GetGerritHost')
   @mock.patch('git_cl.Changelist.GetGerritProject')
   @mock.patch('git_cl.Changelist.GetRemoteBranch')
   @mock.patch('owners_client.OwnersClient.BatchListOwners')
   def getDescriptionForUploadTest(
       self, mockBatchListOwners=None, mockGetRemoteBranch=None,
-      mockGetGerritProject=None, mockGetGerritHost=None, mockGetBranch=None,
+      mockGetGerritProject=None, mockGetGerritHost=None,
+      mockGetCommonAncestorWithUpstream=None, mockGetBranch=None,
       mockFetchDescription=None, mockGetBugPrefix=None,
       mockIsCodeOwnersEnabled=None,
       initial_description='desc', bug=None, fixed=None, branch='branch',
@@ -1531,6 +1533,7 @@ class TestGitCl(unittest.TestCase):
     mockIsCodeOwnersEnabled.return_value = True
     mockGetBranch.return_value = branch
     mockGetBugPrefix.return_value = 'prefix'
+    mockGetCommonAncestorWithUpstream.return_value = 'upstream'
     mockGetRemoteBranch.return_value = ('origin', 'refs/remotes/origin/main')
     mockFetchDescription.return_value = 'desc'
     mockBatchListOwners.side_effect = lambda ps: {

tests/owners_client_test.py

@@ -44,14 +44,11 @@ class DepotToolsClientTest(unittest.TestCase):
     self.fopen = self.repo.open_for_reading
     self.addCleanup(mock.patch.stopall)
     self.client = owners_client.DepotToolsClient(
-        '/', 'refs/heads/main', self.fopen, self.repo)
+        '/', 'branch', self.fopen, self.repo)
 
   @mock.patch('scm.GIT.CaptureStatus')
   @mock.patch('scm.GIT.GetOldContents')
-  @mock.patch('scm.GIT.FetchUpstreamTuple')
-  def testListOwners(
-      self, mockFetchUpstreamTuple, mockGetOldContents, mockCaptureStatus):
-    mockFetchUpstreamTuple.return_value = ('remote', 'refs/heads/blah')
+  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'),
@@ -61,8 +58,7 @@ class DepotToolsClientTest(unittest.TestCase):
     self.assertEqual(
         ['*', 'missing@example.com'],
         self.client.ListOwners('bar/everyone/foo.txt'))
-    mockCaptureStatus.assert_called_once_with(
-        '/', 'refs/remotes/remote/main')
+    mockCaptureStatus.assert_called_once_with('/', 'branch')
 
 
 class GerritClientTest(unittest.TestCase):
@@ -330,7 +326,7 @@ class GetCodeOwnersClientTest(unittest.TestCase):
   def testGetCodeOwnersClient_DepotToolsClient(self):
     gerrit_util.IsCodeOwnersEnabled.return_value = False
     self.assertIsInstance(
-        owners_client.GetCodeOwnersClient('root', 'host', 'project', 'branch'),
+        owners_client.GetCodeOwnersClient('root', 'branch', '', '', ''),
         owners_client.DepotToolsClient)