From e0a10585da93e31358c822caf2dc9f016ad1f7ec Mon Sep 17 00:00:00 2001 From: "tandrii@chromium.org" Date: Fri, 1 Apr 2016 19:20:31 +0000 Subject: [PATCH] Revert of Gerrit git cl: implement git cl patch. (patchset #7 id:120001 of https://codereview.chromium.org/1852593002/ ) Reason for revert: just in case. Original issue's description: > Gerrit git cl: implement git cl patch. > > BUG=579182 > > Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=299644 TBR=andybons@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=579182 Review URL: https://codereview.chromium.org/1848393002 git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@299645 0039d316-1c4b-4281-b951-d872f2087c98 --- git_cl.py | 373 ++++++++++++++----------------------------- tests/git_cl_test.py | 204 +---------------------- 2 files changed, 125 insertions(+), 452 deletions(-) diff --git a/git_cl.py b/git_cl.py index 4c6bb10ce..b3d1d2eb9 100755 --- a/git_cl.py +++ b/git_cl.py @@ -834,43 +834,6 @@ 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. @@ -1294,20 +1257,6 @@ 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): @@ -1397,26 +1346,6 @@ 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): @@ -1589,94 +1518,6 @@ 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): @@ -1855,63 +1696,6 @@ 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, @@ -3809,6 +3593,15 @@ 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.""" @@ -3818,69 +3611,64 @@ 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. Rietveld only.') + 'before doing anything else.') parser.add_option('--reject', action='store_true', help='failed patches spew .rej files rather than ' - 'attempting a 3-way merge. Rietveld only.') + 'attempting a 3-way merge') parser.add_option('-n', '--no-commit', action='store_true', dest='nocommit', - 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') + 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""") 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 or url') - issue_arg = args[0] + parser.error("Must specify issue number") + + issue_arg = ParseIssueNum(args[0]) - if not issue_arg: + # 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: 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") @@ -3890,8 +3678,84 @@ def CMDpatch(parser, args): RunGit(['checkout', '-b', options.newbranch, Changelist().GetUpstreamBranch()]) - return cl.CMDPatchIssue(issue_arg, options.reject, options.nocommit, - options.directory) + 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 def CMDrebase(parser, args): @@ -4293,7 +4157,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 CMDPatchIssue(). + # "git reset --hard" if there are merging conflicts in PatchIssue(). # 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. @@ -4311,7 +4175,8 @@ def CMDdiff(parser, args): # Create a new branch based on the merge-base RunGit(['checkout', '-q', '-b', TMP_BRANCH, base_branch]) try: - rtn = cl.CMDPatchIssue(issue, reject=False, nocommit=False, directory=None) + # Patch in the latest changes from rietveld. + rtn = PatchIssue(issue, False, False, None, auth_config) if rtn != 0: RunGit(['reset', '--hard']) return rtn @@ -4519,18 +4384,16 @@ 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 - issue_arg = ParseIssueNumberArgument(args[0]) - if issue_arg.valid: + target_issue = ParseIssueNum(args[0]) + if target_issue == None: 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 b8b694fa5..2e869a20e 100755 --- a/tests/git_cl_test.py +++ b/tests/git_cl_test.py @@ -10,7 +10,6 @@ import StringIO import stat import sys import unittest -import urlparse sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) @@ -74,100 +73,6 @@ 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() @@ -194,12 +99,8 @@ 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: @@ -1015,14 +916,6 @@ 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): @@ -1030,52 +923,23 @@ 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): 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() @@ -1085,11 +949,8 @@ class TestGitCl(TestCase): 'Description\n\n' + 'patch from issue 123456 at patchset 60001 ' + '(http://crrev.com/123456#ps60001)'],), ''), - ((['git', 'config', 'branch.master.rietveldissue', '123456'],), ''), + ((['git', 'symbolic-ref', 'HEAD'],), 'master'), ((['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) @@ -1101,57 +962,6 @@ 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)