[libcxx-commits] [PATCH] D99484: Use `GNUInstallDirs` to support custom installation dirs.

[Roman Lebedev via Phabricator via libcxx-commits]

lebedev.ri added a comment.

Thank you!
The changes here look about reasonable to me.
I have not checked if there are some changes that were missed.
Usage of `CMAKE_INSTALL_FULL_` is suspect to me. Are you sure those are correct?



================
Comment at: compiler-rt/cmake/Modules/AddCompilerRT.cmake:513
   install(FILES ${file_name}
-    DESTINATION ${COMPILER_RT_INSTALL_PATH}/share
+    DESTINATION ${COMPILER_RT_INSTALL_PATH}/${CMAKE_INSTALL_FULL_INCLUDEDIR}
     COMPONENT ${component})
----------------
This looks suspect


================
Comment at: compiler-rt/cmake/Modules/AddCompilerRT.cmake:530
     PERMISSIONS OWNER_READ OWNER_WRITE OWNER_EXECUTE GROUP_READ GROUP_EXECUTE WORLD_READ WORLD_EXECUTE
-    DESTINATION ${COMPILER_RT_INSTALL_PATH}/bin)
+    DESTINATION ${COMPILER_RT_INSTALL_PATH}/${CMAKE_INSTALL_FULL_BINDIR})
 endmacro(add_compiler_rt_script src name)
----------------
This looks suspect


================
Comment at: compiler-rt/cmake/Modules/CompilerRTDarwinUtils.cmake:511
     set(DARWIN_macho_embedded_LIBRARY_INSTALL_DIR
-      ${COMPILER_RT_INSTALL_PATH}/lib/macho_embedded)
+      ${COMPILER_RT_INSTALL_PATH}/${CMAKE_INSTALL_FULL_LIBDIR}/macho_embedded)
       
----------------
This looks suspect


================
Comment at: compiler-rt/cmake/Modules/CompilerRTUtils.cmake:389
     get_compiler_rt_target(${arch} target)
-    set(${install_dir} ${COMPILER_RT_INSTALL_PATH}/lib/${target} PARENT_SCOPE)
+    set(${install_dir} ${COMPILER_RT_INSTALL_PATH}/${CMAKE_INSTALL_FULL_LIBDIR}/${target} PARENT_SCOPE)
   else()
----------------
This looks suspect


================
Comment at: compiler-rt/cmake/base-config-ix.cmake:100
   set(COMPILER_RT_LIBRARY_INSTALL_DIR
-    ${COMPILER_RT_INSTALL_PATH}/lib/${COMPILER_RT_OS_DIR})
+    ${COMPILER_RT_INSTALL_PATH}/${CMAKE_INSTALL_FULL_LIBDIR}/${COMPILER_RT_OS_DIR})
 endif()
----------------
This looks suspect


================
Comment at: compiler-rt/include/CMakeLists.txt:72-87
+  DESTINATION ${COMPILER_RT_INSTALL_PATH}/${CMAKE_INSTALL_FULL_INCLUDEDIR}/sanitizer)
 # Install fuzzer headers.
 install(FILES ${FUZZER_HEADERS}
   COMPONENT compiler-rt-headers
   PERMISSIONS OWNER_READ OWNER_WRITE GROUP_READ WORLD_READ
-  DESTINATION ${COMPILER_RT_INSTALL_PATH}/include/fuzzer)
+  DESTINATION ${COMPILER_RT_INSTALL_PATH}/${CMAKE_INSTALL_FULL_INCLUDEDIR}/fuzzer)
 # Install xray headers.
----------------
This looks suspect


================
Comment at: compiler-rt/lib/dfsan/CMakeLists.txt:64-65
 add_dependencies(dfsan dfsan_abilist)
 install(FILES ${dfsan_abilist_filename}
-        DESTINATION ${COMPILER_RT_INSTALL_PATH}/share)
+        DESTINATION ${COMPILER_RT_INSTALL_PATH}/${CMAKE_INSTALL_FULL_DATADIR})
----------------
This looks suspect


================
Comment at: polly/cmake/CMakeLists.txt:85
+set(POLLY_CONFIG_CMAKE_DIR "${POLLY_INSTALL_PREFIX}${CMAKE_INSTALL_PREFIX}/${POLLY_INSTALL_PACKAGE_DIR}")
+set(POLLY_CONFIG_LIBRARY_DIRS "${POLLY_INSTALL_PREFIX}${CMAKE_INSTALL_FULL_LIBDIR}${LLVM_LIBDIR_SUFFIX}")
 if (POLLY_BUNDLED_ISL)
----------------
This looks suspect


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99484