Skip to content

cmake: check llvm-config in separate function, fix cmake error - #14033

Merged
andrewrk merged 1 commit into
ziglang:masterfrom
BratishkaErik:fix-cmake
Dec 27, 2022
Merged

cmake: check llvm-config in separate function, fix cmake error#14033
andrewrk merged 1 commit into
ziglang:masterfrom
BratishkaErik:fix-cmake

Conversation

@BratishkaErik

@BratishkaErikBratishkaErik commented Dec 22, 2022

Copy link
Copy Markdown
Contributor

Pull request #12136 has greatly improved LLVM search logic for CMake (kudos to @topolarity !). HOWEVER, it caused following regression: https://bugs.gentoo.org/886569. So, when targets is missing for LLVM, it prints:

CMake Error at cmake/Findllvm.cmake:90 (continue):
A CONTINUE command was found outside of a proper FOREACH or WHILE loop
scope.
Call Stack (most recent call first):
cmake/Findllvm.cmake:93 (NEED_TARGET)
CMakeLists.txt:138 (find_package)

I thought that this function should be macro instead so that this body replaces all occurencies of NEED_TARGET. HOWEVER 😄 , continue() does not work in macro: https://gitlab.kitware.com/cmake/cmake/-/issues/22052.

This PR moves "checking if llvm-config supports all what we need" in separate function (that saves potential error in variable, and if it is not empty, this llvm-config is marked as unsuitable), and slightly touches output, so now errors look like this:

Trying /usr/lib/llvm/15/bin/llvm-config...
This llvm-config is not suitable for us:
This LLVM is missing target AArch64. Zig requires LLVM to be built with all default targets enabled.
Trying another llvm-config...
CMake Error at cmake/Findllvm.cmake:97 (message):
Suitable llvm-config is not found.
Call Stack (most recent call first):
CMakeLists.txt:138 (find_package)

and this is normal output:

Trying /usr/lib/llvm/15/bin/llvm-config...
This llvm-config is suitable for us, continuing...

@BratishkaErik

Copy link
Copy Markdown
ContributorAuthor

Oops, I forgot that GREATER_EQUAL was added in CMake 3.7. Perhaps we can bump minimal version? It was released 6 years ago.

@BratishkaErikBratishkaErik changed the title cmake: check llvm-config in separate functioncmake: check llvm-config in separate function, fix cmake errorDec 25, 2022
@BratishkaErik

Copy link
Copy Markdown
ContributorAuthor

If possible can you add this to 0.10.1 please? Thanks

@andrewrk
andrewrk merged commit 55c3efc into ziglang:masterDec 27, 2022
@andrewrkandrewrk added this to the 0.10.1 milestone Dec 27, 2022
@andrewrk

Copy link
Copy Markdown
Member

Reverted in b1207b3.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@BratishkaErik@andrewrk