git cl: Rework Changelist class for Rietveld/Gerrit use.
This adds pluggable codereview-specific implementations into
Changelist class. The specific implementation is chosen at
Changelist automatically, with Rietveld being default for
backwards compatibility.
Gerrit implementation for Gerrit is incomplete, and will be
added in later CLs. However, it is sufficient to ensure
current functionality of this tool is not diminished.
Sadly, the base class isn't completely free from Rietveld
assumptions because of presubmit_support. Apparently, PRESUBMIT
scripts can make use of Rietveld instance for RPCs directly.
This use doesn't make sense for Gerrit, which substitutes
rietveld instance with a dummy object, which raises exception
on any attribute access with a diagnostic message.
This also includes refactoring of some related code which
(ab)used ChangeList. Overall, this CL adds a few extra call to
git config in order to determine which codereview to use, but
but it shouldn't have any performance impact.
BUG=579160
Review URL: https://codereview.chromium.org/1805193002
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@299462 0039d316-1c4b-4281-b951-d872f2087c98
diff --git a/tests/git_cl_test.py b/tests/git_cl_test.py
index 7da5aae..5c116e6 100755
--- a/tests/git_cl_test.py
+++ b/tests/git_cl_test.py
@@ -163,20 +163,21 @@
'-M'+similarity, 'fake_ancestor_sha', 'HEAD'],), '+dat')
return [
- ((['git', 'config', 'rietveld.autoupdate'],), ''),
- ((['git', 'config', 'rietveld.server'],),
- 'codereview.example.com'),
((['git', 'symbolic-ref', 'HEAD'],), 'master'),
similarity_call,
((['git', 'symbolic-ref', 'HEAD'],), 'master'),
find_copies_call,
+ ((['git', 'config', 'rietveld.autoupdate'],), ''),
+ ((['git', 'config', 'rietveld.server'],),
+ 'codereview.example.com'),
((['git', 'symbolic-ref', 'HEAD'],), 'master'),
+ ((['git', 'config', 'branch.master.rietveldissue'],), ''),
+ ((['git', 'config', 'branch.master.gerritissue'],), ''),
+ ((['git', 'config', 'gerrit.host'],), ''),
((['git', 'config', 'branch.master.merge'],), 'master'),
((['git', 'config', 'branch.master.remote'],), 'origin'),
((['get_or_create_merge_base', 'master', 'master'],),
'fake_ancestor_sha'),
- ((['git', 'config', 'gerrit.host'],), ''),
- ((['git', 'config', 'branch.master.rietveldissue'],), ''),
] + cls._git_sanity_checks('fake_ancestor_sha', 'master') + [
((['git', 'rev-parse', '--show-cdup'],), ''),
((['git', 'rev-parse', 'HEAD'],), '12345'),
@@ -279,16 +280,18 @@
'svn-remote.svn.fetch trunk/src:refs/remotes/origin/master'),
None),
0)),
- ((['git',
- 'config', 'rietveld.server'],), 'codereview.example.com'),
((['git', 'symbolic-ref', 'HEAD'],), 'refs/heads/working'),
((['git', 'config', '--int', '--get',
'branch.working.git-cl-similarity'],), ''),
((['git', 'symbolic-ref', 'HEAD'],), 'refs/heads/working'),
((['git', 'config', '--int', '--get',
'branch.working.git-find-copies'],), ''),
+ ((['git',
+ 'config', 'rietveld.server'],), 'codereview.example.com'),
((['git', 'symbolic-ref', 'HEAD'],), 'refs/heads/working'),
((['git',
+ 'config', 'branch.working.rietveldissue'],), '12345'),
+ ((['git',
'config', 'branch.working.merge'],), 'refs/heads/master'),
((['git', 'config', 'branch.working.remote'],), 'origin'),
((['git', 'config', 'branch.working.merge'],),
@@ -322,8 +325,6 @@
'.'],),
'M\tPRESUBMIT.py'),
((['git',
- 'config', 'branch.working.rietveldissue'],), '12345'),
- ((['git',
'config', 'branch.working.rietveldpatchset'],), '31137'),
((['git', 'config', 'branch.working.rietveldserver'],),
'codereview.example.com'),
@@ -334,8 +335,6 @@
@classmethod
def _dcommit_calls_bypassed(cls):
return [
- ((['git',
- 'config', 'branch.working.rietveldissue'],), '12345'),
((['git', 'config', 'branch.working.rietveldserver'],),
'codereview.example.com'),
]
@@ -343,7 +342,6 @@
@classmethod
def _dcommit_calls_3(cls):
return [
- ((['git', 'config', 'gerrit.host'],), ''),
((['git',
'diff', '--no-ext-diff', '--stat', '--find-copies-harder',
'-l100000', '-C50', 'fake_ancestor_sha',
@@ -546,22 +544,23 @@
@classmethod
def _gerrit_base_calls(cls):
return [
- ((['git', 'config', 'rietveld.autoupdate'],),
- ''),
- ((['git',
- 'config', 'rietveld.server'],), 'codereview.example.com'),
((['git', 'symbolic-ref', 'HEAD'],), 'master'),
((['git', 'config', '--int', '--get',
'branch.master.git-cl-similarity'],), ''),
((['git', 'symbolic-ref', 'HEAD'],), 'master'),
((['git', 'config', '--int', '--get',
'branch.master.git-find-copies'],), ''),
+ ((['git', 'config', 'rietveld.autoupdate'],), ''),
+ ((['git', 'config', 'rietveld.server'],), ''),
+ ((['git', 'config', 'rietveld.server'],), ''),
((['git', 'symbolic-ref', 'HEAD'],), 'master'),
+ ((['git', 'config', 'branch.master.rietveldissue'],), ''),
+ ((['git', 'config', 'branch.master.gerritissue'],), ''),
+ ((['git', 'config', 'gerrit.host'],), 'True'),
((['git', 'config', 'branch.master.merge'],), 'master'),
((['git', 'config', 'branch.master.remote'],), 'origin'),
((['get_or_create_merge_base', 'master', 'master'],),
'fake_ancestor_sha'),
- ((['git', 'config', 'gerrit.host'],), 'True'),
] + cls._git_sanity_checks('fake_ancestor_sha', 'master') + [
((['git', 'rev-parse', '--show-cdup'],), ''),
((['git', 'rev-parse', 'HEAD'],), '12345'),
@@ -569,9 +568,7 @@
'diff', '--name-status', '--no-renames', '-r',
'fake_ancestor_sha...', '.'],),
'M\t.gitignore\n'),
- ((['git', 'config', 'branch.master.rietveldissue'],), ''),
- ((['git',
- 'config', 'branch.master.rietveldpatchset'],), ''),
+ ((['git', 'config', 'branch.master.gerritpatchset'],), ''),
((['git',
'log', '--pretty=format:%s%n%n%b', 'fake_ancestor_sha...'],),
'foo'),
@@ -662,7 +659,12 @@
]
if squash:
calls += [
- ((['git', 'config', 'branch.master.rietveldissue', '123456'],), ''),
+ ((['git', 'config', 'branch.master.gerritissue', '123456'],), ''),
+ ((['git', 'config', 'branch.master.gerritserver'],), ''),
+ ((['git', 'config', 'remote.origin.url'],),
+ 'https://chromium.googlesource.com/my/repo.git'),
+ ((['git', 'config', 'branch.master.gerritserver',
+ 'https://chromium-review.googlesource.com'],), ''),
((['git', 'rev-parse', 'HEAD'],), 'abcdef0123456789'),
((['git', 'update-ref', '-m', 'Uploaded abcdef0123456789',
'refs/heads/git_cl_uploads/master', 'abcdef0123456789'],),
@@ -886,9 +888,12 @@
self.assertNotEqual(git_cl.main(['diff']), 0)
def _patch_common(self):
- self.mock(git_cl.Changelist, 'GetMostRecentPatchset', lambda x: '60001')
- self.mock(git_cl.Changelist, 'GetPatchSetDiff', lambda *args: None)
- self.mock(git_cl.Changelist, 'GetDescription', lambda *args: 'Description')
+ self.mock(git_cl._RietveldChangelistImpl, 'GetMostRecentPatchset',
+ lambda x: '60001')
+ self.mock(git_cl._RietveldChangelistImpl, 'GetPatchSetDiff',
+ lambda *args: None)
+ self.mock(git_cl.Changelist, 'GetDescription',
+ lambda *args: 'Description')
self.mock(git_cl.Changelist, 'SetIssue', lambda *args: None)
self.mock(git_cl.Changelist, 'SetPatchset', lambda *args: None)
self.mock(git_cl, 'IsGitVersionAtLeast', lambda *args: True)
@@ -908,6 +913,8 @@
'Description\n\n' +
'patch from issue 123456 at patchset 60001 ' +
'(http://crrev.com/123456#ps60001)'],), ''),
+ ((['git', 'symbolic-ref', 'HEAD'],), 'master'),
+ ((['git', 'config', 'branch.master.rietveldserver'],), ''),
]
self.assertEqual(git_cl.main(['patch', '123456']), 0)