[GitHub][CI] Run clang-format natively in ci-ubuntu container

[vbvictor]

Tested in #160193 (run logs).
With this patch, we skip "Install clang-format" step which usually takes 30sec (but can be up to 120sec) but add new step "Initialize containers" which adds 17sec.

If we calculate total time, this patch code-format job will take ~1m50sec vs ~2min for code-format job in main.

But also this patch gives us a lot of opportunities for improvements, e.g.:

• Shrink docker image size by creating a dedicated clang-format container. Most of "Initialize containers" time is spent in pulling so smaller image means less time
• Create venv, install dependencies inside docker build, which would give ~6sec speed up.

[llvmbot]

@llvm/pr-subscribers-github-workflow

Author: Baranov Victor (vbvictor)

Changes

Tested in #160193 (run logs).
With this patch, we skip "Install clang-format" step which usually take 30sec (but can be up to 120sec) but add new step "Initialize containers" which add 17sec.

If we calculate total time, this patch code-format job will take ~1m50sec vs ~2min for code-format job in main.

But also this patch gives us a lot of opportunities for improvements, e.g.:

• Shrink docker image size by creating a dedicated clang-format container. Most of "Initialize containers" time is spent in pulling so smaller image means less time
• Create venv, install dependencies inside docker build, which would give ~6sec speed up.

---

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

1 Files Affected:

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

diff --git a/.github/workflows/pr-code-format.yml b/.github/workflows/pr-code-format.yml
index 61c8680cd72a1..841b601ed3f4f 100644
--- a/.github/workflows/pr-code-format.yml
+++ b/.github/workflows/pr-code-format.yml
@@ -12,6 +12,8 @@ on:
 jobs:
   code_formatter:
     runs-on: ubuntu-24.04
+    container:
+      image: 'ghcr.io/llvm/ci-ubuntu-24.04:latest'
     timeout-minutes: 30
     concurrency:
       group: ${{ github.workflow }}-${{ github.event.pull_request.number }}
@@ -38,24 +40,13 @@ jobs:
         run: |
           echo "Formatting files:"
           echo "$CHANGED_FILES"
-
-      # The clang format version should always be upgraded to the first version
-      # of a release cycle (x.1.0) or the last version of a release cycle, or
-      # if there have been relevant clang-format backports.
-      - name: Install clang-format
-        uses: aminya/setup-cpp@17c11551771948abc5752bbf3183482567c7caf0 # v1.1.1
-        with:
-          clangformat: 21.1.0
-
-      - name: Setup Python env
-        uses: actions/setup-python@42375524e23c412d93fb67b49958b491fce71c38 # v5.4.0
-        with:
-          python-version: '3.11'
-          cache: 'pip'
-          cache-dependency-path: 'llvm/utils/git/requirements_formatting.txt'
+      
+      - name: Create python venv
+        run: |
+          python -m venv venv
 
       - name: Install python dependencies
-        run: pip install -r llvm/utils/git/requirements_formatting.txt
+        run: venv/bin/pip install -r llvm/utils/git/requirements_formatting.txt
 
       - name: Run code formatter
         env:
@@ -64,7 +55,7 @@ jobs:
         # Create an empty comments file so the pr-write job doesn't fail.
         run: |
           echo "[]" > comments &&
-          python ./llvm/utils/git/code-format-helper.py \
+          venv/bin/python ./llvm/utils/git/code-format-helper.py \
             --write-comment-to-file \
             --token ${{ secrets.GITHUB_TOKEN }} \
             --issue-number $GITHUB_PR_NUMBER \

[boomanaiden154]

We probably shouldn't be dropping setup-python. Removing the pip dependency cache (even if it doesn't hit unless it has been warmed on a PR) is probably going to be a significant regression.

[vbvictor]

Removing the pip dependency cache (even if it doesn't hit unless it has been warmed on a PR) is probably going to be a significant regression.

I've checked ~10 different workflow runs (runs.txt) and on average it currently takes ~7sec to complete "Setup Python env + Install python dependencies" steps. In my test run, it took 6sec to "Create venv + install dependencies" so it could be even a speedup (or a very small regression)

Apart from it, I observed that setup-python works poorly inside docker environment and in the past I failed to set up custom python and caching for clang-tidy job.

To sum up for this PR, I think we either can merge without caching and see if we made a net-positive in insights (we can always revert if average job time increased), or wait until a custom-container is made in #161083 so we wouldn't need any cache anymore.

[boomanaiden154]

I've checked ~10 different workflow runs (runs.txt) and on average it currently takes ~7sec to complete "Setup Python env + Install python dependencies" steps. In my test run, it took 6sec to "Create venv + install dependencies" so it could be even a speedup (or a very small regression)

Ack. Even if it took a bit of time, I think this should be a net positive.

Apart from it, I observed that setup-python works poorly inside docker environment and in the past I #154829 (comment) to set up custom python and caching for clang-tidy job.

Yeah, especially in a minimal environment. It needs dependencies.

[boomanaiden154]

I think this is fine for a temporary solution.

The only worry I have about this is that this patch ties the CI versions together, which we ideally don't want to do to avoid churn with the clang format version bumps. That should be temporary though assuming there is a concerted effort to get a clang-format specific container up and working.

[vbvictor]

The only worry I have about this is that this patch ties the CI versions together, which we ideally don't want to do to avoid churn with the clang format version bumps. That should be temporary though assuming there is a concerted effort to get a clang-format specific container up and working.

I'm fine postponing this PR for a reasonable amount of time until a separate container is created. Also will have more time to gather opinions.