Merge pull request #1278 from NickeZ/nickez/build-container-in-ci

Nickez/build container in ci

.ci/README.md

@@ -0,0 +1,31 @@
+CI Design guidelines
+
+* It is more maintainable to create scripts in `.ci` and then call them from the workflows than to
+  have scripts inline in the workflows. However, it is also good to split up scripts in multiple
+  steps and jobs depending on what is being done.
+
+* The docker image is rebuilt if the `Dockerfile` or `.containerversion` file is modified. (In case
+  of a push event it is also automatically published to docker hub).
+
+* If there are changes in the `Dockerfile`, then `.containerversion` must be updated with an
+  unpublished version number.
+
+* We listen to two kinds of events, `pull_request` and `push` using two different workflows,
+  `pr-ci.yml` and `ci.yml`.
+  * On pull request events, github will checkout a version of the tree that is the PR branch merged
+    into the base branch. When we look for what is modifed we can diff HEAD^1 to HEAD. If github
+    didn't do this, it would've missed commits added to the base branch since the PR branch was
+    forked.
+
+     o--o--o--o <-- (base branch, typically 'master', parent 1)
+      \        \
+       \        o <-- (HEAD)
+        \      /
+         o----o <-- Pull requst branch (parent 2)
+
+  * On push events we get hashes of last commit before and after the push. When we look for what
+    changed we can diff github.event.before with HEAD.
+
+       o--o--o--o--o--o <-- github.event.after (HEAD)
+              \
+                github.event.before

.ci/build-container

@@ -0,0 +1,8 @@
+#!/bin/bash
+
+set -e
+
+CONTAINER_REPO=shiftcrypto/firmware_v2
+CONTAINER_VERSION=$(cat .containerversion)
+
+docker build --pull --no-cache -t $CONTAINER_REPO:latest -t $CONTAINER_REPO:$CONTAINER_VERSION .

.ci/check-container-sources-modified

@@ -0,0 +1,14 @@
+#!/bin/bash
+#
+# This script works on merge commits. <rev>^1 means the first parent of <rev>.
+#
+# When the github action creates a temporary merge commit for a pull request, the first parent will
+# be the base (the branch being merged into).
+
+set -e
+
+if git diff --name-only HEAD^1 HEAD | grep -E '^(\.containerversion|Dockerfile)' >/dev/null; then
+	echo "true"
+	exit
+fi
+echo "false"

.ci/check-container-version-published

@@ -0,0 +1,14 @@
+#!/bin/bash
+
+set -e
+
+CONTAINER_REPO=shiftcrypto/firmware_v2
+CONTAINER_VERSION=$(cat .containerversion)
+
+# docker manifest returns 1 (error) if the container doesn't exist and 0 (success) if it does.
+if docker manifest inspect $CONTAINER_REPO:$CONTAINER_VERSION > /dev/null; then
+	 >&2 echo Container version \'$CONTAINER_VERSION\' exists.
+	 echo true
+	 exit
+fi
+echo false

.ci/check-pep8

@@ -11,7 +11,7 @@ set -o pipefail
 command -v git >/dev/null 2>&1 || { echo >&2 "git is missing"; exit 1; }
 
 # grep will exit with 1 if no lines are found
-FILES=$(git --no-pager diff --diff-filter=d --name-only ${TARGET_BRANCH} | grep -v -e "old/" -e "generated/" -e "rust/vendor/" | grep -E ".py\$" || exit 0)
+FILES=$(git --no-pager diff --diff-filter=d --name-only ${TARGET_BRANCH} HEAD | grep -v -e "old/" -e "generated/" -e "rust/vendor/" | grep -E ".py\$" || exit 0)
 if [ -z "${FILES}" ] ; then
        exit 0
 fi

.ci/check-style

@@ -25,10 +25,10 @@ if test -t 1; then
 	fi
 fi
 
-if git --no-pager diff --diff-filter=d --name-only ${TARGET_BRANCH} | grep -v -E "(^src/(rust|ui/fonts)|.*ugui.*|.*base32.*)" | grep -E "^(src|test)" | grep -E "\.(c|h)\$" | xargs -n1 "$CLANGFORMAT" -output-replacements-xml | grep -c "<replacement " >/dev/null; then
+if git --no-pager diff --diff-filter=d --name-only ${TARGET_BRANCH} HEAD | grep -v -E "(^src/(rust|ui/fonts)|.*ugui.*|.*base32.*)" | grep -E "^(src|test)" | grep -E "\.(c|h)\$" | xargs -n1 "$CLANGFORMAT" -output-replacements-xml | grep -c "<replacement " >/dev/null; then
 	echo -e "${red}Not $CLANGFORMAT clean${normal}"
 	# Apply CF to the files
-	git --no-pager diff --diff-filter=d --name-only ${TARGET_BRANCH} | grep -v -E "(^src/(rust|ui/fonts)|.*ugui.*|.*base32.*)" | grep -E "^(src|test)" | grep -E "\.(c|h)\$" | xargs -n1 "$CLANGFORMAT" -i
+	git --no-pager diff --diff-filter=d --name-only ${TARGET_BRANCH} HEAD | grep -v -E "(^src/(rust|ui/fonts)|.*ugui.*|.*base32.*)" | grep -E "^(src|test)" | grep -E "\.(c|h)\$" | xargs -n1 "$CLANGFORMAT" -i
 	# Print list of files that weren't formatted correctly
 	echo -e "Incorrectly formatted files:"
 	git --no-pager diff --name-only

.ci/check-tidy

@@ -36,7 +36,7 @@ for dir in build build-build; do
 	        s/-Wno-cast-function-type//g; s/-mfpu=fpv4-sp-d16//g; s/-Wformat-signedness//g' ${dir}/compile_commands.json
 
 	# Only check our files
-	SOURCES1=$(git --no-pager diff --diff-filter=d --name-only ${TARGET_BRANCH} |\
+	SOURCES1=$(git --no-pager diff --diff-filter=d --name-only ${TARGET_BRANCH} HEAD |\
 		grep -v -E "(^src/(drivers|ui/fonts)|.*ugui.*|.*base32.*)" |\
 		grep -E "^(src)" |\
 		grep -v "^test/unit-test/u2f/" |\

.ci/publish-container

@@ -0,0 +1,9 @@
+#!/bin/bash
+
+set -e
+
+CONTAINER_REPO=shiftcrypto/firmware_v2
+CONTAINER_VERSION=$(cat .containerversion)
+
+docker push $CONTAINER_REPO:latest
+docker push $CONTAINER_REPO:$CONTAINER_VERSION

.ci/pull-container

@@ -0,0 +1,8 @@
+#!/bin/bash
+
+set -e
+
+CONTAINER_REPO=shiftcrypto/firmware_v2
+CONTAINER_VERSION=$(cat .containerversion)
+
+docker pull $CONTAINER_REPO:$CONTAINER_VERSION

.ci/run-container-ci

@@ -16,11 +16,9 @@
 
 # The script runs all CI builds and checks in a Docker container.
 # It accepts two positional arguments:
-# 1. A workspace dir, the root of the git repo clone, or "pull" literal.
-#    In the latter case, CI container image is pulled from a registry.
-# 2. An optional target branch for code style diffs. Defaults to "master" for
-#    push commits and overwritten with TRAVIS_BRANCH env var for pull requests
-#    when run on Travis CI.
+# 1. A workspace dir, the root of the git repo clone, to be mounted in the container.
+# 2. A git revision (see man gitrevisions) to compare against HEAD to filter out modified and new
+#    files. Some scripts only run on that subset.
 
 set -e
 set -x
@@ -29,28 +27,13 @@ CONTAINER_REPO=shiftcrypto/firmware_v2
 CONTAINER_VERSION=$(cat .containerversion)
 CONTAINER=$CONTAINER_REPO:${CONTAINER_VERSION}
 
-if [ "$1" == "pull" ] ; then
-	docker pull "$CONTAINER"
-	exit 0
-fi
-
 WORKSPACE_DIR="$1"
 if [ -z "${WORKSPACE_DIR}" ]; then
   echo "Workspace dir path is empty."
   exit 1
 fi
 
-TARGET_BRANCH="${2:-master}"
-if [ "${TRAVIS}" == "true" ] && [ "${TRAVIS_PULL_REQUEST}" != "false" ] ; then
-	TARGET_BRANCH=${TRAVIS_BRANCH}
-fi
-
-# Fetch origin/master so that we can diff when checking coding style.
-git remote set-branches --add origin ${TARGET_BRANCH}
-git fetch origin
-
-TARGET_BRANCH=origin/${TARGET_BRANCH}
-
+TARGET_BRANCH="$2"
 # The safe.directory config is so that git commands work. even though the repo folder mounted in
 # Docker is owned by root, which can be different from the owner on the host.
 docker run -e TARGET_BRANCH="${TARGET_BRANCH}" \

.github/actions/pr-ci-common/action.yml

@@ -0,0 +1,33 @@
+name: Pull request CI common
+
+inputs:
+  base-sha:
+    required: true
+runs:
+  using: "composite"
+  steps:
+    - name: Check if container files was modified and if container version already exists
+      id: checks
+      shell: bash
+      run: |
+        echo modified=$(./.ci/check-container-sources-modified) >> "$GITHUB_OUTPUT"
+        echo container-published=$(./.ci/check-container-version-published) >> "$GITHUB_OUTPUT"
+
+    - name: Build container image
+      if: steps.checks.outputs.modified == 'true'
+      shell: bash
+      run: |
+        if "${{ steps.checks.outputs.container-published }}" == "true"; then
+          echo "::error::Container modified but version $(cat .containerversion) already published"
+          exit 1
+        fi
+        ./.ci/build-container
+
+    - name: Pull container image
+      if: steps.checks.outputs.modified == 'false'
+      shell: bash
+      run: ./.ci/pull-container
+
+    - name: Run CI in container
+      shell: bash
+      run: ./.ci/run-container-ci ${{github.workspace}} ${{ inputs.base-sha }}

.github/workflows/ci.yml

@@ -8,14 +8,37 @@ on:
       - master
 
 jobs:
-  linux-docker:
+  ci:
     runs-on: ubuntu-22.04
     steps:
       - name: Clone the repo
         uses: actions/checkout@v4
         with:
+          fetch-depth: 0
+          fetch-tags: true
           submodules: recursive
+
+      - name: Check if container should be published
+        id: checks
+        run: echo container-published=$(./.ci/check-container-version-published) >> $GITHUB_OUTPUT
+
+      - name: Build container
+        if: steps.checks.outputs.container-published == 'false'
+        run: ./.ci/build-container
+
+      - name: Login to Docker Hub
+        uses: docker/login-action@v3
+        with:
+          username: ${{ secrets.DOCKER_USERNAME }}
+          password: ${{ secrets.DOCKER_TOKEN }}
+
+      - name: Publish container
+        if: steps.checks.outputs.container-published == 'false'
+        run: ./.ci/publish-container
+
       - name: Pull CI container image
-        run: ./.ci/run-container-ci pull
+        if: steps.checks.outputs.container-published == 'true'
+        run: ./.ci/pull-container
+
       - name: Run CI in container
-        run: ./.ci/run-container-ci ${{github.workspace}}
+        run: ./.ci/run-container-ci ${{github.workspace}} ${{ github.event.before }}

.github/workflows/pr-ci.yml

@@ -12,12 +12,12 @@ jobs:
         uses: actions/checkout@v4
         with:
           submodules: recursive
+          fetch-depth: 0
 
-      - name: Pull container image
-        run: ./.ci/run-container-ci pull
-
-      - name: Run CI in container
-        run: ./.ci/run-container-ci ${{github.workspace}} ${{ github.base_ref }}
+      - name: CI
+        uses: ./.github/actions/pr-ci-common
+        with:
+          base-sha: ${{ github.event.pull_request.base.sha }}
 
   # Generate a list of commits to run CI on
   generate-matrix:
@@ -34,9 +34,15 @@ jobs:
       - name: Create jobs for commits in PR history
         id: set-matrix
         run: |
-          echo matrix=$(.ci/matrix-from-commit-log origin/${{github.base_ref}}..${{ github.event.pull_request.head.sha}}~) >> $GITHUB_OUTPUT
+          echo matrix=$(.ci/matrix-from-commit-log ${{ github.event.pull_request.base.sha }}..${{ github.event.pull_request.head.sha }}~) >> $GITHUB_OUTPUT
 
   # Run this job for every commit in the PR except HEAD.
+  # This job simulates what github does for the PR HEAD commit but for every other commit in the
+  # PR. So for every commit, it creates a merge commit between that commit and the base branch.
+  # Then it runs the CI on that merge commit.
+  # The only caveat is that this file (pr-ci.yml) is already loaded from the PR HEAD merge commit,
+  # and therefore we need to load the `.ci` scripts from the PR HEAD merge commit. The outcome of
+  # that is that changes to the CI is not tested per commit. All commits use the final version.
   pr-commit-ci:
     runs-on: ubuntu-22.04
     needs: [ generate-matrix ]
@@ -58,13 +64,14 @@ jobs:
           GIT_COMMITTER_NAME: Bot
           GIT_COMMITTER_EMAIL: [email protected]
         run: |
-          git fetch origin ${{ matrix.commit }}
+          git fetch origin ${{ matrix.commit }} ${{ github.event.pull_request.merge_commit_sha }}
           git merge --no-ff --no-edit ${{ matrix.commit }}
-          echo "merge commit parents:"
           git log -1 --format="Head %H, Parents %P"
+          # Since the workflow definition is taken from the pull request merge commit, we need to
+          # get the .ci scripts from there as well.
+          git checkout -f ${{ github.event.pull_request.merge_commit_sha }} -- .ci .github
 
-      - name: Pull container image
-        run: ./.ci/run-container-ci pull
-
-      - name: Run CI in container
-        run: ./.ci/run-container-ci ${{github.workspace}} ${{ github.base_ref }}
+      - name: CI
+        uses: ./.github/actions/pr-ci-common
+        with:
+          base-sha: ${{ github.event.pull_request.base.sha }}