Skip to content

Commit

Permalink
Run compilation check as a composite action
Browse files Browse the repository at this point in the history
In our CI pipeline we want to test if all commits of a PR would compile
if checked out individually. This ensures commits don't have dependencies
on changes added afterwards. This makes git bisect automation possible.

Previous approach used a step in the main CI workflow, which is loaded
when a CI workflow starts. This means that the method of compiling each
commit in the entire PR branch will be the one used for the PR HEAD.
This may cause problems if the PR changes any Maven parameters in any
commit except the first one.

Consider for example adding a settings.xml file to Maven parameters
in a second commit of a PR. Since the environment variables from PR HEAD
are in effect for the entire workflow run, the workflow would call Maven
with the `-s settings.xml` option for all commits of the PR.
The commit adding the file might be self-contained, but if it's not
the first commit of the PR, it will trigger a failure for older commits.
  • Loading branch information
MiguelWeezardo authored and hashhar committed Nov 26, 2022
1 parent 241f928 commit faa0385
Show file tree
Hide file tree
Showing 2 changed files with 65 additions and 21 deletions.
35 changes: 35 additions & 0 deletions .github/actions/compile-commit/action.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
name: compile-commit
description: "Compile currently checked out commit"
inputs:
base_ref:
description: 'Git base ref'

runs:
using: composite
steps:
- uses: ./.github/actions/setup
- name: Check if a specified commit compiles
shell: bash
run: |
set -x
# The section below should always contain copies of env variables from .github/workflows/ci.yml to maintain both files in sync
# It's important that these values are NOT passed as parameters, because then their values would always be taken from PR HEAD
# -------
# allow overriding Maven command
export MAVEN="./mvnw"
# maven.wagon.rto is in millis, defaults to 30m
export MAVEN_OPTS="-Xmx512M -XX:+ExitOnOutOfMemoryError -Dmaven.wagon.rto=60000"
export MAVEN_INSTALL_OPTS="-Xmx3G -XX:+ExitOnOutOfMemoryError -Dmaven.wagon.rto=60000"
export MAVEN_COMPILE_COMMITS="-B --strict-checksums --quiet -T C1 -DskipTests -Dmaven.source.skip=true -Dair.check.skip-all=true -Dmaven.javadoc.skip=true --no-snapshot-updates --no-transfer-progress -pl !:trino-server-rpm"
export MAVEN_GIB="-P gib -Dgib.referenceBranch=refs/remotes/origin/${{ inputs.base_ref }}"
# -------
# For building with Maven we need MAVEN_OPTS to equal MAVEN_INSTALL_OPTS
export MAVEN_OPTS="${MAVEN_INSTALL_OPTS}"
$MAVEN package ${MAVEN_COMPILE_COMMITS} ${MAVEN_GIB}
- name: Clean local Maven repo
# Avoid creating a cache entry because this job doesn't download all dependencies
if: steps.cache.outputs.cache-hit != 'true'
shell: bash
run: rm -rf ~/.m2/repository
51 changes: 30 additions & 21 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -84,37 +84,46 @@ jobs:
check-commits:
runs-on: ubuntu-latest
if: github.event_name == 'pull_request'
outputs:
matrix: ${{ steps.set-matrix.outputs.matrix }}
steps:
- uses: actions/checkout@v3
with:
fetch-depth: 0 # checkout all commits to be able to determine merge base
ref: ${{ github.event.pull_request.head.sha }} # checkout PR HEAD commit instead of merge commit
- uses: ./.github/actions/setup
- name: Check Commits
uses: trinodb/github-actions/block-commits@c2991972560c5219d9ae5fb68c0c9d687ffcdd10
with:
action-merge: fail
action-fixup: none
- name: Check if all intermediate commits of this PR compile
run: |
set -x
# Fake identity to tag rebased commits with.
# These commits will be discarded, but the important part is that `git rebase` executes commands between all commits.
git config user.name "Compile all commits builder"
git config user.email "[email protected]"
# Show the entire PR branch and the base ref all the way to the fork point.
# This shows some context on the state of the repo and shows commit hashes without having to scroll through entire repo history.
git log --oneline --decorate=full --graph "$( git merge-base HEAD refs/remotes/origin/${{ github.event.pull_request.base.ref }} )~..refs/remotes/origin/${{ github.event.pull_request.base.ref }}" HEAD
# We can skip compiling the top commit since it's already being compiled and tested in other jobs.
git checkout HEAD~
# If the PR branch contains only 1 commit, checkout will move HEAD to a commit already on the base branch, and the rebase doesn't do anything.
export MAVEN_OPTS="${MAVEN_INSTALL_OPTS}"
git rebase --verbose --exec "$MAVEN package ${MAVEN_COMPILE_COMMITS} ${MAVEN_GIB} || (git status && false)" $( git merge-base HEAD refs/remotes/origin/${{ github.event.pull_request.base.ref }} )
# The local repo is broken at this point, and should be abandoned
- name: Clean local Maven repo
# Avoid creating a cache entry because this job doesn't download all dependencies
if: steps.cache.outputs.cache-hit != 'true'
run: rm -rf ~/.m2/repository
- name: Set matrix
id: set-matrix
# The output from rev-list ends with a newline, so we have to filter out index -1 in jq since it's an empty string
# The HEAD commit of the PR (index -2) can be safely ignored since it's already compiled in other jobs
run: |
export JQ_PIPELINE='split("\n") | .[0:-2] | map({base_ref: "${{ github.event.pull_request.base.ref }}", commit: .}) | select(length > 0) | {include: .}'
git rev-list refs/remotes/origin/${{ github.event.pull_request.base.ref }}..HEAD | jq --raw-input --slurp "$JQ_PIPELINE" > commit-matrix.json
echo "Commit matrix: $(jq '.' commit-matrix.json)"
echo "matrix=$(jq -c '.' commit-matrix.json)" >> $GITHUB_OUTPUT
check-commits-dispatcher:
runs-on: ubuntu-latest
needs: check-commits
if: github.event_name == 'pull_request' && needs.check-commits.outputs.matrix != ''
strategy:
matrix: ${{ fromJson(needs.check-commits.outputs.matrix) }}
steps:
- uses: actions/checkout@v3
with:
fetch-depth: 0 # checkout all commits to be able to determine merge base
ref: ${{ matrix.commit }}
# This composite job must be entirely standalone, and checked out from the correct commit before being executed.
# It can't accept any parameters defined in this workflow, because the values of those parameters would always be taken from
# PR HEAD since that is the commit the workflow is started for. This could lead to problems if those parameters were changed
# in the middle of a PR branch.
- uses: ./.github/actions/compile-commit
with:
base_ref: ${{ matrix.base_ref }}

error-prone-checks:
runs-on: ubuntu-latest
Expand Down

0 comments on commit faa0385

Please sign in to comment.