Skip to content

Commit

Permalink
Run Nixpkgs diff on nixfmt commits merged into the base branch
Browse files Browse the repository at this point in the history
Previously the Nixpkgs diff script would first format the latest base
branch commit, before formatting with each of the PRs commits. This
however causes problems if the PR commits are on top of a much older
commit that has bugs that aren't in the base branch anymore.

More visually, a PR branch graph looks like this:

base: B --- L
       \
        \
PR:      P --- Q

Where
B: Earlier commit on the base branch off of which the PR is branched off (aka merge base)
L: Latest commit on the base branch, fixes a bug
P: First commit of the PR, doesn't contain the bug fix
Q: Second commit of the PR, also doesn't contain the bug fix

Previously CI ran:
- Format with L, with bugfix
- Format with P, without bugfix
- Format with Q, without bugfix

With this change, CI runs:
- Format with L, with bugfix
- Format with (L merged with P), with bugfix
- Format with (L merged with Q), with bugfix
  • Loading branch information
infinisil committed Aug 27, 2024
1 parent 8c4da7b commit a78b63e
Showing 1 changed file with 3 additions and 1 deletion.
4 changes: 3 additions & 1 deletion scripts/sync-pr.sh
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ bodyForCommitIndex() {
echo -e "base: $subject\n\nFormat using the base commit from nixfmt PR $nixfmtPrNumber: $url"
else
url=$nixfmtUrl/pull/$nixfmtPrNumber/commits/$commit
echo -e "$index: $subject\n\nFormat using commit number $index from nixfmt PR $nixfmtPrNumber: $url"
echo -e "$index: $subject\n\nFormat using commit number $index from nixfmt PR $nixfmtPrNumber: $url (merged into the base branch)"
fi
}

Expand Down Expand Up @@ -249,6 +249,8 @@ update() {

step "Checking out nixfmt at $nixfmtCommit"
git -C nixfmt checkout -q "$nixfmtCommit"
step "Merging with the base commit $nixfmtBaseCommit"
git -C nixfmt merge --no-stat --no-edit "$nixfmtBaseCommit"
}

# Format Nixpkgs with a specific nixfmt version and push the result.
Expand Down

0 comments on commit a78b63e

Please sign in to comment.