[llvm] [Github] Fetch all commits in PR for code formatting checks (PR #69766)

Aiden Grossman via llvm-commits llvm-commits at lists.llvm.org
Sat Oct 28 18:23:00 PDT 2023


https://github.com/boomanaiden154 updated https://github.com/llvm/llvm-project/pull/69766

>From f44998a0ee126e3cfcd881d3351deb41c958f77b Mon Sep 17 00:00:00 2001
From: Aiden Grossman <agrossman154 at yahoo.com>
Date: Fri, 20 Oct 2023 14:23:06 -0700
Subject: [PATCH 1/3] [Github] Fetch all commits in PR for code formatting
 checks

This patch ensures that all commits from a PR are fetched before running
the rest of the job. This allows the job to scale to any number of
commits per PR. In addition, this significantly reduces the time taken
in the files-changed step where the job would itself try to unshallow
the clone, but do so in some inefficient way. That step which previously
took ~10 minutes now takes 3 seconds after this patch.
---
 .github/workflows/pr-code-format.yml | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

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

>From eec1483d5ce51f5593edff7936a4e14f2393d3ad Mon Sep 17 00:00:00 2001
From: Aiden Grossman <agrossman154 at yahoo.com>
Date: Sat, 28 Oct 2023 16:44:11 -0700
Subject: [PATCH 2/3] Change checkout depth and move changed files location

---
 .github/workflows/pr-code-format.yml | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/.github/workflows/pr-code-format.yml b/.github/workflows/pr-code-format.yml
index 8ca27c2f6363b16..727f4dff40de52f 100644
--- a/.github/workflows/pr-code-format.yml
+++ b/.github/workflows/pr-code-format.yml
@@ -7,17 +7,24 @@ jobs:
   code_formatter:
     runs-on: ubuntu-latest
     steps:
-      - name: Fetch LLVM sources
-        uses: actions/checkout at v4
-        with:
-          fetch-depth: ${{ github.event.pull_request.commits }}
-
+      # Get changed files before checking out the repository to force the action
+      # to analyze the diff from the Github API rather than looking at the
+      # shallow clone and erroring out, which is significantly more prone to
+      # failure.
       - name: Get changed files
         id: changed-files
         uses: tj-actions/changed-files at v39
         with:
           separator: ","
 
+      - name: Calculate number of commits to fetch (PR)
+        run: echo "PR_FETCH_DEPTH=$(( ${{ github.event.pull_request.commits }} + 1 ))" >> "${GITHUB_ENV}"
+
+      - name: Fetch LLVM sources
+        uses: actions/checkout at v4
+        with:
+          fetch-depth: ${{ env.PR_FETCH_DEPTH }}
+
       - name: "Listed files"
         run: |
           echo "Formatting files:"

>From b2eb425310fa3fcd182b406bc828d53b11c783f7 Mon Sep 17 00:00:00 2001
From: Aiden Grossman <agrossman154 at yahoo.com>
Date: Sat, 28 Oct 2023 17:07:09 -0700
Subject: [PATCH 3/3] Switched refs, added checkout of target for security

---
 .github/workflows/pr-code-format.yml | 32 ++++++++++++++++++++--------
 1 file changed, 23 insertions(+), 9 deletions(-)

diff --git a/.github/workflows/pr-code-format.yml b/.github/workflows/pr-code-format.yml
index 727f4dff40de52f..c021c14f4a49530 100644
--- a/.github/workflows/pr-code-format.yml
+++ b/.github/workflows/pr-code-format.yml
@@ -17,13 +17,27 @@ jobs:
         with:
           separator: ","
 
-      - name: Calculate number of commits to fetch (PR)
+      - name: Calculate number of commits to fetch
         run: echo "PR_FETCH_DEPTH=$(( ${{ github.event.pull_request.commits }} + 1 ))" >> "${GITHUB_ENV}"
 
-      - name: Fetch LLVM sources
+      - name: Fetch PR sources
         uses: actions/checkout at v4
         with:
+          ref: ${{ github.event.pull_request.head.ref }}
           fetch-depth: ${{ env.PR_FETCH_DEPTH }}
+          path: pr-sources
+
+      # We need to make sure that we aren't executing/using any code from the
+      # PR for security reasons as we're using pull_request_target. Checkout
+      # the target branch with the necessary files.
+      - name: Fetch LLVM Sources
+        uses: actions/checkout at v4
+        with:
+          sparse-checkout: |
+            llvm/utils/git/requirements_formatting.txt
+            llvm/utils/git/code-format-helper.py
+          sparse-checkout-cone-mode: false
+          path: llvm-sources
 
       - name: "Listed files"
         run: |
@@ -40,21 +54,21 @@ jobs:
         with:
           python-version: '3.11'
           cache: 'pip'
-          cache-dependency-path: 'llvm/utils/git/requirements_formatting.txt'
+          cache-dependency-path: 'llvm-sources/llvm/utils/git/requirements_formatting.txt'
 
       - name: Install python dependencies
-        run: pip install -r llvm/utils/git/requirements_formatting.txt
+        run: pip install -r llvm-sources/llvm/utils/git/requirements_formatting.txt
 
       - name: Run code formatter
         env:
           GITHUB_PR_NUMBER: ${{ github.event.pull_request.number }}
-          START_REV: ${{ github.event.pull_request.base.sha }}
-          END_REV: ${{ github.event.pull_request.head.sha }}
+          PR_DEPTH: ${{ github.event.pull_request.commits }}
           CHANGED_FILES: ${{ steps.changed-files.outputs.all_changed_files }}
+        working-directory: ./pr-sources
         run: |
-          python llvm/utils/git/code-format-helper.py \
+          python ../llvm-sources/llvm/utils/git/code-format-helper.py \
             --token ${{ secrets.GITHUB_TOKEN }} \
             --issue-number $GITHUB_PR_NUMBER \
-            --start-rev $START_REV \
-            --end-rev $END_REV \
+            --start-rev HEAD~$PR_DEPTH \
+            --end-rev HEAD \
             --changed-files "$CHANGED_FILES"



More information about the llvm-commits mailing list