[libcxx-commits] [PATCH] D125683: [runtimes] Replace LIBCXX_ENABLE_STATIC_ABI_LIBRARY & friends by a new LIBCXX_CXX_ABI choice

[Martin Storsjö via Phabricator via libcxx-commits]

mstorsjo added inline comments.


================
Comment at: libcxx/cmake/caches/MinGW.cmake:3
 
-set(LIBCXX_CXX_ABI libcxxabi CACHE STRING "")
+set(LIBCXX_CXX_ABI libcxxabi-objects CACHE STRING "")
 set(LIBCXXABI_USE_LLVM_UNWINDER ON CACHE BOOL "")
----------------
Unfortunately, setting `LIBCXX_CXX_ABI=libcxxabi-objects` here in the cache file doesn't have any effect, because the `run-buildbot` script invokes cmake with a different parameter:
```
function generate-cmake() {
    generate-cmake-base \
          -DLLVM_ENABLE_RUNTIMES="libcxx;libcxxabi;libunwind" \
          -DLIBCXX_CXX_ABI=libcxxabi \
          "${@}"
}
```
A cmake parameter explicitly set on the command line always overrides what's set in a cache file.


================
Comment at: libcxxabi/CMakeLists.txt:254
   # line or via a cache file), use its expected default value (enabled).
-  if ((LIBCXX_ENABLE_SHARED OR NOT DEFINED LIBCXX_ENABLE_SHARED) AND LIBCXX_ENABLE_STATIC_ABI_LIBRARY)
+  if ((LIBCXX_ENABLE_SHARED OR NOT DEFINED LIBCXX_ENABLE_SHARED) AND LIBCXX_CXX_ABI STREQUAL libcxxabi-objects)
     # Building libcxxabi statically, but intending for it to be statically
----------------
The fallback handling (checking `LIBCXX_ENABLE_STATIC_ABI_LIBRARY`, updating `LIBCXX_CXX_ABI` based on that) in libcxx doesn't have any effect here, since libcxxabi is processed strictly before libcxx, so here we only see the original values of the options.

However, luckily enough, I've posted D125715 a couple days ago, which lets us get rid of this whole problematic piece of code altogether (together with fixing a couple other issues), now that we have object libraries in palce!


================
Comment at: llvm/docs/HowToBuildWindowsItaniumPrograms.rst:159
 
-* ``-DLIBCXX_ENABLE_STATIC_ABI_LIBRARY=ON``
 * ``-DLIBCXX_CXX_ABI=libcxxabi``
 * ``-DLIBCXX_CXX_ABI_INCLUDE_PATHS=<libcxxabi src path>/include``
----------------
If the example instructions used to say `LIBCXX_ENABLE_STATIC_ABI_LIBRARY=ON` before, you should update the `LIBCXX_CXX_ABI` setting here to `libcxxabi-objects` too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125683