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 passing a base branch that doesn't have version info #70

Merged
merged 13 commits into from
Nov 16, 2024
52 changes: 34 additions & 18 deletions cherry_picker/cherry_picker.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import collections
import enum
import functools
import os
import re
import subprocess
Expand Down Expand Up @@ -29,6 +30,7 @@
"check_sha": "7f777ed95a19224294949e1b4ce56bbffcb1fe9f",
"fix_commit_msg": True,
"default_branch": "main",
"require_version_in_branch_name": True,
}
)

Expand Down Expand Up @@ -191,7 +193,9 @@ def upstream(self):
@property
def sorted_branches(self):
"""Return the branches to cherry-pick to, sorted by version."""
return sorted(self.branches, reverse=True, key=version_from_branch)
return sorted(
self.branches, key=functools.partial(version_sort_key, self.config)
)

@property
def username(self):
Expand Down Expand Up @@ -324,7 +328,9 @@ def get_updated_commit_message(self, cherry_pick_branch):
# if that's enabled.
commit_prefix = ""
if self.prefix_commit:
commit_prefix = f"[{get_base_branch(cherry_pick_branch)}] "
commit_prefix = (
f"[{get_base_branch(cherry_pick_branch, config=self.config)}] "
)
updated_commit_message = f"{commit_prefix}{self.get_commit_message(self.commit_sha1)}"

# Add '(cherry picked from commit ...)' to the message
Expand Down Expand Up @@ -571,7 +577,7 @@ def continue_cherry_pick(self):
if cherry_pick_branch.startswith("backport-"):
set_state(WORKFLOW_STATES.CONTINUATION_STARTED)
# amend the commit message, prefix with [X.Y]
base = get_base_branch(cherry_pick_branch)
base = get_base_branch(cherry_pick_branch, config=self.config)
short_sha = cherry_pick_branch[
cherry_pick_branch.index("-") + 1 : cherry_pick_branch.index(base) - 1
]
Expand Down Expand Up @@ -800,7 +806,7 @@ def cherry_pick_cli(
sys.exit(-1)


def get_base_branch(cherry_pick_branch):
def get_base_branch(cherry_pick_branch, *, config):
"""
return '2.7' from 'backport-sha-2.7'

Expand All @@ -823,7 +829,7 @@ def get_base_branch(cherry_pick_branch):

# Subject the parsed base_branch to the same tests as when we generated it
# This throws a ValueError if the base_branch doesn't meet our requirements
version_from_branch(base_branch)
version_sort_key(config, base_branch)

return base_branch

Expand All @@ -843,23 +849,33 @@ def validate_sha(sha):
)


def version_from_branch(branch):
def version_sort_key(config, branch):
Jackenmen marked this conversation as resolved.
Show resolved Hide resolved
"""
return version information from a git branch name
Get sort key based on version information from the given git branch name.

This function can be used as a sort key in list.sort()/sorted() provided that
you additionally pass config as a first argument by e.g. wrapping it with
functools.partial().

Branches with version information come first and are sorted from latest
to oldest version.
Branches without version information come second and are sorted alphabetically.
"""
try:
return tuple(
map(
int,
re.match(r"^.*(?P<version>\d+(\.\d+)+).*$", branch)
.groupdict()["version"]
.split("."),
)
)
match = re.match(r"^.*(?P<version>\d+(\.\d+)+).*$", branch)
Jackenmen marked this conversation as resolved.
Show resolved Hide resolved
raw_version = match.groupdict()["version"].split(".")
except AttributeError as attr_err:
raise ValueError(
f"Branch {branch} seems to not have a version in its name."
) from attr_err
if not branch:
raise ValueError("Branch name is an empty string.") from attr_err
if config["require_version_in_branch_name"]:
raise ValueError(
f"Branch {branch} seems to not have a version in its name."
) from attr_err
# Use '0' to sort regular branch names *after* version numbers
Jackenmen marked this conversation as resolved.
Show resolved Hide resolved
return (1, branch)
else:
# Use '0' to sort version numbers *before* regular branch names
Jackenmen marked this conversation as resolved.
Show resolved Hide resolved
return (0, *(-int(x) for x in raw_version))


def get_current_branch():
Expand Down
51 changes: 41 additions & 10 deletions cherry_picker/test_cherry_picker.py
Original file line number Diff line number Diff line change
Expand Up @@ -144,20 +144,20 @@ def tmp_git_repo_dir(tmpdir, cd, git_init, git_commit, git_config):


@mock.patch("subprocess.check_output")
def test_get_base_branch(subprocess_check_output):
def test_get_base_branch(subprocess_check_output, config):
# The format of cherry-pick branches we create are::
# backport-{SHA}-{base_branch}
subprocess_check_output.return_value = b"22a594a0047d7706537ff2ac676cdc0f1dcb329c"
cherry_pick_branch = "backport-22a594a-2.7"
result = get_base_branch(cherry_pick_branch)
result = get_base_branch(cherry_pick_branch, config=config)
assert result == "2.7"


@mock.patch("subprocess.check_output")
def test_get_base_branch_which_has_dashes(subprocess_check_output):
def test_get_base_branch_which_has_dashes(subprocess_check_output, config):
subprocess_check_output.return_value = b"22a594a0047d7706537ff2ac676cdc0f1dcb329c"
cherry_pick_branch = "backport-22a594a-baseprefix-2.7-basesuffix"
result = get_base_branch(cherry_pick_branch)
result = get_base_branch(cherry_pick_branch, config=config)
assert result == "baseprefix-2.7-basesuffix"


Expand All @@ -166,14 +166,14 @@ def test_get_base_branch_which_has_dashes(subprocess_check_output):
[
"backport-22a594a", # Not enough fields
"prefix-22a594a-2.7", # Not the prefix we were expecting
"backport-22a594a-base", # No version info in the base branch
"backport-22a594a-", # No base branch
],
)
@mock.patch("subprocess.check_output")
def test_get_base_branch_invalid(subprocess_check_output, cherry_pick_branch):
def test_get_base_branch_invalid(subprocess_check_output, cherry_pick_branch, config):
subprocess_check_output.return_value = b"22a594a0047d7706537ff2ac676cdc0f1dcb329c"
with pytest.raises(ValueError):
get_base_branch(cherry_pick_branch)
get_base_branch(cherry_pick_branch, config=config)


@mock.patch("subprocess.check_output")
Expand Down Expand Up @@ -201,18 +201,31 @@ def test_get_author_info_from_short_sha(subprocess_check_output):


@pytest.mark.parametrize(
"input_branches,sorted_branches",
"input_branches,sorted_branches,require_version",
[
(["3.1", "2.7", "3.10", "3.6"], ["3.10", "3.6", "3.1", "2.7"]),
(["3.1", "2.7", "3.10", "3.6"], ["3.10", "3.6", "3.1", "2.7"], True),
(
["stable-3.1", "lts-2.7", "3.10-other", "smth3.6else"],
["3.10-other", "smth3.6else", "stable-3.1", "lts-2.7"],
True,
),
(["3.1", "2.7", "3.10", "3.6"], ["3.10", "3.6", "3.1", "2.7"], False),
(
["stable-3.1", "lts-2.7", "3.10-other", "smth3.6else"],
["3.10-other", "smth3.6else", "stable-3.1", "lts-2.7"],
False,
),
(
["3.7", "3.10", "2.7", "foo", "stable", "branch"],
["3.10", "3.7", "2.7", "branch", "foo", "stable"],
False,
),
],
)
@mock.patch("os.path.exists")
def test_sorted_branch(os_path_exists, config, input_branches, sorted_branches):
def test_sorted_branch(os_path_exists, config, input_branches, sorted_branches, require_version):
os_path_exists.return_value = True
config["require_version_in_branch_name"] = require_version
cp = CherryPicker(
"origin",
"22a594a0047d7706537ff2ac676cdc0f1dcb329c",
Expand All @@ -222,6 +235,21 @@ def test_sorted_branch(os_path_exists, config, input_branches, sorted_branches):
assert cp.sorted_branches == sorted_branches


@mock.patch("os.path.exists")
def test_invalid_branch_empty_string(os_path_exists, config):
os_path_exists.return_value = True
# already tested for require_version_in_branch_name=True below
config["require_version_in_branch_name"] = False
cp = CherryPicker(
"origin",
"22a594a0047d7706537ff2ac676cdc0f1dcb329c",
["3.1", "2.7", "3.10", "3.6", ""],
config=config,
)
with pytest.raises(ValueError):
Jackenmen marked this conversation as resolved.
Show resolved Hide resolved
cp.sorted_branches


@pytest.mark.parametrize(
"input_branches",
[
Expand Down Expand Up @@ -437,6 +465,7 @@ def test_load_full_config(tmp_git_repo_dir, git_add, git_commit):
"team": "python",
"fix_commit_msg": True,
"default_branch": "devel",
"require_version_in_branch_name": True,
},
)

Expand All @@ -460,6 +489,7 @@ def test_load_partial_config(tmp_git_repo_dir, git_add, git_commit):
"team": "python",
"fix_commit_msg": True,
"default_branch": "main",
"require_version_in_branch_name": True,
},
)

Expand Down Expand Up @@ -488,6 +518,7 @@ def test_load_config_no_head_sha(tmp_git_repo_dir, git_add, git_commit):
"team": "python",
"fix_commit_msg": True,
"default_branch": "devel",
"require_version_in_branch_name": True,
},
)

Expand Down
41 changes: 23 additions & 18 deletions readme.rst
Original file line number Diff line number Diff line change
Expand Up @@ -123,32 +123,37 @@ Configuration file example:
check_sha = "f382b5ffc445e45a110734f5396728da7914aeb6"
fix_commit_msg = false
default_branch = "devel"
require_version_in_branch_name = false


Available config options::

team github organization or individual nick,
e.g "aio-libs" for https://github.com/aio-libs/aiohttp
("python" by default)
team github organization or individual nick,
e.g "aio-libs" for https://github.com/aio-libs/aiohttp
("python" by default)

repo github project name,
e.g "aiohttp" for https://github.com/aio-libs/aiohttp
("cpython" by default)
repo github project name,
e.g "aiohttp" for https://github.com/aio-libs/aiohttp
("cpython" by default)

check_sha A long hash for any commit from the repo,
e.g. a sha1 hash from the very first initial commit
("7f777ed95a19224294949e1b4ce56bbffcb1fe9f" by default)
check_sha A long hash for any commit from the repo,
e.g. a sha1 hash from the very first initial commit
("7f777ed95a19224294949e1b4ce56bbffcb1fe9f" by default)

fix_commit_msg Replace # with GH- in cherry-picked commit message.
It is the default behavior for CPython because of external
Roundup bug tracker (https://bugs.python.org) behavior:
#xxxx should point on issue xxxx but GH-xxxx points
on pull-request xxxx.
For projects using GitHub Issues, this option can be disabled.
fix_commit_msg Replace # with GH- in cherry-picked commit message.
It is the default behavior for CPython because of external
Roundup bug tracker (https://bugs.python.org) behavior:
#xxxx should point on issue xxxx but GH-xxxx points
on pull-request xxxx.
For projects using GitHub Issues, this option can be disabled.

default_branch Project's default branch name,
e.g "devel" for https://github.com/ansible/ansible
("main" by default)
default_branch Project's default branch name,
e.g "devel" for https://github.com/ansible/ansible
("main" by default)

require_version_in_branch_name Allow backporting to branches whose names don't contain
something that resembles a version number
(i.e. at least two dot-separated numbers).


To customize the tool for used by other project:
Expand Down