[cfe-dev] Separating clang-tools-extra from clang in LLVM_ENABLE_PROJECTS

[Nico Weber via cfe-dev]

I landed the change change in r354057.

On Thu, Feb 14, 2019 at 3:06 AM Jonas Toth <development at jonas-toth.eu>
wrote:

> Having clang-tools-extra as separate project makes sense in my eyes.
> clang-tools-extra now contains multiple useful projects (clangd is
> probably the "most standalone useful" project)
> so it seems logical to not treat it as an extension to "just clang".
> Am 14.02.19 um 04:58 schrieb Nico Weber via cfe-dev:
>
> On Wed, Feb 13, 2019 at 6:58 PM Mehdi AMINI <joker.eph at gmail.com> wrote:
>
>>
>>
>> On Wed, Feb 13, 2019 at 3:15 PM Reid Kleckner <rnk at google.com> wrote:
>>
>>> The context is https://reviews.llvm.org/D58157
>>>
>>> Back when Mehdi originally introduced the LLVM_ENABLE_PROJECTS cmake
>>> variable, people were discussing the idea of folding clang-tools-extra into
>>> clang, if we moved to multiple git repos away from a monorepo. To support
>>> that goal, this code was added:
>>> https://github.com/llvm/llvm-project/blob/master/llvm/CMakeLists.txt#L144
>>>       # There is a widely spread opinion that clang-tools-extra should
>>> be merged
>>>       # into clang. The following simulates it by always enabling
>>> clang-tools-extra
>>>       # when enabling clang.
>>>       if (proj STREQUAL "clang")
>>>         set(LLVM_EXTERNAL_CLANG_TOOLS_EXTRA_SOURCE_DIR
>>> "${CMAKE_CURRENT_SOURCE_DIR}/../clang-tools-extra")
>>>       endif()
>>>
>>> This makes it so that if you're building clang, you're building
>>> clang-tidy, clangd, etc. However, we have use cases where we want to check
>>> out the monorepo and just build clang, so we wanted to remove that block.
>>>
>>
>> Just to mention for completeness that the use case you mention can be
>> supported also by:
>> 1) building the clang target specifically: `ninja clang`
>>
>
> There's no good way to say "build and test everything but
> clang-tools-extra", though.
>
>
>> 2) the same way we support building without llc/opt/... by setting
>> -DLLVM_BUILD_TOOLS=OFF : there could be a -DCLANG_BUILD_TOOLS=OFF ; having
>> clang-tools-extra be "part of" clang does not *forces* to build them. There
>> are good question to ask about what the *default* should be when
>> configuring and building clang/llvm.
>>
>
> As said on the review, I'm open to that if there's a reason for that, but
> just using the same mechanism for clang-tools-extra that all other toplevel
> dirs get seems simpler.
>
>
>> Having clang-tools-extra considered part of clang is something that can
>> be evaluated separately (and I have no idea what makes sense today on this
>> topic) than the build targets (the same way that people were afraid at the
>> time that "monorepo" meant that we would always build everything, organize
>> the source and the build components are somehow orthogonal concerns).
>>
>
> As far as I know, most people working on clang and clang-tools-extra don't
> think this makes sense today. (I'd claim that was true 2 years ago as
> well.). We'll see if this thread shows differently, though.
>
>
>>
>> Best,
>>
>> --
>> Mehdi
>>
>>
>>
>>> The upshot is that if you use LLVM_ENABLE_PROJECTS today and you want to
>>> build clang tools, you will have to add "clang-tools-extra" to your CMake
>>> invocation after D58157 lands. We've identified the one upstream buildbot
>>> that uses this variable and plan to update it, but if you have downstream
>>> bots using this variable, you may also need to add "clang-tools-extra" to
>>> build clang-tidy & co.
>>>
>>> We have some consensus that this is the direction we want to go in the
>>> code review from Sam Mccall, Shoaib, myself, and Nico, but please let us
>>> know if you think this is the wrong direction.
>>>
>>
> _______________________________________________
> cfe-dev mailing listcfe-dev at lists.llvm.orghttps://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20190214/20d73e13/attachment.html>