Allow specifying custom upstream remote name (#35)

* Allow specifying custom upstream remote name * Add test * Update readme * Parametrize upstream_remote instead of weird ifs * Add two more parametrization variants * Rephrase mention of `--upstream-remote` in README Co-authored-by: Ezio Melotti <[email protected]> * Error out early if remote does not exist * Test that cp.upstream errors on missing remote * Use good-old private attribute instead of the fancy lru_cache * Drop unnecessary f from f-string Co-authored-by: Ezio Melotti <[email protected]>
python · Oct 3, 2022 · 2f85c32 · 2f85c32
1 parent 0ecbf06
commit 2f85c32
Show file tree

Hide file tree

Showing 3 changed files with 112 additions and 8 deletions.
diff --git a/cherry_picker/cherry_picker.py b/cherry_picker/cherry_picker.py
@@ -99,6 +99,7 @@ def __init__(
         commit_sha1,
         branches,
         *,
+        upstream_remote=None,
         dry_run=False,
         push=True,
         prefix_commit=True,
@@ -128,13 +129,17 @@ def __init__(
             click.echo("Dry run requested, listing expected command sequence")
 
         self.pr_remote = pr_remote
+        self.upstream_remote = upstream_remote
         self.commit_sha1 = commit_sha1
         self.branches = branches
         self.dry_run = dry_run
         self.push = push
         self.auto_pr = auto_pr
         self.prefix_commit = prefix_commit
 
+        # the cached calculated value of self.upstream property
+        self._upstream = None
+
         # This is set to the PR number when cherry-picker successfully
         # creates a PR through API.
         self.pr_number = None
@@ -153,14 +158,33 @@ def remember_previous_branch(self):
     @property
     def upstream(self):
         """Get the remote name to use for upstream branches
-        Uses "upstream" if it exists, "origin" otherwise
+
+        Uses the remote passed to `--upstream-remote`.
+        If this flag wasn't passed, it uses "upstream" if it exists or "origin" otherwise.
         """
+        # the cached calculated value of the property
+        if self._upstream is not None:
+            return self._upstream
+
         cmd = ["git", "remote", "get-url", "upstream"]
+        if self.upstream_remote is not None:
+            cmd[-1] = self.upstream_remote
+
         try:
             self.run_cmd(cmd)
         except subprocess.CalledProcessError:
-            return "origin"
-        return "upstream"
+            if self.upstream_remote is not None:
+                raise ValueError(f"There is no remote with name {cmd[-1]!r}.")
+            cmd[-1] = "origin"
+            try:
+                self.run_cmd(cmd)
+            except subprocess.CalledProcessError:
+                raise ValueError(
+                    "There are no remotes with name 'upstream' or 'origin'."
+                )
+
+        self._upstream = cmd[-1]
+        return self._upstream
 
     @property
     def sorted_branches(self):
@@ -605,6 +629,13 @@ def is_mirror(self) -> bool:
     help="git remote to use for PR branches",
     default="origin",
 )
+@click.option(
+    "--upstream-remote",
+    "upstream_remote",
+    metavar="REMOTE",
+    help="git remote to use for upstream branches",
+    default=None,
+)
 @click.option(
     "--abort",
     "abort",
@@ -662,6 +693,7 @@ def cherry_pick_cli(
     ctx,
     dry_run,
     pr_remote,
+    upstream_remote,
     abort,
     status,
     push,
@@ -681,6 +713,7 @@ def cherry_pick_cli(
             pr_remote,
             commit_sha1,
             branches,
+            upstream_remote=upstream_remote,
             dry_run=dry_run,
             push=push,
             auto_pr=auto_pr,

diff --git a/cherry_picker/test_cherry_picker.py b/cherry_picker/test_cherry_picker.py
@@ -57,6 +57,12 @@ def git_init():
     return lambda: subprocess.run(git_init_cmd, check=True)
 
 
+@pytest.fixture
+def git_remote():
+    git_remote_cmd = "git", "remote"
+    return lambda *extra_args: (subprocess.run(git_remote_cmd + extra_args, check=True))
+
+
 @pytest.fixture
 def git_add():
     git_add_cmd = "git", "add"
@@ -238,6 +244,62 @@ def test_get_cherry_pick_branch(os_path_exists, config):
     assert cp.get_cherry_pick_branch("3.6") == "backport-22a594a-3.6"
 
 
+@pytest.mark.parametrize(
+    "remote_name,upstream_remote",
+    (
+        ("upstream", None),
+        ("upstream", "upstream"),
+        ("origin", None),
+        ("origin", "origin"),
+        ("python", "python"),
+    ),
+)
+def test_upstream_name(remote_name, upstream_remote, config, tmp_git_repo_dir, git_remote):
+    git_remote("add", remote_name, "https://github.com/python/cpython.git")
+    if remote_name != "origin":
+        git_remote("add", "origin", "https://github.com/miss-islington/cpython.git")
+
+    branches = ["3.6"]
+    with mock.patch("cherry_picker.cherry_picker.validate_sha", return_value=True):
+        cp = CherryPicker(
+            "origin",
+            "22a594a0047d7706537ff2ac676cdc0f1dcb329c",
+            branches,
+            config=config,
+            upstream_remote=upstream_remote,
+        )
+    assert cp.upstream == remote_name
+
+
+@pytest.mark.parametrize(
+    "remote_to_add,remote_name,upstream_remote",
+    (
+        (None, "upstream", None),
+        ("origin", "upstream", "upstream"),
+        (None, "origin", None),
+        ("upstream", "origin", "origin"),
+        ("origin", "python", "python"),
+        (None, "python", None),
+    ),
+)
+def test_error_on_missing_remote(remote_to_add, remote_name, upstream_remote, config, tmp_git_repo_dir, git_remote):
+    git_remote("add", "some-remote-name", "https://github.com/python/cpython.git")
+    if remote_to_add is not None:
+        git_remote("add", remote_to_add, "https://github.com/miss-islington/cpython.git")
+
+    branches = ["3.6"]
+    with mock.patch("cherry_picker.cherry_picker.validate_sha", return_value=True):
+        cp = CherryPicker(
+            "origin",
+            "22a594a0047d7706537ff2ac676cdc0f1dcb329c",
+            branches,
+            config=config,
+            upstream_remote=upstream_remote,
+        )
+    with pytest.raises(ValueError):
+        cp.upstream
+
+
 def test_get_pr_url(config):
     branches = ["3.6"]
 

diff --git 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] [--auto-pr/--no-auto-pr] <commit_sha1> <branches>
+   cherry_picker [--pr-remote REMOTE] [--upstream-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|
@@ -41,6 +41,8 @@ Requires Python 3.7.
 The cherry picking script assumes that if an ``upstream`` remote is defined, then
 it should be used as the source of upstream changes and as the base for
 cherry-pick branches. Otherwise, ``origin`` is used for that purpose.
+You can override this behavior with the ``--upstream-remote`` option
+(e.g. ``--upstream-remote python`` to use a remote named ``python``).
 
 Verify that an ``upstream`` remote is set to the CPython repository:
 
@@ -73,7 +75,7 @@ From the cloned CPython directory:
 
 .. code-block:: console
 
-    (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>
+    (venv) $ cherry_picker [--pr-remote REMOTE] [--upstream-remote REMOTE] [--dry-run] [--config-path CONFIG-PATH] [--abort/--continue] [--status] [--push/--no-push] [--auto-pr/--no-auto-pr] <commit_sha1> <branches>
 
 
 Commit sha1
@@ -94,9 +96,11 @@ Options
 
 ::
 
-    --dry-run           Dry Run Mode.  Prints out the commands, but not executed.
-    --pr-remote REMOTE  Specify the git remote to push into.  Default is 'origin'.
-    --status            Do `git status` in cpython directory.
+    --dry-run                 Dry Run Mode.  Prints out the commands, but not executed.
+    --pr-remote REMOTE        Specify the git remote to push into.  Default is 'origin'.
+    --upstream-remote REMOTE  Specify the git remote to use for upstream branches.
+                              Default is 'upstream' or 'origin' if the former doesn't exist.
+    --status                  Do `git status` in cpython directory.
 
 
 Additional options::
@@ -252,6 +256,11 @@ steps it would execute without actually executing any of them. For example:
 This will generate pull requests through a remote other than ``origin``
 (e.g. ``pr``)
 
+`--upstream-remote` option
+--------------------------
+
+This will generate branches from a remote other than ``upstream``/``origin``
+(e.g. ``python``)
 
 `--status` option
 -----------------