Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow users to commit to the main branch while full release running #40

Conversation

rtyley
Copy link
Member

@rtyley rtyley commented Jun 21, 2024

This should fix #37 - ie, a Full Main-Branch release should still succeed, even if someone adds additional commits to the main branch while the release workflow is running.

Note that this change in behaviour only applies to Full Main-Branch releases, not Preview releases, as Preview releases don't ever attempt to update any branch.

Avoid making a merge commit if possible

To avoid the noise of an additional merge commit (ie a 3rd commit) we prefer a fast-forward, and only create a merge commit if it's necessary - that's when someone has done additional commits on the main branch.

These diagrams show how the resulting git history will look after a release:

A release where no extra work was done on main branch (fast-forward possible)

---
config:
  gitGraph:
    parallelCommits: true
    showCommitLabel: true
---
gitGraph
   commit id: "commit before release"
   commit id: "release commit" tag: "v1.0.0"
   commit id: "set snapshot"
Loading

A release where extra work added to main branch during release (requires a merge)

---
config:
  gitGraph:
    parallelCommits: true
    showCommitLabel: true
---
gitGraph
   commit id: "commit before release"
   branch release-workflow/temporary/xxx
   checkout release-workflow/temporary/xxx
   commit id: "release commit" tag: "v1.0.0"
   checkout main
   commit id: "extra work" type: HIGHLIGHT
   merge release-workflow/temporary/xxx
   commit id: "set snapshot"
Loading

Two full Main-Branch releases can't run concurrently

Although we can now handle extra work on the main branch, running two releases at once is not recommended!

Using the GitHub API to create & merge commits, rather than git

We're now using the GitHub API, rather than git, to do both these operations:

Testing

Here are two successful full-main-branch release runs using this branch:

  • No extra commits image ...fast-forward of default branch to include release commit succeeded image
  • Extra commits during release image
    gh: Update is not a fast forward (HTTP 422)
    ...fast-forward failed (commits added to default branch while release running?), will attempt a merge instead
    
    image

rtyley added a commit to guardian/etag-caching that referenced this pull request Jun 21, 2024
@rtyley rtyley force-pushed the allow-users-to-commit-to-the-default-branch-while-release-is-running branch 5 times, most recently from 87c6426 to 142aa28 Compare June 23, 2024 16:00
rtyley added a commit to guardian/etag-caching that referenced this pull request Jun 23, 2024
@rtyley rtyley force-pushed the allow-users-to-commit-to-the-default-branch-while-release-is-running branch from 142aa28 to 4740182 Compare June 23, 2024 16:15
rtyley added a commit to guardian/etag-caching that referenced this pull request Jun 23, 2024
echo "...fast-forward of default branch to include release commit succeeded"
else
echo "...fast-forward failed (commits added to default branch while release running?), will attempt a merge instead"
gh api --silent --method POST /repos/:owner/:repo/merges -f "base=$GITHUB_REF_NAME" -f "head=$RELEASE_COMMIT_ID"
Copy link
Member Author

@rtyley rtyley Jun 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GitHub API: POST /repos/:owner/:repo/merges merges a branch (that doesn't need to be a PR branch) into the base branch.

RELEASE_COMMIT_ID: ${{ needs.push-release-commit.outputs.release_commit_id }}
run: |
if gh api --silent --method PATCH /repos/:owner/:repo/git/refs/heads/$GITHUB_REF_NAME -f "sha=$RELEASE_COMMIT_ID"; then
echo "...fast-forward of default branch to include release commit succeeded"
Copy link
Member Author

@rtyley rtyley Jun 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GitHub API: PATCH /repos/:owner/:repo/git/refs/heads/{branch_name} updates a branch to point at a new commit. It only allows fast-forward changes - not rewriting history, unless you use the force param - and we don't.

Previously we used git push origin $RELEASE_COMMIT_ID:refs/heads/$GITHUB_REF_NAME to do this, but using the GitHub API, we don't need to rely on cloning enough of the git repo's history (fetch-depth: 2) to find the old position of the branch.

This should fix #37 - ie, a Full Main-Branch release should still succeed, even if someone adds
additional commits to the `main` branch while the release workflow is running.

Note that this change in behaviour only applies to Full Main-Branch releases, not Preview releases
(#19), as Preview releases don't
ever attempt to update any branch.

To avoid the noise of an additional merge commit (ie a 3rd commit) we prefer a fast-forward, and
only create a merge commit if it's _necessary_ - ie because we have to merge on top of some commits
that someone else has added.

## Two full Main-Branch releases can't run concurrently

Although we can now handle extra work on the `main` branch, running **two releases at once** is _not_ recommended!

## Using the GitHub API to create & merge commits, rather than `git`

We're now using the GitHub API, rather than `git`, to do both these operations:

* [Update the branch](https://docs.github.com/en/rest/git/refs?apiVersion=2022-11-28#update-a-reference) to try fast-forwarding - using the GitHub API means we don't need to clone a deep history of the repo to make that update
* [Merge the branch](https://docs.github.com/en/rest/branches/branches?apiVersion=2022-11-28#merge-a-branch) if the fast-forward fails - using the GitHub API means that the merge commit will be signed/verified by GitHub too, [just like other commits created by the GitHub App](#26).
@rtyley rtyley force-pushed the allow-users-to-commit-to-the-default-branch-while-release-is-running branch from 4740182 to 5f9ba7b Compare June 25, 2024 16:19
rtyley added a commit to guardian/etag-caching that referenced this pull request Jun 25, 2024
@rtyley rtyley requested review from Divs-B and JamieB-gu June 25, 2024 16:37
@Divs-B
Copy link

Divs-B commented Jun 26, 2024

Thanks for already working into this @rtyley 👍 we recently had error in release when additional merge happend while the release was in progress so looking forward to have this change.

Changes looks really promising 🌟 specially where you are telling either to 'point' to additional commit because i think when branches are not divergent or to do 'merge' if they are . (please corect if this not sounds right as i am not experienced much in these).

@rtyley
Copy link
Member Author

rtyley commented Jun 26, 2024

specially where you are telling either to 'point' to additional commit because i think when branches are not divergent or to do 'merge' if they are

Thanks @Divs-B! Yep, this bit here:

if gh api --silent --method PATCH /repos/:owner/:repo/git/refs/heads/$GITHUB_REF_NAME -f "sha=$RELEASE_COMMIT_ID"; then
echo "...fast-forward of default branch to include release commit succeeded"
else
echo "...fast-forward failed (commits added to default branch while release running?), will attempt a merge instead"
gh api --silent --method POST /repos/:owner/:repo/merges -f "base=$GITHUB_REF_NAME" -f "head=$RELEASE_COMMIT_ID"
fi

The first attempt, trying to do the 'fast-forward', is in the if statement condition on line 353 - and only if that fails do we get to line 357 where instead we do a merge.

Thanks so much! Is the PR ok for you to approve?

@Divs-B
Copy link

Divs-B commented Jun 26, 2024

Thanks @rtyley for explaining above. The PR looks good to go, cant wait to test 🙌

@rtyley rtyley merged commit df07f53 into main Jun 26, 2024
@rtyley
Copy link
Member Author

rtyley commented Jun 26, 2024

The PR looks good to go, cant wait to test 🙌

Oh yeah, go commit-crazy during releases!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

🔒 Sign fails if main branch has new commits since release workflow run started
2 participants