From aed2e4927bb564c27220a17ab885761ae3c8a9a5 Mon Sep 17 00:00:00 2001 From: Chris de Graaf Date: Fri, 20 Dec 2019 23:13:49 +0700 Subject: [PATCH] Add release branch management (#44) * Add release branch management (#34) * Checkout release branch locally * Less ugly handling of Git commands that can fail Originally I wanted this to just be a `check` keyword to `git`, but mypy does not realize that the return type depends on the keyword's value. So we can't "safely" do things like `git("log").splitlines()` without disabling typing checking. * Fix PR creation --- README.md | 11 +++++++ action.yml | 4 +++ tagbot/__init__.py | 9 ++++++ tagbot/__main__.py | 5 ++++ tagbot/repo.py | 45 ++++++++++++++++++++++++++++- test/test_repo.py | 70 +++++++++++++++++++++++++++++++++++++++++++-- test/test_tagbot.py | 15 ++++++---- 7 files changed, 151 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index 4b3711c0..7bd90ca9 100644 --- a/README.md +++ b/README.md @@ -52,6 +52,17 @@ with: registry: MyOrg/MyRegistry ``` +### Release Branch Management + +If you're using [PkgDev](https://github.com/JuliaLang/PkgDev.jl) to release your packages, TagBot can manage the merging and deletion of the release branches that it creates. +To enable this feature, use the `branches` input: + +```yml +with: + token: ${{ secrets.GITHUB_TOKEN }} + branches: true +``` + ### Pre-Release Hooks If you want to make something happen just before releases are created, for example creating annotated, GPG-signed tags, you can do so with the `dispatch` input: diff --git a/action.yml b/action.yml index fe96152f..4839aeda 100644 --- a/action.yml +++ b/action.yml @@ -9,6 +9,10 @@ inputs: description: Owner/name of the registry repository required: false default: JuliaRegistries/General + branches: + description: Whether or not to merge release branches + required: false + default: false dispatch: description: Whether or not to create a repository dispatch event prior to making releases required: false diff --git a/tagbot/__init__.py b/tagbot/__init__.py index ae5f5c23..36b6b360 100644 --- a/tagbot/__init__.py +++ b/tagbot/__init__.py @@ -50,3 +50,12 @@ def git(*argv: str, repo: Optional[str] = None) -> str: info(p.stderr.decode()) raise Abort(f"Git command '{cmd}' failed") return out.strip() + + +def git_check(*argv: str, repo: Optional[str] = None) -> bool: + """Run a Git command that can fail.""" + try: + git(*argv, repo=repo) + return True + except Abort: + return False diff --git a/tagbot/__main__.py b/tagbot/__main__.py index b61d15cc..138e08ad 100644 --- a/tagbot/__main__.py +++ b/tagbot/__main__.py @@ -5,6 +5,7 @@ from .repo import Repo repo_name = os.getenv("GITHUB_REPOSITORY", "") +branches = os.getenv("INPUT_BRANCHES", "false") == "true" dispatch = os.getenv("INPUT_DISPATCH", "false") == "true" registry_name = os.getenv("INPUT_REGISTRY", "") token = os.getenv("INPUT_TOKEN", "") @@ -24,6 +25,8 @@ for version, sha in versions.items(): info(f"Processing version {version} ({sha})") try: + if branches: + repo.handle_release_branch(version) log = repo.changelog(version) repo.create_release(version, sha, log) except Abort as e: @@ -31,4 +34,6 @@ from . import STATUS +info(f"Exiting with status {STATUS}") + exit(STATUS) diff --git a/tagbot/repo.py b/tagbot/repo.py index 892992e6..2595a7d6 100644 --- a/tagbot/repo.py +++ b/tagbot/repo.py @@ -9,7 +9,7 @@ from github import Github, UnknownObjectException from github.Requester import requests -from . import DELTA, Abort, git, debug, info, warn, error +from . import DELTA, Abort, git, git_check, debug, info, warn, error from .changelog import get_changelog @@ -68,6 +68,13 @@ def _commit_from_tree(self, tree: str) -> Optional[str]: return c return None + def _fetch_branch(self, master: str, branch: str) -> bool: + """Try to checkout a remote branch, and return whether or not it succeeded.""" + if not git_check("checkout", branch, repo=self._dir()): + return False + git("checkout", master, repo=self._dir()) + return True + def _tag_exists(self, version: str) -> bool: """Check whether or not a tag exists locally.""" return bool(git("tag", "--list", version, repo=self._dir())) @@ -132,6 +139,28 @@ def _versions(self, min_age: Optional[timedelta] = None) -> Dict[str, str]: versions = toml.loads(contents.decoded_content.decode()) return {v: versions[v]["git-tree-sha1"] for v in versions} + def _can_fast_forward(self, master: str, branch: str) -> bool: + """Check whether master can be fast-forwarded to branch.""" + return git_check( + "merge-base", "--is-ancestor", master, branch, repo=self._dir() + ) + + def _merge_and_delete_branch(self, master: str, branch: str) -> None: + """Merge a branch into master and delete the branch.""" + git("checkout", master, repo=self._dir()) + git("merge", branch, repo=self._dir()) + git("push", "origin", master, repo=self._dir()) + git("push", "-d", "origin", branch, repo=self._dir()) + + def _create_release_branch_pr(self, version: str, master: str, branch: str) -> None: + """Create a pull request for the release branch.""" + self.__repo.create_pull( + title=f"Merge release branch for {version}", + body="", + head=branch, + base=master, + ) + def new_versions(self) -> Dict[str, str]: """Get all new versions of the package.""" current = self._versions() @@ -153,6 +182,20 @@ def create_dispatch_event(self, payload: Dict[str, Any]) -> None: ) debug(f"Dispatch response code: {resp.status_code}") + def handle_release_branch(self, version: str) -> None: + """Merge an existing release branch or create a PR to merge it.""" + master = self.__repo.default_branch + branch = f"release-{version[1:]}" + if not self._fetch_branch(master, branch): + info(f"Release branch {branch} does not exist") + return + if self._can_fast_forward(master, branch): + info("Release branch can be fast-forwarded") + self._merge_and_delete_branch(master, branch) + else: + info("Release branch cannot be fast-forwarded, creating pull request") + self._create_release_branch_pr(version, master, branch) + def changelog(self, version: str) -> Optional[str]: """Get the changelog for a new version.""" return get_changelog( diff --git a/test/test_repo.py b/test/test_repo.py index 85383d23..1b7a6fe0 100644 --- a/test/test_repo.py +++ b/test/test_repo.py @@ -1,5 +1,3 @@ -import tagbot - from datetime import datetime, timedelta from io import StringIO from unittest.mock import Mock, call, patch @@ -56,6 +54,18 @@ def test_commit_from_tree(git): assert r._commit_from_tree("c") is None +@patch("tagbot.repo.git_check", side_effect=[True, False]) +@patch("tagbot.repo.git", return_value="") +def test_fetch_branch(git, git_check): + r = Repo("", "", "") + r._dir = lambda: "dir" + assert r._fetch_branch("master", "foo") + git_check.assert_called_with("checkout", "foo", repo="dir") + git.assert_called_with("checkout", "master", repo="dir") + assert not r._fetch_branch("master", "bar") + git_check.assert_called_with("checkout", "bar", repo="dir") + + @patch("tagbot.repo.git", side_effect=["v1.2.3", ""]) def test_tag_exists(git): r = Repo("", "", "") @@ -155,6 +165,44 @@ def test_versions(debug, Github): debug.assert_called_with("Versions.toml was not found") +@patch("tagbot.repo.git_check", side_effect=[True, False]) +def test_can_fast_forward(git_check): + r = Repo("", "", "") + r._dir = lambda: "dir" + assert r._can_fast_forward("master1", "branch1") + git_check.assert_called_with( + "merge-base", "--is-ancestor", "master1", "branch1", repo="dir" + ) + assert not r._can_fast_forward("master2", "branch2") + git_check.assert_called_with( + "merge-base", "--is-ancestor", "master2", "branch2", repo="dir", + ) + + +@patch("tagbot.repo.git") +def test_merge_and_delete_branch(git): + r = Repo("", "", "") + r._dir = lambda: "dir" + r._merge_and_delete_branch("master", "branch") + git.assert_has_calls( + [ + call("checkout", "master", repo="dir"), + call("merge", "branch", repo="dir"), + call("push", "origin", "master", repo="dir"), + call("push", "-d", "origin", "branch", repo="dir"), + ] + ) + + +@patch("tagbot.repo.Github") +def test_create_release_branch_pr(Github): + r = Repo("", "", "") + r._create_release_branch_pr("v1.2.3", "master", "branch") + r._Repo__repo.create_pull.assert_called_once_with( + title="Merge release branch for v1.2.3", body="", head="branch", base="master", + ) + + def test_new_versions(): r = Repo("", "", "") r._versions = ( @@ -166,6 +214,24 @@ def test_new_versions(): assert r.new_versions() == {"2.3.4": "bcd"} +@patch("tagbot.repo.Github") +def test_handle_release_branch(Github): + r = Repo("", "", "") + r._Repo__repo.default_branch = "master" + r._fetch_branch = Mock(side_effect=[False, True, True]) + r._can_fast_forward = Mock(side_effect=[True, False]) + r._merge_and_delete_branch = Mock() + r._create_release_branch_pr = Mock() + r.handle_release_branch("v1.2.3") + r._fetch_branch.assert_called_with("master", "release-1.2.3") + r.handle_release_branch("v2.3.4") + r._merge_and_delete_branch.assert_called_once_with("master", "release-2.3.4") + r.handle_release_branch("v3.4.5") + r._create_release_branch_pr.assert_called_once_with( + "v3.4.5", "master", "release-3.4.5" + ) + + @patch("tagbot.repo.Github") @patch("requests.post") def test_create_dispatch_event(post, Github): diff --git a/test/test_tagbot.py b/test/test_tagbot.py index 4aee4fce..411fa697 100644 --- a/test/test_tagbot.py +++ b/test/test_tagbot.py @@ -1,4 +1,4 @@ -from unittest.mock import call, patch +from unittest.mock import Mock, call, patch import tagbot as tb @@ -15,11 +15,8 @@ def test_loggers(print): print.assert_has_calls(calls) -@patch("subprocess.run") +@patch("subprocess.run", return_value=Mock(stdout=b"hello\n", stderr=b"", returncode=0)) def test_git(run): - run.return_value.stdout = b"hello\n" - run.return_value.stderr = b"" - run.return_value.returncode = 0 assert tb.git("a", "b") == "hello" assert tb.git("c", "d", repo=None) == "hello" assert tb.git("e", "f", repo="foo") @@ -29,3 +26,11 @@ def test_git(run): call(["git", "-C", "foo", "e", "f"], capture_output=True), ] run.assert_has_calls(calls) + + +@patch("tagbot.git", side_effect=["", tb.Abort()]) +def test_git_check(git): + assert tb.git_check("a") is True + git.assert_called_with("a", repo=None) + assert tb.git_check("b", repo="dir") is False + git.assert_called_with("b", repo="dir")