aiden
/
depot_tools
mirror of https://chromium.googlesource.com/chromium/tools/depot_tools
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

[git_cl] Remove rietveld CMDland implementation.

Browse Source 
R=agable@chromium.org, tandrii@chromium.org

Change-Id: I2bfed54a9a1bde3335340e5949eac313c3fdda32
Reviewed-on: https://chromium-review.googlesource.com/961970
Commit-Queue: Robbie Iannucci <iannucci@chromium.org>
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
changes/70/961970/3
Robert Iannucci 7 years ago committed by Commit Bot
parent 456b0d648f
commit 2e73d43870
2 changed files with 21 additions and 440 deletions
Show Stats Download Patch File Download Diff File

233
git_cl.py
Unescape Escape View File

@ -5079,219 +5079,28 @@ def CMDland(parser, args):
cl = Changelist(auth_config=auth_config)
# TODO(tandrii): refactor this into _RietveldChangelistImpl method.
if cl.IsGerrit():
if options.message:
# This could be implemented, but it requires sending a new patch to
# Gerrit, as Gerrit unlike Rietveld versions messages with patchsets.
# Besides, Gerrit has the ability to change the commit message on submit
# automatically, thus there is no need to support this option (so far?).
parser.error('-m MESSAGE option is not supported for Gerrit.')
if options.contributor:
parser.error(
'-c CONTRIBUTOR option is not supported for Gerrit.\n'
'Before uploading a commit to Gerrit, ensure it\'s author field is '
'the contributor\'s "name <email>". If you can\'t upload such a '
'commit for review, contact your repository admin and request'
'"Forge-Author" permission.')
if not cl.GetIssue():
DieWithError('You must upload the change first to Gerrit.\n'
' If you would rather have `git cl land` upload '
'automatically for you, see http://crbug.com/642759')
return cl._codereview_impl.CMDLand(options.force, options.bypass_hooks,
options.verbose)
current = cl.GetBranch()
remote, upstream_branch = cl.FetchUpstreamTuple(cl.GetBranch())
if remote == '.':
print()
print('Attempting to push branch %r into another local branch!' % current)
print()
print('Either reparent this branch on top of origin/master:')
print(' git reparent-branch --root')
print()
print('OR run `git rebase-update` if you think the parent branch is ')
print('already committed.')
print()
print(' Current parent: %r' % upstream_branch)
return 1
if not args:
# Default to merging against our best guess of the upstream branch.
args = [cl.GetUpstreamBranch()]
if options.contributor:
if not re.match('^.*\s<\S+@\S+>$', options.contributor):
print("Please provide contributor as 'First Last <email@example.com>'")
return 1
base_branch = args[0]
if git_common.is_dirty_git_tree('land'):
return 1
# This rev-list syntax means "show all commits not in my branch that
# are in base_branch".
upstream_commits = RunGit(['rev-list', '^' + cl.GetBranchRef(),
base_branch]).splitlines()
if upstream_commits:
print('Base branch "%s" has %d commits '
'not in this branch.' % (base_branch, len(upstream_commits)))
print('Run "git merge %s" before attempting to land.' % base_branch)
return 1
merge_base = RunGit(['merge-base', base_branch, 'HEAD']).strip()
if not options.bypass_hooks:
author = None
if options.contributor:
author = re.search(r'\<(.*)\>', options.contributor).group(1)
hook_results = cl.RunHook(
committing=True,
may_prompt=not options.force,
verbose=options.verbose,
change=cl.GetChange(merge_base, author))
if not hook_results.should_continue():
return 1
# Check the tree status if the tree status URL is set.
status = GetTreeStatus()
if 'closed' == status:
print('The tree is closed. Please wait for it to reopen. Use '
'"git cl land --bypass-hooks" to commit on a closed tree.')
return 1
elif 'unknown' == status:
print('Unable to determine tree status. Please verify manually and '
'use "git cl land --bypass-hooks" to commit on a closed tree.')
return 1
change_desc = ChangeDescription(options.message)
if not change_desc.description and cl.GetIssue():
change_desc = ChangeDescription(cl.GetDescription())
if not change_desc.description:
if not cl.GetIssue() and options.bypass_hooks:
change_desc = ChangeDescription(CreateDescriptionFromLog([merge_base]))
else:
print('No description set.')
print('Visit %s/edit to set it.' % (cl.GetIssueURL()))
return 1
# Keep a separate copy for the commit message, because the commit message
# contains the link to the Rietveld issue, while the Rietveld message contains
# the commit viewvc url.
if cl.GetIssue():
change_desc.update_reviewers(
get_approving_reviewers(cl.GetIssueProperties()), [])
commit_desc = ChangeDescription(change_desc.description)
if cl.GetIssue():
# Xcode won't linkify this URL unless there is a non-whitespace character
# after it. Add a period on a new line to circumvent this. Also add a space
# before the period to make sure that Gitiles continues to correctly resolve
# the URL.
commit_desc.append_footer('Review-Url: %s .' % cl.GetIssueURL())
if not cl.IsGerrit():
parser.error('rietveld is not supported')
if options.message:
# This could be implemented, but it requires sending a new patch to
# Gerrit, as Gerrit unlike Rietveld versions messages with patchsets.
# Besides, Gerrit has the ability to change the commit message on submit
# automatically, thus there is no need to support this option (so far?).
parser.error('-m MESSAGE option is not supported for Gerrit.')
if options.contributor:
commit_desc.append_footer('Patch from %s.' % options.contributor)
print('Description:')
print(commit_desc.description)
branches = [merge_base, cl.GetBranchRef()]
if not options.force:
print_stats(branches)
# We want to squash all this branch's commits into one commit with the proper
# description. We do this by doing a "reset --soft" to the base branch (which
# keeps the working copy the same), then landing that.
# Delete the special branches if they exist.
for branch in [MERGE_BRANCH, CHERRY_PICK_BRANCH]:
showref_cmd = ['show-ref', '--quiet', '--verify', 'refs/heads/%s' % branch]
result = RunGitWithCode(showref_cmd)
if result[0] == 0:
RunGit(['branch', '-D', branch])
# We might be in a directory that's present in this branch but not in the
# trunk. Move up to the top of the tree so that git commands that expect a
# valid CWD won't fail after we check out the merge branch.
rel_base_path = settings.GetRelativeRoot()
if rel_base_path:
os.chdir(rel_base_path)
# Stuff our change into the merge branch.
# We wrap in a try...finally block so if anything goes wrong,
# we clean up the branches.
retcode = -1
revision = None
try:
RunGit(['checkout', '-q', '-b', MERGE_BRANCH])
RunGit(['reset', '--soft', merge_base])
if options.contributor:
RunGit(
[
'commit', '--author', options.contributor,
'-m', commit_desc.description,
])
else:
RunGit(['commit', '-m', commit_desc.description])
remote, branch = cl.FetchUpstreamTuple(cl.GetBranch())
mirror = settings.GetGitMirror(remote)
if mirror:
pushurl = mirror.url
git_numberer_enabled = _is_git_numberer_enabled(pushurl, branch)
else:
pushurl = remote # Usually, this is 'origin'.
git_numberer_enabled = _is_git_numberer_enabled(
RunGit(['config', 'remote.%s.url' % remote]).strip(), branch)
retcode = PushToGitWithAutoRebase(
pushurl, branch, commit_desc.description, git_numberer_enabled)
if retcode == 0:
revision = RunGit(['rev-parse', 'HEAD']).strip()
if git_numberer_enabled:
change_desc = ChangeDescription(
RunGit(['show', '-s', '--format=%B', 'HEAD']).strip())
except: # pylint: disable=bare-except
if _IS_BEING_TESTED:
logging.exception('this is likely your ACTUAL cause of test failure.\n'
+ '-' * 30 + '8<' + '-' * 30)
logging.error('\n' + '-' * 30 + '8<' + '-' * 30 + '\n\n\n')
raise
finally:
# And then swap back to the original branch and clean up.
RunGit(['checkout', '-q', cl.GetBranch()])
RunGit(['branch', '-D', MERGE_BRANCH])
RunGit(['branch', '-D', CHERRY_PICK_BRANCH], error_ok=True)
if not revision:
print('Failed to push. If this persists, please file a bug.')
return 1
if cl.GetIssue():
viewvc_url = settings.GetViewVCUrl()
if viewvc_url and revision:
change_desc.append_footer(
'Committed: %s%s' % (viewvc_url, revision))
elif revision:
change_desc.append_footer('Committed: %s' % (revision,))
print('Closing issue '
'(you may be prompted for your codereview password)...')
cl.UpdateDescription(change_desc.description)
cl.CloseIssue()
props = cl.GetIssueProperties()
patch_num = len(props['patchsets'])
comment = "Committed patchset #%d (id:%d) manually as %s" % (
patch_num, props['patchsets'][-1], revision)
if options.bypass_hooks:
comment += ' (tree was closed).' if GetTreeStatus() == 'closed' else '.'
else:
comment += ' (presubmit successful).'
cl.RpcServer().add_comment(cl.GetIssue(), comment)
if os.path.isfile(POSTUPSTREAM_HOOK):
RunCommand([POSTUPSTREAM_HOOK, merge_base], error_ok=True)
return 0
parser.error(
'-c CONTRIBUTOR option is not supported for Gerrit.\n'
'Before uploading a commit to Gerrit, ensure it\'s author field is '
'the contributor\'s "name <email>". If you can\'t upload such a '
'commit for review, contact your repository admin and request'
'"Forge-Author" permission.')
if not cl.GetIssue():
DieWithError('You must upload the change first to Gerrit.\n'
' If you would rather have `git cl land` upload '
'automatically for you, see http://crbug.com/642759')
return cl._codereview_impl.CMDLand(options.force, options.bypass_hooks,
options.verbose)
def PushToGitWithAutoRebase(remote, branch, original_description,

228
tests/git_cl_test.py
Unescape Escape View File

@ -1003,234 +1003,6 @@ class TestGitCl(TestCase):
'desc\n\nBUG=500658\nBUG=proj:1234',
[])
def _land_rietveld_calls(self, repo_url, git_numberer_enabled=False,
parent_msg=None,
new_msg=None,
fail_after_parent_msg=False,
fail_cherry_pick=False,
fail_push_count=0,
debug=False):
if debug:
# Very useful due to finally clause in git cl land raising exceptions and
# shadowing real cause of failure.
self.mock(git_cl, '_IS_BEING_TESTED', True)
else:
self.mock(git_cl.sys, 'stdout', StringIO.StringIO())
if git_numberer_enabled:
# Special mocks to check validity of timestamp.
original_git_amend_head = git_cl._git_amend_head
def _git_amend_head_mock(msg, tstamp):
self._mocked_call(['git_amend_head committer timestamp', tstamp])
return original_git_amend_head(msg, tstamp)
self.mock(git_cl, '_git_amend_head', _git_amend_head_mock)
self.mock(git_cl, '_is_git_numberer_enabled', lambda url, ref:
self._mocked_call('_is_git_numberer_enabled', url, ref))
self.mock(RietveldMock, 'update_description', staticmethod(
lambda i, d: self._mocked_call(['update_description', i, d])))
self.mock(RietveldMock, 'add_comment', staticmethod(
lambda i, c: self._mocked_call(['add_comment', i, c])))
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.merge'],), 'refs/heads/master'),
((['git', 'config', 'branch.feature.remote'],), 'origin'),
((['git', 'config', 'branch.feature.merge'],), 'refs/heads/master'),
((['git', 'config', 'branch.feature.remote'],), 'origin'),
((['git', 'rev-list', '^feature', 'refs/remotes/origin/master'],),
''), # No commits to rebase, according to local view of origin.
((['git', 'merge-base', 'refs/remotes/origin/master', 'HEAD'],),
'fake_ancestor_sha'),
] + self._git_sanity_checks('fake_ancestor_sha', 'feature') + [
((['git', 'rev-parse', '--show-cdup'],), ''),
((['git', 'rev-parse', 'HEAD'],), 'fake_sha'),
((['git', '-c', 'core.quotePath=false', 'diff',
'--name-status', '--no-renames', '-r', 'fake_ancestor_sha...', '.'],),
'M\tfile1.cpp'),
((['git', 'config', 'branch.feature.rietveldpatchset'],), '20001'),
((['git', 'config', 'branch.feature.rietveldserver'],),
'https://codereview.chromium.org'),
((['git', 'config', 'user.email'],), 'user@e.mail'),
((['git', 'config', 'rietveld.tree-status-url'],), CERR1),
((['git', 'diff', '--no-ext-diff', '--stat', '-l100000', '-C50',
'fake_ancestor_sha', 'feature'],),
# This command just prints something like this:
# file1.cpp | 53 ++++++--
# 1 file changed, 33 insertions(+), 20 deletions(-)\n
''),
((['git', 'show-ref', '--quiet', '--verify',
'refs/heads/git-cl-commit'],),
''), # 0 return code means branch exists.
((['git', 'branch', '-D', 'git-cl-commit'],), ''),
((['git', 'show-ref', '--quiet', '--verify',
'refs/heads/git-cl-cherry-pick'],),
CERR1), # This means git-cl-cherry-pick branch does not exist.
((['git', 'rev-parse', '--show-cdup'],), ''),
((['git', 'checkout', '-q', '-b', 'git-cl-commit'],), ''),
((['git', 'reset', '--soft', 'fake_ancestor_sha'],), ''),
((['git', 'commit', '-m',
'Issue: 123\n\nR=john@chromium.org\n\n'
'Review-Url: https://codereview.chromium.org/123 .'],), ''),
((['git', 'config', 'branch.feature.merge'],), 'refs/heads/master'),
((['git', 'config', 'branch.feature.remote'],), 'origin'),
((['git', 'config', '--get', 'remote.origin.url'],),
repo_url),
((['git', 'config', 'remote.origin.url'],), repo_url),
(('_is_git_numberer_enabled', repo_url, 'refs/heads/master'),
git_numberer_enabled),
# Auto-rebase init.
((['git', 'rev-parse', 'HEAD'],), 'sha1_for_cherry_pick'),
]
# Auto-rebase loops.
for attempt in xrange(fail_push_count + 1):
self.calls += [
# Auto-rebase loop start.
((['git', 'retry', 'fetch', 'origin',
'+refs/heads/master:refs/heads/git-cl-cherry-pick'],), ''),
((['git', 'checkout', 'refs/heads/git-cl-cherry-pick'],), ''),
# NOTE: if fail_push_count>0, this sha1_of_parent should probably be
# different in each loop iteration for truly realistic test.
((['git', 'rev-parse', 'HEAD'],), 'sha1_of_parent'),
((['git', 'cherry-pick', 'sha1_for_cherry_pick'],),
callError(1, 'failed to cherry pick') if fail_cherry_pick
else ''),
]
if fail_cherry_pick:
return
if git_numberer_enabled:
assert parent_msg
self.calls += [
((['git', 'show', '-s', '--format=%B', 'sha1_of_parent'],),
parent_msg),
]
if fail_after_parent_msg:
return
assert new_msg
self.calls += [
((['git', 'show', '-s', '--format=%ct', 'sha1_of_parent'],),
'1480022355'), # Committer's unix timestamp.
((['git', 'show', '-s', '--format=%ct', 'HEAD'],),
'1480024000'),
((['git_amend_head committer timestamp', 1480024000],), ''),
((['git', 'commit', '--amend', '-m', new_msg],), ''),
]
self.calls += [
((['git', 'push', '--porcelain', 'origin', 'HEAD:refs/heads/master'],),
CERR1 if attempt < fail_push_count else ''),
]
# End of autorebase + push.
self.calls += [
((['git', 'rev-parse', 'HEAD'],), 'sha1_committed'),
]
if git_numberer_enabled:
self.calls += [
((['git', 'show', '-s', '--format=%B', 'HEAD'],), new_msg),
]
# Cleanup calls.
self.calls += [
((['git', 'checkout', '-q', 'feature'],), ''),
((['git', 'branch', '-D', 'git-cl-commit'],), ''),
((['git', 'branch', '-D', 'git-cl-cherry-pick'],), ''),
]
def test_land_rietveld(self):
self._land_rietveld_calls(
repo_url='https://chromium.googlesource.com/infra/infra',
git_numberer_enabled=False,
debug=False)
self.calls += [
((['git', 'config', 'rietveld.viewvc-url'],),
'https://chromium.googlesource.com/infra/infra/+/'),
((['update_description', 123,
'Issue: 123\n\nR=john@chromium.org\n\nCommitted: '
'https://chromium.googlesource.com/infra/infra/+/sha1_committed'],),
''),
((['add_comment', 123, 'Committed patchset #2 (id:20001) manually as '
'sha1_committed (presubmit successful).'],), ''),
]
git_cl.main(['land'])
def test_land_rietveld_fail_cherry_pick(self):
self._land_rietveld_calls(
repo_url='https://chromium.googlesource.com/infra/infra',
git_numberer_enabled=False,
fail_cherry_pick=True,
debug=False)
self.calls += [
((['git', '-c', 'core.quotePath=false', 'diff',
'--name-status', '--diff-filter=U'],),
'U path/file1\n'
'U file2.cpp\n'),
((['git', 'cherry-pick', '--abort'],), ''),
((['git', 'checkout', '-q', 'feature'],), ''),
((['git', 'branch', '-D', 'git-cl-commit'],), ''),
((['git', 'branch', '-D', 'git-cl-cherry-pick'],), ''),
]
git_cl.main(['land'])
def test_land_rietveld_git_numberer_fail_1_push(self):
return self.test_land_rietveld_git_numberer(fail_push_count=1)
def test_land_rietveld_git_numberer(self, fail_push_count=0):
self._land_rietveld_calls(
repo_url='https://chromium.googlesource.com/chromium/src',
git_numberer_enabled=True,
parent_msg=('This is parent commit.\n'
'\n'
'Cr-Commit-Position: refs/heads/master@{#543}\n'
'Cr-Branched-From: refs/svn/2014@{#2208}'),
new_msg=('Issue: 123\n\nR=john@chromium.org\n'
'\n'
'Review-Url: https://codereview.chromium.org/123 .\n'
'Cr-Commit-Position: refs/heads/master@{#544}\n'
'Cr-Branched-From: refs/svn/2014@{#2208}'),
fail_push_count=fail_push_count,
debug=False)
self.calls += [
((['git', 'config', 'rietveld.viewvc-url'],),
'https://chromium.googlesource.com/chromium/src/+/'),
((['update_description', 123,
'Issue: 123\n\nR=john@chromium.org\n'
'\n'
'Review-Url: https://codereview.chromium.org/123 .\n'
'Cr-Commit-Position: refs/heads/master@{#544}\n'
'Cr-Branched-From: refs/svn/2014@{#2208}\n'
'Committed: '
'https://chromium.googlesource.com/chromium/src/+/sha1_committed'],),
''),
((['add_comment', 123, 'Committed patchset #2 (id:20001) manually as '
'sha1_committed (presubmit successful).'],), ''),
]
git_cl.main(['land'])
def test_land_rietveld_git_numberer_bad_parent(self):
self._land_rietveld_calls(
repo_url='https://chromium.googlesource.com/v8/v8',
git_numberer_enabled=True,
parent_msg='This is parent commit with no footer.',
fail_after_parent_msg=True,
debug=False)
self.calls += [
# Cleanup calls to restore original state.
((['git', 'checkout', '-q', 'feature'],), ''),
((['git', 'branch', '-D', 'git-cl-commit'],), ''),
((['git', 'branch', '-D', 'git-cl-cherry-pick'],), ''),
]
with self.assertRaises(ValueError) as cm:
git_cl.main(['land'])
self.assertEqual(cm.exception.message,
'Unable to infer commit position from footers')
def test_git_numberer_not_whitelisted_repo(self):
self.calls = []
res = git_cl._is_git_numberer_enabled(

Write Preview
Loading…
Cancel
Save