From 2836a02f55cd943760ab4949d5846cc5fb1fadcb Mon Sep 17 00:00:00 2001 From: Gavin Mak Date: Fri, 30 Aug 2024 15:35:36 +0000 Subject: [PATCH] Make "git cl cherry-pick" handle failed cherry picks better "Better" means: 1. Retrying a failed gerrit_util.CherryPick only once instead of 5 times for faster feedback since more retries don't help. 2. Gracefully handling GerritErrors raised by gerrit_util.CherryPick This CL also fixes a minor bug where the "Remaining commit(s) to cherry pick" message would always print even if there were no more commits left. Bug: 341792235 Change-Id: I1712a6b080b14396463f3bceeeac84772f1253b1 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/5825626 Commit-Queue: Gavin Mak Reviewed-by: Joanna Wang --- gerrit_util.py | 6 +++++- git_cl.py | 35 +++++++++++++++++++++-------------- 2 files changed, 26 insertions(+), 15 deletions(-) diff --git a/gerrit_util.py b/gerrit_util.py index a3c16f366..b5dd326ce 100644 --- a/gerrit_util.py +++ b/gerrit_util.py @@ -1413,7 +1413,11 @@ def CherryPick(host, change, destination, revision='current', message=None): if message: body['message'] = message conn = CreateHttpConn(host, path, reqtype='POST', body=body) - return ReadHttpJsonResponse(conn) + + # If a cherry pick fails due to a merge conflict, Gerrit returns 409. + # Retrying more than once probably won't help since the merge conflict will + # still exist. + return ReadHttpJsonResponse(conn, max_tries=2) def CherryPickCommit(host, project, commit, destination): diff --git a/git_cl.py b/git_cl.py index 918007732..6b4227aa8 100755 --- a/git_cl.py +++ b/git_cl.py @@ -4569,26 +4569,37 @@ def CMDcherry_pick(parser, args): print(f'Creating chain of {len(change_ids_to_message)} cherry pick(s)...') + def print_any_remaining_commits(): + if not change_ids_to_commit: + return + print('Remaining commit(s) to cherry pick:') + for commit in change_ids_to_commit.values(): + print(f' {commit}') + # Gerrit only supports cherry picking one commit per change, so we have # to cherry pick each commit individually and create a chain of CLs. parent_change_num = options.parent_change_num for change_id, orig_message in change_ids_to_message.items(): change_ids_to_commit.pop(change_id) message = _create_commit_message(orig_message, options.bug) + orig_subj_line = orig_message.splitlines()[0] # Create a cherry pick first, then rebase. If we create a chained CL # then cherry pick, the change will lose its relation to the parent. - # TODO(b/341792235): Don't retry more than once on merge conflicts and - # handle them gracefully. - new_change_info = gerrit_util.CherryPick(host, - change_id, - options.branch, - message=message) + try: + new_change_info = gerrit_util.CherryPick(host, + change_id, + options.branch, + message=message) + except gerrit_util.GerritError as e: + print(f'Failed to create cherry pick "{orig_subj_line}": {e}. ' + 'Please resolve any merge conflicts.') + print_any_remaining_commits() + return 1 + new_change_id = new_change_info['id'] new_change_num = new_change_info['_number'] new_change_url = gerrit_util.GetChangePageUrl(host, new_change_num) - - orig_subj_line = orig_message.splitlines()[0] print(f'Created cherry pick of "{orig_subj_line}": {new_change_url}') if parent_change_num: @@ -4603,13 +4614,9 @@ def CMDcherry_pick(parser, args): print('Once resolved, you can continue the CL chain with ' f'`--parent-change-num={new_change_num}` to specify ' 'which change the chain should start with.\n') - - if change_ids_to_message: - print('Remaining commit(s) to cherry pick:') - for commit in change_ids_to_commit.values(): - print(f' {commit}') - + print_any_remaining_commits() return 1 + parent_change_num = new_change_num return 0