In D58157 LLVM_EXTERNAL_CLANG_TOOLS_EXTRA_SOURCE_DIR is set only when you enable clang-tools-extra but in D96787 has been added by default. This should be fixed and use the old variant with CMAKE_SOURCE_DIR, otherwise we have build failures when you don't specify clang-tools-extra in LLVM_ENABLE_PROJECTS even though it's going to be built.
I am not an expert on Cmake nor understand what is going on here fully, but here are my 2 cents.
D58157 didn't remove LLVM_EXTERNAL_CLANG_TOOLS_EXTRA_SOURCE_DIR, it just stopped enabling CTE whenever clang was enabled.
All of LLVM_EXTERNAL_$PROJECT variables are set based on the values passed into LLVM_ENABLE_PROJECTS in https://github.com/llvm/llvm-project/blob/main/llvm/CMakeLists.txt#L105.
So in theory this cmake file should only be evaluated when CTE is enabled and LLVM_EXTERNAL_CLANG_TOOLS_EXTRA_SOURCE_DIR should always be set when CTE is enabled.
The correct fix would be to figure out why one of the above statements are not holding, i.e.:
- This cmake file is evaluated/executed despite CTE being off (this shouldn't be the case)
- LLVM_EXTERNAL_CLANG_TOOLS_EXTRA_SOURCE_DIR is not set, even though CTE is part of LLVM_ENABLE_PROJECTS.
If you don't specify LLVM_ENABLE_PROJECTS clang-tools-extra will still be built but without having set LLVM_EXTERNAL_CLANG_TOOLS_EXTRA_SOURCE_DIR.
This feels like the failure of the first case i mentioned, i.e. some cmake file under CTE being executed even though it is disabled. Can you provide some repro steps? For example when I do:
git clone email@example.com:llvm/llvm-project.git cd llvm-project && mkdir build_test && cd build_test cmake -G Ninja ../llvm/ -DCMAKE_BUILD_TYPE="RELEASE" ninja all
this finishes without any errors. as expected clang-tools-extra/clangd/quality/CompletionModel.cmake doesn't interfere with the build when clang-tools-extra is not mentioned in LLVM_ENABLE_PROJECTS.
also there are lots of build bots building different subsets of LLVM in different configurations and none of them have been broken by this change. so it would be great to understand the reason behind the breakage you are experiencing.
On llvm main checkout from the github repo, I'm building it with the following:
cmake -GNinja -DCMAKE_C_COMPILER=/builds/worker/fetches/gcc/bin/gcc -DCMAKE_CXX_COMPILER=/builds/worker/fetches/gcc/bin/g++ -DCMAKE_ASM_COMPILER=/builds/worker/fetches/gcc/bin/gcc -DCMAKE_LINKER=/builds/worker/fetches/binutils/bin/ld -DCMAKE_AR=/builds/worker/fetches/binutils/bin/ar -DCMAKE_C_FLAGS= -DCMAKE_CXX_FLAGS= -DCMAKE_ASM_FLAGS= -DCMAKE_EXE_LINKER_FLAGS=-Wl,-Bsymbolic-functions -fuse-ld=gold -Wl,--gc-sections -Wl,--icf=safe -DCMAKE_SHARED_LINKER_FLAGS=-Wl,-Bsymbolic-functions -fuse-ld=gold -Wl,--gc-sections -Wl,--icf=safe -DCMAKE_BUILD_TYPE=Release -DCMAKE_INSTALL_PREFIX=/builds/worker/fetches/llvm-project/build/stage1/clang -DLLVM_TARGETS_TO_BUILD=X86 -DLLVM_ENABLE_ASSERTIONS=OFF -DPYTHON_EXECUTABLE=/usr/bin/python2.7 -DLLVM_TOOL_LIBCXX_BUILD=ON -DLLVM_ENABLE_BINDINGS=OFF -DLLVM_EXPERIMENTAL_TARGETS_TO_BUILD=WebAssembly -DLLVM_BINUTILS_INCDIR=/builds/worker/fetches/gcc/include -DLLVM_ENABLE_LIBXML2=FORCE_ON -DCMAKE_SYSROOT=/builds/worker/fetches/sysroot -DLLVM_LINK_LLVM_DYLIB=ON -DCMAKE_RANLIB=/builds/worker/fetches/binutils/bin/ranlib /builds/worker/fetches/llvm-project/llvm
As you can see I don't specify LLVM_ENABLE_PROJECTS but still looking at the build failure, it seems that clang-tools-extra is being built, or parts of it are:
ninja: error: '/clangd/quality/CompletionModelCodegen.py', needed by 'tools/clang/tools/extra/clangd/CompletionModel.cpp', missing and no known rule to make it
If I specify only clang or clang-tools-extra in LLVM_ENABLE_PROJECTS` this will not happen.
Apparently Phabricator detected the presence of "revert D100625" in my alternative D101851's description and marked that as reverting this one. Of course, as this hadn't been committed, it cannot have been reverted either. Could you confirm that things work for you now, @Abpostelnicu?
Sorry for not noticing this before, this is pretty much what I came up with too and what's committed now.