[llvm] r302499 - Revert "Revert "CMake: Move sphinx detection into AddSphinxTarget.cmake""

[Tom Stellard via llvm-commits]

On 05/12/2017 02:49 PM, Adam Nemet wrote:
> 
>> On May 12, 2017, at 11:44 AM, Tom Stellard <tstellar at redhat.com> wrote:
>>
>> On 05/12/2017 02:31 PM, Adam Nemet wrote:
>>> Hi Tom,
>>>
>>>> On May 8, 2017, at 6:41 PM, Tom Stellard via llvm-commits <llvm-commits at lists.llvm.org> wrote:
>>>>
>>>> Author: tstellar
>>>> Date: Mon May  8 20:41:28 2017
>>>> New Revision: 302499
>>>>
>>>> URL: http://llvm.org/viewvc/llvm-project?rev=302499&view=rev
>>>> Log:
>>>> Revert "Revert "CMake: Move sphinx detection into AddSphinxTarget.cmake""
>>>>
>>>> This reverts commit r302054.
>>>>
>>>> Re-commit now that I have fixes for clang/lld.
>>>>
>>>> Modified:
>>>>   llvm/trunk/cmake/config-ix.cmake
>>>>   llvm/trunk/cmake/modules/AddSphinxTarget.cmake
>>>>   llvm/trunk/docs/CMakeLists.txt
>>>>
>>>> Modified: llvm/trunk/cmake/config-ix.cmake
>>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/cmake/config-ix.cmake?rev=302499&r1=302498&r2=302499&view=diff
>>>> ==============================================================================
>>>> --- llvm/trunk/cmake/config-ix.cmake (original)
>>>> +++ llvm/trunk/cmake/config-ix.cmake Mon May  8 20:41:28 2017
>>>> @@ -530,16 +530,6 @@ else()
>>>>  message(STATUS "Doxygen disabled.")
>>>> endif()
>>>>
>>>> -if (LLVM_ENABLE_SPHINX)
>>>> -  message(STATUS "Sphinx enabled.")
>>>> -  find_package(Sphinx REQUIRED)
>>>> -  if (LLVM_BUILD_DOCS)
>>>> -    add_custom_target(sphinx ALL)
>>>> -  endif()
>>>> -else()
>>>> -  message(STATUS "Sphinx disabled.")
>>>> -endif()
>>>> -
>>>> set(LLVM_BINDINGS "")
>>>> if(WIN32)
>>>>  message(STATUS "Go bindings disabled.")
>>>>
>>>> Modified: llvm/trunk/cmake/modules/AddSphinxTarget.cmake
>>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/cmake/modules/AddSphinxTarget.cmake?rev=302499&r1=302498&r2=302499&view=diff
>>>> ==============================================================================
>>>> --- llvm/trunk/cmake/modules/AddSphinxTarget.cmake (original)
>>>> +++ llvm/trunk/cmake/modules/AddSphinxTarget.cmake Mon May  8 20:41:28 2017
>>>> @@ -1,3 +1,16 @@
>>>> +
>>>> +# Create sphinx target
>>>> +if (LLVM_ENABLE_SPHINX AND NOT TARGET sphinx)
>>>> +  message(STATUS "Sphinx enabled.")
>>>> +  find_package(Sphinx REQUIRED)
>>>> +  if (LLVM_BUILD_DOCS)
>>>> +    add_custom_target(sphinx ALL)
>>>> +  endif()
>>>> +else()
>>>> +  message(STATUS "Sphinx disabled.")
>>>> +endif()
>>>
>>> Looks like this broke the docs-llvm-man target when configuring the llvm and clang trees together.
>>>
>>> You should be able to reproduce with SPHINX_OUTPUT_MAN=On.
>>>
>>> I am not a cmake expert, but looks like that SPHINX_FOUND has a local scope when AddSphinxTarget is included from tools/clang/docs.  Then we get to llvm at which point the sphinx target is already there so we won’t go looking for sphinx and thus SPHINX_FOUND won’t be set.
>>>
>>> Please revert or investigate.
>>>
>>
>> I can't reproduce this on my machine, can you send me all
>> the options you passed to cmake?
> 
> Here you go:
> 
> cmake .. -G Ninja -DLLVM_TARGETS_TO_BUILD="X86;ARM;AArch64" -DLLVM_BUILD_DOCS=On -DLLVM_ENABLE_SPHINX=On -DSPHINX_WARNINGS_AS_ERRORS=Off -DSPHINX_OUTPUT_HTML=On -DSPHINX_OUTPUT_MAN=On
> 
>>  Also, did you try removing
>> your CMakeCache.txt and rebuilding?
> 
> Yes, this was with a clean build dir.
> 
>> Does your clang checkout have
>> r302500?
> 
> Yes.
> 
> Adam
> 

Should be fixed by: https://reviews.llvm.org/D33146

Can you test it?

-Tom


>>
>>
>> -Tom
>>
>>
>>> Thanks,
>>> Adam
>>>
>>>
>>>> +
>>>> +
>>>> # Handy function for creating the different Sphinx targets.
>>>> #
>>>> # ``builder`` should be one of the supported builders used by
>>>>
>>>> Modified: llvm/trunk/docs/CMakeLists.txt
>>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/docs/CMakeLists.txt?rev=302499&r1=302498&r2=302499&view=diff
>>>> ==============================================================================
>>>> --- llvm/trunk/docs/CMakeLists.txt (original)
>>>> +++ llvm/trunk/docs/CMakeLists.txt Mon May  8 20:41:28 2017
>>>> @@ -103,8 +103,8 @@ endif()
>>>> endif()
>>>>
>>>> if (LLVM_ENABLE_SPHINX)
>>>> +  include(AddSphinxTarget)
>>>>  if (SPHINX_FOUND)
>>>> -    include(AddSphinxTarget)
>>>>    if (${SPHINX_OUTPUT_HTML})
>>>>      add_sphinx_target(html llvm)
>>>>    endif()
>>>>
>>>>
>>>> _______________________________________________
>>>> llvm-commits mailing list
>>>> llvm-commits at lists.llvm.org
>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>>
>>
>