[libcxx-commits] [PATCH] D137337: Replace LLVM_LIBDIR_SUFFIX by CMAKE_INSTALL_LIBDIR

[John Ericson via Phabricator via libcxx-commits]

Ericson2314 added inline comments.


================
Comment at: bolt/lib/RuntimeLibs/RuntimeLibrary.cpp:32
   SmallString<128> LibPath = llvm::sys::path::parent_path(Dir);
-  llvm::sys::path::append(LibPath, "lib" LLVM_LIBDIR_SUFFIX);
+  llvm::sys::path::append(LibPath, CMAKE_INSTALL_LIBDIR);
   if (!llvm::sys::fs::exists(LibPath)) {
----------------
mgorny wrote:
> Well, one thing I immediately notice is that technically `CMAKE_INSTALL_LIBDIR` can be an absolute path, while in many locations you are assuming it will always be relative. Not saying that's a blocker but I think you should add checks for that into `CMakeLists.txt`.
Yes I was working on this, and I intend to use `CMAKE_INSTALL_LIBDIR` with an absolute path.

OK if this isn't supported initially but we should ovoid regressing where possible.


================
Comment at: clang/include/clang/Config/config.h.cmake:39
 /* Multilib basename for libdir. */
-#define CLANG_INSTALL_LIBDIR_BASENAME "${CLANG_INSTALL_LIBDIR_BASENAME}"
 
----------------
I had called it `_BASENAME` in anticipation that it would be the basename of `CMAKE_INSTALL_LIBDIR` eventually when `CMAKE_INSTALL_LIBDIR` is not a relative path.


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

https://reviews.llvm.org/D137337