[libcxx-commits] [PATCH] D132526: [libcxxabi] Unbreak `LLVM_ENABLE_RUNTIMES=libcxxabi` build v2

[Louis Dionne via Phabricator via libcxx-commits]

ldionne requested changes to this revision.
ldionne added a comment.
This revision now requires changes to proceed.

Taking a step back, is there a reason why we don't simply require `libcxx` from being in `LLVM_ENABLE_RUNTIMES` when `libcxxabi` is in it? After all, `libcxxabi` has always been an implementation detail of libc++, and while it makes sense to use libc++ without libc++abi, the reverse doesn't make sense.

In other words, I question the use case of passing `LLVM_ENABLE_RUNTIMES=libcxxabi` without `libcxx`. Can you expand on this?



================
Comment at: libcxxabi/cmake/Modules/HandleLibCXX.cmake:1-2
+set(LIBCXXABI_LIBCXX_PATH "${LIBCXXABI_SOURCE_DIR}/../libcxx" CACHE PATH
+    "Specify path to libc++ source.")
+
----------------
This should not be customizeable. We're in the monorepo, so we can hardcode the path of libc++ from libc++abi.


================
Comment at: libcxxabi/cmake/Modules/HandleLibCXX.cmake:4-6
+option(LIBCXXABI_USE_PREBUILT_LIBCXX_HEADERS
+    "Use prebuilt libc++ headers because we are not also building libcc++."
+    OFF)
----------------
phosek wrote:
> Ericson2314 wrote:
> > phosek wrote:
> > > Ericson2314 wrote:
> > > > My `TARGET cxx-headers` trick didn't work because that target isn't defined until afterwords, since `libcxx` mostly depends on`libcxxabi`.
> > > Could we use `LIBCXXABI_LIBCXX_INCLUDE` instead of introducing another option? That is, if `LIBCXXABI_LIBCXX_INCLUDE` is not empty, it implies using prebuilt libc++ headers.
> > I suppose so, should we rename it something like `LIBCXXABI_LIBCXX_INCLUDE_PREBUILT` if so?
> > 
> > Per the TODO deprecated `libcxxabi/test/lit.site.cfg.in` uses this variable and I am not sure why. So that gave me some trepidation about changing things more.
> How about something like `LIBCXXABI_EXTERNAL_LIBCXX_INCLUDES`?
> Per the TODO deprecated libcxxabi/test/lit.site.cfg.in

Is that still relevant? I can't find it anymore.


================
Comment at: libcxxabi/cmake/Modules/HandleLibCXX.cmake:5
+option(LIBCXXABI_USE_PREBUILT_LIBCXX_HEADERS
+    "Use prebuilt libc++ headers because we are not also building libcc++."
+    OFF)
----------------



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132526