diff --git a/cherry_picker/cherry_picker.py b/cherry_picker/cherry_picker.py index bb4b848..dd9cd9e 100755 --- a/cherry_picker/cherry_picker.py +++ b/cherry_picker/cherry_picker.py @@ -317,6 +317,21 @@ def amend_commit_message(self, cherry_pick_branch): click.echo(cpe.output) return updated_commit_message + def pause_after_committing(self, cherry_pick_branch): + click.echo( + f""" +Finished cherry-pick {self.commit_sha1} into {cherry_pick_branch} \U0001F600 +--no-push option used. +... Stopping here. +To continue and push the changes: +$ cherry_picker --continue + +To abort the cherry-pick and cleanup: +$ cherry_picker --abort +""" + ) + self.set_paused_state() + def push_to_remote(self, base_branch, head_branch, commit_message=""): """git push """ set_state(WORKFLOW_STATES.PUSHING_TO_REMOTE) @@ -445,19 +460,7 @@ def backport(self): if not self.is_mirror(): self.cleanup_branch(cherry_pick_branch) else: - click.echo( - f""" -Finished cherry-pick {self.commit_sha1} into {cherry_pick_branch} \U0001F600 ---no-push option used. -... Stopping here. -To continue and push the changes: - $ cherry_picker --continue - -To abort the cherry-pick and cleanup: - $ cherry_picker --abort -""" - ) - self.set_paused_state() + self.pause_after_committing(cherry_pick_branch) return # to preserve the correct state set_state(WORKFLOW_STATES.BACKPORT_LOOP_END) reset_stored_previous_branch() @@ -470,14 +473,19 @@ def abort_cherry_pick(self): if self.initial_state != WORKFLOW_STATES.BACKPORT_PAUSED: raise ValueError("One can only abort a paused process.") - cmd = ["git", "cherry-pick", "--abort"] try: - set_state(WORKFLOW_STATES.ABORTING) - click.echo(self.run_cmd(cmd)) - set_state(WORKFLOW_STATES.ABORTED) - except subprocess.CalledProcessError as cpe: - click.echo(cpe.output) - set_state(WORKFLOW_STATES.ABORTING_FAILED) + validate_sha("CHERRY_PICK_HEAD") + except ValueError: + pass + else: + cmd = ["git", "cherry-pick", "--abort"] + try: + set_state(WORKFLOW_STATES.ABORTING) + click.echo(self.run_cmd(cmd)) + set_state(WORKFLOW_STATES.ABORTED) + except subprocess.CalledProcessError as cpe: + click.echo(cpe.output) + set_state(WORKFLOW_STATES.ABORTING_FAILED) # only delete backport branch created by cherry_picker.py if get_current_branch().startswith("backport-"): self.cleanup_branch(get_current_branch()) @@ -504,30 +512,39 @@ def continue_cherry_pick(self): cherry_pick_branch.index("-") + 1 : cherry_pick_branch.index(base) - 1 ] self.commit_sha1 = get_full_sha_from_short(short_sha) - updated_commit_message = self.get_updated_commit_message(cherry_pick_branch) - if self.dry_run: - click.echo( - f" dry-run: git commit -a -m '{updated_commit_message}' --allow-empty" - ) - else: - cmd = [ - "git", - "commit", - "-a", - "-m", - updated_commit_message, - "--allow-empty", - ] - self.run_cmd(cmd) - - self.push_to_remote(base, cherry_pick_branch) - if not self.is_mirror(): - self.cleanup_branch(cherry_pick_branch) - - click.echo("\nBackport PR:\n") - click.echo(updated_commit_message) - set_state(WORKFLOW_STATES.BACKPORTING_CONTINUATION_SUCCEED) + commits = get_commits_from_backport_branch(base) + if len(commits) == 1: + commit_message = self.amend_commit_message(cherry_pick_branch) + else: + commit_message = self.get_updated_commit_message(cherry_pick_branch) + if self.dry_run: + click.echo( + f" dry-run: git commit -a -m '{commit_message}' --allow-empty" + ) + else: + cmd = [ + "git", + "commit", + "-a", + "-m", + commit_message, + "--allow-empty", + ] + self.run_cmd(cmd) + + if self.push: + self.push_to_remote(base, cherry_pick_branch) + + if not self.is_mirror(): + self.cleanup_branch(cherry_pick_branch) + + click.echo("\nBackport PR:\n") + click.echo(commit_message) + set_state(WORKFLOW_STATES.BACKPORTING_CONTINUATION_SUCCEED) + else: + self.pause_after_committing(cherry_pick_branch) + return # to preserve the correct state else: click.echo( @@ -795,6 +812,13 @@ def get_author_info_from_short_sha(short_sha): return author +def get_commits_from_backport_branch(cherry_pick_branch): + cmd = ["git", "log", "--format=%H", f"{cherry_pick_branch}.."] + output = subprocess.check_output(cmd, stderr=subprocess.STDOUT) + commits = output.strip().decode("utf-8").splitlines() + return commits + + def normalize_commit_message(commit_message): """ Return a tuple of title and body from the commit message diff --git a/cherry_picker/test_cherry_picker.py b/cherry_picker/test_cherry_picker.py index 6f733e3..c2ef1f5 100644 --- a/cherry_picker/test_cherry_picker.py +++ b/cherry_picker/test_cherry_picker.py @@ -18,6 +18,7 @@ from_git_rev_read, get_author_info_from_short_sha, get_base_branch, + get_commits_from_backport_branch, get_current_branch, get_full_sha_from_short, get_sha1_from, @@ -101,6 +102,12 @@ def git_cherry_pick(): ) +@pytest.fixture +def git_reset(): + git_reset_cmd = "git", "reset" + return lambda *extra_args: (subprocess.run(git_reset_cmd + extra_args, check=True)) + + @pytest.fixture def git_config(): git_config_cmd = "git", "config" @@ -857,8 +864,10 @@ def test_backport_success( assert get_state() == WORKFLOW_STATES.UNSET +@pytest.mark.parametrize("already_committed", (True, False)) +@pytest.mark.parametrize("push", (True, False)) def test_backport_pause_and_continue( - tmp_git_repo_dir, git_branch, git_add, git_commit, git_checkout + tmp_git_repo_dir, git_branch, git_add, git_commit, git_checkout, git_reset, already_committed, push ): cherry_pick_target_branches = ("3.8",) pr_remote = "origin" @@ -879,16 +888,27 @@ def test_backport_pause_and_continue( pr_remote, scm_revision, cherry_pick_target_branches, push=False ) - with mock.patch.object(cherry_picker, "checkout_branch"), mock.patch.object( - cherry_picker, "fetch_upstream" - ), mock.patch.object( + with mock.patch.object(cherry_picker, "fetch_upstream"), mock.patch.object( cherry_picker, "amend_commit_message", return_value="commit message" ): cherry_picker.backport() + assert len(get_commits_from_backport_branch(cherry_pick_target_branches[0])) == 1 assert get_state() == WORKFLOW_STATES.BACKPORT_PAUSED - cherry_picker.initial_state = get_state() + if not already_committed: + git_reset("HEAD~1") + assert len(get_commits_from_backport_branch(cherry_pick_target_branches[0])) == 0 + + with mock.patch("cherry_picker.cherry_picker.validate_sha", return_value=True): + cherry_picker = CherryPicker(pr_remote, "", [], push=push) + + commit_message = f"""[{cherry_pick_target_branches[0]}] commit message +(cherry picked from commit xxxxxxyyyyyy) + + +Co-authored-by: Author Name """ + with mock.patch( "cherry_picker.cherry_picker.wipe_cfg_vals_from_git_cfg" ), mock.patch( @@ -900,21 +920,29 @@ def test_backport_pause_and_continue( "cherry_picker.cherry_picker.get_current_branch", return_value="backport-xxx-3.8", ), mock.patch.object( - cherry_picker, - "get_updated_commit_message", - return_value="""[3.8] commit message -(cherry picked from commit xxxxxxyyyyyy) - - -Co-authored-by: Author Name """, - ), mock.patch.object( + cherry_picker, "amend_commit_message", return_value=commit_message + ) as amend_commit_message, mock.patch.object( + cherry_picker, "get_updated_commit_message", return_value=commit_message + ) as get_updated_commit_message, mock.patch.object( cherry_picker, "checkout_branch" ), mock.patch.object( cherry_picker, "fetch_upstream" + ), mock.patch.object( + cherry_picker, "cleanup_branch" ): cherry_picker.continue_cherry_pick() - assert get_state() == WORKFLOW_STATES.BACKPORTING_CONTINUATION_SUCCEED + if already_committed: + amend_commit_message.assert_called_once() + get_updated_commit_message.assert_not_called() + else: + get_updated_commit_message.assert_called_once() + amend_commit_message.assert_not_called() + + if push: + assert get_state() == WORKFLOW_STATES.BACKPORTING_CONTINUATION_SUCCEED + else: + assert get_state() == WORKFLOW_STATES.BACKPORT_PAUSED def test_continue_cherry_pick_invalid_state(tmp_git_repo_dir): @@ -955,18 +983,6 @@ def test_abort_cherry_pick_invalid_state(tmp_git_repo_dir): cherry_picker.abort_cherry_pick() -def test_abort_cherry_pick_fail(tmp_git_repo_dir): - set_state(WORKFLOW_STATES.BACKPORT_PAUSED) - - with mock.patch("cherry_picker.cherry_picker.validate_sha", return_value=True): - cherry_picker = CherryPicker("origin", "xxx", []) - - with mock.patch("cherry_picker.cherry_picker.wipe_cfg_vals_from_git_cfg"): - cherry_picker.abort_cherry_pick() - - assert get_state() == WORKFLOW_STATES.ABORTING_FAILED - - def test_abort_cherry_pick_success( tmp_git_repo_dir, git_branch, git_add, git_commit, git_checkout, git_cherry_pick ):