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

[Nico Weber via lldb-commits]

On Fri, Mar 15, 2019 at 10:04 AM Pavel Labath <pavel at labath.sk> wrote:

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

I am in the "document build prereqs and require them" camp but nothing else
in LLVM's CMake build is, as far as I can tell. In fact, if that was
project philosophy we wouldn't need all these cmake config checks, and yet
we have them. So I think this should be consistent with the rest of LLVM.


> 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 think that's the status quo already.


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

I'm not sure I agree with that; if it fails at build time I can at least
still build everything that's not lldb.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20190315/23a31a6d/attachment.html>