compnerd added inline comments.
================ Comment at: llvm/CMakeLists.txt:589 CACHE STRING "Doxygen-generated HTML documentation install directory") -set(LLVM_INSTALL_OCAMLDOC_HTML_DIR "share/doc/llvm/ocaml-html" +set(LLVM_INSTALL_OCAMLDOC_HTML_DIR "${CMAKE_INSTALL_DOCDIR}/${project}/ocaml-html" CACHE STRING "OCamldoc-generated HTML documentation install directory") ---------------- Why the change from `llvm` -> `${project}`? (not that it really makes a difference) ================ Comment at: llvm/cmake/modules/AddSphinxTarget.cmake:77 if (CMAKE_INSTALL_MANDIR) - set(INSTALL_MANDIR ${CMAKE_INSTALL_MANDIR}/) + set(INSTALL_MANDIR "${CMAKE_INSTALL_MANDIR}/") else() ---------------- Nit: trailing slash is unnecessary, `CMAKE_INSTALL_MANDIR` should be defined, and if not, you do not want installation into `/` anyway. ================ Comment at: llvm/cmake/modules/CMakeLists.txt:3 -set(LLVM_INSTALL_PACKAGE_DIR lib${LLVM_LIBDIR_SUFFIX}/cmake/llvm) +set(LLVM_INSTALL_PACKAGE_DIR lib${LLVM_LIBDIR_SUFFIX}/cmake/llvm CACHE STRING "Path for CMake subdirectory (defaults to 'cmake/llvm')") set(llvm_cmake_builddir "${LLVM_BINARY_DIR}/${LLVM_INSTALL_PACKAGE_DIR}") ---------------- Why is this variable being put into the cache now? ================ Comment at: llvm/cmake/modules/LLVMInstallSymlink.cmake:7 set(DESTDIR $ENV{DESTDIR}) - set(bindir "${DESTDIR}${CMAKE_INSTALL_PREFIX}/${outdir}/") + set(bindir "${DESTDIR}${outdir}/") ---------------- Nit: trailing slash shouldn't be there. ================ Comment at: llvm/tools/llvm-config/llvm-config.cpp:361 + { + SmallString<256> Path(StringRef(LLVM_INSTALL_INCLUDEDIR)); + sys::fs::make_absolute(ActivePrefix, Path); ---------------- Why the temporary `StringRef`? Can you not just initialize `Path` with the literal? ================ Comment at: llvm/tools/llvm-config/llvm-config.cpp:366 + { + SmallString<256> Path(StringRef(LLVM_INSTALL_BINDIR)); + sys::fs::make_absolute(ActivePrefix, Path); ---------------- Similar Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100810/new/ https://reviews.llvm.org/D100810 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits