diff --git a/git_cl.py b/git_cl.py index b3d1d2eb9..4c6bb10ce 100755 --- a/git_cl.py +++ b/git_cl.py @@ -834,6 +834,43 @@ def GetCurrentBranch(): return None +class _ParsedIssueNumberArgument(object): + def __init__(self, issue=None, patchset=None, hostname=None): + self.issue = issue + self.patchset = patchset + self.hostname = hostname + + @property + def valid(self): + return self.issue is not None + + +class _RietveldParsedIssueNumberArgument(_ParsedIssueNumberArgument): + def __init__(self, *args, **kwargs): + self.patch_url = kwargs.pop('patch_url', None) + super(_RietveldParsedIssueNumberArgument, self).__init__(*args, **kwargs) + + +def ParseIssueNumberArgument(arg): + """Parses the issue argument and returns _ParsedIssueNumberArgument.""" + fail_result = _ParsedIssueNumberArgument() + + if arg.isdigit(): + return _ParsedIssueNumberArgument(issue=int(arg)) + if not arg.startswith('http'): + return fail_result + url = gclient_utils.UpgradeToHttps(arg) + try: + parsed_url = urlparse.urlparse(url) + except ValueError: + return fail_result + for cls in _CODEREVIEW_IMPLEMENTATIONS.itervalues(): + tmp = cls.ParseIssueURL(parsed_url) + if tmp is not None: + return tmp + return fail_result + + class Changelist(object): """Changelist works with one changelist in local branch. @@ -1257,6 +1294,20 @@ class Changelist(object): ('%s\nMaybe your depot_tools is out of date?\n' 'If all fails, contact maruel@') % e) + def CMDPatchIssue(self, issue_arg, reject, nocommit, directory): + """Fetches and applies the issue patch from codereview to local branch.""" + if issue_arg.isdigit(): + parsed_issue_arg = _RietveldParsedIssueNumberArgument(int(issue_arg)) + else: + # Assume url. + parsed_issue_arg = self._codereview_impl.ParseIssueURL( + urlparse.urlparse(issue_arg)) + if not parsed_issue_arg or not parsed_issue_arg.valid: + DieWithError('Failed to parse issue argument "%s". ' + 'Must be an issue number or a valid URL.' % issue_arg) + return self._codereview_impl.CMDPatchWithParsedIssue( + parsed_issue_arg, reject, nocommit, directory) + # Forward methods to codereview specific implementation. def CloseIssue(self): @@ -1346,6 +1397,26 @@ class _ChangelistCodereviewBase(object): """Returns the most recent patchset number from the codereview site.""" raise NotImplementedError() + def CMDPatchWithParsedIssue(self, parsed_issue_arg, reject, nocommit, + directory): + """Fetches and applies the issue. + + Arguments: + parsed_issue_arg: instance of _ParsedIssueNumberArgument. + reject: if True, reject the failed patch instead of switching to 3-way + merge. Rietveld only. + nocommit: do not commit the patch, thus leave the tree dirty. Rietveld + only. + directory: switch to directory before applying the patch. Rietveld only. + """ + raise NotImplementedError() + + @staticmethod + def ParseIssueURL(parsed_url): + """Parses url and returns instance of _ParsedIssueNumberArgument or None if + failed.""" + raise NotImplementedError() + class _RietveldChangelistImpl(_ChangelistCodereviewBase): def __init__(self, changelist, auth_config=None, rietveld_server=None): @@ -1518,6 +1589,94 @@ class _RietveldChangelistImpl(_ChangelistCodereviewBase): def GetRieveldObjForPresubmit(self): return self.RpcServer() + def CMDPatchWithParsedIssue(self, parsed_issue_arg, reject, nocommit, + directory): + # TODO(maruel): Use apply_issue.py + + # PatchIssue should never be called with a dirty tree. It is up to the + # caller to check this, but just in case we assert here since the + # consequences of the caller not checking this could be dire. + assert(not git_common.is_dirty_git_tree('apply')) + assert(parsed_issue_arg.valid) + self._changelist.issue = parsed_issue_arg.issue + if parsed_issue_arg.hostname: + self._rietveld_server = 'https://%s' % parsed_issue_arg.hostname + + if parsed_issue_arg.patch_url: + assert parsed_issue_arg.patchset + patchset = parsed_issue_arg.patchset + patch_data = urllib2.urlopen(parsed_issue_arg.patch_url).read() + else: + patchset = parsed_issue_arg.patchset or self.GetMostRecentPatchset() + patch_data = self.GetPatchSetDiff(self.GetIssue(), patchset) + + # Switch up to the top-level directory, if necessary, in preparation for + # applying the patch. + top = settings.GetRelativeRoot() + if top: + os.chdir(top) + + # Git patches have a/ at the beginning of source paths. We strip that out + # with a sed script rather than the -p flag to patch so we can feed either + # Git or svn-style patches into the same apply command. + # re.sub() should be used but flags=re.MULTILINE is only in python 2.7. + try: + patch_data = subprocess2.check_output( + ['sed', '-e', 's|^--- a/|--- |; s|^+++ b/|+++ |'], stdin=patch_data) + except subprocess2.CalledProcessError: + DieWithError('Git patch mungling failed.') + logging.info(patch_data) + + # We use "git apply" to apply the patch instead of "patch" so that we can + # pick up file adds. + # The --index flag means: also insert into the index (so we catch adds). + cmd = ['git', 'apply', '--index', '-p0'] + if directory: + cmd.extend(('--directory', directory)) + if reject: + cmd.append('--reject') + elif IsGitVersionAtLeast('1.7.12'): + cmd.append('--3way') + try: + subprocess2.check_call(cmd, env=GetNoGitPagerEnv(), + stdin=patch_data, stdout=subprocess2.VOID) + except subprocess2.CalledProcessError: + print 'Failed to apply the patch' + return 1 + + # If we had an issue, commit the current state and register the issue. + if not nocommit: + RunGit(['commit', '-m', (self.GetDescription() + '\n\n' + + 'patch from issue %(i)s at patchset ' + '%(p)s (http://crrev.com/%(i)s#ps%(p)s)' + % {'i': self.GetIssue(), 'p': patchset})]) + self.SetIssue(self.GetIssue()) + self.SetPatchset(patchset) + print "Committed patch locally." + else: + print "Patch applied to index." + return 0 + + @staticmethod + def ParseIssueURL(parsed_url): + if not parsed_url.scheme or not parsed_url.scheme.startswith('http'): + return None + # Typical url: https://domain/[/[other]] + match = re.match('/(\d+)(/.*)?$', parsed_url.path) + if match: + return _RietveldParsedIssueNumberArgument( + issue=int(match.group(1)), + hostname=parsed_url.netloc) + # Rietveld patch: https://domain/download/issue_.diff + match = re.match(r'/download/issue(\d+)_(\d+).diff$', parsed_url.path) + if match: + return _RietveldParsedIssueNumberArgument( + issue=int(match.group(1)), + patchset=int(match.group(2)), + hostname=parsed_url.netloc, + patch_url=gclient_utils.UpgradeToHttps(parsed_url.geturl())) + return None + class _GerritChangelistImpl(_ChangelistCodereviewBase): def __init__(self, changelist, auth_config=None): @@ -1696,6 +1855,63 @@ class _GerritChangelistImpl(_ChangelistCodereviewBase): print('Issue %s has been submitted.' % self.GetIssueURL()) return 0 + def CMDPatchWithParsedIssue(self, parsed_issue_arg, reject, nocommit, + directory): + assert not reject + assert not nocommit + assert not directory + assert parsed_issue_arg.valid + + self._changelist.issue = parsed_issue_arg.issue + + if parsed_issue_arg.hostname: + self._gerrit_host = parsed_issue_arg.hostname + self._gerrit_server = 'https://%s' % self._gerrit_host + + detail = self._GetChangeDetail(['ALL_REVISIONS']) + + if not parsed_issue_arg.patchset: + # Use current revision by default. + revision_info = detail['revisions'][detail['current_revision']] + patchset = int(revision_info['_number']) + else: + patchset = parsed_issue_arg.patchset + for revision_info in detail['revisions'].itervalues(): + if int(revision_info['_number']) == parsed_issue_arg.patchset: + break + else: + DieWithError('Couldn\'t find patchset %i in issue %i' % + (parsed_issue_arg.patchset, self.GetIssue())) + + fetch_info = revision_info['fetch']['http'] + RunGit(['fetch', fetch_info['url'], fetch_info['ref']]) + RunGit(['cherry-pick', 'FETCH_HEAD']) + self.SetIssue(self.GetIssue()) + self.SetPatchset(patchset) + print('Committed patch for issue %i pathset %i locally' % + (self.GetIssue(), self.GetPatchset())) + return 0 + + @staticmethod + def ParseIssueURL(parsed_url): + if not parsed_url.scheme or not parsed_url.scheme.startswith('http'): + return None + # Gerrit's new UI is https://domain/c/[/[patchset]] + # But current GWT UI is https://domain/#/c/[/[patchset]] + # Short urls like https://domain/ can be used, but don't allow + # specifying the patchset (you'd 404), but we allow that here. + if parsed_url.path == '/': + part = parsed_url.fragment + else: + part = parsed_url.path + match = re.match('(/c)?/(\d+)(/(\d+)?/?)?$', part) + if match: + return _ParsedIssueNumberArgument( + issue=int(match.group(2)), + patchset=int(match.group(4)) if match.group(4) else None, + hostname=parsed_url.netloc) + return None + _CODEREVIEW_IMPLEMENTATIONS = { 'rietveld': _RietveldChangelistImpl, @@ -3593,15 +3809,6 @@ def CMDland(parser, args): return SendUpstream(parser, args, 'land') -def ParseIssueNum(arg): - """Parses the issue number from args if present otherwise returns None.""" - if re.match(r'\d+', arg): - return arg - if arg.startswith('http'): - return re.sub(r'.*/(\d+)/?', r'\1', arg) - return None - - @subcommand.usage('') def CMDpatch(parser, args): """Patches in a code review.""" @@ -3611,64 +3818,69 @@ def CMDpatch(parser, args): help='with -b, clobber any existing branch') parser.add_option('-d', '--directory', action='store', metavar='DIR', help='Change to the directory DIR immediately, ' - 'before doing anything else.') + 'before doing anything else. Rietveld only.') parser.add_option('--reject', action='store_true', help='failed patches spew .rej files rather than ' - 'attempting a 3-way merge') + 'attempting a 3-way merge. Rietveld only.') parser.add_option('-n', '--no-commit', action='store_true', dest='nocommit', - help="don't commit after patch applies") - - group = optparse.OptionGroup(parser, - """Options for continuing work on the current issue uploaded -from a different clone (e.g. different machine). Must be used independently from -the other options. No issue number should be specified, and the branch must have -an issue number associated with it""") - group.add_option('--reapply', action='store_true', - dest='reapply', - help="""Reset the branch and reapply the issue. -CAUTION: This will undo any local changes in this branch""") + help='don\'t commit after patch applies. Rietveld only.') + + + group = optparse.OptionGroup( + parser, + 'Options for continuing work on the current issue uploaded from a ' + 'different clone (e.g. different machine). Must be used independently ' + 'from the other options. No issue number should be specified, and the ' + 'branch must have an issue number associated with it') + group.add_option('--reapply', action='store_true', dest='reapply', + help='Reset the branch and reapply the issue.\n' + 'CAUTION: This will undo any local changes in this ' + 'branch') group.add_option('--pull', action='store_true', dest='pull', - help="Performs a pull before reapplying.") + help='Performs a pull before reapplying.') parser.add_option_group(group) auth.add_auth_options(parser) (options, args) = parser.parse_args(args) auth_config = auth.extract_auth_config_from_options(options) + cl = Changelist(auth_config=auth_config) + issue_arg = None if options.reapply : if len(args) > 0: - parser.error("--reapply implies no additional arguments.") + parser.error('--reapply implies no additional arguments.') - cl = Changelist() issue_arg = cl.GetIssue() upstream = cl.GetUpstreamBranch() if upstream == None: - parser.error("No upstream branch specified. Cannot reset branch") + parser.error('No upstream branch specified. Cannot reset branch') RunGit(['reset', '--hard', upstream]) if options.pull: RunGit(['pull']) else: if len(args) != 1: - parser.error("Must specify issue number") - - issue_arg = ParseIssueNum(args[0]) + parser.error('Must specify issue number or url') + issue_arg = args[0] - # The patch URL works because ParseIssueNum won't do any substitution - # as the re.sub pattern fails to match and just returns it. - if issue_arg == None: + if not issue_arg: parser.print_help() return 1 + if cl.IsGerrit(): + if options.reject: + parser.error('--reject is not supported with Gerrit codereview.') + if options.nocommit: + parser.error('--nocommit is not supported with Gerrit codereview.') + if options.directory: + parser.error('--directory is not supported with Gerrit codereview.') + # We don't want uncommitted changes mixed up with the patch. if git_common.is_dirty_git_tree('patch'): return 1 - # TODO(maruel): Use apply_issue.py - # TODO(ukai): use gerrit-cherry-pick for gerrit repository? - if options.newbranch: if options.reapply: parser.error("--reapply excludes any option other than --pull") @@ -3678,84 +3890,8 @@ CAUTION: This will undo any local changes in this branch""") RunGit(['checkout', '-b', options.newbranch, Changelist().GetUpstreamBranch()]) - return PatchIssue(issue_arg, options.reject, options.nocommit, - options.directory, auth_config) - - -def PatchIssue(issue_arg, reject, nocommit, directory, auth_config): - # PatchIssue should never be called with a dirty tree. It is up to the - # caller to check this, but just in case we assert here since the - # consequences of the caller not checking this could be dire. - assert(not git_common.is_dirty_git_tree('apply')) - - # TODO(tandrii): implement for Gerrit. - if type(issue_arg) is int or issue_arg.isdigit(): - # Input is an issue id. Figure out the URL. - issue = int(issue_arg) - cl = Changelist(issue=issue, codereview='rietveld', auth_config=auth_config) - patchset = cl.GetMostRecentPatchset() - patch_data = cl._codereview_impl.GetPatchSetDiff(issue, patchset) - else: - # Assume it's a URL to the patch. Default to https. - issue_url = gclient_utils.UpgradeToHttps(issue_arg) - match = re.match(r'(.*?)/download/issue(\d+)_(\d+).diff', issue_url) - if not match: - DieWithError('Must pass an issue ID or full URL for ' - '\'Download raw patch set\'') - issue = int(match.group(2)) - cl = Changelist(issue=issue, codereview='rietveld', - rietveld_server=match.group(1), auth_config=auth_config) - patchset = int(match.group(3)) - patch_data = urllib2.urlopen(issue_arg).read() - - # Switch up to the top-level directory, if necessary, in preparation for - # applying the patch. - top = settings.GetRelativeRoot() - if top: - os.chdir(top) - - # Git patches have a/ at the beginning of source paths. We strip that out - # with a sed script rather than the -p flag to patch so we can feed either - # Git or svn-style patches into the same apply command. - # re.sub() should be used but flags=re.MULTILINE is only in python 2.7. - try: - patch_data = subprocess2.check_output( - ['sed', '-e', 's|^--- a/|--- |; s|^+++ b/|+++ |'], stdin=patch_data) - except subprocess2.CalledProcessError: - DieWithError('Git patch mungling failed.') - logging.info(patch_data) - - # We use "git apply" to apply the patch instead of "patch" so that we can - # pick up file adds. - # The --index flag means: also insert into the index (so we catch adds). - cmd = ['git', 'apply', '--index', '-p0'] - if directory: - cmd.extend(('--directory', directory)) - if reject: - cmd.append('--reject') - elif IsGitVersionAtLeast('1.7.12'): - cmd.append('--3way') - try: - subprocess2.check_call(cmd, env=GetNoGitPagerEnv(), - stdin=patch_data, stdout=subprocess2.VOID) - except subprocess2.CalledProcessError: - print 'Failed to apply the patch' - return 1 - - # If we had an issue, commit the current state and register the issue. - if not nocommit: - RunGit(['commit', '-m', (cl.GetDescription() + '\n\n' + - 'patch from issue %(i)s at patchset ' - '%(p)s (http://crrev.com/%(i)s#ps%(p)s)' - % {'i': issue, 'p': patchset})]) - cl = Changelist(codereview='rietveld', auth_config=auth_config, - rietveld_server=cl.GetCodereviewServer()) - cl.SetIssue(issue) - cl.SetPatchset(patchset) - print "Committed patch locally." - else: - print "Patch applied to index." - return 0 + return cl.CMDPatchIssue(issue_arg, options.reject, options.nocommit, + options.directory) def CMDrebase(parser, args): @@ -4157,7 +4293,7 @@ def CMDdiff(parser, args): parser.error('Unrecognized args: %s' % ' '.join(args)) # Uncommitted (staged and unstaged) changes will be destroyed by - # "git reset --hard" if there are merging conflicts in PatchIssue(). + # "git reset --hard" if there are merging conflicts in CMDPatchIssue(). # Staged changes would be committed along with the patch from last # upload, hence counted toward the "last upload" side in the final # diff output, and this is not what we want. @@ -4175,8 +4311,7 @@ def CMDdiff(parser, args): # Create a new branch based on the merge-base RunGit(['checkout', '-q', '-b', TMP_BRANCH, base_branch]) try: - # Patch in the latest changes from rietveld. - rtn = PatchIssue(issue, False, False, None, auth_config) + rtn = cl.CMDPatchIssue(issue, reject=False, nocommit=False, directory=None) if rtn != 0: RunGit(['reset', '--hard']) return rtn @@ -4384,16 +4519,18 @@ def CMDformat(parser, args): @subcommand.usage('') def CMDcheckout(parser, args): """Checks out a branch associated with a given Rietveld issue.""" + # TODO(tandrii): consider adding this for Gerrit? _, args = parser.parse_args(args) if len(args) != 1: parser.print_help() return 1 - target_issue = ParseIssueNum(args[0]) - if target_issue == None: + issue_arg = ParseIssueNumberArgument(args[0]) + if issue_arg.valid: parser.print_help() return 1 + target_issue = issue_arg.issue key_and_issues = [x.split() for x in RunGit( ['config', '--local', '--get-regexp', r'branch\..*\.rietveldissue']) diff --git a/tests/git_cl_test.py b/tests/git_cl_test.py index 2e869a20e..b8b694fa5 100755 --- a/tests/git_cl_test.py +++ b/tests/git_cl_test.py @@ -10,6 +10,7 @@ import StringIO import stat import sys import unittest +import urlparse sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) @@ -73,6 +74,100 @@ class AuthenticatorMock(object): return True +class TestGitClBasic(unittest.TestCase): + def _test_ParseIssueUrl(self, func, url, issue, patchset, hostname, fail): + parsed = urlparse.urlparse(url) + result = func(parsed) + if fail: + self.assertIsNone(result) + return None + self.assertIsNotNone(result) + self.assertEqual(result.issue, issue) + self.assertEqual(result.patchset, patchset) + self.assertEqual(result.hostname, hostname) + return result + + def test_ParseIssueURL_rietveld(self): + def test(url, issue=None, patchset=None, hostname=None, patch_url=None, + fail=None): + result = self._test_ParseIssueUrl( + git_cl._RietveldChangelistImpl.ParseIssueURL, + url, issue, patchset, hostname, fail) + if not fail: + self.assertEqual(result.patch_url, patch_url) + + test('http://codereview.chromium.org/123', + 123, None, 'codereview.chromium.org') + test('https://codereview.chromium.org/123', + 123, None, 'codereview.chromium.org') + test('https://codereview.chromium.org/123/', + 123, None, 'codereview.chromium.org') + test('https://codereview.chromium.org/123/whatever', + 123, None, 'codereview.chromium.org') + test('http://codereview.chromium.org/download/issue123_4.diff', + 123, 4, 'codereview.chromium.org', + patch_url='https://codereview.chromium.org/download/issue123_4.diff') + # This looks like bad Gerrit, but is actually valid Rietveld. + test('https://chrome-review.source.com/123/4/', + 123, None, 'chrome-review.source.com') + + test('https://codereview.chromium.org/deadbeaf', fail=True) + test('https://codereview.chromium.org/api/123', fail=True) + test('bad://codereview.chromium.org/123', fail=True) + test('http://codereview.chromium.org/download/issue123_4.diffff', fail=True) + + def test_ParseIssueURL_gerrit(self): + def test(url, issue=None, patchset=None, hostname=None, fail=None): + self._test_ParseIssueUrl( + git_cl._GerritChangelistImpl.ParseIssueURL, + url, issue, patchset, hostname, fail) + + test('http://chrome-review.source.com/c/123', + 123, None, 'chrome-review.source.com') + test('https://chrome-review.source.com/c/123/', + 123, None, 'chrome-review.source.com') + test('https://chrome-review.source.com/c/123/4', + 123, 4, 'chrome-review.source.com') + test('https://chrome-review.source.com/#/c/123/4', + 123, 4, 'chrome-review.source.com') + test('https://chrome-review.source.com/c/123/4', + 123, 4, 'chrome-review.source.com') + test('https://chrome-review.source.com/123', + 123, None, 'chrome-review.source.com') + test('https://chrome-review.source.com/123/4', + 123, 4, 'chrome-review.source.com') + + test('https://chrome-review.source.com/c/123/1/whatisthis', fail=True) + test('https://chrome-review.source.com/c/abc/', fail=True) + test('ssh://chrome-review.source.com/c/123/1/', fail=True) + + def test_ParseIssueNumberArgument(self): + def test(arg, issue=None, patchset=None, hostname=None, fail=False): + result = git_cl.ParseIssueNumberArgument(arg) + self.assertIsNotNone(result) + if fail: + self.assertFalse(result.valid) + else: + self.assertEqual(result.issue, issue) + self.assertEqual(result.patchset, patchset) + self.assertEqual(result.hostname, hostname) + + test('123', 123) + test('', fail=True) + test('abc', fail=True) + test('123/1', fail=True) + test('123a', fail=True) + test('ssh://chrome-review.source.com/#/c/123/4/', fail=True) + # Rietveld. + test('https://codereview.source.com/123', + 123, None, 'codereview.source.com') + test('https://codereview.source.com/www123', fail=True) + # Gerrrit. + test('https://chrome-review.source.com/c/123/4', + 123, 4, 'chrome-review.source.com') + test('https://chrome-review.source.com/bad/123/4', fail=True) + + class TestGitCl(TestCase): def setUp(self): super(TestGitCl, self).setUp() @@ -99,8 +194,12 @@ class TestGitCl(TestCase): # It's important to reset settings to not have inter-tests interference. git_cl.settings = None + def tearDown(self): try: + # Note: has_failed returns True if at least 1 test ran so far, current + # included, has failed. That means current test may have actually ran + # fine, and the check for no leftover calls would be skipped. if not self.has_failed(): self.assertEquals([], self.calls) finally: @@ -916,6 +1015,14 @@ class TestGitCl(TestCase): def test_patch_when_dirty(self): # Patch when local tree is dirty self.mock(git_common, 'is_dirty_git_tree', lambda x: True) + self.calls = [ + ((['git', 'symbolic-ref', 'HEAD'],), 'master'), + ((['git', 'config', 'branch.master.rietveldissue'],), ''), + ((['git', 'config', 'branch.master.gerritissue'],), ''), + ((['git', 'config', 'rietveld.autoupdate'],), ''), + ((['git', 'config', 'gerrit.host'],), ''), + ((['git', 'config', 'rietveld.server'],), 'codereview.example.com'), + ] self.assertNotEqual(git_cl.main(['patch', '123456']), 0) def test_diff_when_dirty(self): @@ -923,23 +1030,52 @@ 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): + def _patch_common(self, is_gerrit=False): self.mock(git_cl._RietveldChangelistImpl, 'GetMostRecentPatchset', lambda x: '60001') self.mock(git_cl._RietveldChangelistImpl, 'GetPatchSetDiff', lambda *args: None) + self.mock(git_cl._GerritChangelistImpl, '_GetChangeDetail', + lambda *args: { + 'current_revision': '7777777777', + 'revisions': { + '1111111111': { + '_number': 1, + 'fetch': {'http': { + 'url': 'https://chromium.googlesource.com/my/repo', + 'ref': 'refs/changes/56/123456/1', + }}, + }, + '7777777777': { + '_number': 7, + 'fetch': {'http': { + 'url': 'https://chromium.googlesource.com/my/repo', + 'ref': 'refs/changes/56/123456/7', + }}, + }, + }, + }) 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) self.calls = [ + ((['git', 'symbolic-ref', 'HEAD'],), 'master'), + ((['git', 'config', 'branch.master.rietveldissue'],), ''), + ((['git', 'config', 'branch.master.gerritissue'],), ''), ((['git', 'config', 'rietveld.autoupdate'],), ''), - ((['git', 'config', 'rietveld.server'],), 'codereview.example.com'), - ((['git', 'rev-parse', '--show-cdup'],), ''), - ((['sed', '-e', 's|^--- a/|--- |; s|^+++ b/|+++ |'],), ''), ] + if is_gerrit: + self.calls += [ + ((['git', 'config', 'gerrit.host'],), 'true'), + ] + else: + self.calls += [ + ((['git', 'config', 'gerrit.host'],), ''), + ((['git', 'config', 'rietveld.server'],), 'codereview.example.com'), + ((['git', 'rev-parse', '--show-cdup'],), ''), + ((['sed', '-e', 's|^--- a/|--- |; s|^+++ b/|+++ |'],), ''), + ] def test_patch_successful(self): self._patch_common() @@ -949,8 +1085,11 @@ class TestGitCl(TestCase): 'Description\n\n' + 'patch from issue 123456 at patchset 60001 ' + '(http://crrev.com/123456#ps60001)'],), ''), - ((['git', 'symbolic-ref', 'HEAD'],), 'master'), + ((['git', 'config', 'branch.master.rietveldissue', '123456'],), ''), ((['git', 'config', 'branch.master.rietveldserver'],), ''), + ((['git', 'config', 'branch.master.rietveldserver', + 'https://codereview.example.com'],), ''), + ((['git', 'config', 'branch.master.rietveldpatchset', '60001'],), ''), ] self.assertEqual(git_cl.main(['patch', '123456']), 0) @@ -962,6 +1101,57 @@ class TestGitCl(TestCase): ] self.assertNotEqual(git_cl.main(['patch', '123456']), 0) + def test_gerrit_patch_successful(self): + self._patch_common(is_gerrit=True) + self.calls += [ + ((['git', 'fetch', 'https://chromium.googlesource.com/my/repo', + 'refs/changes/56/123456/7'],), ''), + ((['git', 'cherry-pick', 'FETCH_HEAD'],), ''), + ((['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', '123456']), 0) + + def test_gerrit_patch_url_successful(self): + self._patch_common(is_gerrit=True) + self.calls += [ + ((['git', 'fetch', 'https://chromium.googlesource.com/my/repo', + 'refs/changes/56/123456/1'],), ''), + ((['git', 'cherry-pick', 'FETCH_HEAD'],), ''), + ((['git', 'config', 'branch.master.gerritissue', '123456'],), ''), + ((['git', 'config', 'branch.master.gerritserver', + 'https://chromium-review.googlesource.com'],), ''), + ((['git', 'config', 'branch.master.gerritpatchset', '1'],), ''), + ] + self.assertEqual(git_cl.main( + ['patch', 'https://chromium-review.googlesource.com/#/c/123456/1']), 0) + + def test_gerrit_patch_conflict(self): + self._patch_common(is_gerrit=True) + self.mock(git_cl, 'DieWithError', + lambda msg: self._mocked_call(['DieWithError', msg])) + class SystemExitMock(Exception): + pass + self.calls += [ + ((['git', 'fetch', 'https://chromium.googlesource.com/my/repo', + 'refs/changes/56/123456/1'],), ''), + ((['git', 'cherry-pick', 'FETCH_HEAD'],), + '', subprocess2.CalledProcessError(1, '', '', '', '')), + ((['DieWithError', 'git cherry-pick FETCH_HEAD" failed.\n'],), + '', SystemExitMock()), + ] + with self.assertRaises(SystemExitMock): + git_cl.main(['patch', + 'https://chromium-review.googlesource.com/#/c/123456/1']) + + if __name__ == '__main__': git_cl.logging.basicConfig( level=git_cl.logging.DEBUG if '-v' in sys.argv else git_cl.logging.ERROR)