This is an archive of the discontinued LLVM Phabricator instance.

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

Authored by jroelofs on Mar 13 2017, 11:52 AM.

Diff Detail

Event Timeline

jroelofs created this revision.Mar 13 2017, 11:52 AM
jroelofs updated this revision to Diff 91601.Mar 13 2017, 11:53 AM
EricWF edited edge metadata.Mar 27 2017, 7:19 PM

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.

beanz edited edge metadata.Mar 28 2017, 10:57 AM

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.

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.

@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 accepted this revision.Mar 29 2017, 3:11 PM

Looks reasonable to me. Thanks!

This revision is now accepted and ready to land.Mar 29 2017, 3:11 PM
jroelofs closed this revision.Apr 5 2017, 8:00 AM
jroelofs updated this revision to Diff 94232.

r299547