Merge bitcoin#28573: github actions: Fix test-one-commit when parent of head is merge commit

88c8e3a github actions: Fix test-one-commit when parent of head is merge commit (Ryan Ofsky)

Pull request description:

  Instead of figuring out the commit *after* the last merge and rebasing on that with a ~1 suffix, just figure out the last merge commit directly and rebase on it. This way, if HEAD happens to be a merge commit, the rebase just succeeds immediately without blank variables or errors.

  Explanation of the problem from bitcoin#28497 (comment):

  > The problem is that the PR only contains a one commit after the last merge, so the job _should_ be skipped, but the `pull_request.commits != 1` check is not smart enough to skip it because the PR is based on another PR and has merge ancestor commits. So specifically what happens is that after HEAD~ is checked out, the new HEAD  is a merge commit, so the range `$(git log --merges -1 --format=%H)..HEAD` is equivalent to HEAD..HEAD, which is empty, so the `COMMIT_AFTER_LAST_MERGE` variable is empty and the rebase command fails.

  Note: In the current version of this PR, the "test each commit" job is skipped, because this PR only contains a single commit. But I manually verified the code works in earlier versions of the PR that included dummy commits.

ACKs for top commit:
  maflcko:
    lgtmrecr ACK 88c8e3a
  RandyMcMillan:
    utACK 88c8e3a

Tree-SHA512: a6865b5c8b96eb0b622b3255971a3cf050dd0f5a356cdfcf7f0cbb659e4a363612e8e62b3ae4fd6b5d9a40bc29176891bc4690659b026c5ef8feea25c8e263cc

Author: fanquake

.github/workflows/ci.yml

@@ -31,19 +31,41 @@ jobs:
     env:
       MAX_COUNT: 6
     steps:
-      - run: echo "FETCH_DEPTH=$((${{ github.event.pull_request.commits }} + 2))" >> "$GITHUB_ENV"
+      - name: Determine fetch depth
+        run: echo "FETCH_DEPTH=$((${{ github.event.pull_request.commits }} + 2))" >> "$GITHUB_ENV"
       - uses: actions/checkout@v4
         with:
           ref: ${{ github.event.pull_request.head.sha }}
           fetch-depth: ${{ env.FETCH_DEPTH }}
-      - run: |
+      - name: Determine commit range
+        run: |
+          # Checkout HEAD~ and find the test base commit
+          # Checkout HEAD~ because it would be wasteful to rerun tests on the PR
+          # head commit that are already run by other jobs.
           git checkout HEAD~
-          echo "COMMIT_AFTER_LAST_MERGE=$(git log $(git log --merges -1 --format=%H)..HEAD --format=%H --max-count=${{ env.MAX_COUNT }} | tail -1)" >> "$GITHUB_ENV"
+          # Figure out test base commit by listing ancestors of HEAD, excluding
+          # ancestors of the most recent merge commit, limiting the list to the
+          # newest MAX_COUNT ancestors, ordering it from oldest to newest, and
+          # taking the first one.
+          #
+          # If the branch contains up to MAX_COUNT ancestor commits after the
+          # most recent merge commit, all of those commits will be tested. If it
+          # contains more, only the most recent MAX_COUNT commits will be
+          # tested.
+          #
+          # In the command below, the ^@ suffix is used to refer to all parents
+          # of the merge commit as described in:
+          # https://git-scm.com/docs/git-rev-parse#_other_rev_parent_shorthand_notations
+          # and the ^ prefix is used to exclude these parents and all their
+          # ancestors from the rev-list output as described in:
+          # https://git-scm.com/docs/git-rev-list
+          echo "TEST_BASE=$(git rev-list -n$((${{ env.MAX_COUNT }} + 1)) --reverse HEAD ^$(git rev-list -n1 --merges HEAD)^@ | head -1)" >> "$GITHUB_ENV"
       - run: sudo apt install clang ccache build-essential libtool autotools-dev automake pkg-config bsdmainutils python3-zmq libevent-dev libboost-dev libsqlite3-dev libdb++-dev systemtap-sdt-dev libminiupnpc-dev libnatpmp-dev libqt5gui5 libqt5core5a libqt5dbus5 qttools5-dev qttools5-dev-tools qtwayland5 libqrencode-dev -y
       - name: Compile and run tests
         run: |
+          # Run tests on commits after the last merge commit and before the PR head commit
           # Use clang++, because it is a bit faster and uses less memory than g++
-          git rebase --exec "echo Running test-one-commit on \$( git log -1 ) && ./autogen.sh && CC=clang CXX=clang++ ./configure && make clean && make -j $(nproc) check && ./test/functional/test_runner.py -j $(( $(nproc) * 2 ))" ${{ env.COMMIT_AFTER_LAST_MERGE }}~1
+          git rebase --exec "echo Running test-one-commit on \$( git log -1 ) && ./autogen.sh && CC=clang CXX=clang++ ./configure && make clean && make -j $(nproc) check && ./test/functional/test_runner.py -j $(( $(nproc) * 2 ))" ${{ env.TEST_BASE }}
 
   macos-native-x86_64:
     name: 'macOS 13 native, x86_64, no depends, sqlite only, gui'