From 565974c3de7d71e4af089c347c8b8222b431bd77 Mon Sep 17 00:00:00 2001 From: Stan Ulbrych Date: Mon, 26 Oct 2026 15:21:18 +0000 Subject: [PATCH] Keep the backport branch when pushing it fails --- cherry_picker/cherry_picker.py | 20 +++++ cherry_picker/test_cherry_picker.py | 111 ++++++++++++++++++++++++++++ 2 files changed, 131 insertions(+) diff --git a/cherry_picker/cherry_picker.py b/cherry_picker/cherry_picker.py index a71a1ac..d54d5c7 100755 --- a/cherry_picker/cherry_picker.py +++ b/cherry_picker/cherry_picker.py @@ -410,6 +410,18 @@ def pause_after_committing(self, cherry_pick_branch): To continue and push the changes: $ cherry_picker --continue +To abort the cherry-pick and cleanup: +$ cherry_picker --abort +""") + self.set_paused_state() + + def pause_after_push_failure(self, cherry_pick_branch): + print(f""" +Failed to push {cherry_pick_branch} to {self.pr_remote} ☹ +... Stopping here. The branch {cherry_pick_branch} has been kept. +To retry the push: +$ cherry_picker --continue + To abort the cherry-pick and cleanup: $ cherry_picker --abort """) @@ -573,6 +585,10 @@ def backport(self): print(self.get_exit_message(maint_branch)) self.set_paused_state() raise + if get_state() == WORKFLOW_STATES.PUSHING_TO_REMOTE_FAILED: + # Keep the branch so that the work isn't lost. + self.pause_after_push_failure(cherry_pick_branch) + return # to preserve the correct state if not self.is_mirror(): self.cleanup_branch(cherry_pick_branch) else: @@ -661,6 +677,10 @@ def continue_cherry_pick(self): if self.push: self.push_to_remote(base, cherry_pick_branch) + if get_state() == WORKFLOW_STATES.PUSHING_TO_REMOTE_FAILED: + # Keep the branch so that the work isn't lost. + self.pause_after_push_failure(cherry_pick_branch) + return # to preserve the correct state if not self.is_mirror(): self.cleanup_branch(cherry_pick_branch) diff --git a/cherry_picker/test_cherry_picker.py b/cherry_picker/test_cherry_picker.py index 35e8e14..2365895 100644 --- a/cherry_picker/test_cherry_picker.py +++ b/cherry_picker/test_cherry_picker.py @@ -1151,12 +1151,61 @@ def test_backport_success( mock.patch.object( cherry_picker, "amend_commit_message", return_value="commit message" ), + mock.patch.object(cherry_picker, "push_to_remote", side_effect=_fake_push), ): cherry_picker.backport() assert get_state() == WORKFLOW_STATES.UNSET +def test_backport_push_fail_keeps_branch( + tmp_git_repo_dir, git_branch, git_add, git_commit, git_checkout, git_remote +): + cherry_pick_target_branches = ("3.15",) + pr_remote = "origin" + test_file = "some.file" + tmp_git_repo_dir.join(test_file).write("some contents") + # A remote that doesn't exist, so pushing to it fails. + git_remote("add", pr_remote, str(tmp_git_repo_dir.join("nonexistent"))) + git_branch(cherry_pick_target_branches[0]) + git_branch( + f"{pr_remote}/{cherry_pick_target_branches[0]}", cherry_pick_target_branches[0] + ) + git_add(test_file) + git_commit("Add a test file") + scm_revision = get_sha1_from("HEAD") + + with mock.patch("cherry_picker.cherry_picker.validate_sha", return_value=True): + cherry_picker = CherryPicker( + pr_remote, scm_revision, cherry_pick_target_branches + ) + backport_branch_name = cherry_picker.get_cherry_pick_branch( + cherry_pick_target_branches[0] + ) + + with ( + mock.patch.object(cherry_picker, "fetch_upstream"), + mock.patch.object( + cherry_picker, "amend_commit_message", return_value="commit message" + ), + mock.patch.object(cherry_picker, "cleanup_branch") as cleanup_branch, + ): + cherry_picker.backport() + + cleanup_branch.assert_not_called() + assert get_state() == WORKFLOW_STATES.BACKPORT_PAUSED + assert get_current_branch() == backport_branch_name + assert len(get_commits_from_backport_branch(cherry_pick_target_branches[0])) == 1 + + +def _fake_push(*args, **kwargs): + set_state(WORKFLOW_STATES.PUSHED_TO_REMOTE) + + +def _fake_push_fail(*args, **kwargs): + set_state(WORKFLOW_STATES.PUSHING_TO_REMOTE_FAILED) + + @pytest.mark.parametrize("already_committed", (True, False)) @pytest.mark.parametrize("push", (True, False)) def test_backport_pause_and_continue( @@ -1235,6 +1284,7 @@ def test_backport_pause_and_continue( ) as get_updated_commit_message, mock.patch.object(cherry_picker, "checkout_branch"), mock.patch.object(cherry_picker, "fetch_upstream"), + mock.patch.object(cherry_picker, "push_to_remote", side_effect=_fake_push), mock.patch.object(cherry_picker, "cleanup_branch"), ): cherry_picker.continue_cherry_pick() @@ -1252,6 +1302,67 @@ def test_backport_pause_and_continue( assert get_state() == WORKFLOW_STATES.BACKPORT_PAUSED +def test_continue_cherry_pick_push_fail_keeps_branch( + tmp_git_repo_dir, git_branch, git_add, git_commit, git_checkout, git_remote +): + cherry_pick_target_branches = ("3.8",) + pr_remote = "origin" + test_file = "some.file" + tmp_git_repo_dir.join(test_file).write("some contents") + git_remote("add", pr_remote, "https://github.com/python/cpython.git") + git_branch(cherry_pick_target_branches[0]) + git_branch( + f"{pr_remote}/{cherry_pick_target_branches[0]}", cherry_pick_target_branches[0] + ) + git_add(test_file) + git_commit("Add a test file") + scm_revision = get_sha1_from("HEAD") + + git_checkout(cherry_pick_target_branches[0]) # simulate backport method logic + + with mock.patch("cherry_picker.cherry_picker.validate_sha", return_value=True): + cherry_picker = CherryPicker( + pr_remote, scm_revision, cherry_pick_target_branches, push=False + ) + + with ( + mock.patch.object(cherry_picker, "fetch_upstream"), + mock.patch.object( + cherry_picker, "amend_commit_message", return_value="commit message" + ), + ): + cherry_picker.backport() + + assert get_state() == WORKFLOW_STATES.BACKPORT_PAUSED + + with mock.patch("cherry_picker.cherry_picker.validate_sha", return_value=True): + cherry_picker = CherryPicker(pr_remote, "", [], push=True) + + with ( + mock.patch("cherry_picker.cherry_picker.wipe_cfg_vals_from_git_cfg"), + mock.patch( + "cherry_picker.cherry_picker.get_full_sha_from_short", + return_value="xxxxxxyyyyyy", + ), + mock.patch("cherry_picker.cherry_picker.get_base_branch", return_value="3.8"), + mock.patch( + "cherry_picker.cherry_picker.get_current_branch", + return_value="backport-xxx-3.8", + ), + mock.patch.object( + cherry_picker, "amend_commit_message", return_value="commit message" + ), + mock.patch.object(cherry_picker, "checkout_branch"), + mock.patch.object(cherry_picker, "fetch_upstream"), + mock.patch.object(cherry_picker, "push_to_remote", side_effect=_fake_push_fail), + mock.patch.object(cherry_picker, "cleanup_branch") as cleanup_branch, + ): + cherry_picker.continue_cherry_pick() + + cleanup_branch.assert_not_called() + assert get_state() == WORKFLOW_STATES.BACKPORT_PAUSED + + def test_continue_cherry_pick_invalid_state(tmp_git_repo_dir): assert get_state() == WORKFLOW_STATES.UNSET