Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Github] Add initial version of precommit checks #80951

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

boomanaiden154
Copy link
Contributor

This patch adds an initial version of the LLVM precommit checks. These checks should be reasonably performant (full test cycle in ~30 minutes with a fully warm cache, ~80 minutes with a completely cold cache) and should catch regressions within LLVM.

This is mainly intended to test the scalability of the current design and to start eliminating issues before we begin to scale to other subprojects.

This patch adds an initial version of the LLVM precommit checks. These
checks should be reasonably performant (full test cycle in ~30 minutes
with a fully warm cache, ~80 minutes with a completely cold cache) and
should catch regressions within LLVM.

This is mainly intended to test the scalability of the current design
and to start eliminating issues before we begin to scale to other
subprojects.
@llvmbot
Copy link
Collaborator

llvmbot commented Feb 7, 2024

@llvm/pr-subscribers-github-workflow

Author: Aiden Grossman (boomanaiden154)

Changes

This patch adds an initial version of the LLVM precommit checks. These checks should be reasonably performant (full test cycle in ~30 minutes with a fully warm cache, ~80 minutes with a completely cold cache) and should catch regressions within LLVM.

This is mainly intended to test the scalability of the current design and to start eliminating issues before we begin to scale to other subprojects.


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

1 Files Affected:

  • (added) .github/workflows/precommit.yaml (+47)
diff --git a/.github/workflows/precommit.yaml b/.github/workflows/precommit.yaml
new file mode 100644
index 0000000000000..8285a19b684f3
--- /dev/null
+++ b/.github/workflows/precommit.yaml
@@ -0,0 +1,47 @@
+name: "Precommit tests"
+
+permissions:
+  contents: read
+
+on:
+  pull_request:
+    branches:
+      - main
+    paths:
+      - '.github/workflows/precommit.yaml'
+      - 'llvm/**'
+
+jobs:
+  build-llvm:
+    name: "Build and test LLVM"
+    runs-on: ubuntu-latest
+    container:
+      image: ghcr.io/llvm/ci-ubuntu-22.04:latest
+    steps:
+      - name: Fetch LLVM sources
+        uses: actions/checkout@v4
+        with:
+          fetch-depth: 1
+      - name: Setup ccache
+        uses: hendrikmuhs/ccache-action@v1
+        with:
+          max-size: 500M
+          variant: sccache
+          key: precommit-linux
+      - name: Configure LLVM
+        run: |
+          cmake -B llvm-build -GNinja \
+            -DCMAKE_BUILD_TYPE=Release \
+            -DCMAKE_C_COMPILER_LAUNCHER=sccache \
+            -DCMAKE_CXX_COMPILER_LAUNCHER=sccache \
+            -DLLVM_ENABLE_ASSERTIONS=ON \
+            -DCMAKE_C_COMPILER=clang \
+            -DCMAKE_CXX_COMPILER=clang++ \
+            -DLLVM_LIT_ARGS="-v --no-progress-bar" \
+            ./llvm
+      - name: Build LLVM
+        run: |
+          ninja -C llvm-build llvm-test-depends
+      - name: Check LLVM
+        run: |
+          ninja -C llvm-build check-llvm

@boomanaiden154
Copy link
Contributor Author

I think this is in a workable enough state at this point given the recent container work. After this (and fixing any following issues), I plan on working on the following:

  • Enabling more subprojects using logic from the existing premerge checks to select what to build to test (as per the RFC discussion).
  • Introducing a windows docker container/workflow to get testing coverage back there in hopefully a reasonably performant manner, potentially only running after Linux tests have passed to reduce job load.
  • Potentially moving to label-triggered tests so we could potentially make this required at some point and could enable opt-in tests.

Probably a couple other misc things in there. This will help us figure out system scalability however and get things rolling in those directions.

@boomanaiden154 boomanaiden154 changed the title t[Github] Add initial version of precommit checks [Github] Add initial version of precommit checks Feb 7, 2024
Copy link
Collaborator

@tstellar tstellar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before this gets committed, we need to make sure we get consensus on the RFC thread. I'm not sure we are there yet.

.github/workflows/precommit.yaml Outdated Show resolved Hide resolved
.github/workflows/precommit.yaml Outdated Show resolved Hide resolved
.github/workflows/precommit.yaml Outdated Show resolved Hide resolved
@@ -0,0 +1,47 @@
name: "Precommit tests"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think we'll be able to use the same file for Windows testing? If not, I would call this "Linux Precommit Tests" and change the filename too.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm planning on a separate file per OS. Github doesn't have native runs-on support for containers on Windows, so we'll have to run the docker commands explicitly. The container image would need to be different either way, which means we would at least need separate jobs. I'm hoping to make sure common parts are pulled out into something common, but the OSes are different (at least as exposed to GHA for the way I want to use it).

.github/workflows/precommit.yaml Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants