[libcxx-commits] [PATCH] D120719: [runtimes] Always configure libc++abi before libc++

[Petr Hosek via Phabricator via libcxx-commits]

phosek added a comment.

In D120719#3372589 <https://reviews.llvm.org/D120719#3372589>, @ldionne wrote:

> In D120719#3371499 <https://reviews.llvm.org/D120719#3371499>, @phosek wrote:
>
>> In D120719#3371282 <https://reviews.llvm.org/D120719#3371282>, @ldionne wrote:
>>
>>> Gentle ping on this @phosek . I spoke to @beanz offline who said he was "neutral" on this change (I guess he doesn't quite like it but also won't push back). This isn't nice, but I think it's a necessary evil and it will allow greatly cleaning up how we pick up the ABI library from within libc++. Does this look OK to you?
>>
>> I'd like to do one additional experiment to see if there was a way to achieve this with generator expressions, I'll report back hopefully within a day if that's fine with you?
>
> Yes, that's fine with me. My goal is to be able to land https://reviews.llvm.org/D120727. The way I test D120727 <https://reviews.llvm.org/D120727> right now is:
>
>   cmake -S runtimes -B <build> -GNinja \
>           -DCMAKE_BUILD_TYPE=Release \
>           -DLLVM_ENABLE_RUNTIMES="libcxx;libcxxabi;libunwind" \
>           -DLIBCXX_ENABLE_STATIC_ABI_LIBRARY=ON \
>           -DLIBCXX_STATICALLY_LINK_ABI_IN_SHARED_LIBRARY=ON \
>           -DLIBCXX_STATICALLY_LINK_ABI_IN_STATIC_LIBRARY=ON
>
> Without this patch, it fails due to `cxxabi_shared` not being a known target when we configure libcxx. If we apply this patch, it works. Also note that using `LIBCXX_ENABLE_STATIC_ABI_LIBRARY` & friends is relevant -- if you remove them, it's easy to implement D120727 <https://reviews.llvm.org/D120727> without needing this patch right here, you just have to make `libcxx-abi-static` be an `INTERFACE` library that links against `cxxabi_static` (even if it isn't defined yet). But that approach breaks when you use `LIBCXX_ENABLE_STATIC_ABI_LIBRARY` & friends, since we need to get `$<TARGET_LINKER_FILE:libcxx-abi-static>`.

@ldionne this is what I had in mind:

  cmake_minimum_required(VERSION 3.13.4)
  project(Test C)
  
  add_library(A STATIC a.c)
  
  add_library(C SHARED c.c)
  target_link_libraries(C PRIVATE "$<GENEX_EVAL:$<TARGET_LINKER_FILE:B>>")
  
  add_library(B IMPORTED STATIC)
  set_target_properties(B PROPERTIES IMPORTED_LOCATION "$<TARGET_LINKER_FILE:A>")

It seems to be working as expected in this minimal example.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120719