diff --git a/README.md b/README.md index 3e4f6ce..07e32bf 100644 --- a/README.md +++ b/README.md @@ -137,33 +137,38 @@ repo = "aiohttp" 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) - -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) - -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) +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) + +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. + +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: diff --git a/cherry_picker/cherry_picker.py b/cherry_picker/cherry_picker.py index 95229b1..2bc5a81 100755 --- a/cherry_picker/cherry_picker.py +++ b/cherry_picker/cherry_picker.py @@ -4,6 +4,7 @@ import collections import enum +import functools import os import re import subprocess @@ -31,6 +32,7 @@ "check_sha": "7f777ed95a19224294949e1b4ce56bbffcb1fe9f", "fix_commit_msg": True, "default_branch": "main", + "require_version_in_branch_name": True, } ) @@ -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(compute_version_sort_key, self.config) + ) @property def username(self): @@ -333,7 +337,7 @@ def get_updated_commit_message(self, cherry_pick_branch): updated_commit_message = self.get_commit_message(self.commit_sha1) if self.prefix_commit: updated_commit_message = remove_commit_prefix(updated_commit_message) - base_branch = get_base_branch(cherry_pick_branch) + base_branch = get_base_branch(cherry_pick_branch, config=self.config) updated_commit_message = f"[{base_branch}] {updated_commit_message}" # Add '(cherry picked from commit ...)' to the message @@ -600,7 +604,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 ] @@ -831,7 +835,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' @@ -855,7 +859,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) + compute_version_sort_key(config, base_branch) return base_branch @@ -876,14 +880,31 @@ def validate_sha(sha): ) -def version_from_branch(branch): +def compute_version_sort_key(config, branch): """ - 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. """ m = re.search(r"\d+(?:\.\d+)+", branch) - if not m: + if m: + raw_version = m[0].split(".") + # Use 0 to sort version numbers *before* regular branch names + return (0, *(-int(x) for x in raw_version)) + + if not branch: + raise ValueError("Branch name is an empty string.") + if config["require_version_in_branch_name"]: raise ValueError(f"Branch {branch} seems to not have a version in its name.") - return tuple(map(int, m[0].split("."))) + + # Use 1 to sort regular branch names *after* version numbers + return (1, branch) def get_current_branch(): diff --git a/cherry_picker/test_cherry_picker.py b/cherry_picker/test_cherry_picker.py index 88c3cde..7b77d18 100644 --- a/cherry_picker/test_cherry_picker.py +++ b/cherry_picker/test_cherry_picker.py @@ -149,20 +149,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" @@ -171,14 +171,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") @@ -206,18 +206,33 @@ 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", @@ -227,6 +242,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, match=r"^Branch name is an empty string\.$"): + cp.sorted_branches + + @pytest.mark.parametrize( "input_branches", [ @@ -460,6 +490,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, }, ) @@ -483,6 +514,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, }, ) @@ -511,6 +543,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, }, )