Repository: python/cherry-picker

Issue:
cherry_picker fails on branch cleanup in non-main worktree
**Every time** when I run `cherry_picker` I get the following error:
```
🐍 🍒 ⛏
Usage: cherry_picker [OPTIONS] [COMMIT_SHA1] [BRANCHES]...
Try "cherry_picker -h" for help.

Error: Run state cherry-picker.state=CHECKING_OUT_DEFAULT_BRANCH in Git config is not known.
Perhaps it has been set by a newer version of cherry-picker. Try upgrading.
Valid states are: BACKPORT_PAUSED, UNSET. If this looks suspicious, raise an issue at https://github.com/python/core-workflow/issues/new.
As the last resort you can reset the runtime state stored in Git config using the following command: `git config --local --remove-section cherry-picker`
```
`cherry_picker` has been updated, it is 1.3.2.

Every time I need to run `git config --local --remove-section cherry-picker` to make `cherry_picker` working. This becomes annoying. Can it be fixed permanently?

Discussion:
jack1142: There is also a problem that git worktrees share the `git config` (without the use of special configuration) so trying to cherry-pick in a worktree while there's a paused cherry-pick in another worktree is bound to fail. I consider it out of scope for this PR though and regardless, I'm not really sure what would be a good solution for that problem.
If someone runs into that, they should probably just make a separate issue as #17 was reporting a different issue and it's somewhat of a coincidence that we also found the issue with shared git config.

Patch:
diff --git a/cherry_picker/cherry_picker.py b/cherry_picker/cherry_picker.py
index 1233cbd..b131509 100755
--- a/cherry_picker/cherry_picker.py
+++ b/cherry_picker/cherry_picker.py
@@ -38,6 +38,9 @@ WORKFLOW_STATES = enum.Enum(
     CHECKING_OUT_DEFAULT_BRANCH
     CHECKED_OUT_DEFAULT_BRANCH
 
+    CHECKING_OUT_PREVIOUS_BRANCH
+    CHECKED_OUT_PREVIOUS_BRANCH
+
     PUSHING_TO_REMOTE
     PUSHED_TO_REMOTE
     PUSHING_TO_REMOTE_FAILED
@@ -128,12 +131,21 @@ class CherryPicker:
         self.auto_pr = auto_pr
         self.prefix_commit = prefix_commit
 
+        # This is set to the PR number when cherry-picker successfully
+        # creates a PR through API.
+        self.pr_number = None
+
     def set_paused_state(self):
         """Save paused progress state into Git config."""
         if self.chosen_config_path is not None:
             save_cfg_vals_to_git_cfg(config_path=self.chosen_config_path)
         set_state(WORKFLOW_STATES.BACKPORT_PAUSED)
 
+    def remember_previous_branch(self):
+        """Save the current branch into Git config to be able to get back to it later."""
+        current_branch = get_current_branch()
+        save_cfg_vals_to_git_cfg(previous_branch=current_branch)
+
     @property
     def upstream(self):
         """Get the remote name to use for upstream branches
@@ -180,24 +192,29 @@ class CherryPicker:
         output = subprocess.check_output(cmd, stderr=subprocess.STDOUT)
         return output.decode("utf-8")
 
-    def checkout_branch(self, branch_name):
-        """git checkout -b <branch_name>"""
-        cmd = [
-            "git",
-            "checkout",
-            "-b",
-            self.get_cherry_pick_branch(branch_name),
-            f"{self.upstream}/{branch_name}",
-        ]
+    def checkout_branch(self, branch_name, *, create_branch=False):
+        """git checkout [-b] <branch_name>"""
+        if create_branch:
+            checked_out_branch = self.get_cherry_pick_branch(branch_name)
+            cmd = [
+                "git",
+                "checkout",
+                "-b",
+                checked_out_branch,
+                f"{self.upstream}/{branch_name}",
+            ]
+        else:
+            checked_out_branch = branch_name
+            cmd = ["git", "checkout", branch_name]
         try:
             self.run_cmd(cmd)
         except subprocess.CalledProcessError as err:
             click.echo(
-                f"Error checking out the branch {self.get_cherry_pick_branch(branch_name)}."
+                f"Error checking out the branch {branch_name}."
             )
             click.echo(err.output)
             raise BranchCheckoutException(
-                f"Error checking out the branch {self.get_cherry_pick_branch(branch_name)}."
+                f"Error checking out the branch {branch_name}."
             )
 
     def get_commit_message(self, commit_sha):
@@ -221,11 +238,23 @@ class CherryPicker:
         """git checkout default branch"""
         set_state(WORKFLOW_STATES.CHECKING_OUT_DEFAULT_BRANCH)
 
-        cmd = "git", "checkout", self.config["default_branch"]
-        self.run_cmd(cmd)
+        self.checkout_branch(self.config["default_branch"])
 
         set_state(WORKFLOW_STATES.CHECKED_OUT_DEFAULT_BRANCH)
 
+    def checkout_previous_branch(self):
+        """git checkout previous branch"""
+        set_state(WORKFLOW_STATES.CHECKING_OUT_PREVIOUS_BRANCH)
+
+        previous_branch = load_val_from_git_cfg("previous_branch")
+        if previous_branch is None:
+            self.checkout_default_branch()
+            return
+
+        self.checkout_branch(previous_branch)
+
+        set_state(WORKFLOW_STATES.CHECKED_OUT_PREVIOUS_BRANCH)
+
     def status(self):
         """
         git status
@@ -331,7 +360,9 @@ Co-authored-by: {get_author_info_from_short_sha(self.commit_sha1)}"""
         url = CREATE_PR_URL_TEMPLATE.format(config=self.config)
         response = requests.post(url, headers=request_headers, json=data)
         if response.status_code == requests.codes.created:
-            click.echo(f"Backport PR created at {response.json()['html_url']}")
+            response_data = response.json()
+            click.echo(f"Backport PR created at {response_data['html_url']}")
+            self.pr_number = response_data['number']
         else:
             click.echo(response.status_code)
             click.echo(response.text)
@@ -357,7 +388,12 @@ Co-authored-by: {get_author_info_from_short_sha(self.commit_sha1)}"""
         Switch to the default branch before that.
         """
         set_state(WORKFLOW_STATES.REMOVING_BACKPORT_BRANCH)
-        self.checkout_default_branch()
+        try:
+            self.checkout_previous_branch()
+        except BranchCheckoutException:
+            click.echo(f"branch {branch} NOT deleted.")
+            set_state(WORKFLOW_STATES.REMOVING_BACKPORT_BRANCH_FAILED)
+            return
         try:
             self.delete_branch(branch)
         except subprocess.CalledProcessError:
@@ -372,6 +408,7 @@ Co-authored-by: {get_author_info_from_short_sha(self.commit_sha1)}"""
             raise click.UsageError("At least one branch must be specified.")
         set_state(WORKFLOW_STATES.BACKPORT_STARTING)
         self.fetch_upstream()
+        self.remember_previous_branch()
 
         set_state(WORKFLOW_STATES.BACKPORT_LOOPING)
         for maint_branch in self.sorted_branches:
@@ -379,7 +416,7 @@ Co-authored-by: {get_author_info_from_short_sha(self.commit_sha1)}"""
             click.echo(f"Now backporting '{self.commit_sha1}' into '{maint_branch}'")
 
             cherry_pick_branch = self.get_cherry_pick_branch(maint_branch)
-            self.checkout_branch(maint_branch)
+            self.checkout_branch(maint_branch, create_branch=True)
             commit_message = ""
             try:
                 self.cherry_pick()
@@ -413,6 +450,7 @@ To abort the cherry-pick and cleanup:
                     self.set_paused_state()
                     return  # to preserve the correct state
             set_state(WORKFLOW_STATES.BACKPORT_LOOP_END)
+        reset_stored_previous_branch()
         reset_state()
 
     def abort_cherry_pick(self):
@@ -434,6 +472,7 @@ To abort the cherry-pick and cleanup:
         if get_current_branch().startswith("backport-"):
             self.cleanup_branch(get_current_branch())
 
+        reset_stored_previous_branch()
         reset_stored_config_ref()
         reset_state()
 
@@ -493,6 +532,7 @@ To abort the cherry-pick and cleanup:
             )
             set_state(WORKFLOW_STATES.CONTINUATION_FAILED)
 
+        reset_stored_previous_branch()
         reset_stored_config_ref()
         reset_state()
 
@@ -822,6 +862,11 @@ def reset_stored_config_ref():
         """Config file pointer is not stored in Git config."""
 
 
+def reset_stored_previous_branch():
+    """Remove the previous branch information from Git config."""
+    wipe_cfg_vals_from_git_cfg("previous_branch")
+
+
 def reset_state():
     """Remove the progress state from Git config."""
     wipe_cfg_vals_from_git_cfg("state")
diff --git a/readme.rst b/readme.rst
index 42949a2..6579ca6 100644
--- a/readme.rst
+++ b/readme.rst
@@ -1,6 +1,6 @@
 Usage (from a cloned CPython directory) ::
 
-   cherry_picker [--pr-remote REMOTE] [--dry-run] [--config-path CONFIG-PATH] [--status] [--abort/--continue] [--push/--no-push] <commit_sha1> <branches>
+   cherry_picker [--pr-remote REMOTE] [--dry-run] [--config-path CONFIG-PATH] [--status] [--abort/--continue] [--push/--no-push] [--auto-pr/--no-auto-pr] <commit_sha1> <branches>
 
 |pyversion status|
 |pypi status|
@@ -69,7 +69,7 @@ From the cloned CPython directory:
 
 ::
 
-    (venv) $ cherry_picker [--pr-remote REMOTE] [--dry-run] [--config-path CONFIG-PATH] [--abort/--continue] [--status] [--push/--no-push] <commit_sha1> <branches>
+    (venv) $ cherry_picker [--pr-remote REMOTE] [--dry-run] [--config-path CONFIG-PATH] [--abort/--continue] [--status] [--push/--no-push] [--auto-pr/--no-auto-pr] <commit_sha1> <branches>
 
 
 Commit sha1
@@ -100,6 +100,8 @@ Additional options::
     --abort        Abort current cherry-pick and clean up branch
     --continue     Continue cherry-pick, push, and clean up branch
     --no-push      Changes won't be pushed to remote
+    --no-auto-pr   PR creation page won't be automatically opened in the web browser or
+                   if GH_AUTH is set, the PR won't be automatically opened through API.
     --config-path  Path to config file
                    (`.cherry_picker.toml` from project root by default)
 
@@ -269,6 +271,14 @@ cherry-pick additional commits, by::
 
    $ git cherry-pick -x <commit_sha1>
 
+`--no-auto-pr` option
+---------------------
+
+PR creation page won't be automatically opened in the web browser or
+if GH_AUTH is set, the PR won't be automatically opened through API.
+This can be useful if your terminal is not capable of opening a useful web browser,
+or if you use cherry-picker with a different Git hosting than GitHub.
+
 `--config-path` option
 ----------------------
 


Test patch:
diff --git a/cherry_picker/test_cherry_picker.py b/cherry_picker/test_cherry_picker.py
index a2919b0..7f8a793 100644
--- a/cherry_picker/test_cherry_picker.py
+++ b/cherry_picker/test_cherry_picker.py
@@ -84,6 +84,14 @@ def git_commit():
     )
 
 
+@pytest.fixture
+def git_worktree():
+    git_worktree_cmd = "git", "worktree"
+    return lambda *extra_args: (
+        subprocess.run(git_worktree_cmd + extra_args, check=True)
+    )
+
+
 @pytest.fixture
 def git_cherry_pick():
     git_cherry_pick_cmd = "git", "cherry-pick"
@@ -100,12 +108,13 @@ def git_config():
 
 @pytest.fixture
 def tmp_git_repo_dir(tmpdir, cd, git_init, git_commit, git_config):
-    cd(tmpdir)
+    repo_dir = tmpdir.mkdir("tmp-git-repo")
+    cd(repo_dir)
     git_init()
     git_config("--local", "user.name", "Monty Python")
     git_config("--local", "user.email", "bot@python.org")
     git_commit("Initial commit", "--allow-empty")
-    yield tmpdir
+    yield repo_dir
 
 
 @mock.patch("subprocess.check_output")
@@ -545,6 +554,11 @@ def test_paused_flow(tmp_git_repo_dir, git_add, git_commit):
             WORKFLOW_STATES.CHECKING_OUT_DEFAULT_BRANCH,
             WORKFLOW_STATES.CHECKED_OUT_DEFAULT_BRANCH,
         ),
+        (
+            "checkout_previous_branch",
+            WORKFLOW_STATES.CHECKING_OUT_PREVIOUS_BRANCH,
+            WORKFLOW_STATES.CHECKED_OUT_PREVIOUS_BRANCH,
+        ),
     ),
 )
 def test_start_end_states(method_name, start_state, end_state, tmp_git_repo_dir):
@@ -552,6 +566,7 @@ def test_start_end_states(method_name, start_state, end_state, tmp_git_repo_dir)
 
     with mock.patch("cherry_picker.cherry_picker.validate_sha", return_value=True):
         cherry_picker = CherryPicker("origin", "xxx", [])
+    cherry_picker.remember_previous_branch()
     assert get_state() == WORKFLOW_STATES.UNSET
 
     def _fetch(cmd):
@@ -572,6 +587,22 @@ def test_cleanup_branch(tmp_git_repo_dir, git_checkout):
     git_checkout("-b", "some_branch")
     cherry_picker.cleanup_branch("some_branch")
     assert get_state() == WORKFLOW_STATES.REMOVED_BACKPORT_BRANCH
+    assert get_current_branch() == "main"
+
+
+def test_cleanup_branch_checkout_previous_branch(tmp_git_repo_dir, git_checkout, git_worktree):
+    assert get_state() == WORKFLOW_STATES.UNSET
+
+    with mock.patch("cherry_picker.cherry_picker.validate_sha", return_value=True):
+        cherry_picker = CherryPicker("origin", "xxx", [])
+    assert get_state() == WORKFLOW_STATES.UNSET
+
+    git_checkout("-b", "previous_branch")
+    cherry_picker.remember_previous_branch()
+    git_checkout("-b", "some_branch")
+    cherry_picker.cleanup_branch("some_branch")
+    assert get_state() == WORKFLOW_STATES.REMOVED_BACKPORT_BRANCH
+    assert get_current_branch() == "previous_branch"
 
 
 def test_cleanup_branch_fail(tmp_git_repo_dir):
@@ -585,6 +616,19 @@ def test_cleanup_branch_fail(tmp_git_repo_dir):
     assert get_state() == WORKFLOW_STATES.REMOVING_BACKPORT_BRANCH_FAILED
 
 
+def test_cleanup_branch_checkout_fail(tmp_git_repo_dir, tmpdir, git_checkout, git_worktree):
+    assert get_state() == WORKFLOW_STATES.UNSET
+
+    with mock.patch("cherry_picker.cherry_picker.validate_sha", return_value=True):
+        cherry_picker = CherryPicker("origin", "xxx", [])
+    assert get_state() == WORKFLOW_STATES.UNSET
+
+    git_checkout("-b", "some_branch")
+    git_worktree("add", str(tmpdir.mkdir("test-worktree")), "main")
+    cherry_picker.cleanup_branch("some_branch")
+    assert get_state() == WORKFLOW_STATES.REMOVING_BACKPORT_BRANCH_FAILED
+
+
 def test_cherry_pick(tmp_git_repo_dir, git_add, git_branch, git_commit, git_checkout):
     cherry_pick_target_branches = ("3.8",)
     pr_remote = "origin"