Repository navigation
[clang] do not search again for LLVM if already found by parent project - #23160
ferdymercury wants to merge 1 commit into
Conversation
In ROOT, for builtin_llvm=OFF we have the sequence find_package(LLVM) # searchs for system package, which sets DYLIB to ON set(LLVM_LINK_LLVM_DYLIB OFF) # override DYLIB to OFF, we want static add_subdirectory(clang) # this will call find_package LLVM again See root-project#13420 Since LLVM22 though, this fails: Even if we call for a second time set(LLVM_LINK_LLVM_DYLIB OFF) after the add_subdirectory line, it's not enough, because that variable was ON all throughout the child clang/CMakeLists.txt and has already configured some stuff with the wrong flag. Avoid the issue by preventing the double-search of LLVM and thus the double overwrite. Fixes root-project#18387 Fixes root-project#23155
Test Results 23 files 23 suites 3d 13h 49m 20s ⏱️ Results for commit 3f21c9a. ♻️ This comment has been updated with latest results. |
|
Sorry, I have to remove the review request because I have to decided it's just easier to patch my system Clang build rather than arguing for patches that make |
|
Yes, I don't see the point of adding more downstream patches for this case. I've said it before, the problem should be fixed "properly" by making it possible to link against a provided |
Yep, I tried that PR as patch on my local build last week, but that does not help since it seems it moves the problem somewhere else. When applying that patch and building, I got: Reproducer: |
…in_llvm=OFF Alternative to root-project#23160 that does not require any upstream or downstream patches In ROOT, for builtin_llvm=OFF we have the sequence ``` find_package(LLVM) # searchs for system package, which sets DYLIB to ON set(LLVM_LINK_LLVM_DYLIB OFF) # override DYLIB to OFF, we want static add_subdirectory(clang) # this will call find_package LLVM again ``` See root-project#13420 Since LLVM22 though, this fails: Even if we call for a second time `set(LLVM_LINK_LLVM_DYLIB OFF)` after the `add_subdirectory` line, it's not enough, because that variable was ON all throughout the child `clang/CMakeLists.txt` and has already configured some stuff with the wrong flag. Avoid the issue by preventing the double-search of LLVM and thus the double overwrite, by temporarily overwriting the find_package(LLVM) calls. Fixes root-project#18387 Fixes root-project#23155
…in_llvm=OFF Alternative to root-project#23160 that does not require any upstream or downstream patches In ROOT, for builtin_llvm=OFF we have the sequence ``` find_package(LLVM) # searchs for system package, which sets DYLIB to ON set(LLVM_LINK_LLVM_DYLIB OFF) # override DYLIB to OFF, we want static add_subdirectory(clang) # this will call find_package LLVM again ``` See root-project#13420 Since LLVM22 though, this fails: Even if we call for a second time `set(LLVM_LINK_LLVM_DYLIB OFF)` after the `add_subdirectory` line, it's not enough, because that variable was ON all throughout the child `clang/CMakeLists.txt` and has already configured some stuff with the wrong flag. Avoid the issue by preventing the double-search of LLVM and thus the double overwrite, by temporarily overwriting the find_package(LLVM) calls. Fixes root-project#18387 Fixes root-project#23155
Alternative to #23160 that does not require any upstream or downstream patches In ROOT, for builtin_llvm=OFF we have the sequence ``` find_package(LLVM) # searchs for system package, which sets DYLIB to ON set(LLVM_LINK_LLVM_DYLIB OFF) # override DYLIB to OFF, we want static add_subdirectory(clang) # this will call find_package LLVM again ``` See #13420 Since LLVM22 though, this fails: Even if we call for a second time `set(LLVM_LINK_LLVM_DYLIB OFF)` after the `add_subdirectory` line, it's not enough, because that variable was ON all throughout the child `clang/CMakeLists.txt` and has already configured some stuff with the wrong flag. Avoid the issue by preventing the double-search of LLVM and thus the double overwrite, by temporarily overwriting the find_package(LLVM) calls. Fixes #18387 Fixes #23155
Alternative to root-project#23160 that does not require any upstream or downstream patches In ROOT, for builtin_llvm=OFF we have the sequence ``` find_package(LLVM) # searchs for system package, which sets DYLIB to ON set(LLVM_LINK_LLVM_DYLIB OFF) # override DYLIB to OFF, we want static add_subdirectory(clang) # this will call find_package LLVM again ``` See root-project#13420 Since LLVM22 though, this fails: Even if we call for a second time `set(LLVM_LINK_LLVM_DYLIB OFF)` after the `add_subdirectory` line, it's not enough, because that variable was ON all throughout the child `clang/CMakeLists.txt` and has already configured some stuff with the wrong flag. Avoid the issue by preventing the double-search of LLVM and thus the double overwrite, by temporarily overwriting the find_package(LLVM) calls. Fixes root-project#18387 Fixes root-project#23155
This Pull request:
Changes or fixes:
In ROOT, for builtin_llvm=OFF we have the sequence
See #13420
Since LLVM22 though, this fails:
Even if we call for a second time
set(LLVM_LINK_LLVM_DYLIB OFF)after theadd_subdirectoryline, it's not enough, because that variable was ON all throughout the childclang/CMakeLists.txtand has already configured some stuff with the wrong flag.Avoid the issue by preventing the double-search of LLVM and thus the double overwrite.
Fixes #18387
Fixes #23155
Upstream PR: llvm/llvm-project#218965
Reproducer
Checklist: