-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
[libc++] Use GitHub-provided runners for the windows CI #79326
Conversation
3445df8
to
2f5a9d2
Compare
@mstorsjo Could you take a look here? I've tried a bunch of stuff, but couldn't really get things to run properly. |
Sure, I can try to set this up. I have tested running libcxx tests on Windows GitHub Actions runners before, and got it working before (for a subset of configs, not using the libcxx |
I got it working for all of our Windows CI configs. See the two topmost commits at https://github.com/mstorsjo/llvm-project/commits/libcxx-gh-win-runners. The one setting up the actions stuff is quite straightforward. The other one fixes a seriously annoying issue in how CMake picks python, please read the commit message thoroughly. Also, running the libcxx tests on Github runners used to be quite annoying as they used to be quite limited (2 cores, 8 GB memory only), so earlier, I've had to set |
3b4bf6e
to
0439ebc
Compare
@llvm/pr-subscribers-libcxx Author: Nikolas Klauser (philnik777) ChangesFull diff: https://github.com/llvm/llvm-project/pull/79326.diff 4 Files Affected:
diff --git a/.github/workflows/libcxx-build-and-test.yaml b/.github/workflows/libcxx-build-and-test.yaml
index 5727b956dc6dd2..3682ab6f61386a 100644
--- a/.github/workflows/libcxx-build-and-test.yaml
+++ b/.github/workflows/libcxx-build-and-test.yaml
@@ -199,3 +199,44 @@ jobs:
**/CMakeError.log
**/CMakeOutput.log
**/crash_diagnostics/*
+ windows:
+ runs-on: windows-2022
+ strategy:
+ fail-fast: false
+ matrix:
+ include:
+ - { config: clang-cl-dll, mingw: false }
+ - { config: clang-cl-static, mingw: false }
+ - { config: clang-cl-no-vcruntime, mingw: false }
+ - { config: clang-cl-debug, mingw: false }
+ - { config: clang-cl-static-crt, mingw: false }
+ - { config: mingw-dll, mingw: true }
+ - { config: mingw-static, mingw: true }
+ - { config: mingw-dll-i686, mingw: true }
+ steps:
+ - uses: actions/checkout@v4
+ - name: Install dependencies
+ run: |
+ choco install -y ninja wget
+ pip install psutil
+ - name: Install a current LLVM
+ if: ${{ matrix.mingw != true }}
+ run: |
+ choco install -y llvm --version=17.0.6
+ - name: Install llvm-mingw
+ if: ${{ matrix.mingw == true }}
+ run: |
+ curl -LO https://github.com/mstorsjo/llvm-mingw/releases/download/20231128/llvm-mingw-20231128-ucrt-x86_64.zip
+ powershell Expand-Archive llvm-mingw*.zip -DestinationPath .
+ del llvm-mingw*.zip
+ mv llvm-mingw* c:\llvm-mingw
+ echo "c:\llvm-mingw\bin" | Out-File -FilePath $Env:GITHUB_PATH -Encoding utf8 -Append
+ - name: Add Git Bash to the path
+ run: |
+ echo "c:\Program Files\Git\usr\bin" | Out-File -FilePath $Env:GITHUB_PATH -Encoding utf8 -Append
+ - name: Set up the MSVC dev environment
+ if: ${{ matrix.mingw != true }}
+ uses: ilammy/msvc-dev-cmd@v1
+ - name: Build and test
+ run: |
+ bash libcxx/utils/ci/run-buildbot ${{ matrix.config }}
diff --git a/libcxx/trigger b/libcxx/trigger
new file mode 100644
index 00000000000000..e69de29bb2d1d6
diff --git a/libcxx/utils/ci/buildkite-pipeline.yml b/libcxx/utils/ci/buildkite-pipeline.yml
index a7c44dab709391..e42262620d5fb0 100644
--- a/libcxx/utils/ci/buildkite-pipeline.yml
+++ b/libcxx/utils/ci/buildkite-pipeline.yml
@@ -57,56 +57,6 @@ environment_definitions:
steps:
-- group: ':windows: Windows'
- steps:
- - label: Clang-cl (DLL)
- command: bash libcxx/utils/ci/run-buildbot clang-cl-dll
- agents:
- queue: windows
- <<: *common
-
- - label: Clang-cl (Static)
- command: bash libcxx/utils/ci/run-buildbot clang-cl-static
- agents:
- queue: windows
- <<: *common
-
- - label: Clang-cl (no vcruntime exceptions)
- command: bash libcxx/utils/ci/run-buildbot clang-cl-no-vcruntime
- <<: *common
- agents:
- queue: windows
-
- - label: Clang-cl (Debug mode)
- command: bash libcxx/utils/ci/run-buildbot clang-cl-debug
- agents:
- queue: windows
- <<: *common
-
- - label: Clang-cl (Static CRT)
- command: bash libcxx/utils/ci/run-buildbot clang-cl-static-crt
- agents:
- queue: windows
- <<: *common
-
- - label: MinGW (DLL, x86_64)
- command: bash libcxx/utils/ci/run-buildbot mingw-dll
- agents:
- queue: windows
- <<: *common
-
- - label: MinGW (Static, x86_64)
- command: bash libcxx/utils/ci/run-buildbot mingw-static
- agents:
- queue: windows
- <<: *common
-
- - label: MinGW (DLL, i686)
- command: bash libcxx/utils/ci/run-buildbot mingw-dll-i686
- agents:
- queue: windows
- <<: *common
-
- group: ':mac: Apple'
steps:
- label: MacOS x86_64
diff --git a/runtimes/CMakeLists.txt b/runtimes/CMakeLists.txt
index 742334328fd32a..634ffe710b06b8 100644
--- a/runtimes/CMakeLists.txt
+++ b/runtimes/CMakeLists.txt
@@ -155,6 +155,9 @@ set(LLVM_COMPILER_CHECKED ON)
include(AddLLVM)
include(HandleLLVMOptions)
+# Loot at the PATH first to avoid a version mismatch between the command-line
+# python and the CMake-found version
+set(Python3_FIND_REGISTRY LAST)
find_package(Python3 REQUIRED COMPONENTS Interpreter)
# Host triple is used by tests to check if they are running natively.
|
@llvm/pr-subscribers-github-workflow Author: Nikolas Klauser (philnik777) ChangesFull diff: https://github.com/llvm/llvm-project/pull/79326.diff 4 Files Affected:
diff --git a/.github/workflows/libcxx-build-and-test.yaml b/.github/workflows/libcxx-build-and-test.yaml
index 5727b956dc6dd23..3682ab6f61386a4 100644
--- a/.github/workflows/libcxx-build-and-test.yaml
+++ b/.github/workflows/libcxx-build-and-test.yaml
@@ -199,3 +199,44 @@ jobs:
**/CMakeError.log
**/CMakeOutput.log
**/crash_diagnostics/*
+ windows:
+ runs-on: windows-2022
+ strategy:
+ fail-fast: false
+ matrix:
+ include:
+ - { config: clang-cl-dll, mingw: false }
+ - { config: clang-cl-static, mingw: false }
+ - { config: clang-cl-no-vcruntime, mingw: false }
+ - { config: clang-cl-debug, mingw: false }
+ - { config: clang-cl-static-crt, mingw: false }
+ - { config: mingw-dll, mingw: true }
+ - { config: mingw-static, mingw: true }
+ - { config: mingw-dll-i686, mingw: true }
+ steps:
+ - uses: actions/checkout@v4
+ - name: Install dependencies
+ run: |
+ choco install -y ninja wget
+ pip install psutil
+ - name: Install a current LLVM
+ if: ${{ matrix.mingw != true }}
+ run: |
+ choco install -y llvm --version=17.0.6
+ - name: Install llvm-mingw
+ if: ${{ matrix.mingw == true }}
+ run: |
+ curl -LO https://github.com/mstorsjo/llvm-mingw/releases/download/20231128/llvm-mingw-20231128-ucrt-x86_64.zip
+ powershell Expand-Archive llvm-mingw*.zip -DestinationPath .
+ del llvm-mingw*.zip
+ mv llvm-mingw* c:\llvm-mingw
+ echo "c:\llvm-mingw\bin" | Out-File -FilePath $Env:GITHUB_PATH -Encoding utf8 -Append
+ - name: Add Git Bash to the path
+ run: |
+ echo "c:\Program Files\Git\usr\bin" | Out-File -FilePath $Env:GITHUB_PATH -Encoding utf8 -Append
+ - name: Set up the MSVC dev environment
+ if: ${{ matrix.mingw != true }}
+ uses: ilammy/msvc-dev-cmd@v1
+ - name: Build and test
+ run: |
+ bash libcxx/utils/ci/run-buildbot ${{ matrix.config }}
diff --git a/libcxx/trigger b/libcxx/trigger
new file mode 100644
index 000000000000000..e69de29bb2d1d64
diff --git a/libcxx/utils/ci/buildkite-pipeline.yml b/libcxx/utils/ci/buildkite-pipeline.yml
index a7c44dab7093911..e42262620d5fb01 100644
--- a/libcxx/utils/ci/buildkite-pipeline.yml
+++ b/libcxx/utils/ci/buildkite-pipeline.yml
@@ -57,56 +57,6 @@ environment_definitions:
steps:
-- group: ':windows: Windows'
- steps:
- - label: Clang-cl (DLL)
- command: bash libcxx/utils/ci/run-buildbot clang-cl-dll
- agents:
- queue: windows
- <<: *common
-
- - label: Clang-cl (Static)
- command: bash libcxx/utils/ci/run-buildbot clang-cl-static
- agents:
- queue: windows
- <<: *common
-
- - label: Clang-cl (no vcruntime exceptions)
- command: bash libcxx/utils/ci/run-buildbot clang-cl-no-vcruntime
- <<: *common
- agents:
- queue: windows
-
- - label: Clang-cl (Debug mode)
- command: bash libcxx/utils/ci/run-buildbot clang-cl-debug
- agents:
- queue: windows
- <<: *common
-
- - label: Clang-cl (Static CRT)
- command: bash libcxx/utils/ci/run-buildbot clang-cl-static-crt
- agents:
- queue: windows
- <<: *common
-
- - label: MinGW (DLL, x86_64)
- command: bash libcxx/utils/ci/run-buildbot mingw-dll
- agents:
- queue: windows
- <<: *common
-
- - label: MinGW (Static, x86_64)
- command: bash libcxx/utils/ci/run-buildbot mingw-static
- agents:
- queue: windows
- <<: *common
-
- - label: MinGW (DLL, i686)
- command: bash libcxx/utils/ci/run-buildbot mingw-dll-i686
- agents:
- queue: windows
- <<: *common
-
- group: ':mac: Apple'
steps:
- label: MacOS x86_64
diff --git a/runtimes/CMakeLists.txt b/runtimes/CMakeLists.txt
index 742334328fd32af..634ffe710b06b8e 100644
--- a/runtimes/CMakeLists.txt
+++ b/runtimes/CMakeLists.txt
@@ -155,6 +155,9 @@ set(LLVM_COMPILER_CHECKED ON)
include(AddLLVM)
include(HandleLLVMOptions)
+# Loot at the PATH first to avoid a version mismatch between the command-line
+# python and the CMake-found version
+set(Python3_FIND_REGISTRY LAST)
find_package(Python3 REQUIRED COMPONENTS Interpreter)
# Host triple is used by tests to check if they are running natively.
|
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.
TBH I think this LGTM if the CI passes.
I do expect that we'll hit resource limitations pretty quickly, but I guess we can cross that bridge when we get to it. This might entail setting up self-hosted runners for Windows.
@@ -199,3 +199,44 @@ jobs: | |||
**/CMakeError.log | |||
**/CMakeOutput.log | |||
**/crash_diagnostics/* | |||
windows: |
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.
When we're ready to merge this, we should add needs: [ stage1, stage2, stage3 ]
to only trigger the Windows CI if the other stages are happy.
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.
I'd rather only wait for stage1, since the GitHub-provided agents are quite slow. I think it's quite unlikely that we'll actually hit resource limits, since we have up to 60 concurrent agents available, i.e. more than 7 libc++ patches uploaded in an hour that don't fail on stage1. We can still put that higher later if we do hit limitations at some point.
Can you limit the total number of jobs to 40? We need to have some spare capacity for running some of the other jobs, like the PR labeler, code_formatter, etc. We may be able to bump it higher in the future, but I think 40 is a good start. |
I thought this was possible, but I haven't found a way to actually limit the number of jobs. I f you know how to implement this then that is great, but otherwise do you have an estimate of the number of concurrent machines you will need to handle the peak load? |
Hm, yeah. I thought it could be done with |
@philnik777 OK, let's just keep it as-is and see how it goes. |
0439ebc
to
775d9d5
Compare
775d9d5
to
c28f055
Compare
@@ -155,6 +155,9 @@ set(LLVM_COMPILER_CHECKED ON) | |||
include(AddLLVM) | |||
include(HandleLLVMOptions) | |||
|
|||
# Loot at the PATH first to avoid a version mismatch between the command-line | |||
# python and the CMake-found version | |||
set(Python3_FIND_REGISTRY LAST) |
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.
FYI - ideally we wouldn't need to have things like this in the main CMakeLists.txt.
Ideally we'd set it in the CI environment, as we happen to need it there due to how the Github runners are set up. It is possible to pass this on the CMake command line, I think, like -DPython3_FIND_REGISTRY=LAST
, but the run-buildbot
script doesn't support passing extra command line parameters to cmake.
On the other hand - this doesn't really hurt either; the cmake default behaviour is really really surprising here, and this makes it less surprising. It makes things a little inconsistent to keep setting this in the runtimes/CMakeLists.txt
only - I wonder if we could set it e.g. cmake/Modules/CMakePolicy.cmake
as a sort of project-wide default policy?
Anyway, just venting my thoughts on the matter - no urgent action needed.
Co-authored-by: Martin Storsjö martin@martin.st