diff --git a/README.md b/README.md index af2f4ad..319caeb 100644 --- a/README.md +++ b/README.md @@ -6,6 +6,7 @@ Utility script that synchronizes a local repository with all remotes: - fast-forwards all local branches that have a remote upstream - pushes upstream changes to the default push remote - fast-forwards or deletes branches associated with merged PRs to the merge commit + - deletes merged fork branches from your push remote if they are unchanged since the merge Not all git configuration is taken into account; please open an issue if this causes problems. @@ -27,6 +28,8 @@ Squash commits break this model, as they deliberately do not record the informat If you have `remote.pushdefault` set and `$GITHUB_TOKEN` in your environment, `git sync` will query the last 50 PRs from each remote, and if it finds a merge commit for a local branch, will fast-forward that branch to the merge commit. This gives git enough information to reenable the safer workflows. +GitHub can be configured to delete the PR branch on merge, but not everything that merges PRs respects that setting. `git sync` will delete any leftover merged PR branch from your push remote, provided the branch is still at the commit it was merged at, and the PR was raised from a fork (that is, the branch is in your own repository, not one shared with colleagues). + ## Installing diff --git a/git_sync/__init__.py b/git_sync/__init__.py index 6a0c412..de4ea17 100644 --- a/git_sync/__init__.py +++ b/git_sync/__init__.py @@ -6,6 +6,7 @@ from .git import ( GitError, Remote, + delete_merged_remote_branches, fast_forward_to_downstream, fetch_and_fast_forward_to_upstream, get_branches_with_remote_upstreams, @@ -39,7 +40,7 @@ def get_description( description += "\nPulls all remotes, including updating the current branch if safe." if push_remote and any(remote.name != push_remote for remote in remotes): description += f"\nPushes any upstream changes to {push_remote.decode()}." - if domains: + if domains and push_remote: domains_with_tokens = sorted( domain for domain in domains if github_token(domain) ) @@ -49,6 +50,8 @@ def get_description( if domains_with_tokens: description += ( "\nMerged PR branches will be fast-forwarded or, if safe, deleted." + f"\nMerged fork branches will be deleted from {push_remote.decode()}" + " if unchanged since the merge." ) if domains_without_tokens: description += "\n" @@ -122,6 +125,10 @@ async def git_sync() -> None: await update_merged_prs( push_remote_url, pull_requests, allow_delete=args.allow_delete ) + if args.allow_delete: + await delete_merged_remote_branches( + push_remote, push_remote_url, pull_requests + ) def main() -> None: diff --git a/git_sync/git.py b/git_sync/git.py index 68afe50..0751112 100644 --- a/git_sync/git.py +++ b/git_sync/git.py @@ -208,3 +208,39 @@ async def update_merged_prs( merged_hash=merged_hash, allow_delete=allow_delete, ) + + +def is_stale_merged_fork_branch(pr: PullRequest, push_remote_url: str) -> bool: + """Return true if the PR branch is in our fork and unchanged since the merge. + + Restricted to cross-repository PRs so that we never delete a branch from a + repository shared with colleagues. + """ + return ( + pr.merged_hash is not None + and pr.is_cross_repository + and push_remote_url in pr.repo_urls + and bool(pr.hashes) + and pr.head_hash == pr.hashes[0] + ) + + +async def delete_merged_remote_branches( + push_remote: bytes, push_remote_url: str, prs: Iterable[PullRequest] +) -> None: + """Delete branches of merged PRs from our fork. + + GitHub is normally configured to do this on merge, but not every merge respects + that setting. + """ + branch_names = [ + pr.branch_name for pr in prs if is_stale_merged_fork_branch(pr, push_remote_url) + ] + if not branch_names: + return + remote = push_remote.decode() + for branch_name in branch_names: + print(f"Deleting {branch_name} from {remote}") + # A branch may have been deleted since we queried the API; git will report + # any failures itself, and the rest of the deletions still apply + await git("push", push_remote, "--delete", *branch_names, check_return=False) diff --git a/git_sync/github.py b/git_sync/github.py index 4f7684d..e2bddca 100644 --- a/git_sync/github.py +++ b/git_sync/github.py @@ -60,6 +60,10 @@ class PullRequest: """All commits pushed to the PR, newest first.""" merged_hash: str | None """The commit hash of the PR merge commit, if it exists.""" + head_hash: str | None = None + """The current commit hash of the PR branch, or None if it has been deleted.""" + is_cross_repository: bool = False + """Whether the PR branch is in a different repository to the PR target.""" COMMITS_PAGE_SIZE = 100 @@ -94,6 +98,12 @@ def pr_query(owner: str, name: str) -> str: sshUrl url }} + headRef {{ + target {{ + oid + }} + }} + isCrossRepository {commits_query()} mergeCommit {{ oid @@ -174,6 +184,8 @@ class PartialPullRequest: branch_name: str repo_urls: frozenset[str] merged_hash: str | None + head_hash: str | None = None + is_cross_repository: bool = False oids: list[str] = field(default_factory=list) """Commits fetched so far, oldest first.""" cursor: str | None = None @@ -195,6 +207,8 @@ def to_pull_request(self) -> PullRequest: repo_urls=self.repo_urls, hashes=tuple(reversed(self.oids)), merged_hash=self.merged_hash, + head_hash=self.head_hash, + is_cross_repository=self.is_cross_repository, ) @@ -204,6 +218,8 @@ def partial_pull_request(pr_data: dict[str, Any]) -> PartialPullRequest: branch_name=pr_data["headRefName"], repo_urls=frozenset(repo_urls(pr_data)), merged_hash=(pr_data.get("mergeCommit") or {}).get("oid"), + head_hash=((pr_data.get("headRef") or {}).get("target") or {}).get("oid"), + is_cross_repository=bool(pr_data.get("isCrossRepository")), ) pr.add_page(pr_data["commits"]) return pr diff --git a/pyproject.toml b/pyproject.toml index 494125f..67be574 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "git-sync" -version = "0.5.1" +version = "0.6.0" description = "Synchronize local git repo with remotes" authors = [{ name = "Alice Purcell", email = "alicederyn@gmail.com" }] requires-python = ">= 3.12" diff --git a/tests/integration/gitutils.py b/tests/integration/gitutils.py index 9fb1e1b..8405c0e 100644 --- a/tests/integration/gitutils.py +++ b/tests/integration/gitutils.py @@ -78,6 +78,29 @@ def setup_upstreams(**upstreams: str) -> None: ) +def setup_remote(name: str, **branches: str) -> None: + """Create a bare repository as a remote, with the given branches pushed to it.""" + remote_path = Path.cwd().parent / name + subprocess.run(["git", "init", "--bare", "-q", str(remote_path)], check=True) + subprocess.run(["git", "remote", "add", name, str(remote_path)], check=True) + for branch_name, commit_hash in branches.items(): + subprocess.run( + ["git", "push", "-q", name, f"{commit_hash}:refs/heads/{branch_name}"], + check=True, + ) + subprocess.run(["git", "fetch", "-q", name], check=True) + + +def remote_branches(name: str) -> dict[str, str]: + lines = run_split_stdout( + ["git", "ls-remote", "--heads", name], + ) + return { + ref.removeprefix("refs/heads/"): commit_hash + for commit_hash, ref in (line.split("\t") for line in lines) + } + + def all_branches() -> dict[str, str]: return { branch_name: subprocess.run( diff --git a/tests/integration/test_delete_merged_remote_branches.py b/tests/integration/test_delete_merged_remote_branches.py new file mode 100644 index 0000000..c025e03 --- /dev/null +++ b/tests/integration/test_delete_merged_remote_branches.py @@ -0,0 +1,142 @@ +from git_sync.git import delete_merged_remote_branches +from git_sync.github import PullRequest + +from .gitutils import ( + create_commit, + remote_branches, + setup_branches, + setup_remote, + squash_merge, +) + +FORK_URL = "https://github.com/me/example.git" # Dummy URL for test +PUSH_REMOTE = b"origin" + + +def merged_fork_pr(*, hashes: tuple[str, ...], head_hash: str | None) -> PullRequest: + return PullRequest( + branch_name="my_pr", + repo_urls=frozenset([FORK_URL]), + hashes=hashes, + merged_hash="merge", + head_hash=head_hash, + is_cross_repository=True, + ) + + +async def test_delete_unchanged_merged_fork_branch() -> None: + # Given a merged PR whose fork branch is still at the merged commit + commit_a = create_commit("main", file="A\n") + commit_b = create_commit(commit_a, file="B\n") + commit_c = squash_merge(commit_a, commit_b) + setup_branches(main=commit_c, active_branch="main") + setup_remote("origin", main=commit_c, my_pr=commit_b) + pr = merged_fork_pr(hashes=(commit_b,), head_hash=commit_b) + + # When we delete merged remote branches + await delete_merged_remote_branches(PUSH_REMOTE, FORK_URL, [pr]) + + # Then the fork branch is deleted + assert remote_branches("origin") == {"main": commit_c} + + +async def test_keep_fork_branch_with_commits_added_since_merge() -> None: + # Given a merged PR whose fork branch has moved on + commit_a = create_commit("main", file="A\n") + commit_b = create_commit(commit_a, file="B\n") + commit_c = squash_merge(commit_a, commit_b) + commit_d = create_commit(commit_b, file="C\n") + setup_branches(main=commit_c, active_branch="main") + setup_remote("origin", main=commit_c, my_pr=commit_d) + pr = merged_fork_pr(hashes=(commit_b,), head_hash=commit_d) + + # When we delete merged remote branches + await delete_merged_remote_branches(PUSH_REMOTE, FORK_URL, [pr]) + + # Then the fork branch is left alone + assert remote_branches("origin") == {"main": commit_c, "my_pr": commit_d} + + +async def test_keep_already_deleted_fork_branch() -> None: + # Given a merged PR whose branch has already been deleted + commit_a = create_commit("main", file="A\n") + commit_b = create_commit(commit_a, file="B\n") + commit_c = squash_merge(commit_a, commit_b) + setup_branches(main=commit_c, active_branch="main") + setup_remote("origin", main=commit_c, my_pr=commit_b) + pr = merged_fork_pr(hashes=(commit_b,), head_hash=None) + + # When we delete merged remote branches + await delete_merged_remote_branches(PUSH_REMOTE, FORK_URL, [pr]) + + # Then the identically named remote branch is left alone + assert remote_branches("origin") == {"main": commit_c, "my_pr": commit_b} + + +async def test_keep_branch_from_same_repository() -> None: + """A branch in the PR's target repository may belong to a colleague.""" + # Given a merged PR raised from a branch in the target repository + commit_a = create_commit("main", file="A\n") + commit_b = create_commit(commit_a, file="B\n") + commit_c = squash_merge(commit_a, commit_b) + setup_branches(main=commit_c, active_branch="main") + setup_remote("origin", main=commit_c, my_pr=commit_b) + pr = PullRequest( + branch_name="my_pr", + repo_urls=frozenset([FORK_URL]), + hashes=(commit_b,), + merged_hash="merge", + head_hash=commit_b, + is_cross_repository=False, + ) + + # When we delete merged remote branches + await delete_merged_remote_branches(PUSH_REMOTE, FORK_URL, [pr]) + + # Then the branch is left alone + assert remote_branches("origin") == {"main": commit_c, "my_pr": commit_b} + + +async def test_keep_branch_in_another_repository() -> None: + # Given a merged PR raised from a fork that is not our push remote + commit_a = create_commit("main", file="A\n") + commit_b = create_commit(commit_a, file="B\n") + commit_c = squash_merge(commit_a, commit_b) + setup_branches(main=commit_c, active_branch="main") + setup_remote("origin", main=commit_c, my_pr=commit_b) + pr = PullRequest( + branch_name="my_pr", + repo_urls=frozenset(["https://github.com/someone-else/example.git"]), + hashes=(commit_b,), + merged_hash="merge", + head_hash=commit_b, + is_cross_repository=True, + ) + + # When we delete merged remote branches + await delete_merged_remote_branches(PUSH_REMOTE, FORK_URL, [pr]) + + # Then the identically named branch on our push remote is left alone + assert remote_branches("origin") == {"main": commit_c, "my_pr": commit_b} + + +async def test_keep_unmerged_fork_branch() -> None: + # Given an unmerged PR + commit_a = create_commit("main", file="A\n") + commit_b = create_commit(commit_a, file="B\n") + setup_branches(main=commit_a, active_branch="main") + setup_remote("origin", main=commit_a, my_pr=commit_b) + pr = PullRequest( + branch_name="my_pr", + repo_urls=frozenset([FORK_URL]), + hashes=(commit_b,), + merged_hash=None, + head_hash=commit_b, + is_cross_repository=True, + ) + + # When we delete merged remote branches + await delete_merged_remote_branches(PUSH_REMOTE, FORK_URL, [pr]) + + # Then the fork branch is left alone + assert remote_branches("origin") == {"main": commit_a, "my_pr": commit_b} diff --git a/tests/test_github.py b/tests/test_github.py index 84f3588..721a9e8 100644 --- a/tests/test_github.py +++ b/tests/test_github.py @@ -74,6 +74,8 @@ async def test_successful_fetch_with_multiple_repos_and_prs( "sshUrl": "git@github.com:owner1/repo1.git", "url": "https://github.com/owner1/repo1", }, + "headRef": {"target": {"oid": "commit3"}}, + "isCrossRepository": True, "commits": commits("commit1", "commit2", "commit3"), "mergeCommit": {"oid": "merge1"}, }, @@ -84,6 +86,8 @@ async def test_successful_fetch_with_multiple_repos_and_prs( "sshUrl": "git@github.com:owner1/repo1.git", "url": "https://github.com/owner1/repo1", }, + "headRef": {"target": {"oid": "commit4"}}, + "isCrossRepository": False, "commits": commits("commit4"), "mergeCommit": None, }, @@ -100,6 +104,8 @@ async def test_successful_fetch_with_multiple_repos_and_prs( "sshUrl": "git@github.com:owner2/repo2.git", "url": "https://github.com/owner2/repo2", }, + "headRef": None, # Branch deleted on merge + "isCrossRepository": True, "commits": commits("commit5", "commit6"), "mergeCommit": {"oid": "merge2"}, }, @@ -130,6 +136,8 @@ async def test_successful_fetch_with_multiple_repos_and_prs( ), hashes=("commit3", "commit2", "commit1"), # Newest first merged_hash="merge1", + head_hash="commit3", + is_cross_repository=True, ) # Second PR - without merge commit @@ -144,6 +152,8 @@ async def test_successful_fetch_with_multiple_repos_and_prs( ), hashes=("commit4",), merged_hash=None, + head_hash="commit4", + is_cross_repository=False, ) # Third PR - from different repo @@ -158,6 +168,8 @@ async def test_successful_fetch_with_multiple_repos_and_prs( ), hashes=("commit6", "commit5"), merged_hash="merge2", + head_hash=None, + is_cross_repository=True, )