-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[clang][ci] Remove unnecessary BuildKite jobs for Clang #93233
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
Conversation
1. Remove the format-checking job from the BuildKite pipeline. We now have a monorepo-wide format checker implemented with Github Actions, so that should not be necessary anymore. 2. Stop building and testing Clang on Windows via the clang-ci pipeline. We already do that in the github-pull-requests pipeline, so that's just duplicate work. 3. Stop testing Clang on Linux in the clang-ci pipeline. We already do that in the github-pull-requests pipeline too, so that's also duplicate work. For now we still build Clang because the other jobs in the clang-ci pipeline require its artifacts, but that could be improved.
@llvm/pr-subscribers-clang Author: Louis Dionne (ldionne) Changes
Full diff: https://github.com/llvm/llvm-project/pull/93233.diff 2 Files Affected:
diff --git a/clang/utils/ci/buildkite-pipeline.yml b/clang/utils/ci/buildkite-pipeline.yml
index 7a679176038c6..86cfcf35cc867 100644
--- a/clang/utils/ci/buildkite-pipeline.yml
+++ b/clang/utils/ci/buildkite-pipeline.yml
@@ -17,18 +17,7 @@ env:
# LLVM RELEASE bump version
LLVM_HEAD_VERSION: "17"
steps:
- - label: "Format"
- commands:
- - "clang/utils/ci/run-buildbot check-format"
- agents:
- queue: "linux"
- retry:
- automatic:
- - exit_status: -1 # Agent was lost
- limit: 2
- timeout_in_minutes: 120
-
- - label: "Building and testing clang (Linux)"
+ - label: "Building Clang (Linux)"
commands:
- "clang/utils/ci/run-buildbot build-clang"
agents:
@@ -39,21 +28,9 @@ steps:
limit: 2
timeout_in_minutes: 120
- - label: "Building and testing clang (Windows)"
- commands:
- - "C:\\BuildTools\\Common7\\Tools\\VsDevCmd.bat -arch=amd64 -host_arch=amd64"
- - "bash clang/utils/ci/run-buildbot build-clang-windows"
- agents:
- queue: "windows"
- retry:
- automatic:
- - exit_status: -1 # Agent was lost
- limit: 2
- timeout_in_minutes: 120
-
- wait
- - label: "Running libc++ test suite in C++03"
+ - label: "Testing libc++ with just-built Clang (C++03)"
commands:
- "clang/utils/ci/run-buildbot generic-cxx03"
artifact_paths:
@@ -70,7 +47,7 @@ steps:
limit: 2
timeout_in_minutes: 120
- - label: "Running libc++ test suite in C++26"
+ - label: "Testing libc++ with just-built Clang (C++26)"
commands:
- "clang/utils/ci/run-buildbot generic-cxx26"
artifact_paths:
@@ -87,7 +64,7 @@ steps:
limit: 2
timeout_in_minutes: 120
- - label: "Running libc++ test suite with Clang Modules"
+ - label: "Testing libc++ with just-built Clang (w/ Clang Modules)"
commands:
- "clang/utils/ci/run-buildbot generic-modules"
artifact_paths:
diff --git a/clang/utils/ci/run-buildbot b/clang/utils/ci/run-buildbot
index f47ffb5cbd38d..6490d8e8f5b3b 100755
--- a/clang/utils/ci/run-buildbot
+++ b/clang/utils/ci/run-buildbot
@@ -69,13 +69,6 @@ cmake --version
ninja --version
case "${BUILDER}" in
-check-format)
- echo "*** Checking for trailing whitespace left in Clang source files ***"
- if grep -rnI '[[:blank:]]$' clang/lib clang/include clang/docs; then
- echo "*** Trailing whitespace has been found in Clang source files as described above ***"
- exit 1
- fi
-;;
build-clang)
mkdir install
# We use Release here to avoid including debug information. Otherwise, the
@@ -96,23 +89,6 @@ build-clang)
ccache -s
tar -cJvf install.tar.xz install/
buildkite-agent artifact upload --debug install.tar.xz
-
- ninja -C ${BUILD_DIR} check-clang
-;;
-build-clang-windows)
- cmake -S llvm -B ${BUILD_DIR} -G Ninja \
- -D CMAKE_C_COMPILER_LAUNCHER=sccache \
- -D CMAKE_CXX_COMPILER_LAUNCHER=sccache \
- -D CMAKE_BUILD_TYPE=Release \
- -D CMAKE_INSTALL_PREFIX=install-windows \
- -D LLVM_ENABLE_PROJECTS="clang;compiler-rt" \
- -D LLVM_ENABLE_ASSERTIONS=ON \
- -D LLVM_BUILD_EXAMPLES=ON \
- -D COMPILER_RT_BUILD_LIBFUZZER=OFF \
- -D COMPILER_RT_BUILD_ORC=OFF
-
- ninja -C ${BUILD_DIR} install-clang install-clang-resource-headers
- ninja -C ${BUILD_DIR} check-clang
;;
generic-cxx03)
buildkite-agent artifact download install.tar.xz .
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we aren't running the tests any more, can we build clang with -DLLVM_TARGETS_TO_BUILD=Native
Do we even need to build Clang and run libc++ jobs? I don't even see them linked in libc++ PR, even besides the fact that I think libc++ is fully tested with GitHub Actions. |
What we're testing is that the changes to Clang didn't break libc++. We're testing Clang via libc++ -- we're not testing libc++ itself. So yes, this is still relevant. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, I'll try to add this testing to "GitHub Pull Requests" later so that we can avoid building Clang twice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks, this should help free up some builder resources.
I will merge once the CI has passed. If someone else sees the CI green, feel free to merge too. @Endilll We could indeed "flatten" the We would instead expand to the pipeline itself and it would run inside the The same treatment could potentially be applied to libc++ although there will be other challenges (namely generating a valid pipeline YAML inside |
Following the discussion in #93233 (comment), this patch merges `clang-ci` pipeline into main `GitHub Pull Requests` pipeline. `clang-ci` enables additional test coverage for Clang by compiling it, and then using it to compile and test libc++, libc++abi, and libunwind in C++03, C++26, and Clang Modules modes. Additional work we skip and total time savings we should see: 1. Checking out the repo to generate the clang-ci pipeline (2 minutes) 2. Building Clang (3.5 minutes) 3. Uploading the artifacts once, then downloading them 3 times and unpacking 3 times (0.5 minutes) Note that because previously-split jobs for each mode are now under a single Linux job, it now takes around 8 minutes more see the Linux CI results despite total time savings. The primary goal of this patch is to reduce the load of CI by removing duplicated work. I consider this goal achieved. I could keep the job parallelism we had (3 libc++ jobs depending on a main Linux job), but I don't consider it worth the effort and opportunity cost, because parallelism is not helping once the pool of builders is fully subscribed.
Remove the format-checking job from the BuildKite pipeline. We now have a monorepo-wide format checker implemented with Github Actions, so that should not be necessary anymore.
Stop building and testing Clang on Windows via the clang-ci pipeline. We already do that in the github-pull-requests pipeline, so that's just duplicate work.
Stop testing Clang on Linux in the clang-ci pipeline. We already do that in the github-pull-requests pipeline too, so that's also duplicate work. For now we still build Clang because the other jobs in the clang-ci pipeline require its artifacts, but that could be improved.