diff --git a/.github/workflows/c++-code-formatting.yml b/.github/workflows/c++-code-formatting.yml index 4d024b23..02ffeb8b 100644 --- a/.github/workflows/c++-code-formatting.yml +++ b/.github/workflows/c++-code-formatting.yml @@ -15,11 +15,13 @@ name: C++ code formatting reusable workflow permissions: {} env: + # GitHub also provides github.event.pull_request.{head,base}.sha, but these + # aren't always the latest commits on their respective branches (e.g. see + # https://github.com/AliceO2Group/AliceO2/pull/12499). Using them might lead + # to false positives in the errors we show. BASE_BRANCH: ${{ github.event.pull_request.base.ref }} PR_BRANCH: ${{ github.event.pull_request.head.ref }} PR_NUMBER: ${{ github.event.pull_request.number }} - HEAD_SHA: ${{ github.event.pull_request.head.sha }} - BASE_SHA: ${{ github.event.pull_request.base.sha }} jobs: clang-format: @@ -61,14 +63,13 @@ jobs: id: clang_format run: | set -x - # $BASE_SHA is the latest commit on the branch the PR will be merged - # into, NOT the commit this PR derives from! For that, we need to find - # the latest common ancestor between the PR and the branch we are - # merging into. - base_commit=$(git merge-base HEAD "$BASE_SHA") + # $BASE_BRANCH is the branch the PR will be merged into, NOT the + # commit this PR derives from! For that, we need to find the latest + # common ancestor between the PR and the branch we are merging into. + base_commit=$(git merge-base HEAD "$BASE_BRANCH") # Find changed files, ignoring binary files. readarray -d '' commit_files < \ - <(git diff -z --diff-filter d --name-only --merge-base "$BASE_SHA") + <(git diff -z --diff-filter d --name-only "$base_commit") [ ${#commit_files[@]} -gt 0 ] || { echo "No files to check"; exit 0; } # Check for invalid file extensions for C++ code. @@ -156,6 +157,11 @@ jobs: # many commits back that point is. fetch-depth: 0 + # Fetch the PR's head commit to find the common ancestor. + - name: Fetch PR branch + run: | + git fetch origin "$BASE_BRANCH" "pull/$PR_NUMBER/head:$PR_BRANCH" + - name: Check copyright headers env: # The expected copyright notice. Comment markers ("//" or "#") are @@ -177,7 +183,7 @@ jobs: # Find changed C++ and CMake files. Keep the file extensions in sync # with the ones in the "case" statement below! readarray -d '' files < \ - <(git diff -z --diff-filter d --name-only --merge-base "$BASE_SHA" \ + <(git diff -z --diff-filter d --name-only --merge-base "$BASE_BRANCH" \ -- '*.cxx' '*.h' '*.C' '*.cmake' '*/CMakeLists.txt') # Run copyright notice check. Comment lines start with "//" for C++ # files and "#" for CMake files. @@ -314,7 +320,7 @@ jobs: run: | # Find changed files, ignoring binary files. readarray -d '' files < \ - <(git diff -z --diff-filter d --name-only --merge-base "$BASE_SHA" | + <(git diff -z --diff-filter d --name-only --merge-base "$BASE_BRANCH" | while read -rd '' filename; do file -bi "$filename" | grep -q charset=binary || printf "%s\\0" "$filename"