diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index fb6f879..5b6742b 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -20,7 +20,7 @@ jobs: bash tests/test_conflict_resolution_resume.sh bash tests/test_conflict_absorbed_resolution.sh bash tests/test_merge_commit_merge.sh - bash tests/test_rebase_merge_skip.sh + bash tests/test_rebase_merge.sh e2e-tests: name: E2E Tests diff --git a/README.md b/README.md index cf6b223..137071f 100644 --- a/README.md +++ b/README.md @@ -134,7 +134,7 @@ jobs: ### Notes -* Built for squash merges. A PR merged with a merge commit keeps its history, so the action only retargets its children and deletes the branch. Rebase merges are not supported: the action detects them through GitHub's commit-PR association (the merge method itself is recorded nowhere) and comments on each child PR instead of acting. +* Works with squash merges and rebase merges: both land the branch's content as new commits without merging its history, so the children are re-parented onto the landed tip (the squash commit, or the last copied commit of a rebase merge). A PR merged with a merge commit keeps its history, so the action only retargets its children and deletes the branch. * If a merge hits a conflict, you'll need to resolve it manually; pushing the resolution automatically continues the stack update * Very large stacks might hit GitHub rate limits * After retargeting, GitHub sometimes keeps rendering a PR's diff against its old, deleted base, so the PR appears to contain already-merged changes. The branch itself is correct (`git diff ...HEAD` shows the real diff). Pushing any commit to the PR usually makes GitHub recompute. Sometimes it doesn't. Tough luck. diff --git a/action.yml b/action.yml index eb69e0e..176bd4a 100644 --- a/action.yml +++ b/action.yml @@ -1,5 +1,5 @@ name: 'Stacked PR Update Action' -description: 'Automatically fixes stacked PRs after a base PR is squash-merged' +description: 'Automatically fixes stacked PRs after a base PR is squash- or rebase-merged' author: 'GitHub Actions' inputs: diff --git a/tests/mock_gh.sh b/tests/mock_gh.sh index e2ff8ff..d96b1d5 100755 --- a/tests/mock_gh.sh +++ b/tests/mock_gh.sh @@ -29,16 +29,6 @@ elif [[ "$1" == "pr" && "$2" == "comment" ]]; then fi # Just log the comment command echo "Mock: gh pr comment $3" -elif [[ "$1" == "api" && "$2" == repos/*/commits/*/pulls ]]; then - # Which PRs introduced this trunk commit (already --jq filtered to bare - # numbers). The merge commit belongs to the merged PR, and so does any sha - # listed in MOCK_REBASE_COPIES (space-separated); anything else was not - # introduced by a PR. SQUASH_COMMIT and PR_NUMBER come from the test's env. - sha="${2#*/commits/}" - sha="${sha%/pulls}" - if [[ "$sha" == "$SQUASH_COMMIT" || " ${MOCK_REBASE_COPIES:-} " == *" $sha "* ]]; then - echo "$PR_NUMBER" - fi else echo "Unknown gh command: $@" >&2 exit 1 diff --git a/tests/test_rebase_merge_skip.sh b/tests/test_rebase_merge.sh old mode 100755 new mode 100644 similarity index 51% rename from tests/test_rebase_merge_skip.sh rename to tests/test_rebase_merge.sh index 7af6c50..6403994 --- a/tests/test_rebase_merge_skip.sh +++ b/tests/test_rebase_merge.sh @@ -1,10 +1,11 @@ #!/bin/bash # -# A PR merged with "Rebase and merge" is not supported: the commits on the -# target are new copies, so neither retargeting children as-is nor the squash -# sequence gives a correct result. The action must detect the rebase (the -# commit below the merge sha was also introduced by this PR, per GitHub's -# commit-PR association), comment on the children, and leave the stack alone. +# A PR merged with "Rebase and merge" lands its commits as new copies on the +# target, exactly like a squash spread over several commits: content without +# history. The action treats it like a squash: SQUASH_COMMIT (the PR's +# merge_commit_sha) is the last copy, and the child is re-parented onto it in +# one merge. The intermediate copies never enter the 3-way merge, so they can +# raise no spurious conflicts. set -ueo pipefail @@ -28,7 +29,8 @@ log_cmd git add file.txt log_cmd git commit -m "Initial commit" simulate_push main -# feature1: two commits, so the rebase leaves a copy below the merge sha +# feature1: two commits, so the rebase leaves an intermediate copy below the +# merge sha log_cmd git checkout -b feature1 sed -i '2s/.*/Feature 1 change A/' file.txt log_cmd git add file.txt @@ -54,38 +56,47 @@ log_cmd git cherry-pick feature1~1 feature1 MERGE_COMMIT=$(git rev-parse HEAD) simulate_push main -FEATURE2_BEFORE=$(git rev-parse feature2) - OUT=$(env \ SQUASH_COMMIT="$MERGE_COMMIT" \ MERGED_BRANCH=feature1 \ PR_NUMBER=1 \ TARGET_BRANCH=main \ - MOCK_REBASE_COPIES="$(git rev-parse "$MERGE_COMMIT~")" \ GH="$SCRIPT_DIR/mock_gh.sh" \ GIT="$SCRIPT_DIR/mock_git.sh" \ "$SCRIPT_DIR/../update-pr-stack.sh" 2>&1) echo "$OUT" -if ! grep -q "rebase merges are not supported" <<<"$OUT"; then - echo "❌ Expected the rebase-merge skip message" - exit 1 -fi -if ! grep -q "Mock: gh pr comment 2" <<<"$OUT"; then - echo "❌ The child PR must be told the stack was not updated" +if ! git merge-base --is-ancestor "$MERGE_COMMIT" feature2; then + echo "❌ feature2 must be re-parented onto the rebased tip" + log_cmd git log --graph --oneline --all exit 1 fi -if grep -q "pr edit" <<<"$OUT"; then - echo "❌ No PR must be retargeted" +if ! grep -q "Mock: gh pr edit 2 --base main" <<<"$OUT"; then + echo "❌ The child PR must be retargeted to main" exit 1 fi -if grep -q "push origin :feature1" <<<"$OUT"; then - echo "❌ The merged branch must be kept" +if ! grep -q "push origin :feature1" <<<"$OUT"; then + echo "❌ The merged branch must be deleted" exit 1 fi -if [[ "$(git rev-parse feature2)" != "$FEATURE2_BEFORE" ]]; then - echo "❌ feature2's head must not be rewritten" + +# The child's PR diff against its new base shows only its own change: the +# parent's changes arrive through the rebased copies on main, not the diff. +EXPECTED_DIFF=$(cat <<'EOF' +-line 14 ++Feature 2 content +EOF +) +ACTUAL_DIFF=$(log_cmd git diff main...feature2 | grep '^[+-]' | grep -v '^[+-][+-][+-]') +if [[ "$ACTUAL_DIFF" == "$EXPECTED_DIFF" ]]; then + echo "✅ Triple-dot diff main...feature2 shows only feature2's unique change" +else + echo "❌ Triple-dot diff is wrong" + echo "Expected:" + echo "$EXPECTED_DIFF" + echo "Actual:" + echo "$ACTUAL_DIFF" exit 1 fi -echo "✅ Rebase merge detected: children warned, stack left alone" +echo "✅ Rebase merge handled like a squash: child re-parented, retargeted, branch deleted" diff --git a/update-pr-stack.sh b/update-pr-stack.sh index f93de17..c001839 100755 --- a/update-pr-stack.sh +++ b/update-pr-stack.sh @@ -3,7 +3,8 @@ # Updates PR stack after merging a PR # # Required environment variables (squash-merge mode): -# SQUASH_COMMIT - The hash of the squash commit that was merged +# SQUASH_COMMIT - The merged PR's merge_commit_sha: the squash commit, or the +# last copied commit of a rebase merge # MERGED_BRANCH - The name of the branch that was merged and will be deleted # TARGET_BRANCH - The name of the branch that the PR was merged into # PR_NUMBER - The number of the PR that was merged @@ -99,58 +100,6 @@ has_squash_commit() { && git merge-base --is-ancestor SQUASH_COMMIT "$BRANCH" } -# Args: a commit sha. Echoes the numbers of the pull requests that introduced -# the commit to the repository, one per line. -commit_pull_numbers() { - gh api "repos/{owner}/{repo}/commits/$1/pulls" --jq '.[].number' \ - || { echo "❌ Could not list the pull requests that introduced commit $1" >&2; return 1; } -} - -# Args: the merge commit sha, the merged PR's number. The association is -# computed asynchronously, some time after the merge. The merge commit always -# belongs to the merged PR, so once it shows up the index has caught up with -# this merge; until then, an empty answer for any commit of the merge means -# nothing. Exits if the association never appears. -wait_for_pull_association() { - local MERGE_SHA="$1" PR_NUMBER="$2" - local NUMBERS - for _ in $(seq 1 24); do - NUMBERS=$(commit_pull_numbers "$MERGE_SHA") || exit 1 - if grep -qx "$PR_NUMBER" <<<"$NUMBERS"; then - return 0 - fi - sleep "${ASSOCIATION_POLL_SECONDS:-5}" - done - echo "❌ GitHub never associated $MERGE_SHA with PR #$PR_NUMBER; cannot tell a squash from a rebase" >&2 - exit 1 -} - -# Args: the merged PR's number. The event payload does not say which merge -# method was used (GitHub records it nowhere), but GitHub associates every -# trunk commit with the PR that introduced it. A squash introduces a single -# commit, so the commit below SQUASH_COMMIT belongs to an older PR or to -# none; a rebase introduces a copy of each PR commit, so with two or more -# commits the one below SQUASH_COMMIT still belongs to this PR. A -# single-commit PR merges identically under rebase and squash and correctly -# reads as a squash here. -is_rebase_merge() { - local PR_NUMBER="$1" - local MERGE_SHA PARENT_SHA NUMBERS - MERGE_SHA=$(git rev-parse SQUASH_COMMIT) - PARENT_SHA=$(git rev-parse SQUASH_COMMIT~) - - NUMBERS=$(commit_pull_numbers "$PARENT_SHA") || exit 1 - if [[ -z "$NUMBERS" ]]; then - # Ambiguous: "no PR introduced this commit" (a squash on top of a - # direct push) and "not indexed yet" (a rebase copy) both come back - # empty. Wait until the index has caught up with this merge, then ask - # again; this time empty really means no PR. - wait_for_pull_association "$MERGE_SHA" "$PR_NUMBER" - NUMBERS=$(commit_pull_numbers "$PARENT_SHA") || exit 1 - fi - grep -qx "$PR_NUMBER" <<<"$NUMBERS" -} - # Echoes " " for each open PR based on the merged branch. # try, not run: callers run this in a command substitution, where a die would # only leave the subshell, so they capture the output and die themselves. An @@ -425,19 +374,11 @@ main() { return 0 fi - # Rebase merges are not supported: the copies on the target are new - # commits, so a child retargeted as-is would show its parent's changes in - # its diff, and the squash sequence can raise spurious conflicts against - # the intermediate copies. Tell the children and leave everything alone. - if is_rebase_merge "$PR_NUMBER"; then - echo "⚠️ '$MERGED_BRANCH' looks rebase-merged; rebase merges are not supported, leaving the stack alone" - CHILDREN=$(list_child_prs) || die "could not list the PRs based on $MERGED_BRANCH" - while read -r NUMBER BRANCH; do - [[ -n "$BRANCH" ]] || continue - run gh pr comment "$NUMBER" --body "ℹ️ The base branch \`$MERGED_BRANCH\` of this PR was merged with \"Rebase and merge\", which autorestack does not support. Update this PR manually. \`$MERGED_BRANCH\` was kept so this PR stays open." - done <<<"$CHILDREN" - return 0 - fi + # A squash merge and a rebase merge both land the branch's content as new + # commits without merging its history, and both take the path below. + # SQUASH_COMMIT (the PR's merge_commit_sha) is the squash commit or the + # last rebased copy; either way its tree carries the merged branch's full + # content, which is all the single-merge re-parent reads from it. # Find all PRs directly targeting the merged PR's head CHILDREN=$(list_child_prs) || die "could not list the PRs based on $MERGED_BRANCH"