From e3d23d89e08d429d82cd3764c9e429417c75d3aa Mon Sep 17 00:00:00 2001 From: bc1cindy Date: Thu, 8 Oct 2026 13:43:21 -0300 Subject: [PATCH 1/2] Find where a series forked from a rewritten base A parent rewritten in a stack leaves its old tips unreachable, so they are read from the repository activity log in place of a reflog. --- README.md | 4 ++- action.yml | 2 +- post-range-diff-comment.sh | 30 +++++++++++++++++++---- tests/test_action.py | 50 +++++++++++++++++++++++++++++++++----- 4 files changed, 73 insertions(+), 13 deletions(-) diff --git a/README.md b/README.md index 3be8301..8e9c954 100644 --- a/README.md +++ b/README.md @@ -22,7 +22,9 @@ jobs: Fast-forward pushes are skipped. Each side is diffed against its own merge base with the base branch, so a rebase onto a newer base is not mistaken for -changes. Inputs and outputs are described in [action.yml](action.yml). +changes. A base branch that was itself rewritten, as a parent in a stack, is +recognized from its force pushes in the repository's activity log. Inputs and +outputs are described in [action.yml](action.yml). `pull_request_target` gives fork PRs a token that can comment. It is safe here because nothing from the PR runs: the checkout is the base branch, the action diff --git a/action.yml b/action.yml index a8258fe..0d9d212 100644 --- a/action.yml +++ b/action.yml @@ -7,7 +7,7 @@ branding: color: purple inputs: token: - description: "Token with `pull-requests: write`." + description: "Token with `contents: read` and `pull-requests: write`." default: ${{ github.token }} before: description: Head commit before the push. diff --git a/post-range-diff-comment.sh b/post-range-diff-comment.sh index 1e085d0..ff79727 100755 --- a/post-range-diff-comment.sh +++ b/post-range-diff-comment.sh @@ -6,7 +6,7 @@ # BEFORE, AFTER head commit before and after the push # BASE_REF the pull request's base branch # PR_NUMBER the pull request number -# GH_TOKEN token with pull-requests: write +# GH_TOKEN token with contents: read and pull-requests: write # CREATION_FACTOR optional, passed to --creation-factor # MAX_COMMENT_BYTES cap on the comment body # POST_COMMENT "false" to render without posting @@ -42,15 +42,35 @@ if git merge-base --is-ancestor "$BEFORE" "$AFTER"; then fi echo "forced=true" >>"$OUT" +# When the base branch was itself rewritten, as a parent in a stack, its old +# tips are unreachable from it, and a series left on one would seem to fork +# from further back. GitHub's activity log stands in for the reflog that +# `git merge-base --fork-point` reads. +base_tips=("origin/$BASE_REF") +if rewound="$(gh api --paginate --method GET --jq '.[].before' "repos/$REPO/activity" \ + -f ref="refs/heads/$BASE_REF" -f activity_type=force_push -F per_page=100)"; then + for tip in $rewound; do + if git fetch --quiet --no-tags origin "$tip" 2>/dev/null; then + base_tips+=("$tip") + fi + done +else + echo "::warning::could not list force pushes of $BASE_REF; if it was rewritten, its old commits will be counted in the series" +fi + # Diff each side against where it forked from the base branch, so a rebase # onto a newer base does not drag the base's new commits into the comparison. -# Fall back to the symmetric difference if either side is unrelated to base. -if old_base="$(git merge-base "origin/$BASE_REF" "$BEFORE")" && - new_base="$(git merge-base "origin/$BASE_REF" "$AFTER")"; then +# Fall back to the symmetric difference if either side is unrelated to base, +# or if a tip of the base contains it: its range would be empty, which +# range-diff rejects. +if old_base="$(git merge-base "$BEFORE" "${base_tips[@]}")" && + new_base="$(git merge-base "$AFTER" "${base_tips[@]}")" && + [[ $old_base != "$(git rev-parse "$BEFORE^{commit}")" ]] && + [[ $new_base != "$(git rev-parse "$AFTER^{commit}")" ]]; then old_range="$old_base..$BEFORE" new_range="$new_base..$AFTER" else - echo "::warning::no merge base with $BASE_REF; using $BEFORE...$AFTER" + echo "::warning::no fork point from $BASE_REF; using $BEFORE...$AFTER" old_range="$AFTER..$BEFORE" new_range="$BEFORE..$AFTER" fi diff --git a/tests/test_action.py b/tests/test_action.py index 579ed97..ec499df 100644 --- a/tests/test_action.py +++ b/tests/test_action.py @@ -35,13 +35,27 @@ def git(self, *args, cwd=None): ["git", *args], cwd=cwd or self.origin, env=self.env, text=True ).strip() - def commit(self, content, message, amend=False): - (self.origin / "file").write_text(content) - self.git("add", "file") + def commit(self, content, message, amend=False, path="file"): + (self.origin / path).write_text(content) + self.git("add", path) self.git("commit", "-q", "-m", message, *(["--amend"] if amend else [])) return self.git("rev-parse", "HEAD") - def run_action(self, *, shallow=False, packaged=False, forced=True): + def run_action( + self, + *, + shallow=False, + packaged=False, + forced=True, + base_ref="main", + rewound=(), + ): + # Stands in for the activity log: prints the old tips of the base branch. + stub = self.root / "stub" + stub.mkdir() + gh = stub / "gh" + gh.write_text("#!/bin/sh\n" + "".join(f"echo {tip}\n" for tip in rewound)) + gh.chmod(0o755) checkout = self.root / "checkout" self.git( "clone", @@ -57,13 +71,14 @@ def run_action(self, *, shallow=False, packaged=False, forced=True): self.env, BEFORE=self.before, AFTER=self.after, - BASE_REF="main", + BASE_REF=base_ref, PR_NUMBER="1", GITHUB_REPOSITORY="test/repo", GITHUB_SERVER_URL="https://github.com", GITHUB_OUTPUT=str(output), GITHUB_STEP_SUMMARY=str(summary), POST_COMMENT="false", + PATH=f"{stub}{os.pathsep}{os.environ['PATH']}", ) command = ( [PACKAGE] @@ -86,13 +101,36 @@ def run_action(self, *, shallow=False, packaged=False, forced=True): self.assertIn("range-diff", result.stdout) self.assertIn(self.before, result.stdout) self.assertIn(self.after, result.stdout) - self.assertEqual(summary.read_text(), result.stdout) + self.assertTrue(result.stdout.endswith(summary.read_text())) else: self.assertFalse(summary.exists()) + return result.stdout def test_force_push_full_checkout(self): self.run_action() + def test_rewritten_parent_in_a_stack(self): + self.git("checkout", "-q", "-b", "child") + self.before = self.commit("child\n", "child", path="child") + self.git("checkout", "-q", "topic") + old_parent = self.after + new_parent = self.commit("newer\n", "change", amend=True) + self.git("checkout", "-q", "child") + self.git("rebase", "-q", "--onto", "topic", "child~1") + self.after = self.git("rev-parse", "HEAD") + collected = "0123456789" * 4 + out = self.run_action(base_ref="topic", rewound=[collected, old_parent]) + self.assertIn(f"git range-diff {old_parent[:7]}", out) + self.assertIn(f"{new_parent[:7]}..{self.after[:7]}", out) + self.assertIn(": 1 unchanged", out) + + def test_series_an_old_base_tip_contained(self): + self.git("checkout", "-q", "-b", "child") + self.before = self.commit("child\n", "child", path="child") + self.git("branch", "merged", self.before) + self.after = self.commit("child again\n", "child", amend=True, path="child") + self.run_action(base_ref="topic", rewound=[self.before]) + def test_force_push_shallow_checkout(self): self.run_action(shallow=True) From 582337537ebb04df266e99df1733f6244a2e14ad Mon Sep 17 00:00:00 2001 From: bc1cindy Date: Thu, 8 Oct 2026 13:51:33 -0300 Subject: [PATCH 2/2] Describe the base and series length in the summary Says whether a force push kept the base or moved it to a descendant, an ancestor or a diverged one, and how many commits the series had before and after. --- README.md | 8 ++++-- post-range-diff-comment.sh | 4 +++ render.py | 57 +++++++++++++++++++++++++++++++++++++- tests/fixtures/basic.md | 2 +- tests/test_action.py | 23 +++++++++++++-- tests/test_render.py | 20 +++++++++++++ 6 files changed, 106 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index 8e9c954..8f8a91d 100644 --- a/README.md +++ b/README.md @@ -22,9 +22,11 @@ jobs: Fast-forward pushes are skipped. Each side is diffed against its own merge base with the base branch, so a rebase onto a newer base is not mistaken for -changes. A base branch that was itself rewritten, as a parent in a stack, is -recognized from its force pushes in the repository's activity log. Inputs and -outputs are described in [action.yml](action.yml). +changes. The summary line says how many commits the series had before and +after, and whether its base is the same, a descendant or an ancestor of the +old one, or diverged from it. A base branch that was itself rewritten, as a +parent in a stack, is recognized from its force pushes in the repository's +activity log. Inputs and outputs are described in [action.yml](action.yml). `pull_request_target` gives fork PRs a token that can comment. It is safe here because nothing from the PR runs: the checkout is the base branch, the action diff --git a/post-range-diff-comment.sh b/post-range-diff-comment.sh index ff79727..2af5d47 100755 --- a/post-range-diff-comment.sh +++ b/post-range-diff-comment.sh @@ -69,10 +69,12 @@ if old_base="$(git merge-base "$BEFORE" "${base_tips[@]}")" && [[ $new_base != "$(git rev-parse "$AFTER^{commit}")" ]]; then old_range="$old_base..$BEFORE" new_range="$new_base..$AFTER" + read -r base_behind base_ahead < <(git rev-list --left-right --count "$old_base...$new_base") else echo "::warning::no fork point from $BASE_REF; using $BEFORE...$AFTER" old_range="$AFTER..$BEFORE" new_range="$BEFORE..$AFTER" + old_base="" new_base="" base_behind=0 base_ahead=0 fi args=(--no-color) @@ -90,6 +92,8 @@ render() { python3 "$HERE/render.py" \ --repo-url "$SERVER/$REPO" --before "$BEFORE" --after "$AFTER" \ --old-range "$pretty_old" --new-range "$pretty_new" \ + --old-base "$old_base" --new-base "$new_base" \ + --base-behind "$base_behind" --base-ahead "$base_ahead" \ --pushed-at "$PUSHED_AT" --max-bytes "$1" <"$WORK/range-diff.txt" } render "${MAX_COMMENT_BYTES:-65000}" >"$WORK/comment.md" diff --git a/render.py b/render.py index a58416f..9b91eff 100755 --- a/render.py +++ b/render.py @@ -52,6 +52,26 @@ def main(argv: list[str]) -> int: required=True, help="second range-diff argument, shown for reproduction", ) + ap.add_argument( + "--old-base", + required=True, + help="where the old series forked from the base, empty if unknown", + ) + ap.add_argument( + "--new-base", required=True, help="where the new series forks from the base" + ) + ap.add_argument( + "--base-behind", + type=int, + required=True, + help="commits in the old base missing from the new one", + ) + ap.add_argument( + "--base-ahead", + type=int, + required=True, + help="commits in the new base missing from the old one", + ) ap.add_argument( "--max-bytes", type=int, @@ -72,6 +92,10 @@ def main(argv: list[str]) -> int: after=args.after, old_range=args.old_range, new_range=args.new_range, + old_base=args.old_base, + new_base=args.new_base, + base_behind=args.base_behind, + base_ahead=args.base_ahead, max_bytes=args.max_bytes, pushed_at=args.pushed_at, ) @@ -118,18 +142,27 @@ def render( new_range: str, max_bytes: int, pushed_at: str, + old_base: str, + new_base: str, + base_behind: int, + base_ahead: int, ) -> str: counts = {op: sum(1 for p in pairs if p.op == op) for op in OP_LABEL} tally = ( ", ".join(f"{n} {OP_LABEL[op]}" for op, n in counts.items() if n) or "no commits" ) + overview = [tally, describe_series(pairs)] + if old_base: + overview.append( + describe_base(old_base, new_base, base_behind, base_ahead, repo_url) + ) head = [ f"", "
", ( f"range-diff for force push {sha_html(before, repo_url)} → " - f"{sha_html(after, repo_url)} {relative_time(pushed_at)}: {tally}" + f"{sha_html(after, repo_url)} {relative_time(pushed_at)}: {'; '.join(overview)}" ), "", f"reproduce: git range-diff {html.escape(old_range)} {html.escape(new_range)}", @@ -190,6 +223,28 @@ def render_interdiffs(pairs: list[Pair]) -> str: return "\n\n".join(blocks) +def describe_series(pairs: list[Pair]) -> str: + old = sum(p.lnum != "-" for p in pairs) + new = sum(p.rnum != "-" for p in pairs) + return commits(new) if old == new else f"{old} → {commits(new)}" + + +def describe_base(old: str, new: str, behind: int, ahead: int, repo_url: str) -> str: + if old == new: + return f"same base {sha_html(old, repo_url)}" + if not behind: + relation = f"descendant, {commits(ahead)} ahead" + elif not ahead: + relation = f"ancestor, {commits(behind)} behind" + else: + relation = f"diverged, {ahead} ahead and {behind} behind" + return f"base {sha_html(old, repo_url)} → {sha_html(new, repo_url)} ({relation})" + + +def commits(n: int) -> str: + return f"{n} commit" if n == 1 else f"{n} commits" + + def number_width(pairs: list[Pair]) -> int: return max((len(n) for p in pairs for n in (p.lnum, p.rnum)), default=1) diff --git a/tests/fixtures/basic.md b/tests/fixtures/basic.md index 879176d..8093ce4 100644 --- a/tests/fixtures/basic.md +++ b/tests/fixtures/basic.md @@ -1,6 +1,6 @@
-range-diff for force push deecb4b7a4 → 031a72677e September 17, 2026 00:10 UTC: 1 modified, 1 unchanged, 1 dropped, 1 added +range-diff for force push deecb4b7a4 → 031a72677e September 17, 2026 00:10 UTC: 1 modified, 1 unchanged, 1 dropped, 1 added; 3 commits; base 1111111111 → 2222222222 (descendant, 12 commits ahead) reproduce: git range-diff 9b127fb^..deecb4b 9b127fb^..031a726 diff --git a/tests/test_action.py b/tests/test_action.py index ec499df..be2844d 100644 --- a/tests/test_action.py +++ b/tests/test_action.py @@ -107,7 +107,15 @@ def run_action( return result.stdout def test_force_push_full_checkout(self): - self.run_action() + self.assertIn("; 1 commit; same base ", out) - self.assertIn(": 1 unchanged", out) + self.assertIn("1 unchanged; 1 commit; base ', out) + self.assertIn("(diverged, 1 ahead and 1 behind)", out) + + def test_unrelated_base_is_not_described(self): + self.git("checkout", "-q", "main") + self.commit("rewritten\n", "base", amend=True, path="other") + self.git("checkout", "-q", "topic") + self.git("rebase", "-q", "--onto", "main", "topic~1") + self.after = self.git("rev-parse", "HEAD") + self.assertNotIn("base 2: bbbb b\n") + self.assertEqual(render.describe_series(grown), "1 → 2 commits") + self.assertEqual(render.describe_series(grown[:1]), "1 commit") + + def test_base_relation(self): + def base(new, behind, ahead): + return render.describe_base(OLD_BASE, new, behind, ahead, REPO) + + self.assertTrue(base(OLD_BASE, 0, 0).startswith("same base &\n") out = render.render_listing(pairs, REPO)