[llvm-commits] [llvm] r114747 - in /llvm/trunk: CMakeLists.txt cmake/modules/AddLLVM.cmake unittests/CMakeLists.txt utils/unittest/CMakeLists.txt

[Óscar Fuentes]

Michael Spencer <bigcheesegs at gmail.com>
writes:

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

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)

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

Please explain in more detail what the problem is and maybe we can find
a better implementation. The current one removes a convenience from the
user and is unnecessarily complex (why not

if( LLVM_INCLUDE_X )
  add_subdirectory(X)
endif()

instead of changing add_llvm_X)

In any case, removing an option (LLVM_BUILD_TOOLS) and introducing a new
one (LLVM_INCLUDE_TOOLS) requires updating docs/CMake.html.

Oh, and next time it would be a good idea to discuss the patch before
committing it.