[Openmp-commits] [PATCH] D117945: [openmp][cmake] Use `GNUInstallDirs` to support custom installation dirs

[John Ericson via Phabricator via Openmp-commits]

Ericson2314 marked an inline comment as done.
Ericson2314 added inline comments.


================
Comment at: openmp/runtime/src/CMakeLists.txt:361
     install(CODE "execute_process(COMMAND \"\${CMAKE_COMMAND}\" -E copy \"${LIBOMP_LIB_FILE}\"
-      \"${alias}${LIBOMP_LIBRARY_SUFFIX}\" WORKING_DIRECTORY \"\${CMAKE_INSTALL_PREFIX}/bin\")")
+      \"${alias}${LIBOMP_LIBRARY_SUFFIX}\" WORKING_DIRECTORY \"\${CMAKE_INSTALL_BINDIR}\")")
     install(CODE "execute_process(COMMAND \"\${CMAKE_COMMAND}\" -E copy \"${LIBOMP_IMP_LIB_FILE}\"
----------------
mstorsjo wrote:
> This change seems to have broken one aspect: When doing `ninja install` I now get a warning saying `Error copying file "libomp.dll" to "libiomp5md.dll".`, and `libiomp5md.dll` isn't installed.
> 
> I believe the reason is that the inline cmake snippet is written to `runtime/src/cmake_install.cmake` and then executed on install, but on install, `${CMAKE_INSTALL_BINDIR}` isn't set (as `GNUInstallDirs` isn't included there). Should this maybe expand `${CMAKE_INSTALL_BINDIR}` right here instead of deferring it to the install cmake, or what's the right course of action?
Did in D118528!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117945