Improve CMakeLists.txt

[graeme-a-stewart]

This is a large change to CMakeLists, with the headline that it fixes the compilation of wrapit on AlmaLinux9.

Simplify the configuration to use the system clang/llvm installation (use of CLANG_JLL from the Julia installation seems very fragile and I could not get it to work properly)

Use the "unified" new clang targets (libclang clang-cpp) instead of the deprecated split libraries.

Use recommended llvm recipe for the code llvm libraries.

Update CMake minimum version to 3.12, which throws no warnings on configuration with the latest releases (released 5 years ago, so not onerous!).

Update README with instructions for Alma and updated instructions for OS X brew install location.

[grasph]

Hello Graeme,

Thanks for making it build on Alma/RH 9 sharing your work with this PR.

The CLANG_JLL is for building BinaryBuilder. It's experimental. Especially, it needs to be updated in order to include the setting of clang resource path which I recently introduced in the cmake phase. The option should be kept.

Is there a motivation to add dependency on the full clang C++ API (clang-ccp)?

The code uses the C API of clang. There are some undesirable C++ API calls to work around few C API limitations, I hope will go away in some future clang versions. I would prefer to keep track of the clang C++ libraries we depend on. Use of individual dependencies is advertised in the clang documentation in the LibASTMatchersTutorial section, so I would expect it is ok to do so. Let me know if you have concerns with this approach.

Cheers, Philippe.

[grasph]

@graeme-a-stewart after having a try, I see that Alma has its own version of clang cmake files.

It looks like clang installations differ significantly from one distribution to another, each applying its own patch. I am wondering if the instructions should not be to install clang from the source distributed by LLVM team.

[graeme-a-stewart]

Hi @grasph - thanks for the comments!

I have restored the option to build with CLANG_JLL set. So is BinaryBuilder building wrapit? I couldn't see this in Yggdrasil.

I would really not try and tell people to download the source of LLVM and build it, it's way too heavy a requirement when we can get wrapit to build nicely on major platforms out of the box. On Alma9 I think we need to accept that libclang-cpp is needed. Without it there are a bunch of linker errors like

undefined reference to `clang::QualType::getAsString[abi:cxx11]() const'

I also can't see another way to build on RHEL and clones, which are the major platform for HEP.

[graeme-a-stewart]

A couple of other things...

Does this line do anything? I couldn't see the variable being used:

set(SEARCH_CLANG_CMAKE TRUE
    CACHE BOOL "If true use the Clang cmake configuration files found on the system.")

And, can we setup a CI here, so that the platforms we would like to support get tested?

[grasph]

Hello Graeme,

1. The clang-cpp dependency switch from static to dynamic the linkage to the C++ API part, which I did not realize first. The shared library is a single one, while for static code is split in many. The dynamic library looks better supported in the different Linux distribution.

So it's actually a good think to move to clang-cpp. I've validated it on Debian.

2. I expect SEARCH_CLANG_CMAKE can be removed.
3. It is not yet on Yggdrasil, but I am working on it. I will fix the Clang_JLL part.
4. A CI running the tests would be great. I'm not familiar with CI and I've missed so far time to invest on it. If you can help on this front, that would be excellent.

Philippe.

[grasph]

PR merged on main branch in commit da4d651.

[graeme-a-stewart]

Thanks for the merge, @grasph!

If you want to make an issue for the CI and assign it to me then I will work on it in the next week (I have some experience and examples I can harvest!).

[grasph]

Thanks @graeme-a-stewart! Please find the issue here: #45. I was not able to assign the issue to you name.