[PATCH] D49587: [CMake] Support exporting runtimes using the CMake export

[Petr Hosek via Phabricator via llvm-commits]

phosek marked 3 inline comments as done.
phosek added inline comments.


================
Comment at: libcxx/lib/CMakeLists.txt:101
   elseif (LIBCXXABI_ENABLE_STATIC_UNWINDER AND (TARGET unwind_static OR HAVE_LIBUNWIND))
-    add_interface_library(unwind_static)
+    if (NOT LIBCXXABI_ENABLE_STATIC_UNWINDER)
+      add_interface_library(unwind_static)
----------------
smeenai wrote:
> Seems like an unrelated change? I can see how it was exposed by adding the exported target support, but it still seems like it could go in separately.
D57872 is the right way which is the dependency of this change, once it lands I'll rebase this.


================
Comment at: llvm/runtimes/CMakeLists.txt:220
+  get_property(LLVM_RUNTIMES GLOBAL PROPERTY LLVM_RUNTIMES)
+  export(TARGETS ${LLVM_RUNTIMES} FILE ${LLVM_RUNTIMES_FILE})
+
----------------
smeenai wrote:
> Why do we need to collect targets manually instead of just using the `export(EXPORT)` signature?
I just followed what LLVM already does for `LLVMExports` but I'm fine using `export(EXPORT)` if that's preferred.


================
Comment at: llvm/runtimes/CMakeLists.txt:223
+  install(EXPORT LLVMRuntimes
+          DESTINATION lib${LLVM_LIBDIR_SUFFIX}/clang/${CLANG_VERSION}/${LLVM_DEFAULT_TARGET_TRIPLE}
+          COMPONENT cmake-exports)
----------------
smeenai wrote:
> Is this assuming the per-triple resource directory layout?
Yes, that's already used for the runtimes build: https://github.com/llvm/llvm-project/blob/master/llvm/runtimes/CMakeLists.txt#L345


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D49587