Skip to content

[interpreter] Use find_package({LLVM,Clang})#17737

Merged
hahnjo merged 2 commits into
root-project:masterfrom
hahnjo:cling-find
Feb 18, 2025
Merged

[interpreter] Use find_package({LLVM,Clang})#17737
hahnjo merged 2 commits into
root-project:masterfrom
hahnjo:cling-find

Conversation

@hahnjo

@hahnjo hahnjo commented Feb 17, 2025

Copy link
Copy Markdown
Member

This gives CMake full information about (transitive) library dependencies.

Fixes #17461

@hahnjo
[cling] Print correct LLVM CMake directory
d8a4bda 
LLVM_DIR can be set as a hint for find_package, but LLVMConfig.cmake
sets LLVM_CMAKE_DIR.
@hahnjo
[interpreter] Use find_package({LLVM,Clang})
471c3be 
This gives CMake full information about (transitive) library
dependencies.

Fixes root-project#17461
@hahnjo hahnjo self-assigned this Feb 17, 2025
@hahnjo hahnjo requested a review from bellenot as a code owner February 17, 2025 12:36
@hahnjo hahnjo mentioned this pull request Feb 17, 2025
Open
devajithvs
devajithvs approved these changes Feb 17, 2025
View reviewed changes

@devajithvs devajithvs left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

vgvassilev
vgvassilev approved these changes Feb 17, 2025
View reviewed changes

@vgvassilev vgvassilev left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great to see that’s finally possible. Did you make sure it works with -Dbuiltin_clang,llvm=Off?

@hahnjo

hahnjo commented Feb 17, 2025

Copy link
Copy Markdown
Member Author

Did you make sure it works with -Dbuiltin_clang,llvm=Off?

Yes, builtin_llvm=OFF continues to work. This PR fixes builtin_llvm=OFF builtin_clang=OFF (see #17461) with LLVM and Clang installed together. I believe this should finally also allow to have them installed separately (for example build Clang against a system-installed LLVM), but I didn't tests this (yet). What does not work is builtin_clang=OFF with LLVM and Clang built with LLVM_LINK_LLVM_DYLIB - see #17698. Given that builtin_clang=OFF was completely broken before, I think this doesn't need to be in this PR.

@github-actions

github-actions Bot commented Feb 17, 2025

Copy link
Copy Markdown

Test Results

    18 files      18 suites   4d 6h 43m 6s ⏱️
 2 722 tests  2 722 ✅ 0 💤 0 ❌
47 294 runs  47 294 ✅ 0 💤 0 ❌

Results for commit 471c3be.

@hahnjo hahnjo merged commit 5c51aaf into root-project:master Feb 18, 2025
@hahnjo hahnjo deleted the cling-find branch February 18, 2025 07:17
@hahnjo hahnjo mentioned this pull request Feb 18, 2025
Merged
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vgvassilev vgvassilev vgvassilev approved these changes

@devajithvs devajithvs devajithvs approved these changes

@bellenot bellenot Awaiting requested review from bellenot bellenot is a code owner

Assignees

@hahnjo hahnjo

Labels

in:Build System in:Cling

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

builtin_clang=OFF broken because of missing transitive dependencies

3 participants

@hahnjo @vgvassilev @devajithvs