Increase git fetch depth

[llvm-beanz]

We've gotten ~750 commits in the last 7 days. Upping the fetch depth to 2000 will make it more likely that PRs up to 2 weeks old will have enough fetch history to find a common parent. This might address some of the failures we're seeing in the clang-format action where it cannot find a common base commit.

[llvmbot]

@llvm/pr-subscribers-github-workflow

Author: Chris B (llvm-beanz)

Changes

We've gotten ~750 commits in the last 7 days. Upping the fetch depth to 2000 will make it more likely that PRs up to 2 weeks old will have enough fetch history to find a common parent. This might address some of the failures we're seeing in the clang-format action where it cannot find a common base commit.

---

Full diff: https://github.com/llvm/llvm-project/pull/70946.diff

1 Files Affected:

• (modified) .github/workflows/pr-code-format.yml (+2-2)

diff --git a/.github/workflows/pr-code-format.yml b/.github/workflows/pr-code-format.yml
index 3a91ffb0b1ad9a7..5978d503f6f35d3 100644
--- a/.github/workflows/pr-code-format.yml
+++ b/.github/workflows/pr-code-format.yml
@@ -10,14 +10,14 @@ jobs:
       - name: Fetch LLVM sources
         uses: actions/checkout@v4
         with:
-          fetch-depth: 2
+          fetch-depth: 2000 # Fetches only the last 2000 commits
 
       - name: Get changed files
         id: changed-files
         uses: tj-actions/changed-files@v39
         with:
           separator: ","
-          fetch_depth: 100 # Fetches only the last 10 commits
+          fetch_depth: 2000 # Fetches only the last 2000 commits
 
       - name: "Listed files"
         run: |

[tstellar]

I still don't quite understand why so much history is needed.

[boomanaiden154]

LGTM with one comment.

This is a fairly bandaid fix. The changed-files action takes an incredible amount of time to pull commits. Still haven't gotten to the bottom of that, my current fix is to just move it to before the checkout to force it to grab the info from the API. While testing the code formatting action, I've also seen quite a few cases where a fetch-depth of 2000 still won't be good enough to find the merge base. That case is uncommon though.

I have a proper fix in #69766, but of course it broke fetching anything as soon as I pushed it, so it's been reverted for now. Still working on debugging it/reading up on what could be going wrong.

Landing this in the meantime sounds good to me though.

[boomanaiden154]

You can keep the fetch depth here 2. This action only checks out the base of the PR (which is the target branch) and doesn't actually checkout any PR code. The changed-files action is the one that actually checks out the code.

[llvm-beanz]

I'm also experimenting with a more permanent fix. I was thinking about adding something like:

     - name: Find merge base
        env:
          PR_NUMBER: ${{ github.event.number }}
        run: |
          until git merge-base $GITHUB_SHA pull/$PR_NUMBER
            do
            depth=$((depth+1000))
            git fetch --depth=$depth origin $GITHUB_HEAD_REF
            git fetch --depth=$depth origin refs/pull/$PR_NUMBER/head:pull/$PR_NUMBER
            if [ "$depth" -gt "10000" ]
            then
              exit 1
            fi
          done

That would incrementally walk up the depth until it either finds a merge base or tries too many times.

[boomanaiden154]

The action does something similar internally (I believe). I think if we want something that works but is inefficient, just fetching the whole repository would work. That significantly increases the time the action takes to run (makes it ~30 minutes from my experience with the documentation action), but it should always work.

I still think #69766 is the ideal solution. It works and is fast from all my testing, there's just something that breaks when it actually gets deployed. I just haven't had the time to reproduce the issue on my fork and fix it.

[llvm-beanz]

Hopefully my change will reduce the frequency enough that we don't have too much pain between now and when you can revisit.

Thanks!

[boomanaiden154]

Hopefully. Thanks for the patch!