[libc++] Move the check-generated-files job to Github Actions

[ldionne]

This allows running these quick checks faster than in our Buildkite pipeline, which has much more latency. This will also avoid blocking the rest of the testing pipeline in case the generated-files checks are failing.

[llvmbot]

@llvm/pr-subscribers-github-workflow

@llvm/pr-subscribers-libcxx

Author: Louis Dionne (ldionne)

Changes

This allows running these quick checks faster than in our Buildkite pipeline, which has much more latency. This will also avoid blocking the rest of the testing pipeline in case the generated-files checks are failing.

---

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

2 Files Affected:

• (added) .github/workflows/libcxx-check-generated-files.yml (+29)
• (modified) libcxx/utils/ci/buildkite-pipeline.yml (-18)

diff --git a/.github/workflows/libcxx-check-generated-files.yml b/.github/workflows/libcxx-check-generated-files.yml
new file mode 100644
index 000000000000000..609065c1e1c38aa
--- /dev/null
+++ b/.github/workflows/libcxx-check-generated-files.yml
@@ -0,0 +1,29 @@
+name: "Check libc++ generated files"
+on:
+  pull_request_target:
+    paths:
+      - 'libcxx/**'
+permissions:
+  pull-requests: write
+
+jobs:
+  check_generated_files:
+    runs-on: ubuntu-latest
+    steps:
+      - name: Fetch LLVM sources
+        uses: actions/checkout@v4
+        with:
+          fetch-depth: 2
+
+      - name: Install clang-format
+        uses: aminya/setup-cpp@v1
+        with:
+          clangformat: 17.0.1
+
+      - name: Install Ninja
+        uses: seanmiddleditch/gha-setup-ninja@master
+        with:
+          destination: build/ninja-install
+
+      - name: Check generated files
+        run: libcxx/utils/ci/run-buildbot check-generated-output
diff --git a/libcxx/utils/ci/buildkite-pipeline.yml b/libcxx/utils/ci/buildkite-pipeline.yml
index ebfb35eee91e1ed..8866a33d04d5a2d 100644
--- a/libcxx/utils/ci/buildkite-pipeline.yml
+++ b/libcxx/utils/ci/buildkite-pipeline.yml
@@ -33,24 +33,6 @@ steps:
   #
   # Light pre-commit tests for things like forgetting to update generated files.
   #
-  - label: "Generated output"
-    command: "libcxx/utils/ci/run-buildbot check-generated-output"
-    artifact_paths:
-      - "**/generated_output.patch"
-      - "**/generated_output.status"
-    env:
-        CC: "clang-${LLVM_HEAD_VERSION}"
-        CXX: "clang++-${LLVM_HEAD_VERSION}"
-        CLANG_FORMAT: "/usr/bin/clang-format-${LLVM_STABLE_VERSION}"
-    agents:
-      queue: "libcxx-builders"
-      os: "linux"
-    retry:
-      automatic:
-        - exit_status: -1  # Agent was lost
-          limit: 2
-    timeout_in_minutes: 120
-
   - label: "Documentation"
     command: "libcxx/utils/ci/run-buildbot documentation"
     artifact_paths:

[EricWF]

Can we just use & fix the existing formatting check?

[ldionne]

Can we just use & fix the existing formatting check?

As I explained on Discord:

There are two CI jobs in question here:

1. The formatting job that was set up by @tobiashieta . This job ensures that whatever changes have been touched in a PR are formatted properly except for any file in ignore_format.txt. That job works as intended and the only problem with it is that it skips libc++ headers that have no extension.
2. The check-generated-output job. That job runs various generation scripts to re-generate our modulemaps and a few other things, and then ensures that they have been re-generated by whoever created a PR. As part of this job, we ensure that the list of files to ignore format in (ignore_format.txt) is kept up to date. 90% of the problems we've been talking about in this thread have to do with the fact that this job has a few flaws

This patch tries to incrementally fix issues with (2)

[EricWF]

I would like a response to the comment about security, but otherwise LGTM.

[EricWF]

Are we using these actions elsewhere? Because using random actions associated with the project might be a security risk?

[EricWF]

We could specify a different container and skip a lot of these steps, no?

[tstellar]

We have our own action for installing ninja: llvm/actions/install-ninja

But do we really need this if the job is always going to run on Linux? Can we just apt install it?

[ldionne]

Is it more idiomatic to use llvm/actions/install-ninja or to install it manually? I don't really mind either way. I think I've seen uses: being used more often in actions but I don't mind using apt-get directly either.

[EricWF]

If we build a container and run the job in that container, we'll save time each iteration of the job, no?

[ldionne]

Yes, I do want to investigate setting up a container for libc++ jobs and storing that in the GH Docker registry to gradually replace our current DockerHub thing, but I would like to tackle that as a separate task to improve the CI situation in the short term.

[ldionne]

Sorry for the churn on the patch -- AFAICT this should be ready to review again now. I'm using on: pull_request instead to avoid security issues, and I tested this in ldionne#4.

[tru]

Looks good with some suggestions.

[ldionne]

@tstellar @EricWF Do you folks have more comments on this patch?