[libcxx-commits] [PATCH] D150766: [libcxx] [ci] Add a test configuration with an incomplete sysroot

Mon Jul 10 13:41:14 PDT 2023

ldionne added inline comments.

================
Comment at: libcxx/utils/ci/run-buildbot:648
 ;;
+mingw-incomplete-sysroot)
+    clean
----------------
Is there a reason why we're running this on MinGW? It would make more sense IMO to do this in our Docker image on Linux. At the end of the day, we are trying to test a general property of our CMake build, i.e. that it can be run even if you don't have access to a fully working C++ toolchain. This isn't Windows-specific, right?

================
Comment at: libcxx/utils/ci/run-buildbot:675
+          -DCMAKE_C_COMPILER_WORKS=TRUE \
+          -DCMAKE_CXX_COMPILER_WORKS=TRUE \
+          -DCMAKE_C_COMPILER_TARGET=x86_64-w64-windows-gnu \
----------------
I would set this unconditionally in `runtimes/CMakeLists.txt` with an appropriate comment. I don't think it ever makes sense for us to rely on the default CMake compiler checking.

================
Comment at: libcxx/utils/ci/run-buildbot:681-683
+    # Additionally, the sysroot flag from CMAKE_SYSROOT doesn't end up
+    # propagated into the tests at the moment (neither do flags set via
+    # CMAKE_*_FLAGS).
----------------
> Additionally, the sysroot flag from CMAKE_SYSROOT doesn't end up propagated into the tests

This shouldn't matter, since we're using `-nostdlib++` and `-nostdinc++` to avoid depending on any existing libc++ in the toolchain.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150766/new/

https://reviews.llvm.org/D150766