Authored by labath on Mar 12 2018, 9:12 AM.



This is needed so that external projects (e.g. a standalone build of
lldb) can link to the LLVM shared library via the USE_SHARED argument of
llvm_config. Without this, llvm_config would add LLVM to the link list,
but then also add the constituent static libraries, resulting in
multiply defined symbols.

Diff Detail

labath created this revision.Mar 12 2018, 9:12 AM

I don't see why not but I'm not sure if it has any value by itself. FWICS, LLVM_DYLIB_COMPONENTS is not used anywhere in cmake/*.

It's used in cmake/modules/LLVM-Config.cmake, and that's the part I need. Without this, the llvm_config macro will behave differently in standalone vs. in-tree builds (i.e., it will be mostly broken in the out-of-tree case as this variable will be empty).

mgorny accepted this revision.Mar 12 2018, 10:06 AM

Aye, sorry, I confused it with generated config ;-). It looks sane to me.

This revision is now accepted and ready to land.Mar 12 2018, 10:06 AM

This change seems to make llvm_config() usable for out-of-tree builds, neat! I tried a standalone build of LLDB and it works fine after having rebuilt llvm with this change.

It should also make the example given on llvm-dev behave correctly, with one caveat:

This works:

llvm_config(foo support)

This doesn't (still chooses static libs):

llvm_config(foo USE_SHARED support)

The above is driving me nuts! I don't know enough about CMake to figure it out myself. :(

Scratch that, I see what's going on! The body of the llvm_config macro needs to check if(ARG_USE_SHARED) instead of if(USE_SHARED).

Could you please add this related fix to this change?

One more observation: It might be preferable to avoid defining set(LLVM_DYLIB_COMPONENTS all) in non-DYLIB builds. It prevents llvm_config(foo USE_SHARED ...) from opportunistically linking to the DYLIB while still having the static libraries as a fallback.

I really want projects linking to LLVM to be able to use llvm_config(foo USE_SHARED ...) and not worry about the detail of whether it's a static or dylib-enabled build.

The ARG_USE_SHARED thingy looks like a typo. I'll make a separate patch for that.

The LLVM_DYLIB_COMPONENTS "all" idea also sounds reasonable, but is also quite orthogonal to what I'm trying to do here (making standalone and in-tree builds behave the same way). I don't have time to look into that now, but I do encourage you to propose a patch for that yourself.

You have a point that any further tweaks to llvm_config should be created and reviewed separately. What I described above is not too great either; it shouldn't be required to specify USE_SHARED.

Ideally llvm_config(foo <list-of-components>) would work similar to llvm-config --libs <list-of-components. The latter defaults to using the dylib if LLVM is built with LLVM_LINK_LLVM_DYLIB=ON.

I'm not sure I can come up with a proper patch for this. With three build modes and in/out-of-tree builds, it gets confusing quite fast. πŸ˜•

In order to not derail this much further, I'll proceed to follow bug 35245 and post there if I manage to cobble something together.

This revision was automatically updated to reflect the committed changes.