From a185e2e3c83d805d3306a3f1fe9258ae2e75f231 Mon Sep 17 00:00:00 2001 From: Andrii Shyshkalov Date: Wed, 21 Nov 2018 00:42:55 +0000 Subject: [PATCH] git cl: remove Rietveld implementation entirely. R=ajp, ehmaldonado Change-Id: I59859048d64902669479e666c6c3d37c1fb1394c Reviewed-on: https://chromium-review.googlesource.com/c/1344642 Auto-Submit: Andrii Shyshkalov Reviewed-by: Edward Lesmes Commit-Queue: Andrii Shyshkalov --- git_cl.py | 121 ------------------------------------------- tests/git_cl_test.py | 64 ++++++----------------- 2 files changed, 16 insertions(+), 169 deletions(-) diff --git a/git_cl.py b/git_cl.py index 8b957eb77..06a7c1ef0 100755 --- a/git_cl.py +++ b/git_cl.py @@ -1916,126 +1916,6 @@ class _ChangelistCodereviewBase(object): raise NotImplementedError() -class _RietveldChangelistImpl(_ChangelistCodereviewBase): - - def __init__(self, changelist, auth_config=None, codereview_host=None): - super(_RietveldChangelistImpl, self).__init__(changelist) - assert settings, 'must be initialized in _ChangelistCodereviewBase' - if not codereview_host: - settings.GetDefaultServerUrl() - - self._rietveld_server = codereview_host - self._auth_config = auth_config or auth.make_auth_config() - self._props = None - self._rpc_server = None - - def GetCodereviewServer(self): - if not self._rietveld_server: - # If we're on a branch then get the server potentially associated - # with that branch. - if self.GetIssue(): - self._rietveld_server = gclient_utils.UpgradeToHttps( - self._GitGetBranchConfigValue(self.CodereviewServerConfigKey())) - if not self._rietveld_server: - self._rietveld_server = settings.GetDefaultServerUrl() - return self._rietveld_server - - def EnsureAuthenticated(self, force, refresh=False): - # No checks for Rietveld because we are deprecating Rietveld. - pass - - def EnsureCanUploadPatchset(self, force): - # No checks for Rietveld because we are deprecating Rietveld. - pass - - def FetchDescription(self, force=False): - raise NotImplementedError() - - def GetMostRecentPatchset(self): - raise NotImplementedError() - - def GetIssueProperties(self): - raise NotImplementedError() - - def CannotTriggerTryJobReason(self): - raise NotImplementedError() - - def GetTryJobProperties(self, patchset=None): - raise NotImplementedError() - - def GetIssueOwner(self): - raise NotImplementedError() - - def GetReviewers(self): - raise NotImplementedError() - - def AddComment(self, message, publish=None): - raise NotImplementedError() - - def GetCommentsSummary(self, readable=True): - raise NotImplementedError() - - def GetStatus(self): - print( - 'WARNING! Rietveld is no longer supported.\n' - '\n' - 'If you have old branches in your checkout, please archive/delete them.\n' - ' $ git cl archive --help\n' - '\n' - 'See also PSA https://groups.google.com/a/chromium.org/' - 'forum/#!topic/infra-dev/2DIVzM2wseo\n') - return 'rietveld-not-supported' - - def UpdateDescriptionRemote(self, description, force=False): - raise NotImplementedError() - - def CloseIssue(self): - raise NotImplementedError() - - def SetFlag(self, flag, value): - return self.SetFlags({flag: value}) - - def SetFlags(self, flags): - """Sets flags on this CL/patchset in Rietveld. - """ - raise NotImplementedError - - def RpcServer(self): - """Returns an upload.RpcServer() to access this review's rietveld instance. - """ - raise NotImplementedError - - @classmethod - def IssueConfigKey(cls): - return 'rietveldissue' - - @classmethod - def PatchsetConfigKey(cls): - return 'rietveldpatchset' - - @classmethod - def CodereviewServerConfigKey(cls): - return 'rietveldserver' - - def SetLabels(self, enable_auto_submit, use_commit_queue, cq_dry_run): - raise NotImplementedError - - def SetCQState(self, new_state): - raise NotImplementedError - - def CMDPatchWithParsedIssue(self, parsed_issue_arg, reject, nocommit, - directory, force): - raise NotImplementedError - - @staticmethod - def ParseIssueURL(parsed_url): - raise NotImplementedError - - def CMDUploadChange(self, options, args, custom_cl_base, change): - """Upload the patch to Rietveld.""" - raise NotImplementedError - - class _GerritChangelistImpl(_ChangelistCodereviewBase): def __init__(self, changelist, auth_config=None, codereview_host=None): # auth_config is Rietveld thing, kept here to preserve interface only. @@ -3067,7 +2947,6 @@ class _GerritChangelistImpl(_ChangelistCodereviewBase): _CODEREVIEW_IMPLEMENTATIONS = { - 'rietveld': _RietveldChangelistImpl, 'gerrit': _GerritChangelistImpl, } diff --git a/tests/git_cl_test.py b/tests/git_cl_test.py index 13e279c2d..90a31534a 100755 --- a/tests/git_cl_test.py +++ b/tests/git_cl_test.py @@ -824,7 +824,6 @@ class TestGitCl(TestCase): calls = cls._is_gerrit_calls(True) calls += [ ((['git', 'symbolic-ref', 'HEAD'],), 'master'), - ((['git', 'config', 'branch.master.rietveldissue'],), CERR1), ((['git', 'config', 'branch.master.gerritissue'],), CERR1 if issue is None else str(issue)), ] @@ -1615,10 +1614,6 @@ class TestGitCl(TestCase): actual_codereview=None, codereview_in_url=False): self.mock(git_cl.sys, 'stdout', StringIO.StringIO()) - self.mock(git_cl._RietveldChangelistImpl, 'GetMostRecentPatchset', - lambda x: '60001') - self.mock(git_cl._RietveldChangelistImpl, 'FetchDescription', - lambda *a, **kw: 'Description') self.mock(git_cl, 'IsGitVersionAtLeast', lambda *args: True) if new_branch: @@ -1634,7 +1629,6 @@ class TestGitCl(TestCase): # These calls detect codereview to use. self.calls += [ ((['git', 'symbolic-ref', 'HEAD'],), 'master'), - ((['git', 'config', 'branch.master.rietveldissue'],), CERR1), ((['git', 'config', 'branch.master.gerritissue'],), CERR1), ((['git', 'config', 'rietveld.autoupdate'],), CERR1), ((['git', 'config', 'gerrit.host'],), 'true'), @@ -1787,7 +1781,6 @@ class TestGitCl(TestCase): self.calls = [ ((['git', 'symbolic-ref', 'HEAD'],), 'master'), - ((['git', 'config', 'branch.master.rietveldissue'],), CERR1), ((['git', 'config', 'branch.master.gerritissue'],), CERR1), ((['git', 'config', 'rietveld.autoupdate'],), CERR1), ((['git', 'config', 'gerrit.host'],), 'true'), @@ -1805,10 +1798,6 @@ class TestGitCl(TestCase): def _checkout_calls(self): return [ - ((['git', 'config', '--local', '--get-regexp', - 'branch\\..*\\.rietveldissue'], ), - ('branch.retrying.rietveldissue 1111111111\n' - 'branch.some-fix.rietveldissue 2222222222\n')), ((['git', 'config', '--local', '--get-regexp', 'branch\\..*\\.gerritissue'], ), ('branch.ger-branch.gerritissue 123456\n' @@ -1821,12 +1810,6 @@ class TestGitCl(TestCase): self.calls += [((['git', 'checkout', 'ger-branch'], ), '')] self.assertEqual(0, git_cl.main(['checkout', '123456'])) - def test_checkout_rietveld(self): - """Tests git cl checkout .""" - self.calls = self._checkout_calls() - self.calls += [((['git', 'checkout', 'some-fix'], ), '')] - self.assertEqual(0, git_cl.main(['checkout', '2222222222'])) - def test_checkout_not_found(self): """Tests git cl checkout .""" self.mock(git_cl.sys, 'stdout', StringIO.StringIO()) @@ -1837,8 +1820,6 @@ class TestGitCl(TestCase): """Tests git cl checkout .""" self.mock(git_cl.sys, 'stdout', StringIO.StringIO()) self.calls = [ - ((['git', 'config', '--local', '--get-regexp', - 'branch\\..*\\.rietveldissue'], ), CERR1), ((['git', 'config', '--local', '--get-regexp', 'branch\\..*\\.gerritissue'], ), CERR1), ] @@ -1916,7 +1897,6 @@ class TestGitCl(TestCase): self.calls = [ ((['git', 'symbolic-ref', 'HEAD'],), 'feature'), - ((['git', 'config', 'branch.feature.rietveldissue'],), CERR1), ((['git', 'config', 'branch.feature.gerritissue'],), '123'), ((['git', 'config', 'branch.feature.gerritserver'],), 'https://chromium-review.googlesource.com'), @@ -2078,11 +2058,10 @@ class TestGitCl(TestCase): self.calls = \ [((['git', 'for-each-ref', '--format=%(refname)', 'refs/heads'],), 'refs/heads/master\nrefs/heads/foo\nrefs/heads/bar'), - ((['git', 'config', 'branch.master.rietveldissue'],), '1'), + ((['git', 'config', 'branch.master.gerritissue'],), '456'), + ((['git', 'config', 'branch.foo.gerritissue'],), CERR1), ((['git', 'config', 'rietveld.autoupdate'],), CERR1), - ((['git', 'config', 'rietveld.server'],), 'codereview.example.com'), - ((['git', 'config', 'branch.foo.rietveldissue'],), '456'), - ((['git', 'config', 'branch.bar.rietveldissue'],), CERR1), + ((['git', 'config', 'gerrit.host'],), 'true'), ((['git', 'config', 'branch.bar.gerritissue'],), '789'), ((['git', 'symbolic-ref', 'HEAD'],), 'master'), ((['git', 'tag', 'git-cl-archived-456-foo', 'foo'],), ''), @@ -2101,9 +2080,7 @@ class TestGitCl(TestCase): self.calls = \ [((['git', 'for-each-ref', '--format=%(refname)', 'refs/heads'],), 'refs/heads/master'), - ((['git', 'config', 'branch.master.rietveldissue'],), '1'), - ((['git', 'config', 'rietveld.autoupdate'],), CERR1), - ((['git', 'config', 'rietveld.server'],), 'codereview.example.com'), + ((['git', 'config', 'branch.master.gerritissue'],), '1'), ((['git', 'symbolic-ref', 'HEAD'],), 'master')] self.mock(git_cl, 'get_cl_statuses', @@ -2118,11 +2095,10 @@ class TestGitCl(TestCase): self.calls = \ [((['git', 'for-each-ref', '--format=%(refname)', 'refs/heads'],), 'refs/heads/master\nrefs/heads/foo\nrefs/heads/bar'), - ((['git', 'config', 'branch.master.rietveldissue'],), '1'), + ((['git', 'config', 'branch.master.gerritissue'],), '456'), + ((['git', 'config', 'branch.foo.gerritissue'],), CERR1), ((['git', 'config', 'rietveld.autoupdate'],), CERR1), - ((['git', 'config', 'rietveld.server'],), 'codereview.example.com'), - ((['git', 'config', 'branch.foo.rietveldissue'],), '456'), - ((['git', 'config', 'branch.bar.rietveldissue'],), CERR1), + ((['git', 'config', 'gerrit.host'],), 'true'), ((['git', 'config', 'branch.bar.gerritissue'],), '789'), ((['git', 'symbolic-ref', 'HEAD'],), 'master'),] @@ -2140,12 +2116,11 @@ class TestGitCl(TestCase): self.calls = \ [((['git', 'for-each-ref', '--format=%(refname)', 'refs/heads'],), 'refs/heads/master\nrefs/heads/foo\nrefs/heads/bar'), - ((['git', 'config', 'branch.master.rietveldissue'],), '1'), + ((['git', 'config', 'branch.master.gerritissue'],), '1'), + ((['git', 'config', 'branch.foo.gerritissue'],), '456'), + ((['git', 'config', 'branch.bar.gerritissue'],), CERR1), ((['git', 'config', 'rietveld.autoupdate'],), CERR1), - ((['git', 'config', 'rietveld.server'],), 'codereview.example.com'), - ((['git', 'config', 'branch.foo.rietveldissue'],), '456'), - ((['git', 'config', 'branch.bar.rietveldissue'],), CERR1), - ((['git', 'config', 'branch.bar.gerritissue'],), '789'), + ((['git', 'config', 'gerrit.host'],), 'true'), ((['git', 'symbolic-ref', 'HEAD'],), 'master'), ((['git', 'branch', '-D', 'foo'],), '')] @@ -2162,7 +2137,6 @@ class TestGitCl(TestCase): self.mock(git_cl.sys, 'stdout', out) self.calls = [ ((['git', 'symbolic-ref', 'HEAD'],), 'feature'), - ((['git', 'config', 'branch.feature.rietveldissue'],), CERR1), ((['git', 'config', 'branch.feature.gerritissue'],), '123'), # Let this command raise exception (retcode=1) - it should be ignored. ((['git', 'config', '--unset', 'branch.feature.last-upload-hash'],), @@ -2183,7 +2157,6 @@ class TestGitCl(TestCase): lambda _: 'This is a description\n\nChange-Id: Ideadbeef') self.calls = [ ((['git', 'symbolic-ref', 'HEAD'],), 'feature'), - ((['git', 'config', 'branch.feature.rietveldissue'],), CERR1), ((['git', 'config', 'branch.feature.gerritissue'],), '123'), # Let this command raise exception (retcode=1) - it should be ignored. ((['git', 'config', '--unset', 'branch.feature.last-upload-hash'],), @@ -2204,13 +2177,12 @@ class TestGitCl(TestCase): self.mock(git_cl.sys, 'stdout', out) self.calls = [ ((['git', 'symbolic-ref', 'HEAD'],), 'feature'), - ((['git', 'config', 'branch.feature.rietveldissue'],), '123'), - ((['git', 'config', 'rietveld.autoupdate'],), ''), - ((['git', 'config', 'rietveld.server'],), - 'https://codereview.chromium.org'), - ((['git', 'config', 'branch.feature.rietveldserver'],), ''), + ((['git', 'config', 'branch.feature.gerritissue'],), '123'), + ((['git', 'config', 'branch.feature.gerritserver'],), + 'https://chromium-review.googlesource.com'), (('write_json', 'output.json', - {'issue': 123, 'issue_url': 'https://codereview.chromium.org/123'}), + {'issue': 123, + 'issue_url': 'https://chromium-review.googlesource.com/123'}), ''), ] self.assertEqual(0, git_cl.main(['issue', '--json', 'output.json'])) @@ -2227,7 +2199,6 @@ class TestGitCl(TestCase): self.calls = [ ((['git', 'symbolic-ref', 'HEAD'],), 'feature'), - ((['git', 'config', 'branch.feature.rietveldissue'],), CERR1), ((['git', 'config', 'branch.feature.gerritissue'],), '123456'), ((['git', 'config', 'branch.feature.gerritserver'],), 'https://chromium-review.googlesource.com'), @@ -2279,7 +2250,6 @@ class TestGitCl(TestCase): self.calls = [ ((['git', 'symbolic-ref', 'HEAD'],), 'feature'), - ((['git', 'config', 'branch.feature.rietveldissue'],), CERR1), ((['git', 'config', 'branch.feature.gerritissue'],), '123456'), ((['git', 'config', 'branch.feature.gerritserver'],), 'https://chromium-review.googlesource.com'), @@ -2367,7 +2337,6 @@ class TestGitCl(TestCase): self.calls = [ ((['git', 'symbolic-ref', 'HEAD'],), 'feature'), - ((['git', 'config', 'branch.feature.rietveldissue'],), CERR1), ((['git', 'config', 'branch.feature.gerritissue'],), '123456'), ((['git', 'config', 'branch.feature.gerritserver'],), 'https://chromium-review.googlesource.com'), @@ -2602,7 +2571,6 @@ class TestGitCl(TestCase): self._setup_fetch_try_jobs(most_recent_patchset=13) self.calls += [ ((['git', 'symbolic-ref', 'HEAD'],), 'feature'), - ((['git', 'config', 'branch.feature.rietveldissue'],), CERR1), ((['git', 'config', 'branch.feature.gerritissue'],), '1'), # TODO(tandrii): Uncomment the below if we decide to support checking # patchsets for Gerrit.