[PATCH] D115601: Simplify AddSphinxTarget.cmake by just including `GNUInstallDirs`

John Ericson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun Dec 12 09:14:34 PST 2021


Ericson2314 created this revision.
Herald added a subscriber: mgorny.
Ericson2314 requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

See the discussion in D30906 <https://reviews.llvm.org/D30906> for how it must be robust to downstream
projects that might not use `GNUInstallDirs`.

I think an easier approach is just to include `GNUInstallDirs` so that
we can always rely on it. Looking at its definition in
https://gitlab.kitware.com/cmake/cmake/-/blob/master/Modules/GNUInstallDirs.cmake
it appears safe to include multiple times.

I am trying this out because we will face the same problem many times
over if we use `GNUInstallDirs` more widely, as proposed in D99484 <https://reviews.llvm.org/D99484> and
its parent revisions, since we do not know what downstream projects do
and rather not break them.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D115601

Files:
  llvm/cmake/modules/AddSphinxTarget.cmake


Index: llvm/cmake/modules/AddSphinxTarget.cmake
===================================================================
--- llvm/cmake/modules/AddSphinxTarget.cmake
+++ llvm/cmake/modules/AddSphinxTarget.cmake
@@ -1,3 +1,4 @@
+include(GNUInstallDirs)
 
 # Create sphinx target
 if (LLVM_ENABLE_SPHINX)
@@ -73,15 +74,10 @@
     # Handle installation
     if (NOT LLVM_INSTALL_TOOLCHAIN_ONLY)
       if (builder STREQUAL man)
-        if (CMAKE_INSTALL_MANDIR)
-          set(INSTALL_MANDIR ${CMAKE_INSTALL_MANDIR}/)
-        else()
-          set(INSTALL_MANDIR share/man/)
-        endif()
         # FIXME: We might not ship all the tools that these man pages describe
         install(DIRECTORY "${SPHINX_BUILD_DIR}/" # Slash indicates contents of
                 COMPONENT "${project}-sphinx-man"
-                DESTINATION ${INSTALL_MANDIR}man1)
+                DESTINATION "${CMAKE_INSTALL_MANDIR}/man1")
 
         if(NOT LLVM_ENABLE_IDE)
           add_llvm_install_targets("install-${SPHINX_TARGET_NAME}"


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D115601.393762.patch
Type: text/x-patch
Size: 1013 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20211212/86c22ad3/attachment.bin>


More information about the llvm-commits mailing list