[CMake] Get Clang directory from CLANG_INSTALL_PREFIX

[guitargeek]

If Clang was installed standalone (in particular, not in the LLVM tree),
use the CLANG_INSTALL_PREFIX variable to indicate where it is.

Actually, we can always use CLANG_INSTALL_PREFIX. It's defined by
ClangConfig.cmake. We should also need to take into account
LLVM_LIBDIR_SUFFIX.

Follows up on:

• [CMake] Fixes and improvements for buildng with builtin_clang=OFF #18386
• Revert PR #18386 #18446

[hahnjo]

I disagree with merging any build system related work at this point.

[guitargeek]

Sure, this can also be done after the branching of 6.36!

[github-actions]

Test Results

    18 files      18 suites   3d 17h 17m 17s ⏱️
 2 731 tests  2 731 ✅ 0 💤 0 ❌
47 771 runs  47 771 ✅ 0 💤 0 ❌

Results for commit fbec790.

♻️ This comment has been updated with latest results.

[guitargeek]

@hahnjo, what do you think about this for master?

[hahnjo]

Hm wait, CLANG_INSTALL_PREFIX is not set for me with builtin_clang=OFF... Could be that we have to forward it

[hahnjo]

I think we need

diff --git a/interpreter/CMakeLists.txt b/interpreter/CMakeLists.txt
index e6512a88828..014be83dc28 100644
--- a/interpreter/CMakeLists.txt
+++ b/interpreter/CMakeLists.txt
@@ -407,6 +407,9 @@ if (builtin_clang)
 else()
   find_package(Clang REQUIRED CONFIG)
   message(STATUS "Found Clang ${CLANG_PACKAGE_VERSION} in ${CLANG_CMAKE_DIR}")
+
+  # Forward CLANG_INSTALL_PREFIX so it can be used by core/clingutils/CMakeLists.txt
+  set(CLANG_INSTALL_PREFIX ${CLANG_INSTALL_PREFIX} PARENT_SCOPE)
 endif()
 
 # Reset the compiler flags after compiling LLVM and Clang

It might have worked by accident before if /lib/clang/ happened to contain files for the right Clang version...

[guitargeek]

Hm wait, CLANG_INSTALL_PREFIX is not set for me with builtin_clang... Could be that we have to forward it

But the change is in the else branch of the if (builtin_clang) so it should be alright, no?

I have tested it for my setup: system LLVM without Clang and then standalone Clang in different directory, and it works

[hahnjo]

Hm wait, CLANG_INSTALL_PREFIX is not set for me with builtin_clang... Could be that we have to forward it

But the change is in the else branch of the if (builtin_clang) so it should be alright, no?

Yes sorry, my comment was supposed to say builtin_clang=OFF. It is set in interpreter/CMakeLists.txt, but not propagated to the parent directory.

I have tested it for my setup: system LLVM without Clang and then standalone Clang in different directory, and it works

Can you print the result of CLANG_RESOURCE_DIR_STEM? For me it was /lib/clang which is incorrect, but happened to work...

[guitargeek]

Ok, I'll check. But I will also make some test with a full LLVM build including patched Clang so I'm more sure to understand all cases. So for me the PR is on hold anyway

[guitargeek]

Hi @hahnjo! I now tested this PR with two configurations:

1. System LLVM with no Clang on the system, plus standalone build of patched Clang
2. No LLVM on the system, but custom LLVM build with LLVM_ENABLE_PROJECTS=clang

Both works! By the way, what do you think about doing find_package(Clang) also in core, instead of the set(CLANG_INSTALL_PREFIX ${CLANG_INSTALL_PREFIX} PARENT_SCOPE)? Setting variables in the parent scope is more confusing IMO 🙂 But anyway, both works and I'm happy with how the PR currently is 👍

[hahnjo]

By the way, what do you think about doing find_package(Clang) also in core, instead of the set(CLANG_INSTALL_PREFIX ${CLANG_INSTALL_PREFIX} PARENT_SCOPE)? Setting variables in the parent scope is more confusing IMO

Potentially, but we also compute many other variables in interpreter/CMakeLists.txt and having them all in one place might be the better choice