Skip to content

Commit

Permalink
Pull Request Handler - Use Github SHA if possible (#1130)
Browse files Browse the repository at this point in the history
**Problem**
With `github.event.pull_request.merge_commit_sha` we're not guaranteed
to get all the latest changes from the head branch included in the
build. Often we only get the changes from the second last commit
actions/checkout#518 (comment)

Previously we used the merge branch but with the merge branch it's
possible to push new changes to a PR from a forked repo and have the PR
Build run on unapproved changes.

**Proposed Solution**
Use Github SHA if possible in the PR Build. Otherwise use the merge
branch.

The
[pull_request](https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#pull_request)
trigger will use the github_sha which is the last merge commit on the
GITHUB_REF branch. Thereby it should not be possible for the PR Build to
run on unapproved changes.

The
[pull_request_target](https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#pull_request_target)
trigger cannot use the github_sha because it is the last commit on the
PR base branch. Instead we'll use the merge branch. With the
pull_request_target trigger PR Builds are always triggered right away
which means we'll cancel any currently running PR Build once there's a
new push to the PR.
  • Loading branch information
aholstrup1 authored Jul 2, 2024
1 parent 3e9e4a8 commit 5eeeec5
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ jobs:
uses: actions/checkout@v4
with:
lfs: true
ref: ${{ github.event.pull_request.merge_commit_sha }}
ref: ${{ github.event_name == 'pull_request' && github.sha || format('refs/pull/${0}/merge', github.event.pull_request.number) }}

- name: Initialize the workflow
id: init
Expand Down Expand Up @@ -93,7 +93,7 @@ jobs:
with:
shell: ${{ needs.Initialization.outputs.githubRunnerShell }}
runsOn: ${{ needs.Initialization.outputs.githubRunner }}
checkoutRef: ${{ github.event.pull_request.merge_commit_sha }}
checkoutRef: ${{ github.event_name == 'pull_request' && github.sha || format('refs/pull/${0}/merge', github.event.pull_request.number) }}
parentTelemetryScopeJson: ${{ needs.Initialization.outputs.telemetryScopeJson }}
project: ${{ matrix.project }}
projectName: ${{ matrix.projectName }}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ jobs:
uses: actions/checkout@v4
with:
lfs: true
ref: ${{ github.event.pull_request.merge_commit_sha }}
ref: ${{ github.event_name == 'pull_request' && github.sha || format('refs/pull/${0}/merge', github.event.pull_request.number) }}

- name: Initialize the workflow
id: init
Expand Down Expand Up @@ -93,7 +93,7 @@ jobs:
with:
shell: ${{ needs.Initialization.outputs.githubRunnerShell }}
runsOn: ${{ needs.Initialization.outputs.githubRunner }}
checkoutRef: ${{ github.event.pull_request.merge_commit_sha }}
checkoutRef: ${{ github.event_name == 'pull_request' && github.sha || format('refs/pull/${0}/merge', github.event.pull_request.number) }}
parentTelemetryScopeJson: ${{ needs.Initialization.outputs.telemetryScopeJson }}
project: ${{ matrix.project }}
projectName: ${{ matrix.projectName }}
Expand Down

0 comments on commit 5eeeec5

Please sign in to comment.