diff --git a/git_cl.py b/git_cl.py index e6d2030105..3bf80cb19a 100755 --- a/git_cl.py +++ b/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) diff --git a/owners_client.py b/owners_client.py index 8b2d28530e..4d7dd59429 100644 --- a/owners_client.py +++ b/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) diff --git a/tests/git_cl_test.py b/tests/git_cl_test.py index be35fc7c49..a1515e8aff 100755 --- a/tests/git_cl_test.py +++ b/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: { diff --git a/tests/owners_client_test.py b/tests/owners_client_test.py index 5039ca735a..9e823436a8 100644 --- a/tests/owners_client_test.py +++ b/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)