[rootcling] Remove llvm::cl dependency by using ROOT's optparse

[devajithvs]

This Pull request:

This is a draft PR to test if it is possible to use optparse in rootcling:

• barecling subcommand makes this slightly complicated, but not sure if it is worth adding subcommand feature to optparse.
• TODO: Move from global variables for opt to a RootClingConfig object (not doing this keeps the diff minimal for now).

This patch potentially removes issues like

CommandLine Error: Option 'W' registered more than once!

caused by llvm::cl registering the same options multiple times when ROOT is linked against a shared libLLVM.

(See: #12156)

Changes or fixes:

Checklist:

• tested changes locally
• updated the docs (if necessary)

This PR fixes #

[github-actions]

Test Results

    22 files      22 suites   3d 10h 32m 2s ⏱️
 3 770 tests  3 768 ✅ 0 💤 2 ❌
74 986 runs  74 984 ✅ 0 💤 2 ❌

For more details on these failures, see this check.

Results for commit 195fcf1.

♻️ This comment has been updated with latest results.

[silverweed]

Note: clearly if we want to use optparse.hxx from modules other than main I'll move it to a better location (should it be in core since we use it from core/dictgen?)

[devajithvs]

Note: clearly if we want to use optparse.hxx from modules other than main I'll move it to a better location (should it be in core since we use it from core/dictgen?)

Yes, it would be nice to have the header somewhere in core.

[hahnjo]

For future reference, this is the reverse direction of #4171

[hahnjo]

I just tested removing all LLVM_DYLIB workarounds, and including this PR solves all build errors due to duplicate options registered with llvm::cl::opt. There are a few test failures (~10) that need to be tracked down (in future PRs), but I think this is promising 👍

[vgvassilev]

I am not sure what is the merit of going this direction. If we want to avoid the assert, it is rather a symptom rather than an issue. That is, when we see the assert we almost certainly have screwed up the setup of the llvm linking.

[hahnjo]

That is, when we see the assert we almost certainly have screwed up the setup of the llvm linking.

The goal here is to make this a non-screwup and allow linking against the libLLVM dylib, if the user tells us to

[vgvassilev]

That is, when we see the assert we almost certainly have screwed up the setup of the llvm linking.

The goal here is to make this a non-screwup and allow linking against the libLLVM dylib, if the user tells us to

Yes, and that would cause symbols to clash when dlopening libraries in libCling that bring their own copy of llvm. Of course that assertion won’t fire however later when you deploy root it would cause issues. We’ve been discussing this over many years and there doesn’t seem to be a reasonable way forward. That’s why I wonder if it’s worth revisiting this topic rather than actually fixing llvm to not link libLLVM.so when we request static linking.

[hahnjo]

that would cause symbols to clash when dlopening libraries in libCling that bring their own copy of llvm

When building against an external LLVM, the user tells us (more or less) that they don't care about symbol clashes. Otherwise they would use builtin_llvm=ON in which case we can take all precautions necessary to avoid issues. Nothing changes there, the plan is definitely not to start building libLLVM.so inside ROOT. The goal here is rather to allow linking against libLLVM.so which is an extremely common way distributions build LLVM (all of Arch Linux, Enterprise Linux, Fedora, Ubuntu; didn't check Debian).

[vgvassilev]

that would cause symbols to clash when dlopening libraries in libCling that bring their own copy of llvm

When building against an external LLVM, the user tells us (more or less) that they don't care about symbol clashes. Otherwise they would use builtin_llvm=ON in which case we can take all precautions necessary to avoid issues. Nothing changes there, the plan is definitely not to start building libLLVM.so inside ROOT. The goal here is rather to allow linking against libLLVM.so which is an extremely common way distributions build LLVM (all of Arch Linux, Enterprise Linux, Fedora, Ubuntu; didn't check Debian).

Yes, but what is the end goal? You can't do what you are doing for rootcling to libCling, right? If you do the same strategy you can't load things like libmessa which ships its own llvm.

[hahnjo]

You can't do what you are doing for rootcling to libCling, right?

I don't understand this question, what does "rootcling to libCling" mean?

If you do the same strategy you can't load things like libmessa which ships its own llvm.

I don't see this being the case: libEGL_mesa.so and libGLX_mesa.so link against the system-provided libLLVM.so. If they are the same, they can happily co-exist in the same process.

[vgvassilev]

You can't do what you are doing for rootcling to libCling, right?

I don't understand this question, what does "rootcling to libCling" mean?

If you do the same strategy you can't load things like libmessa which ships its own llvm.

I don't see this being the case: libEGL_mesa.so and libGLX_mesa.so link against the system-provided libLLVM.so. If they are the same, they can happily co-exist in the same process.

What if they are different which is usually the case? Also, some distributions statically linked llvm in messa and who knows what else. I think going in this direction will cause a lot of pain for us because we don't know what the users are going to ask ROOT to dlopen... I think that reintroduces a problem that we worked very hard to avoid. We can probably work it around in a more reliable way if we used a linker script to rename libCling's llvm symbols to avoid clashes. However, I do not see really a benefit rootcling to be using a different version of llvm...