[PATCH] D137337: Replace LLVM_LIBDIR_SUFFIX by CMAKE_INSTALL_LIBDIR

Michał Górny via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Nov 3 12:05:32 PDT 2022


mgorny 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)) {
----------------
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`.


================
Comment at: clang/tools/libclang/CMakeLists.txt:234
           DESTINATION
-            "lib${LLVM_LIBDIR_SUFFIX}/python${PythonVersion}/site-packages")
+          "${CMAKE_INSTALL_LIBDIR}/python${PythonVersion}/site-packages")
 endforeach()
----------------
You seem to be changing indentation here, and below.

On unrelated note, hardcoding site-packages path sounds like a bad idea but that's a problem for another patch.


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

https://reviews.llvm.org/D137337



More information about the cfe-commits mailing list