Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow specifying custom upstream remote name #35

Merged
merged 13 commits into from
Oct 3, 2022
39 changes: 36 additions & 3 deletions cherry_picker/cherry_picker.py
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ def __init__(
commit_sha1,
branches,
*,
upstream_remote=None,
dry_run=False,
push=True,
prefix_commit=True,
Expand Down Expand Up @@ -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
Expand All @@ -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):
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -662,6 +693,7 @@ def cherry_pick_cli(
ctx,
dry_run,
pr_remote,
upstream_remote,
abort,
status,
push,
Expand All @@ -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,
Expand Down
62 changes: 62 additions & 0 deletions cherry_picker/test_cherry_picker.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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"]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
branches = ["3.6"]

Why assign to a variable if you can just pass it directly?

Copy link
Contributor Author

@Jackenmen Jackenmen Jul 9, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because it's consistent with the rest of the code in this file. Or more truthfully... Because I copy-pasted one of the other test cases when making this one :) I'm indifferent to what's done here but I'll wait to change it until one of the maintainers requests it.

with mock.patch("cherry_picker.cherry_picker.validate_sha", return_value=True):
cp = CherryPicker(
"origin",
"22a594a0047d7706537ff2ac676cdc0f1dcb329c",
branches,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
branches,
["3.6"],

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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"]

Expand Down
19 changes: 14 additions & 5 deletions readme.rst
Original file line number Diff line number Diff line change
@@ -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|
Expand Down Expand Up @@ -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:

Expand Down Expand Up @@ -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
Expand All @@ -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::
Expand Down Expand Up @@ -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
-----------------
Expand Down