[PATCH] D132316: [CMake] `${LLVM_BINARY_DIR}/lib(${LLVM_LIBDIR_SUFFIX})?` -> `${LLVM_LIBRARY_DIR}`

[Sebastian Neubauer via Phabricator via llvm-commits]

sebastian-ne added inline comments.


================
Comment at: llvm/tools/llvm-shlib/CMakeLists.txt:89
 
-  set(LLVM_EXPORTED_SYMBOL_FILE ${LLVM_BINARY_DIR}/libllvm-c.exports)
 
----------------
The lib here is not used as a folder, so this should probably not be replaced?
It should probably include CMAKE_CFG_INTDIR though (i.e. `${LLVM_BINARY_DIR}/${CMAKE_CFG_INTDIR}/libllvm-c.exports`), to be consistent with other places, though this only affects the ninja multi-config build. I’m not sure if that works at all and it’s outside of the scope of this patch anyway.


================
Comment at: llvm/tools/llvm-shlib/CMakeLists.txt:91
 
   set(LIB_DIR ${LLVM_BINARY_DIR}/${CMAKE_CFG_INTDIR}/lib${LLVM_LIBDIR_SUFFIX})
   set(LIB_NAME ${LIB_DIR}/${CMAKE_SHARED_LIBRARY_PREFIX}LLVM)
----------------
Should this be `set(LIB_DIR ${LLVM_LIBRARY_DIR})`?


================
Comment at: llvm/tools/llvm-shlib/CMakeLists.txt:139
   foreach(lib ${LIB_NAMES})
     list(APPEND FULL_LIB_NAMES ${LLVM_BINARY_DIR}/${CMAKE_CFG_INTDIR}/lib/${lib}.lib)
   endforeach()
----------------
This should probably use LLVM_LIBRARY_DIR as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132316