[llvm-commits] [llvm] r114747 - in /llvm/trunk: CMakeLists.txt cmake/modules/AddLLVM.cmake unittests/CMakeLists.txt utils/unittest/CMakeLists.txt
bigcheesegs at gmail.com
Fri Sep 24 21:44:32 CDT 2010
On Fri, Sep 24, 2010 at 10:17 PM, Michael Spencer <bigcheesegs at gmail.com> wrote:
> On Fri, Sep 24, 2010 at 9:43 PM, Michael Spencer <bigcheesegs at gmail.com> wrote:
>> On Fri, Sep 24, 2010 at 4:01 PM, Óscar Fuentes <ofv at wanadoo.es> wrote:
>>> Michael Spencer <bigcheesegs at gmail.com>
>>>> This was requested for the msvc build bot as building ALL_BUILD from
>>>> VSBuild doesn't build anything and we needed to disable building
>>>> unittests and examples.
>>> I don't understand this. So if the cmake invocation contains
>>> -DLLVM_BUILD_EXAMPLES=OFF this is a problem?
>> The only way to build (that we could find) clang from the command line
>> using VCBuild is to build the entire solution, which includes the
>> tools, examples, and tests no matter what. LLVM_BUILD_X only controls
>> if they are part of the ALL_BUILD target, and building ALL_BUILD
>> doesn't build anything with VCBuild.
>>> And why you don't want to build examples and unittests on the bot, if
>>> precisely the purpose of the bot is to test the build? (*all* the build,
>>> I guess)
>> This is a clang tester, it slows it down. And the unittests don't
>> build on msvc9 (they do in 10).
>>>> I don't really like the implementation because it may be confusing to
>>>> the user because LLVM_INCLUDE_X is ignored if LLVM_BUILD_X is on. I
>>>> tried getting CMake to update the cache so ccmake and cmake-gui would
>>>> show the change, but it didn't work :(. I would appreciate any help in
>>>> making this implementation more robust.
>>> I don't like the patch either. The previous implementation supported
>>> ignoring the tools and examples on the default build but allowing
>>> building them later as the user demanded. Now if the user wants to build
>>> a tool or example, he must reconfigure and incorporate *all* the tools
>>> examples to the default build.
>> Ugg, the intention was to not change the behavior at all for TOOLS and
>> EXAMPLES. TOOLS is currently the same, but I accidentally changed
>> EXAMPLES. LLVM_INCLUDE_EXAMPLES should default to ON.
>>> Please explain in more detail what the problem is and maybe we can find
>>> a better implementation.
>> There are two options per group. LLVM_BUILD_X and LLVM_INCLUDE_X.
>> LLVM_BUILD_X says that X should be built by ALL_BUILD, and
>> LLVM_INCLUDE_X says that X should be included as a target that is
>> available to be built. LLVM_BUILD_X must imply LLVM_INCLUDE_X, as you
>> cannot build a target that doesn't exist.
>> The problem is that I could not find a way to change the cache value
>> of LLVM_INCLUDE_X displayed in ccache and cmake-gui to ON if
>> LLVM_BUILD_X is ON. This would make it clearer to the user and make
>> the code simpler.
>>> The current one removes a convenience from the
>> It shouldn't. See above.
>>> and is unnecessarily complex (why not
>>> if( LLVM_INCLUDE_X )
>>> instead of changing add_llvm_X)
>> I agree. However, it should be:
>> if( LLVM_INCLUDE_X OR LLVM_BUILD_X )
>> Because of the problem above.
>>> In any case, removing an option (LLVM_BUILD_TOOLS)
>> No options were removed.
>>> and introducing a new
>>> one (LLVM_INCLUDE_TOOLS) requires updating docs/CMake.html.
>> I agree.
>>> Oh, and next time it would be a good idea to discuss the patch before
>>> committing it.
>> It didn't (well, wasn't supposed to) have any functionality changes,
>> it's rather small, and it was needed to fix the build bot. Is this
>> really too much for review after commit?
>> - Michael Spencer
> Attached is a patch that fixes the above (except for auto updating
> - Michael Spencer
There was a bug in that last patch. Correct patch attached.
- Michael Spencer
-------------- next part --------------
A non-text attachment was scrubbed...
Size: 6151 bytes
Desc: not available
Url : http://lists.cs.uiuc.edu/pipermail/llvm-commits/attachments/20100924/b36596f4/attachment.obj
More information about the llvm-commits