phosek added inline comments.
================ 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() ---------------- Ericson2314 wrote: > ldionne wrote: > > lebedev.ri wrote: > > > This looks suspect > > Yeah, I don't understand why this isn't just `CMAKE_INSTALL_LIBDIR` like > > elsewhere. > See the non-line comment I responded to @lebidev.ri with. In sort if > > ``` > ${COMPILER_RT_INSTALL_PATH}/${CMAKE_INSTALL_LIBDIR}/${target} > ``` > > is a relative path, then we end up with > > ``` > ${CMAKE_INSTALL_PREFIX}/${COMPILER_RT_INSTALL_PATH}/${CMAKE_INSTALL_LIBDIR}/${target} > ``` > > instead of > > ``` > ${COMPILER_RT_INSTALL_PATH}/${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_LIBDIR}/${target} > ``` > > as we do with the other per-package prefixes. Also if `CMAKE_INSTALL_LIBDIR` > is already an absolute path, then > ``` > ${COMPILER_RT_INSTALL_PATH}/${CMAKE_INSTALL_FULL_LIBDIR}/${target} > ``` > is the same thing, and closer to the second than the first. I'm not sure if that's desirable. I'm not sure if we still need `COMPILER_RT_INSTALL_PATH`. That variable is only used by `clang/runtime/CMakeLists.txt` which predates `runtimes/CMakeLists.txt` and was AFAIK only ever used by Apple. I think we should consider removing `COMPILER_RT_INSTALL_PATH`. We'll need to check if `clang/runtime/CMakeLists.txt` is still being used or not. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99484/new/ https://reviews.llvm.org/D99484 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits