From b464239c2e332d673e4da97447eb723a35eea066 Mon Sep 17 00:00:00 2001 From: Alice Purcell Date: Mon, 14 Jul 2025 08:42:10 +0100 Subject: [PATCH 1/3] Remove outdated Ruff ANN101 ignore This rule was removed from Ruff and is now generating a warning when we ignore it. --- pyproject.toml | 3 --- 1 file changed, 3 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index a0a84f7..da90760 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -22,9 +22,6 @@ target-version = "py310" [tool.ruff.lint] select = ["ANN", "B", "C4", "E", "F", "I", "PGH", "PLR", "PYI", "RUF", "SIM", "UP", "W"] -ignore = [ - "ANN101", # Deprecated, will be removed -] isort.split-on-trailing-comma = false [tool.setuptools.dynamic] From 1bf9673c3c57062ca4385e13105d439a2b7e86f8 Mon Sep 17 00:00:00 2001 From: Alice Purcell Date: Mon, 14 Jul 2025 09:22:53 +0100 Subject: [PATCH 2/3] Fetch PR commits from GitHub We were relying on the local git repo to determine ancestry; however, if a commit was pushed directly to a PR (for instance, a merge commit generated by GitHub), it is not necessarily available locally. Instead, pull the commit history from GitHub itself. --- git_sync/git.py | 29 ++---- git_sync/github.py | 97 +++++++++++++------ .../test_fast_forward_merged_prs.py | 20 ++-- tests/integration/test_is_ancestor.py | 13 --- 4 files changed, 82 insertions(+), 77 deletions(-) delete mode 100644 tests/integration/test_is_ancestor.py diff --git a/git_sync/git.py b/git_sync/git.py index 44e4dc4..3ce18cc 100644 --- a/git_sync/git.py +++ b/git_sync/git.py @@ -106,17 +106,6 @@ async def get_remote_branches(remote: bytes) -> list[bytes]: return raw_bytes.splitlines() -async def is_ancestor(commit1: _ExecArg, commit2: _ExecArg) -> bool: - """Return true if commit1 is an ancestor of commit2.""" - try: - await git("merge-base", "--is-ancestor", commit1, commit2) - return True - except GitError as e: - if e.returncode == 1: - return False - raise - - async def fetch_and_fast_forward_to_upstream(branches: Iterable[Branch]) -> None: if any(b.is_current for b in branches): await git("pull", "--all") @@ -211,17 +200,11 @@ async def update_merged_prs( if ( merged_hash and branch_name in branch_hashes - and merged_hash != branch_hashes[branch_name] + and branch_hashes[branch_name] in pr.hashes and push_remote_url in pr.repo_urls ): - try: - branch_is_ancestor = await is_ancestor(branch_name, pr.branch_hash) - except GitError: - pass # Probably no longer have the commit hash - else: - if branch_is_ancestor: - await update_merged_pr_branch( - branch_name=branch_name, - merged_hash=merged_hash, - allow_delete=allow_delete, - ) + await update_merged_pr_branch( + branch_name=branch_name, + merged_hash=merged_hash, + allow_delete=allow_delete, + ) diff --git a/git_sync/github.py b/git_sync/github.py index 1859323..5f1cf78 100644 --- a/git_sync/github.py +++ b/git_sync/github.py @@ -49,37 +49,58 @@ def repos_by_domain(urls: Iterable[str]) -> dict[str, list[Repository]]: @dataclass(frozen=True) class PullRequest: branch_name: str + """Name of the branch that backed the PR.""" repo_urls: frozenset[str] - branch_hash: str + """Git and SSH URLs of the repository where the PR is located.""" + hashes: tuple[str, ...] + """All commits pushed to the PR, newest first.""" merged_hash: str | None + """The commit hash of the PR merge commit, if it exists.""" -def gql_query(owner: str, name: str) -> str: +def pr_initial_query(owner: str, name: str) -> str: return f""" repository(owner: "{owner}", name: "{name}" ) {{ pullRequests(orderBy: {{ field: UPDATED_AT, direction: ASC }}, last: 50) {{ nodes {{ - headRefName - headRepository {{ - sshUrl - url - }} + id commits (last: 1) {{ - nodes {{ - commit {{ - oid - }} - }} + totalCount }} - mergeCommit {{ - oid + }} + }} + }} + """ + + +def pr_details_query(pr_node_id: str, commit_count: int) -> str: + return f""" + node(id: "{pr_node_id}") {{ + ... on PullRequest {{ + headRefName + headRepository {{ + sshUrl + url + }} + commits (last: {commit_count}) {{ + nodes {{ + commit {{ + oid + }} }} }} + mergeCommit {{ + oid + }} }} }} """ +def join_queries(queries: Iterable[str]) -> str: + return "{" + "\n".join(f"q{i}: {query}" for i, query in enumerate(queries)) + "}" + + async def fetch_pull_requests_from_domain( token: str, domain: str, repos: list[Repository] ) -> AsyncIterator[PullRequest]: @@ -91,24 +112,38 @@ async def fetch_pull_requests_from_domain( client = GraphQLClient( endpoint=endpoint, headers={"Authorization": f"Bearer {token}"} ) - queries = [ - f"repo{i}: {gql_query(repo.owner, repo.name)}" - for i, repo in enumerate(repos, 1) + + # Query for PRs and commit counts + initial_queries = [ + pr_initial_query(repo.owner, repo.name) for i, repo in enumerate(repos, 1) ] - query = "{" + "\n".join(queries) + "}" - response = await client.query(query) - assert not response.errors - for repo_data in response.data.values(): - for pr_data in repo_data["pullRequests"]["nodes"]: - head_repo = pr_data.get("headRepository") or {} - repo_urls = [head_repo.get("sshUrl"), head_repo.get("url")] - if pr_data["commits"]["nodes"]: - yield PullRequest( - branch_name=pr_data["headRefName"], - repo_urls=frozenset(url for url in repo_urls if url is not None), - branch_hash=pr_data["commits"]["nodes"][0]["commit"]["oid"], - merged_hash=(pr_data.get("mergeCommit") or {}).get("oid"), - ) + initial_response = await client.query(join_queries(initial_queries)) + assert not initial_response.errors + + # Determine what follow-up queries to make + details_queries = [ + pr_details_query(pr_data["id"], pr_data["commits"]["totalCount"]) + for repo_data in initial_response.data.values() + for pr_data in repo_data["pullRequests"]["nodes"] + ] + + # Query for detailed PR information + details_response = await client.query(join_queries(details_queries)) + assert not details_response.errors + + # Yield response data as PullRequest objects + for pr_data in details_response.data.values(): + head_repo = pr_data.get("headRepository") or {} + repo_urls = [head_repo.get("sshUrl"), head_repo.get("url")] + hashes = tuple( + commit["commit"]["oid"] for commit in reversed(pr_data["commits"]["nodes"]) + ) + yield PullRequest( + branch_name=pr_data["headRefName"], + repo_urls=frozenset(url for url in repo_urls if url is not None), + hashes=hashes, + merged_hash=(pr_data.get("mergeCommit") or {}).get("oid"), + ) async def fetch_pull_requests( diff --git a/tests/integration/test_fast_forward_merged_prs.py b/tests/integration/test_fast_forward_merged_prs.py index 69981f1..e3df031 100644 --- a/tests/integration/test_fast_forward_merged_prs.py +++ b/tests/integration/test_fast_forward_merged_prs.py @@ -30,7 +30,7 @@ async def test_delete_merged_inactive_pr_branch() -> None: pr = PullRequest( branch_name="my_pr", repo_urls=frozenset([REPO_URL]), - branch_hash=commit_b, + hashes=(commit_b,), merged_hash=commit_c, ) @@ -57,7 +57,7 @@ async def test_force_inactive_upstream_branch_to_merged_commit() -> None: pr = PullRequest( branch_name="my_pr", repo_urls=frozenset([REPO_URL]), - branch_hash=commit_b, + hashes=(commit_b,), merged_hash=commit_c, ) @@ -85,7 +85,7 @@ async def test_merged_inactive_pr_branch_with_deletion_disabled() -> None: pr = PullRequest( branch_name="my_pr", repo_urls=frozenset([REPO_URL]), - branch_hash=commit_b, + hashes=(commit_b,), merged_hash=commit_c, ) @@ -113,7 +113,7 @@ async def test_delete_merged_active_pr_branch() -> None: pr = PullRequest( branch_name="my_pr", repo_urls=frozenset([REPO_URL]), - branch_hash=commit_b, + hashes=(commit_b,), merged_hash=commit_c, ) @@ -142,7 +142,7 @@ async def test_force_active_upstream_branch_to_merged_commit() -> None: pr = PullRequest( branch_name="my_pr", repo_urls=frozenset([REPO_URL]), - branch_hash=commit_b, + hashes=(commit_b,), merged_hash=commit_c, ) @@ -170,7 +170,7 @@ async def test_merged_active_upstream_branch_with_deletion_disabled() -> None: pr = PullRequest( branch_name="my_pr", repo_urls=frozenset([REPO_URL]), - branch_hash=commit_b, + hashes=(commit_b,), merged_hash=commit_c, ) @@ -195,7 +195,7 @@ async def test_staged_changes_not_lost() -> None: pr = PullRequest( branch_name="my_pr", repo_urls=frozenset([REPO_URL]), - branch_hash=commit_b, + hashes=(commit_b,), merged_hash=commit_c, ) @@ -221,7 +221,7 @@ async def test_unstaged_changes_to_committed_files_not_lost() -> None: pr = PullRequest( branch_name="my_pr", repo_urls=frozenset([REPO_URL]), - branch_hash=commit_b, + hashes=(commit_b,), merged_hash=commit_c, ) @@ -251,7 +251,7 @@ async def test_fastforward_when_pr_had_additional_commits() -> None: pr = PullRequest( branch_name="my_pr", repo_urls=frozenset([REPO_URL]), - branch_hash=commit_c, + hashes=(commit_c, commit_b), merged_hash=commit_d, ) @@ -276,7 +276,7 @@ async def test_no_fastforward_when_branch_has_additional_commits() -> None: pr = PullRequest( branch_name="my_pr", repo_urls=frozenset([REPO_URL]), - branch_hash=commit_b, + hashes=(commit_b,), merged_hash=commit_d, ) diff --git a/tests/integration/test_is_ancestor.py b/tests/integration/test_is_ancestor.py deleted file mode 100644 index c8b68e9..0000000 --- a/tests/integration/test_is_ancestor.py +++ /dev/null @@ -1,13 +0,0 @@ -from git_sync.git import is_ancestor - -from .gitutils import create_commit - - -async def test_is_ancestor_simple() -> None: - # Given two commits - base = create_commit("HEAD", foo="a") - second = create_commit(base, foo="b") - # Then base is an ancestor of second - assert await is_ancestor(base, second) is True - # And second is not an ancestor of base - assert await is_ancestor(second, base) is False From dbc43fc83ef6046b842b045bd6519f84ac6a2988 Mon Sep 17 00:00:00 2001 From: Alice Purcell Date: Mon, 14 Jul 2025 09:39:03 +0100 Subject: [PATCH 3/3] Point release --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index da90760..2c46e4c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "git-sync" -version = "0.4" +version = "0.4.1" description = "Synchronize local git repo with remotes" authors = [{ name = "Alice Purcell", email = "alicederyn@gmail.com" }] requires-python = ">= 3.12"