[llvm] r241663 - Revert r241621, "[CMake] Cleanup tools/CMakeLists.txt to take advantage of the auto-registration that was already partially working."

Chris Bieneman beanz at apple.com
Mon Jul 13 14:05:57 PDT 2015


> On Jul 10, 2015, at 5:14 PM, NAKAMURA Takumi <geek4civic at gmail.com> wrote:
> 
> Chris, thanks for your working.
> 
> 2015-07-11 5:45 GMT+09:00 Chris Bieneman <beanz at apple.com>:
>> Takumi,
>> 
>> How does this sound. I’d like to re-land the patches with the following
>> changes:
>> 
>> (1) Keep the LLVM_EXTERNAL_*_SOURCE_DIR naming
>> (2) Ignore LLVM_EXTERNAL_*_SOURCE_DIR if an in-tree directory exists
>> (3) Only cache LLVM_EXTERNAL_*_SOURCE_DIR variables if they are used
> 
> Sounds good. For now, their default value may null.

So… I tried this, and broke every bot using CMake everywhere.

The problem is that because the value is cached changing the behavior and caching characteristics of the variable can only be done if we clear the CMakeCaches on all the builders. This is just one more reason why renaming the value is probably a good idea.

I’m ready to abandon these patches entirely because this is sinking way too much time for a code cleanup change. It is unfortunate that cleaning this up is proving prohibitively difficult.

-Chris

> 
>> The patches will still:
>> (1) Migrate to implicit adding tools by default
>> (2) Use LLVM_TOOL_*_BUILD to enable and disable tool builds, and to prevent
>> tools from being added more than once
>> (3) Generate LLVM_TOOL_*_BUILD for all tools as advanced options
> 
> I am still uncomfortable about them. Even they were ADVANCED, they
> might occupy many lines in ccmake (or cmake-gui)
> and they would stay in the cache if they were removed again.
> I don't imagine any usecases to toggle individual LLVM_TOOL_*_BUILD.
> 
> That said, it's ok as far as the tree is fine.





More information about the llvm-commits mailing list