[Lldb-commits] [lldb] r355103 - [cmake] Move LLDB_DISABLE_LIBEDIT handling code into a central place

[Pavel Labath via lldb-commits]

On 14/03/2019 21:45, Nico Weber wrote:
> I tried LLVM_ENABLE_PROJECTS=all today, and it fails with
> 
> CMake Error: The following variables are used in this project, but they 
> are set to NOTFOUND.
> Please set them or make sure they are set and tested correctly in the 
> CMake files:
> libedit_INCLUDE_DIRS
>     used as include directory in directory 
> /usr/local/google/home/thakis/src/llvm-build-project/CMakeFiles/CMakeTmp
>     used as include directory in directory 
> /usr/local/google/home/thakis/src/llvm-build-project/CMakeFiles/CMakeTmp
>     used as include directory in directory 
> /usr/local/google/home/thakis/src/llvm-build-project/CMakeFiles/CMakeTmp
> libedit_LIBRARIES
>      linked by target "cmTC_43ea9" in directory 
> /usr/local/google/home/thakis/src/llvm-build-project/CMakeFiles/CMakeTmp
> 
> CMake Error at /usr/share/cmake-3.10/Modules/CheckTypeSize.cmake:114 
> (try_compile):
>    Failed to configure test project build system.
> Call Stack (most recent call first):
>    /usr/share/cmake-3.10/Modules/CheckTypeSize.cmake:239 
> (__check_type_size_impl)
>    
> /usr/local/google/home/thakis/src/llvm-project/lldb/cmake/modules/LLDBConfig.cmake:106 
> (check_type_size)
>    /usr/local/google/home/thakis/src/llvm-project/lldb/CMakeLists.txt:15 
> (include)
> 
> 
> 
> Is that expected?
> 

Whether it is "expected" to fail the build if you don't have libedit is 
a bit of a complicated question and the answer depends on which side of 
the "should dependencies auto-disable themselves when not found" debate. 
However, it's certainly not expected that the build fails in this way, 
as there is a find_package(LibEdit REQUIRED) call two lines above this 
check.

I've done some digging around, and it seems this is due to a bug in our 
our libedit-finding code. r356263 will fix that, so now the build will 
fail slightly earlier and with a slightly better error message.

If we want to change the libedit dependency to auto-disabling, then we 
should start a separate thread about that. I believe that's not 
something that this commit has caused. What it may have done is that it 
changed the failure from a build-time thing (we would fail when trying 
to compile code using libedit) to a configure-time failure. That I would 
consider to be an improvement. :)