[ci] Restrict clang-format to PR changes

Related github discussion: https://github.com/orgs/community/discussions/59677

For a given PR, the value of `github.event.pull_request.base.sha` always
corresponds to the SHA of the head/tip of the base branch at the moment
of either creation of the PR or the last force-push (regardless of whether
that force-push actually rebased the PR branch against base or was just
modifying an existing commit in it) to the PR branch.

In addition the merge commit is updated whenever a new commit is push
onto the PR branch but this merge commit is not against the (fixed)
`github.event.pull_request.base.sha` but is instead against the (moving)
tip of the target branch.

Currently (before this commit), our clang-format action will check the
differences between that commit and the merge commit.

In practice this means that if a PR languish and one or more PR is
actually merged into the master branch then the comparison will
now involved not only the commit introduced the PR but also all
the commits added to master since the PR creation (or its last
force push).

There is 2 general solutions to replace the `github.event.pull_request.base.sha`:

1. Find and use the non PR branch parent of the merge commit
2. Find and use the common ancestor of the master and the PR

This commit implement 2nd options by using `git merge-base` with the
arguments 'tip of the PR branch' and `github.event.pull_request.base.sha`
which is guaranteed to be ahead of/have a commong ancestor with
the PR branch.

Until there is a resolution to the above mentioned github discussion
this is more complex than before but more stable.

NOTE: This matters in our case since most existing files and some
PRs are merge without passing clang-format (in part because clang-format
still gives some significantly sub-optimal result in a few important
cases)

As a concrete example, the PR root-project#18587
was at some point in a state where the merge commit was:

*   354e754 - (3 hours ago) Merge e54a1ea into 59c28d5 — Philippe Canal

The tip of master was:

* | 59c28d5 - (5 hours ago) [nfc] mention deprecated ConcatFileName (root-project#18755) — ferdymercury

but the `github.event.pull_request.base.sha` was pointing to:

* | bda0b84 - (3 days ago) [cmake] Check if cplusplus is 23 by default — ferdymercury

As seen below this meant that the clang-format stage was reporting about
a lot more than what that PR had actually changed.

*   354e754 - (3 hours ago) Merge e54a1ea into 59c28d5 — Philippe Canal
|\
| * e54a1ea - (3 hours ago) [NFC] clang-format — Philippe Canal
| * 4c9bbf3 - (4 days ago) meta: Extend ClassInfo loading test — Philippe Canal
| * 7137bf0 - (2 weeks ago) roottest: switch test from DEPENDS to FIXTURES. — Philippe Canal
| * ca68ceb - (2 weeks ago) meta: Correct TClass::LoadClassInfo. — Philippe Canal
* | 59c28d5 - (5 hours ago) [nfc] mention deprecated ConcatFileName (root-project#18755) — ferdymercury
* | 0fe2f1f - (2 days ago) [RF] Fix memory leak of ConcatFileName return value in RooWorkspace — Jonas Rembser
* | eb0b903 - (6 hours ago) core: Add semantic ROOT::EnableImplicitMT (root-project#18694) — Philippe Canal
* | c814428 - (9 hours ago) [jsroot] dev 16/05/2025 with ws private members — Sergey Linev
* | bbff7ab - (9 hours ago) [qtweb] rename unload handler — Sergey Linev
* | af9d1e4 - (10 hours ago) [webgui][ui5] use websocket.isStandalone() — Sergey Linev
* | 768f741 - (10 hours ago) [webgui][ui5] replace title by tooltip in CheckBox — Sergey Linev
* | 2056891 - (10 hours ago) [webgui][ui5] use isChannel function of ws handler — Sergey Linev
* | 600573f - (2 days ago) [df] Add test for nested data members with same name — Vincenzo Eduardo Padulano
* | c8fd528 - (5 weeks ago) [tree] Speedup retrieval of branch names — Vincenzo Eduardo Padulano
* | 98d8160 - (33 hours ago) [Tutorials] Add Python dependencies for df101 and rf617. — Stephan Hageboeck
* | dd272ab - (15 hours ago) [math] add DeltaR to GenVector (root-project#18728) — ferdymercury
* | ddd8512 - (13 days ago) [test] Add test case — ferdymercury
* | ea465be - (2 weeks ago) [tree,hist] Fix parsing bugs in TTreeFormula and TFormula_v5 — ferdymercury
* | bda0b84 - (3 days ago) [cmake] Check if cplusplus is 23 by default — ferdymercury
* | 3049b5e - (2 weeks ago) [metacling] copy-paste from RTypes ClassDefBase into interpreted version — ferdymercury
* | aac51aa - (2 days ago) [MC] Correctly report stability of particles — Danilo Piparo
* | 6d6be1a - (2 days ago) [webgui] adjust stressGraphics_web.ref file — Sergey Linev
* | a198988 - (2 days ago) [jsroot] dev 15/05/2025 without RHist classes — Sergey Linev
* | b65a5d6 - (2 days ago) [roottest] fix leak by using TSystem::ConcatFileName — Sergey Linev
* | 470e732 - (2 days ago) [math] fix harmless initialization bug in halfsamplemode algorithm — ferdymercury
* | be3d7c1 - (2 days ago) [Math][GenVector] Substitute `M_PI*` with `TMath` function calls (root-project#18659) — Monica Dessole (origin/master, origin/HEAD)
* | 9a5f6ca - (7 days ago) [RF][HF] Don't create output directory if not needed — Jonas Rembser
* | a4a0933 - (7 days ago) [RF][HF] Add configuration parameter to disable workspace writing — Jonas Rembser
* | 6d570f6 - (7 days ago) [RF][HF] Make sure that RooMsgService is correctly reset — Jonas Rembser
* | 42a1c56 - (10 days ago) [core] Add tests for TClass lookup of STL containers. — Stephan Hageboeck
* | 9fe1956 - (10 days ago) [core] Fix a TClass lookup for STL containers. — Stephan Hageboeck
* | be95a45 - (4 days ago) [NFC] Move functions to anonymous namespace/clang-format TClassEdit. — Stephan Hageboeck
* | d0e79a5 - (2 days ago) [math] Add HalfSampleMode calculation to TMath — ferdymercury
* | ca36f75 - (3 days ago) [ci] Enable C++23 for macOS Beta and 15 — Danilo Piparo
* | 8ddf345 - (3 days ago) [nfc][DF] Clarify documentation of RDFHistoModels — ferdymercury
* | 073e81f - (3 days ago) [roottest] fine-tuning of io/xml test — Sergey Linev
* | 17500f5 - (4 days ago) [roottest] modernize io-xml tests — Sergey Linev
* | 078afaa - (4 days ago) [roottest] use cmake for io-xml tests — Sergey Linev
* | 85b49c7 - (3 days ago) [nfc][meta] Code formatting — Danilo Piparo
* | 65df0a9 - (3 days ago) [meta] Test handling of C11 _Atomic type specifier — Danilo Piparo
* | db23e0b - (3 days ago) [meta] Properly handle C11 atomic type specifier in normalisation routines — Danilo Piparo
|/
* 7a222ec - (4 days ago) build: Actually record dep from RooFitJSONInterface to Core. — Philippe Canal

Author: pcanal

.ci/format_script.sh

@@ -1,14 +1,19 @@
 #!/usr/bin/env bash
 
-echo "Running clang-format against branch $TRAVIS_BRANCH, with hash $BASE_COMMIT"
+if [ -z "$MERGE_BASE" ]; then
+  echo "Warning: merging base was not found!  Using the tip of $TRAVIS_BRANCH instead."
+  MERGE_BASE=$BASE_COMMIT
+fi
+
+echo "Running clang-format from $TRAVIS_PULL_REQUEST_BRANCH against branch $TRAVIS_BRANCH, with hash $MERGE_BASE"
 clang-format --version
-COMMIT_FILES=$(git diff --name-status $BASE_COMMIT | grep -i -v '.mjs$' | grep -i -v LinkDef | grep -v -E '^D +' | sed -E 's,^.[[:space:]]+,,')
+COMMIT_FILES=$(git diff --name-status $MERGE_BASE | grep -i -v '.mjs$' | grep -i -v LinkDef | grep -v -E '^D +' | sed -E 's,^.[[:space:]]+,,')
 
 RESULT_OUTPUT="no modified files to format"
 if [ ! -z "$COMMIT_FILES" ]; then
   # If COMMIT_FILES is empty, git-clang-format will take all the modified files
   # Therefore, we only actually run git-clang-format if there is anything to check
-  RESULT_OUTPUT="$(git-clang-format --commit $BASE_COMMIT --diff --binary `which clang-format` $COMMIT_FILES)"
+  RESULT_OUTPUT="$(git-clang-format --commit $MERGE_BASE --diff --binary `which clang-format` $COMMIT_FILES)"
 fi
 
 if [ "$RESULT_OUTPUT" == "no modified files to format" ] \
@@ -20,14 +25,14 @@ else
   echo  -e "clang-format failed with the following diff:\n"
   echo "$RESULT_OUTPUT"
 
-  echo -e "\nTo reproduce it locally please run"
-  echo -e "\tgit checkout $TRAVIS_PULL_REQUEST_BRANCH"
-  echo -e "\tgit-clang-format --commit $BASE_COMMIT --diff --binary $(which clang-format) # adjust to point to the local clang-format"
+  echo -e "\n\nTo reproduce it locally please run (skip the first 2 steps if already have the branch checked out):"
+  echo -e "\tgit checkout -b \$USER-$TRAVIS_PULL_REQUEST_BRANCH master"
+  echo -e "\tgit pull $TRAVIS_PULL_REQUEST_REPO $TRAVIS_PULL_REQUEST_BRANCH"
+  echo -e "\tgit-clang-format --commit $MERGE_BASE --diff --binary $(which clang-format) # adjust to point to the local clang-format"
 
   echo -e "\nConsider running the following to apply the code formatting changes without bloating the history."
-  echo -e "\t\tgit checkout $TRAVIS_PULL_REQUEST_BRANCH"
   echo -e "\t\tgit rebase -i -x \"git-clang-format master && git commit -a --allow-empty --fixup=HEAD\" --strategy-option=theirs origin/master"
-  echo -e "\t Then inspect the results with git log --oneline"
+  echo -e "\t Then inspect the results with 'git log --oneline' and 'git show'"
   echo -e "\t Then squash without poluting the history with: git rebase --autosquash -i master"
 
   exit 1

.github/workflows/code_analysis.yml

@@ -20,12 +20,18 @@ jobs:
     runs-on: ubuntu-latest
     env:
       TRAVIS_BRANCH: ${{ github.base_ref }} 
+      TRAVIS_PULL_REQUEST_REPO: ${{ github.event.pull_request.head.repo.html_url }}
       TRAVIS_PULL_REQUEST_BRANCH: ${{ github.head_ref }}
       BASE_COMMIT: ${{ github.event.pull_request.base.sha }}
     steps:
     - uses: actions/checkout@v4
+      with:
+        fetch-depth: 1024
+        ref: ${{ github.event.pull_request.head.sha }}
     - name: Fetch base sha
-      run: git fetch --depth=1 origin +${{github.event.pull_request.base.sha}}:origin/base_sha
+      run: git fetch --depth=1024 origin +${{github.event.pull_request.base.sha}}:origin/base_sha
+    - name: Determine merge base
+      run: echo "MERGE_BASE=$(git merge-base ${{ github.event.pull_request.base.sha }} HEAD)" >> $GITHUB_ENV
     - name: install clang-format
       run: sudo apt-get install -y clang-format
     - name: run clang-format script