-
Notifications
You must be signed in to change notification settings - Fork 11.9k
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
[libcxx] [ci] Add a test configuration with an incomplete sysroot #107089
Conversation
When bringing up a new cross compiler from scratch, we build libunwind/libcxx in a setup where the toolchain is incomplete and unable to perform the normal linker checks; this requires a few special cases in the CMake files. We simulate that scenario by removing the libc++ headers, libunwind and libc++ libraries from the installed toolchain. We need to set CMAKE_CXX_COMPILER_WORKS since CMake fails to probe the compiler. We need to set CMAKE_CXX_COMPILER_TARGET, since LLVM's heuristics fail when CMake hasn't been able to probe the environment properly. (This is normal; one has to set those options when setting up such a toolchain from scratch.) This adds CI coverage for these build scenarios, which otherwise seldom are tested by some build flow (but are essential when setting up a cross compiler from scratch).
@llvm/pr-subscribers-libcxx @llvm/pr-subscribers-github-workflow Author: Martin Storsjö (mstorsjo) ChangesWhen bringing up a new cross compiler from scratch, we build libunwind/libcxx in a setup where the toolchain is incomplete and unable to perform the normal linker checks; this requires a few special cases in the CMake files. We simulate that scenario by removing the libc++ headers, libunwind and libc++ libraries from the installed toolchain. We need to set CMAKE_CXX_COMPILER_WORKS since CMake fails to probe the compiler. We need to set CMAKE_CXX_COMPILER_TARGET, since LLVM's heuristics fail when CMake hasn't been able to probe the environment properly. (This is normal; one has to set those options when setting up such a toolchain from scratch.) This adds CI coverage for these build scenarios, which otherwise seldom are tested by some build flow (but are essential when setting up a cross compiler from scratch). Full diff: https://github.com/llvm/llvm-project/pull/107089.diff 2 Files Affected:
diff --git a/.github/workflows/libcxx-build-and-test.yaml b/.github/workflows/libcxx-build-and-test.yaml
index 1a26a699db8e01..b5e60781e00064 100644
--- a/.github/workflows/libcxx-build-and-test.yaml
+++ b/.github/workflows/libcxx-build-and-test.yaml
@@ -242,6 +242,7 @@ jobs:
- { config: mingw-dll, mingw: true }
- { config: mingw-static, mingw: true }
- { config: mingw-dll-i686, mingw: true }
+ - { config: mingw-incomplete-sysroot, mingw: true }
steps:
- uses: actions/checkout@v4
- name: Install dependencies
@@ -260,6 +261,12 @@ jobs:
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: Simulate a from-scratch build of llvm-mingw
+ if: ${{ matrix.config == 'mingw-incomplete-sysroot' }}
+ run: |
+ rm -r c:\llvm-mingw\include\c++
+ rm -r c:\llvm-mingw\*-w64-mingw32\lib\libc++*
+ rm -r c:\llvm-mingw\*-w64-mingw32\lib\libunwind*
- name: Add Git Bash to the path
run: |
echo "c:\Program Files\Git\usr\bin" | Out-File -FilePath $Env:GITHUB_PATH -Encoding utf8 -Append
diff --git a/libcxx/utils/ci/run-buildbot b/libcxx/utils/ci/run-buildbot
index 102f1669e63b84..1387353d2a0f17 100755
--- a/libcxx/utils/ci/run-buildbot
+++ b/libcxx/utils/ci/run-buildbot
@@ -715,6 +715,29 @@ mingw-dll-i686)
-C "${MONOREPO_ROOT}/libcxx/cmake/caches/MinGW.cmake"
check-runtimes
;;
+mingw-incomplete-sysroot)
+ # When bringing up a new cross compiler from scratch, we build
+ # libunwind/libcxx in a setup where the toolchain is incomplete and
+ # unable to perform the normal linker checks; this requires a few
+ # special cases in the CMake files.
+ #
+ # Building in an incomplete setup requires setting CMAKE_*_COMPILER_WORKS,
+ # as CMake fails to probe the compiler. This case also requires
+ # setting CMAKE_CXX_COMPILER_TARGET, as LLVM's heuristics for setting
+ # the triple fails when CMake hasn't been able to probe the environment.
+ # (This is what one has to do when building the initial libunwind/libcxx
+ # for a new toolchain.)
+ clean
+ generate-cmake \
+ -DCMAKE_C_COMPILER_WORKS=TRUE \
+ -DCMAKE_CXX_COMPILER_WORKS=TRUE \
+ -DCMAKE_C_COMPILER_TARGET=x86_64-w64-windows-gnu \
+ -DCMAKE_CXX_COMPILER_TARGET=x86_64-w64-windows-gnu \
+ -C "${MONOREPO_ROOT}/libcxx/cmake/caches/MinGW.cmake"
+ # Only test that building succeeds; there's not much extra value in running
+ # the tests here, as it would be equivalent to the mingw-dll config above.
+ ${NINJA} -vC "${BUILD_DIR}"
+;;
aix)
clean
generate-cmake -C "${MONOREPO_ROOT}/libcxx/cmake/caches/AIX.cmake" \
|
This was earlier posted for review in https://reviews.llvm.org/D150766. Since that version, the setup of this test environment has gotten simpler, since we're running the Windows tests on GitHub Actions runners, where each run is in a separate clean container, where we're installing the toolchain ourselves. So it's easier to unconditionally remove things from the toolchain, to properly simulate the incomplete sysroot setup. The previous review asked whether this really is Windows specific - it isn't. It should certainly be possible to have the same situation in a Linux environment too. But exactly which files to remove from a full working environment, in order to simulate this from-scratch bringup stage, is quite environment specific. It's much easier to do with a cross compiler that come with their own sysroot, rather than a native toolchain that uses whatever it can find in So to test the same on Linux, you'd probably need to have a standalone cross toolchain setup (targeting e.g. musl or so?). And we don't have that readily available, as far as I know. While this testing setup uses the exact same tooling we're already using, and gives at least some sort of test coverage for this important scenario. Right now, there are discussions in #90332 about things that do touch upon the from-scratch incomplete sysroot cases - having even some sort of test coverage of that in the CI setup would be valuable. CC @petrhosek |
-DCMAKE_C_COMPILER_WORKS=TRUE \ | ||
-DCMAKE_CXX_COMPILER_WORKS=TRUE \ | ||
-DCMAKE_C_COMPILER_TARGET=x86_64-w64-windows-gnu \ | ||
-DCMAKE_CXX_COMPILER_TARGET=x86_64-w64-windows-gnu \ |
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.
As part of (or as a prerequisite to) this patch, shouldn't we fix these "bugs" so that we don't have to work around them here?
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 don’t consider this a bug that reasonably can be fixed.
At this stage, we can’t even link a trivial C application (the toolchain implicitly tries to link libunwind, which is yet to be built), so the cmake tests that normally deduce the target triple simply fall flat.
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.
And as for needing to pass the *_COMPILER_WORKS
flag, that’s also required to get cmake pass the very first sanity checks. It is possible to avoid that by setting CMAKE_TRY_COMPILE_TARGET_TYPE
to STATIC_LIBRARY
, but that gives false positives on all further checks for whether libraries exist.
We’ve looked into reducing the number of extra flags one has to set when doing the initial build, but these couple flags are the ones that the caller do need to set in such situations. Trying to get rid of it isn’t worth it - we’ve tried.
…m#107089) When bringing up a new cross compiler from scratch, we build libunwind/libcxx in a setup where the toolchain is incomplete and unable to perform the normal linker checks; this requires a few special cases in the CMake files. We simulate that scenario by removing the libc++ headers, libunwind and libc++ libraries from the installed toolchain. We need to set CMAKE_CXX_COMPILER_WORKS since CMake fails to probe the compiler. We need to set CMAKE_CXX_COMPILER_TARGET, since LLVM's heuristics fail when CMake hasn't been able to probe the environment properly. (This is normal; one has to set those options when setting up such a toolchain from scratch.) This adds CI coverage for these build scenarios, which otherwise seldom are tested by some build flow (but are essential when setting up a cross compiler from scratch).
When bringing up a new cross compiler from scratch, we build libunwind/libcxx in a setup where the toolchain is incomplete and unable to perform the normal linker checks; this requires a few special cases in the CMake files.
We simulate that scenario by removing the libc++ headers, libunwind and libc++ libraries from the installed toolchain.
We need to set CMAKE_CXX_COMPILER_WORKS since CMake fails to probe the compiler. We need to set CMAKE_CXX_COMPILER_TARGET, since LLVM's heuristics fail when CMake hasn't been able to probe the environment properly. (This is normal; one has to set those options when setting up such a toolchain from scratch.)
This adds CI coverage for these build scenarios, which otherwise seldom are tested by some build flow (but are essential when setting up a cross compiler from scratch).