[libcxx-commits] [PATCH] D60276: [libc++] Localize CMake code only related to the shared library

[Shoaib Meenai via Phabricator via libcxx-commits]

smeenai added a comment.

I really like the direction this is going.



================
Comment at: libcxx/lib/CMakeLists.txt:177
+    if (APPLE)
+      target_link_options(cxx_shared PRIVATE "-Wl,-force_load" "$<TARGET_LINKER_FILE:${LIBCXX_CXX_STATIC_ABI_LIBRARY}>")
+    else()
----------------
`target_link_options` was added in CMake 3.13, so you're gonna have to use `target_link_libraries` to specify linker options (which is unfortunate, but our minimum CMake version is 3.4.3).

How come you're using a generator expression here whereas the previous code was just using LIBCXX_CXX_STATIC_ABI_LIBRARY directly? The need for that might go away with the switch to `target_link_libraries`.


================
Comment at: libcxx/lib/CMakeLists.txt:185
+  else()
+    target_link_libraries(cxx_shared PRIVATE "${LIBCXX_CXX_SHARED_ABI_LIBRARY}")
+  endif()
----------------
We were previously using `add_interface_library` for this, which would populate the list of libraries used to form the libc++ linker script. I don't see a replacement for that logic here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60276