[Clang][Docs] Fix man page build

[aidengrossman]

Summary

This patch fixes the man page build. It currently doesn't work as
SOURCE_DIR isn't set correctly (just undefined) within the
add_sphinx_target function. This patch also moves around the creation of
targets for autogenerated rst files so that both the man page and html
build can depend upon them as before only the html build depended on
them.

[aidengrossman]

Posting this as when I was setting up a new dev environment with -DLLVM_ENABLE_SPHINX, it defaults to having SPHINX_OUTPUT_HTML and SPHINX_OUTPUT_MAN on, and the man build was broken so when I tried to build the default target, it failed. This patch fixes the clang man page build.

Not sure this is the ideal approach. I'd really like to remove the redundancy between the generated targets reference in gen_docs_depends and their values as specified in the calls to gen_rst_files_from_td, but CMake doesn't really seem to have appropriate data structures (a list of tuples or something similar) to accomplish this. Very open to suggestions here though since I'm not that familiar with CMake.

[tstellar]

I think we could remove some of the duplication by making the docs_target parameter into a list and passing both docs-clang-html and docs-clang-man to it.

[aidengrossman]

In D149809#4317610, @tstellar wrote:

I think we could remove some of the duplication by making the docs_target parameter into a list and passing both docs-clang-html and docs-clang-man to it.

If I'm understanding things correctly we wouldn't be able to do this for either the gen_docs_depends or gen_rst_file_from_td functions because someone might have the HTML docs build turned on and the man pages build turned off or vice versa. We can only add dependencies to the target after it's created as far as I'm aware.

[tstellar]

In D149809#4317763, @aidengrossman wrote:

In D149809#4317610, @tstellar wrote:

I think we could remove some of the duplication by making the docs_target parameter into a list and passing both docs-clang-html and docs-clang-man to it.

If I'm understanding things correctly we wouldn't be able to do this for either the gen_docs_depends or gen_rst_file_from_td functions because someone might have the HTML docs build turned on and the man pages build turned off or vice versa. We can only add dependencies to the target after it's created as far as I'm aware.

I tried to explain what I was thinking with the suggested edits. Does that make sense?

clang/docs/CMakeLists.txt

93

99

107–124

123–126

126

136–137

[aidengrossman]

Adjust based on reviewer suggestions.

[aidengrossman]

Thank you very much for your patience and writing everything out. That makes a lot more sense. The resulting code definitely has a lot more desirable properties. I've updated the diff in accordance with your suggestions.

[aidengrossman]

Ping

[tstellar]

LGTM.

clang/docs/CMakeLists.txt

if (LLVM_ENABLE_DOXYGEN)
 
   if (NOT LLVM_INSTALL_TOOLCHAIN_ONLY)
     install(DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/doxygen/html
       DESTINATION docs/html)
   endif()
 endif()
 endif()
 
-function (gen_rst_file_from_td output_file td_option source docs_target)
+function (gen_rst_file_from_td output_file td_option source docs_targets)
   if (NOT EXISTS "${CMAKE_CURRENT_SOURCE_DIR}/${source}")
     message(FATAL_ERROR "Cannot find source file: ${source} in ${CMAKE_CURRENT_SOURCE_DIR}")
   endif()
   get_filename_component(TABLEGEN_INCLUDE_DIR "${CMAKE_CURRENT_SOURCE_DIR}/${source}" DIRECTORY)
   list(APPEND LLVM_TABLEGEN_FLAGS "-I${TABLEGEN_INCLUDE_DIR}")
   clang_tablegen(${output_file} ${td_option} SOURCE ${source} TARGET "gen-${output_file}")
-  add_dependencies(${docs_target} "gen-${output_file}")
+  foreach(target ${docs_targets})
+    add_dependencies(${target} gen-${output_file})
+  endforeach()
 endfunction()
 
 if (LLVM_ENABLE_SPHINX)
   include(AddSphinxTarget)
-  if (SPHINX_FOUND)
-    if (${SPHINX_OUTPUT_HTML})
-      add_sphinx_target(html clang SOURCE_DIR "${CMAKE_CURRENT_BINARY_DIR}")
-
-      # Copy rst files to build directory before generating the html
-      # documentation.  Some of the rst files are generated, so they
-      # only exist in the build directory.  Sphinx needs all files in
-      # the same directory in order to generate the html, so we need to
-      # copy all the non-gnerated rst files from the source to the build
-      # directory before we run sphinx.
-      add_custom_target(copy-clang-rst-docs
-        COMMAND "${CMAKE_COMMAND}" -E copy_directory
-        "${CMAKE_CURRENT_SOURCE_DIR}" "${CMAKE_CURRENT_BINARY_DIR}"
+  if (SPHINX_FOUND AND (${SPHINX_OUTPUT_HTML} OR ${SPHINX_OUTPUT_MAN}))
 
-        COMMAND "${CMAKE_COMMAND}" -E copy_if_different
-        "${CMAKE_CURRENT_SOURCE_DIR}/../CodeOwners.rst"
-        "${CMAKE_CURRENT_BINARY_DIR}"
-      )
-      add_dependencies(docs-clang-html copy-clang-rst-docs)
+
+    set(docs_targets "")
+
+    if (${SPHINX_OUTPUT_HTML})
+      add_sphinx_target(html clang SOURCE_DIR "${CMAKE_CURRENT_BINARY_DIR}")
 
       add_custom_command(TARGET docs-clang-html POST_BUILD
         COMMAND "${CMAKE_COMMAND}" -E copy
         "${CMAKE_CURRENT_SOURCE_DIR}/LibASTMatchersReference.html"
         "${CMAKE_CURRENT_BINARY_DIR}/html/LibASTMatchersReference.html")
 
-      # Generated files
-      gen_rst_file_from_td(AttributeReference.rst -gen-attr-docs ../include/clang/Basic/Attr.td docs-clang-html)
-      gen_rst_file_from_td(DiagnosticsReference.rst -gen-diag-docs ../include/clang/Basic/Diagnostic.td docs-clang-html)
-      gen_rst_file_from_td(ClangCommandLineReference.rst -gen-opt-docs ../include/clang/Driver/ClangOptionDocs.td docs-clang-html)
+      list(APPEND docs_targets "docs-clang-html")
     endif()
     if (${SPHINX_OUTPUT_MAN})
-      add_sphinx_target(man clang)
+      add_sphinx_target(man clang SOURCE_DIR "${CMAKE_CURRENT_BINARY_DIR}")
+      list(APPEND docs_targets "docs-clang-man")
     endif()
+
+    # Generated files
+    gen_rst_file_from_td(AttributeReference.rst -gen-attr-docs ../include/clang/Basic/Attr.td "${docs_targets}")
+    gen_rst_file_from_td(DiagnosticsReference.rst -gen-diag-docs ../include/clang/Basic/Diagnostic.td "${docs_targets}")
+    gen_rst_file_from_td(ClangCommandLineReference.rst -gen-opt-docs ../include/clang/Driver/ClangOptionDocs.td "${docs_targets}")
+
+    foreach(target ${docs_targets})
+      add_dependencies(${target} copy-clang-rst-docs)
+    endforeach()
   endif()
 endif()