[PATCH] D126122: Fix RUNPATH not accounting for LLVM_ENABLE_PER_TARGET_RUNTIME_DIR

Thu Feb 2 23:42:09 PST 2023

phosek added inline comments.

================
Comment at: libcxx/src/CMakeLists.txt:184
 endif()

+if (NOT APPLE AND LIBCXX_ENABLE_SHARED AND LIBCXXABI_USE_LLVM_UNWINDER AND
----------------
Once we require CMake >=3.14, we should be able to use https://cmake.org/cmake/help/latest/prop_tgt/BUILD_RPATH_USE_ORIGIN.html. Can you leave a `TODO` comment here for that?

================
Comment at: libcxx/src/CMakeLists.txt:185-186

+if (NOT APPLE AND LIBCXX_ENABLE_SHARED AND LIBCXXABI_USE_LLVM_UNWINDER AND
+        NOT LIBCXXABI_STATICALLY_LINK_UNWINDER_IN_SHARED_LIBRARY AND (TARGET unwind_shared OR HAVE_LIBUNWIND))
+    add_library_flags("-Wl,-rpath,'$$ORIGIN'")
----------------
What's the motivation behind this condition? Why not set this unconditionally?

================
Comment at: libcxx/src/CMakeLists.txt:187
+        NOT LIBCXXABI_STATICALLY_LINK_UNWINDER_IN_SHARED_LIBRARY AND (TARGET unwind_shared OR HAVE_LIBUNWIND))
+    add_library_flags("-Wl,-rpath,'$$ORIGIN'")
+endif()
----------------
This flag may not be supported by all linkers so we need to check if it's supported first.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126122