[PATCH] D23755: cmake: Add an ordering dep between HTML & man Sphinx targets

[Michał Górny via llvm-commits]

mgorny added a comment.

In https://reviews.llvm.org/D23755#525455, @beanz wrote:

> The simple Ninja solution is actually to just specify `USES_TERMINAL` on the target creation which will limit one at a time.
>
> If we do that, then this dependency could be added only for non-ninja generators.
>
>   diff --git a/cmake/modules/AddSphinxTarget.cmake b/cmake/modules/AddSphinxTarget.cmake
>   index 045dc23..fbe4f95 100644
>   --- a/cmake/modules/AddSphinxTarget.cmake
>   +++ b/cmake/modules/AddSphinxTarget.cmake
>   @@ -24,7 +24,8 @@ function (add_sphinx_target builder project)
>                                "${CMAKE_CURRENT_SOURCE_DIR}" # Source
>                                "${SPHINX_BUILD_DIR}" # Output
>                        COMMENT
>   -                    "Generating ${builder} Sphinx documentation for ${project} into \"${SPHINX_BUILD_DIR}\"")
>   +                    "Generating ${builder} Sphinx documentation for ${project} into \"${SPHINX_BUILD_DIR}\""
>   +                    USES_TERMINAL)
>   
>      # When "clean" target is run, remove the Sphinx build directory
>      set_property(DIRECTORY APPEND PROPERTY
>


To be honest, this sounds like a bad hack to me. The command does not use terminal after all but instead you want to rely on a side effect of its current implementation in Ninja. I can imagine that in the future this could change, and cause hard-to-track issues.


https://reviews.llvm.org/D23755