Skip to content

Commit a4eca62

Browse files
committed
ci: Compare against HEAD instead of working tree
When we list modified files we should only check the commits that are being added or are under review. Therefore we should use the format `git diff <commit> <commit>`. Using `git diff <commit>` will diff <commit> against the current working tree, which is only correct if the working tree is pristine. TARGET_BRANCH doesn't have to be limited to being a branch, it can be any kind of git revision. Finally instead of hard coding `master`, we use the `event.before` which has the tip of the branch before the push event.
1 parent 87ef460 commit a4eca62

File tree

6 files changed

+11
-14
lines changed

6 files changed

+11
-14
lines changed

.ci/check-pep8

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ set -o pipefail
1111
command -v git >/dev/null 2>&1 || { echo >&2 "git is missing"; exit 1; }
1212

1313
# grep will exit with 1 if no lines are found
14-
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)
14+
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)
1515
if [ -z "${FILES}" ] ; then
1616
exit 0
1717
fi

.ci/check-style

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,10 @@ if test -t 1; then
2525
fi
2626
fi
2727

28-
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
28+
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
2929
echo -e "${red}Not $CLANGFORMAT clean${normal}"
3030
# Apply CF to the files
31-
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
31+
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
3232
# Print list of files that weren't formatted correctly
3333
echo -e "Incorrectly formatted files:"
3434
git --no-pager diff --name-only

.ci/check-tidy

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ for dir in build build-build; do
3636
s/-Wno-cast-function-type//g; s/-mfpu=fpv4-sp-d16//g; s/-Wformat-signedness//g' ${dir}/compile_commands.json
3737

3838
# Only check our files
39-
SOURCES1=$(git --no-pager diff --diff-filter=d --name-only ${TARGET_BRANCH} |\
39+
SOURCES1=$(git --no-pager diff --diff-filter=d --name-only ${TARGET_BRANCH} HEAD |\
4040
grep -v -E "(^src/(drivers|ui/fonts)|.*ugui.*|.*base32.*)" |\
4141
grep -E "^(src)" |\
4242
grep -v "^test/unit-test/u2f/" |\

.ci/run-container-ci

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,8 @@
1818
# It accepts two positional arguments:
1919
# 1. A workspace dir, the root of the git repo clone, or "pull" literal.
2020
# In the latter case, CI container image is pulled from a registry.
21-
# 2. An optional target branch for code style diffs. Defaults to "master" for
22-
# push commits and overwritten with TRAVIS_BRANCH env var for pull requests
23-
# when run on Travis CI.
21+
# 2. A git revision (see man gitrevisions) to compare against HEAD to filter out modified and new
22+
# files. Some scripts only run on that subset.
2423

2524
set -e
2625
set -x
@@ -40,9 +39,7 @@ if [ -z "${WORKSPACE_DIR}" ]; then
4039
exit 1
4140
fi
4241

43-
TARGET_BRANCH="${2:-master}"
44-
TARGET_BRANCH=origin/${TARGET_BRANCH}
45-
42+
TARGET_BRANCH="$2"
4643
# The safe.directory config is so that git commands work. even though the repo folder mounted in
4744
# Docker is owned by root, which can be different from the owner on the host.
4845
docker run -e TARGET_BRANCH="${TARGET_BRANCH}" \

.github/workflows/ci.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,4 +20,4 @@ jobs:
2020
- name: Pull CI container image
2121
run: ./.ci/run-container-ci pull
2222
- name: Run CI in container
23-
run: ./.ci/run-container-ci ${{github.workspace}}
23+
run: ./.ci/run-container-ci ${{github.workspace}} ${{ github.event.before }}

.github/workflows/pr-ci.yml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ jobs:
1818
run: ./.ci/run-container-ci pull
1919

2020
- name: Run CI in container
21-
run: ./.ci/run-container-ci ${{github.workspace}} ${{ github.base_ref }}
21+
run: ./.ci/run-container-ci ${{github.workspace}} ${{ github.event.pull_request.base.sha }}
2222

2323
# Generate a list of commits to run CI on
2424
generate-matrix:
@@ -35,7 +35,7 @@ jobs:
3535
- name: Create jobs for commits in PR history
3636
id: set-matrix
3737
run: |
38-
echo matrix=$(.ci/matrix-from-commit-log origin/${{github.base_ref}}..${{ github.event.pull_request.head.sha}}~) >> $GITHUB_OUTPUT
38+
echo matrix=$(.ci/matrix-from-commit-log ${{ github.event.pull_request.base.sha }}..${{ github.event.pull_request.head.sha }}~) >> $GITHUB_OUTPUT
3939
4040
# Run this job for every commit in the PR except HEAD.
4141
pr-commit-ci:
@@ -70,4 +70,4 @@ jobs:
7070
run: ./.ci/run-container-ci pull
7171

7272
- name: Run CI in container
73-
run: ./.ci/run-container-ci ${{github.workspace}} ${{ github.base_ref }}
73+
run: ./.ci/run-container-ci ${{github.workspace}} ${{ github.event.pull_request.base.sha }}

0 commit comments

Comments
 (0)