From f56c8c2fd1592d65aff7554c241beecafd42171c Mon Sep 17 00:00:00 2001 From: Aiden Grossman Date: Mon, 30 Oct 2023 12:23:51 -0700 Subject: [PATCH 1/8] Reland "[Github] Fetch all commits in PR for code formatting checks (#69766)" This commit relands 4aa12afb967bd7c5f051f3b72271f787f1a7538b. This was originally reverted as it caused fetch failures due to the usage of the wrong ref. I didn't catch this in testing as it was attempting to check the branch out from origin, where it didn't exist, but did on my fork as I never tested any cross-repo PRs. --- .github/workflows/pr-code-format.yml | 46 ++++++++++++++++++++-------- 1 file changed, 33 insertions(+), 13 deletions(-) diff --git a/.github/workflows/pr-code-format.yml b/.github/workflows/pr-code-format.yml index fa0e69211b14788..4e20bc5d9374788 100644 --- a/.github/workflows/pr-code-format.yml +++ b/.github/workflows/pr-code-format.yml @@ -7,17 +7,37 @@ jobs: code_formatter: runs-on: ubuntu-latest steps: - - name: Fetch LLVM sources - uses: actions/checkout@v4 - with: - fetch-depth: 2 # Fetches only the last 2 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@v39 with: separator: "," - fetch_depth: 2000 # Fetches only the last 2000 commits + + - name: Calculate number of commits to fetch + run: echo "PR_FETCH_DEPTH=$(( ${{ github.event.pull_request.commits }} + 1 ))" >> "${GITHUB_ENV}" + + - name: Fetch PR sources + uses: actions/checkout@v4 + with: + ref: ${{ github.event.pull_request.head.sha }} + 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@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: | @@ -34,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" From dbcd677d4057d4048c17aabf2f369705a7a66c2f Mon Sep 17 00:00:00 2001 From: Aiden Grossman Date: Thu, 2 Nov 2023 00:52:12 -0700 Subject: [PATCH 2/8] Add git log step for debugging --- .github/workflows/pr-code-format.yml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/.github/workflows/pr-code-format.yml b/.github/workflows/pr-code-format.yml index 4e20bc5d9374788..555081471a7ef7b 100644 --- a/.github/workflows/pr-code-format.yml +++ b/.github/workflows/pr-code-format.yml @@ -27,6 +27,13 @@ jobs: fetch-depth: ${{ env.PR_FETCH_DEPTH }} path: pr-sources + - name: Print commits being evaluated + working-directory: ./pr-sources + env: + PR_DEPTH: ${{ github.event.pull_request.commits }} + run: | + git log HEAD~$PR_DEPTH...HEAD + # 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. From 324fcd6d540e95d7a4c1d26126ba13f719fdabde Mon Sep 17 00:00:00 2001 From: Aiden Grossman Date: Fri, 3 Nov 2023 16:05:58 -0700 Subject: [PATCH 3/8] Check merge commits, only run if commit is mergeable --- .github/workflows/pr-code-format.yml | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/.github/workflows/pr-code-format.yml b/.github/workflows/pr-code-format.yml index 555081471a7ef7b..acc70c0b5131839 100644 --- a/.github/workflows/pr-code-format.yml +++ b/.github/workflows/pr-code-format.yml @@ -17,23 +17,13 @@ jobs: with: separator: "," - - name: Calculate number of commits to fetch - run: echo "PR_FETCH_DEPTH=$(( ${{ github.event.pull_request.commits }} + 1 ))" >> "${GITHUB_ENV}" - - name: Fetch PR sources uses: actions/checkout@v4 with: - ref: ${{ github.event.pull_request.head.sha }} - fetch-depth: ${{ env.PR_FETCH_DEPTH }} + ref: "refs/pull/${{ github.event.number }}/merge" + fetch-depth: 2 path: pr-sources - - name: Print commits being evaluated - working-directory: ./pr-sources - env: - PR_DEPTH: ${{ github.event.pull_request.commits }} - run: | - git log HEAD~$PR_DEPTH...HEAD - # 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. @@ -69,13 +59,12 @@ jobs: - name: Run code formatter env: GITHUB_PR_NUMBER: ${{ github.event.pull_request.number }} - PR_DEPTH: ${{ github.event.pull_request.commits }} CHANGED_FILES: ${{ steps.changed-files.outputs.all_changed_files }} working-directory: ./pr-sources run: | python ../llvm-sources/llvm/utils/git/code-format-helper.py \ --token ${{ secrets.GITHUB_TOKEN }} \ --issue-number $GITHUB_PR_NUMBER \ - --start-rev HEAD~$PR_DEPTH \ + --start-rev HEAD^ \ --end-rev HEAD \ --changed-files "$CHANGED_FILES" From 0747f8986beda4cf163f56d222d754975f3104d4 Mon Sep 17 00:00:00 2001 From: Aiden Grossman Date: Fri, 3 Nov 2023 21:46:37 -0700 Subject: [PATCH 4/8] test --- llvm/lib/CodeGen/MLRegAllocEvictAdvisor.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/llvm/lib/CodeGen/MLRegAllocEvictAdvisor.cpp b/llvm/lib/CodeGen/MLRegAllocEvictAdvisor.cpp index 114e7910dc27bbd..769371680642b15 100644 --- a/llvm/lib/CodeGen/MLRegAllocEvictAdvisor.cpp +++ b/llvm/lib/CodeGen/MLRegAllocEvictAdvisor.cpp @@ -10,6 +10,8 @@ // //===----------------------------------------------------------------------===// +// this line is way too fricking long and should definitely be caught by the c++ code formatter if it isn't just dumb as frick + #include "AllocationOrder.h" #include "RegAllocEvictionAdvisor.h" #include "RegAllocGreedy.h" From d6bcc0072e250b072532ec130c5df2c9f0252fe2 Mon Sep 17 00:00:00 2001 From: Aiden Grossman Date: Fri, 3 Nov 2023 21:53:50 -0700 Subject: [PATCH 5/8] Add tst file --- tst | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 tst diff --git a/tst b/tst new file mode 100644 index 000000000000000..139597f9cb07c5d --- /dev/null +++ b/tst @@ -0,0 +1,2 @@ + + From 73a579f7dd8dd395cbfd0e36ddb9d1f5e3db6c55 Mon Sep 17 00:00:00 2001 From: Aiden Grossman Date: Fri, 3 Nov 2023 22:05:50 -0700 Subject: [PATCH 6/8] try something --- .github/workflows/pr-code-format2.yml | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 .github/workflows/pr-code-format2.yml diff --git a/.github/workflows/pr-code-format2.yml b/.github/workflows/pr-code-format2.yml new file mode 100644 index 000000000000000..abae2292017dda3 --- /dev/null +++ b/.github/workflows/pr-code-format2.yml @@ -0,0 +1,11 @@ +name: "Check code formatting" +on: pull_request + +jobs: + code_formatter: + runs-on: ubuntu-latest + steps: + - name: Fetch PR sources + uses: actions/checkout@v4 + - name: Install python dependencies + run: git log -n 1 From 67ec99c5b1106189b26175114112e127f75c3c97 Mon Sep 17 00:00:00 2001 From: Aiden Grossman Date: Fri, 3 Nov 2023 22:12:04 -0700 Subject: [PATCH 7/8] Another test commit --- llvm/lib/CodeGen/MLRegAllocPriorityAdvisor.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/llvm/lib/CodeGen/MLRegAllocPriorityAdvisor.cpp b/llvm/lib/CodeGen/MLRegAllocPriorityAdvisor.cpp index 422781593a9c6bd..748720b82643968 100644 --- a/llvm/lib/CodeGen/MLRegAllocPriorityAdvisor.cpp +++ b/llvm/lib/CodeGen/MLRegAllocPriorityAdvisor.cpp @@ -10,6 +10,8 @@ // //===----------------------------------------------------------------------===// +// this is another test that the code formatter should pick up on and fix, hopefully without too much fuss because gosh dang + #include "AllocationOrder.h" #include "RegAllocGreedy.h" #include "RegAllocPriorityAdvisor.h" From bbe5a81baa19bd67ffede8d52cf0eb3c837a9626 Mon Sep 17 00:00:00 2001 From: Aiden Grossman Date: Fri, 3 Nov 2023 23:37:08 -0700 Subject: [PATCH 8/8] test --- test | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 test diff --git a/test b/test new file mode 100644 index 000000000000000..139597f9cb07c5d --- /dev/null +++ b/test @@ -0,0 +1,2 @@ + +