Revert r297545 - Revert r297516 - Respect CMAKE_INSTALL_MANDIR for sphinx generated manpages

[jroelofs]

Summary

... which now avoids installing things in root.

http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20170306/436814.html
http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20170306/436890.html

Sorry for jumping the gun on it earlier.

[EricWF]

So I can't find any official documentation about CMAKE_INSTALL_MANDIR, nor do I see how it ever gets defined.
The only thing I've found is a CMake module called CMakeInstallDirs.cmake which defines CMAKE_INSTALL_MANDIR but it appears LLVM never includes it.

@jroelofs Can you elaborate on how you ended up with this approach?

PS. Sorry for the delay. I thought I submitted this comment a week ago.

[jroelofs]

https://cmake.org/cmake/help/v3.0/module/GNUInstallDirs.html

[beanz]

The one downside to this approach is that when you use an absolute path as the DESTINATION argument to install you can't change the install root without reconfiguring. When you use relative paths you can.

[jroelofs]

In D30906#712323, @beanz wrote:

The one downside to this approach is that when you use an absolute path as the DESTINATION argument to install you can't change the install root without reconfiguring. When you use relative paths you can.

Mmm, yeah, good point.

[jroelofs]

@beanz what about something like this (untested):

Index: cmake/modules/AddSphinxTarget.cmake
===================================================================
--- cmake/modules/AddSphinxTarget.cmake	(revision 298777)
+++ cmake/modules/AddSphinxTarget.cmake	(working copy)
@@ -48,10 +48,15 @@
     # 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 share/man/man1)
+                DESTINATION ${INSTALL_MANDIR}man1)
 
       elseif (builder STREQUAL html)
         string(TOUPPER "${project}" project_upper)

[beanz]

Looks reasonable to me. Thanks!

[jroelofs]

r299547

cmake/modules/AddSphinxTarget.cmake

function (add_sphinx_target builder project)
   endif()
 
   if (LLVM_BUILD_DOCS)
     add_dependencies(sphinx ${SPHINX_TARGET_NAME})
 
     # 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 share/man/man1)
+                DESTINATION ${INSTALL_MANDIR}man1)
 
       elseif (builder STREQUAL html)
         string(TOUPPER "${project}" project_upper)
         set(${project_upper}_INSTALL_SPHINX_HTML_DIR "share/doc/${project}/html"
             CACHE STRING "HTML documentation install directory for ${project}")
 
         # '/.' indicates: copy the contents of the directory directly into
         # the specified destination, without recreating the last component