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

Fri May 12 12:27:09 PDT 2017

> On May 12, 2017, at 12:22 PM, Tom Stellard <tstellar at redhat.com> wrote:
> 
> 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 <https://reviews.llvm.org/D33146>
> 
> Can you test it?

It was found by one our internal bots.  Let’s just commit it and then you’ll hear from me if the bot is still broken.

Thanks for the quick fix!

Adam

> 
> -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

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170512/a0f01a55/attachment.html>