From dde6462520224897393e13d1a9f421166beac08e Mon Sep 17 00:00:00 2001 From: "tandrii@chromium.org" Date: Wed, 13 Apr 2016 17:11:21 +0000 Subject: [PATCH] git cl: add --gerrit and --rietveld options to force codereview. This makes it possible to override the codereview set in: * repository codereview.settings * cached/set in local .git/config, either repo-wide or current branch only. Examples: cd $SOME_RIETVELD_USING_REPO # Enable Gerrit codereview on it. Contact Infra-Git admin: # https://bugs.chromium.org/p/chromium/issues/entry?template=Infra-Git # Uploading git cl upload --gerrit --squash # Hack, hack, re-upload uses Gerrit automatically. git cl upload --squash # Patching git new-branch patched-in-issue git cl patch --gerrit XXXXXX # (Re-)setting issue git cl issue --gerrit 0 git cl issue --gerrit XXXXXX R=andybons@chromium.org,sergiyb@chromium.org BUG=598681 Review URL: https://codereview.chromium.org/1880243003 git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@299901 0039d316-1c4b-4281-b951-d872f2087c98 --- git_cl.py | 35 +++++++++++++++++++++++++++++++--- tests/git_cl_test.py | 45 ++++++++++++++++++++++++++++++++++---------- 2 files changed, 67 insertions(+), 13 deletions(-) diff --git a/git_cl.py b/git_cl.py index 34c96c058..9bc7d52e7 100755 --- a/git_cl.py +++ b/git_cl.py @@ -2507,6 +2507,29 @@ _CODEREVIEW_IMPLEMENTATIONS = { } +def _add_codereview_select_options(parser): + """Appends --gerrit and --rietveld options to force specific codereview.""" + parser.codereview_group = optparse.OptionGroup( + parser, 'EXPERIMENTAL! Codereview override options') + parser.add_option_group(parser.codereview_group) + parser.codereview_group.add_option( + '--gerrit', action='store_true', + help='Force the use of Gerrit for codereview') + parser.codereview_group.add_option( + '--rietveld', action='store_true', + help='Force the use of Rietveld for codereview') + + +def _process_codereview_select_options(parser, options): + if options.gerrit and options.rietveld: + parser.error('Options --gerrit and --rietveld are mutually exclusive') + options.forced_codereview = None + if options.gerrit: + options.forced_codereview = 'gerrit' + elif options.rietveld: + options.forced_codereview = 'rietveld' + + class ChangeDescription(object): """Contains a parsed form of the change description.""" R_LINE = r'^[ \t]*(TBR|R)[ \t]*=[ \t]*(.*?)[ \t]*$' @@ -3122,7 +3145,9 @@ def CMDissue(parser, args): help='Lookup the branch(es) for the specified issues. If ' 'no issues are specified, all branches with mapped ' 'issues will be listed.') + _add_codereview_select_options(parser) options, args = parser.parse_args(args) + _process_codereview_select_options(parser, options) if options.reverse: branches = RunGit(['for-each-ref', 'refs/heads', @@ -3141,7 +3166,7 @@ def CMDissue(parser, args): print 'Branch for issue number %s: %s' % ( issue, ', '.join(issue_branch_map.get(int(issue)) or ('None',))) else: - cl = Changelist() + cl = Changelist(codereview=options.forced_codereview) if len(args) > 0: try: issue = int(args[0]) @@ -3485,7 +3510,9 @@ def CMDupload(parser, args): orig_args = args add_git_similarity(parser) auth.add_auth_options(parser) + _add_codereview_select_options(parser) (options, args) = parser.parse_args(args) + _process_codereview_select_options(parser, options) auth_config = auth.extract_auth_config_from_options(options) if git_common.is_dirty_git_tree('upload'): @@ -3497,7 +3524,7 @@ def CMDupload(parser, args): # For sanity of test expectations, do this otherwise lazy-loading *now*. settings.GetIsGerrit() - cl = Changelist(auth_config=auth_config) + cl = Changelist(auth_config=auth_config, codereview=options.forced_codereview) return cl.CMDUpload(options, args, orig_args) @@ -3981,10 +4008,12 @@ def CMDpatch(parser, args): parser.add_option_group(group) auth.add_auth_options(parser) + _add_codereview_select_options(parser) (options, args) = parser.parse_args(args) + _process_codereview_select_options(parser, options) auth_config = auth.extract_auth_config_from_options(options) - cl = Changelist(auth_config=auth_config) + cl = Changelist(auth_config=auth_config, codereview=options.forced_codereview) issue_arg = None if options.reapply : diff --git a/tests/git_cl_test.py b/tests/git_cl_test.py index 8eacfbbfe..295d1a0fb 100755 --- a/tests/git_cl_test.py +++ b/tests/git_cl_test.py @@ -1093,7 +1093,7 @@ class TestGitCl(TestCase): self.mock(git_common, 'is_dirty_git_tree', lambda x: True) self.assertNotEqual(git_cl.main(['diff']), 0) - def _patch_common(self, is_gerrit=False): + def _patch_common(self, is_gerrit=False, force_codereview=False): self.mock(git_cl._RietveldChangelistImpl, 'GetMostRecentPatchset', lambda x: '60001') self.mock(git_cl._RietveldChangelistImpl, 'GetPatchSetDiff', @@ -1122,16 +1122,22 @@ class TestGitCl(TestCase): lambda *args: 'Description') self.mock(git_cl, 'IsGitVersionAtLeast', lambda *args: True) - self.calls = [ - ((['git', 'symbolic-ref', 'HEAD'],), 'master'), - ((['git', 'config', 'branch.master.rietveldissue'],), ''), - ((['git', 'config', 'branch.master.gerritissue'],), ''), - ((['git', 'config', 'rietveld.autoupdate'],), ''), - ] - if is_gerrit: - self.calls += [ - ((['git', 'config', 'gerrit.host'],), 'true'), + if not force_codereview: + # These calls detect codereview to use. + self.calls = [ + ((['git', 'symbolic-ref', 'HEAD'],), 'master'), + ((['git', 'config', 'branch.master.rietveldissue'],), ''), + ((['git', 'config', 'branch.master.gerritissue'],), ''), + ((['git', 'config', 'rietveld.autoupdate'],), ''), ] + else: + self.calls = [] + + if is_gerrit: + if not force_codereview: + self.calls += [ + ((['git', 'config', 'gerrit.host'],), 'true'), + ] else: self.calls += [ ((['git', 'config', 'gerrit.host'],), ''), @@ -1182,6 +1188,25 @@ class TestGitCl(TestCase): ] self.assertEqual(git_cl.main(['patch', '123456']), 0) + def test_patch_force_codereview(self): + self._patch_common(is_gerrit=True, force_codereview=True) + self.calls += [ + ((['git', 'fetch', 'https://chromium.googlesource.com/my/repo', + 'refs/changes/56/123456/7'],), ''), + ((['git', 'cherry-pick', 'FETCH_HEAD'],), ''), + ((['git', 'symbolic-ref', 'HEAD'],), 'master'), + ((['git', 'config', 'branch.master.gerritissue', '123456'],), ''), + ((['git', 'config', 'branch.master.gerritserver'],), ''), + ((['git', 'config', 'branch.master.merge'],), 'master'), + ((['git', 'config', 'branch.master.remote'],), 'origin'), + ((['git', 'config', 'remote.origin.url'],), + 'https://chromium.googlesource.com/my/repo'), + ((['git', 'config', 'branch.master.gerritserver', + 'https://chromium-review.googlesource.com'],), ''), + ((['git', 'config', 'branch.master.gerritpatchset', '7'],), ''), + ] + self.assertEqual(git_cl.main(['patch', '--gerrit', '123456']), 0) + def test_gerrit_patch_url_successful(self): self._patch_common(is_gerrit=True) self.calls += [