[llvm] r337366 - [CMake] Export the LLVM_LINK_LLVM_DYLIB setting

[Justin Bogner via llvm-commits]

Philip Pfaffe <philip.pfaffe at gmail.com> writes:
> I can reproduce the not-linking-with-dylib bit, but it doesn't fail during
> linking for me. That's why I missed this in testing. Sorry!
>
> Is this serious enough to revert to unbreak you?

I've reverted it locally to keep things working on my side for now, and
since LLVMConfig.cmake doesn't change much that's not too hard to
maintain short term. I guess it depends on how long it takes to get the
correct fix.

> Alternatively there's two quick fixes for this:
> - Make LLVM_LINK_LLVM_DYLIB an option() in LLVMConfig.cmake. Then it'll
>   show up in the GUI and respects configurations.
> - Wrap the set() in an if().
>
> I'd prefer going the option() route.

That sounds like it would work, but I do see that there are no options
or cache variables in LLVMConfig.cmake today and I'm not sure what the
implications of adding some are. Can you think of anything that might go
wrong by doing this?

There is some precedent for the if() approach, even if it is kind of
strange.

> Adding a CLANG_LINK_LLVM_DYLIB additionally is worth considering, I'd
> prefer doing that as a seperate change though.
>
> Cheers,
> Philip
>
> On Tue, Jul 24, 2018 at 1:30 AM Justin Bogner <mail at justinbogner.com> wrote:
>
>> Philip Pfaffe via llvm-commits <llvm-commits at lists.llvm.org> writes:
>> > Author: pfaffe
>> > Date: Wed Jul 18 01:53:31 2018
>> > New Revision: 337366
>> >
>> > URL: http://llvm.org/viewvc/llvm-project?rev=337366&view=rev
>> > Log:
>> > [CMake] Export the LLVM_LINK_LLVM_DYLIB setting
>> >
>> > Summary:
>> > When building out-of-tree tools, there are several macros available to
>> > automate linking against llvm. An examples is `add_llvm_executable`, or
>> > the clang variant of this.
>> >
>> > These macros use the LLVM_LINK_LLVM_DYLIB option to decide whether to
>> > link against libraries defined by setting LLVM_LINK_COMPONENTS or to
>> > link against libLLVM instead. Currently this is problematic in
>> > out-of-tree targets, because they cannot identify whether this option is
>> > required or even available. If the option was enabled in LLVM's own
>> > build, the clang libraries are built against libLLVM, so a client
>> > linking against those must link against it too. On the other hand the
>> > client can't just always link against it, because it might not be
>> > available.
>>
>> I'm not sure I understand the problem this is solving, but it breaks the
>> case where an out-of-tree target tries to link the dylib when the
>> in-tree build does not. Specifically, if I build llvm with
>> LLVM_LINK_LLVM_DYLIB=Off (but LLVM_BUILD_LLVM_DYLIB=On), and then I try
>> to build clang against this with LLVM_LINK_LLVM_DYLIB=On, clang's
>> setting of the variable seems to be ignored and I get link errors as it
>> looks for static libraries.
>>
>> Can this be made to respect externally setting the variable? Maybe it
>> should only show up if set, for example? Alternatively, do external
>> projects need to implement their own version of the variable, like a
>> CLANG_LINK_LLVM_DYLIB?
>>
>> > This is related to D44391, but that change assumed the client knew
>> > whether they wanted the dylib or not.
>> >
>> > Reviewers: mgorny, beanz, labath
>> >
>> > Reviewed By: mgorny
>> >
>> > Subscribers: bollu, llvm-commits
>> >
>> > Differential Revision: https://reviews.llvm.org/D49193
>> >
>> > Modified:
>> >     llvm/trunk/cmake/modules/LLVMConfig.cmake.in
>> >
>> > Modified: llvm/trunk/cmake/modules/LLVMConfig.cmake.in
>> > URL:
>> >
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/cmake/modules/LLVMConfig.cmake.in?rev=337366&r1=337365&r2=337366&view=diff
>> >
>> ==============================================================================
>> >
>> > --- llvm/trunk/cmake/modules/LLVMConfig.cmake.in (original)
>> > +++ llvm/trunk/cmake/modules/LLVMConfig.cmake.in Wed Jul 18 01:53:31
>> 2018
>> > @@ -13,6 +13,8 @@ set(LLVM_COMMON_DEPENDS @LLVM_COMMON_DEP
>> >
>> >  set(LLVM_AVAILABLE_LIBS @LLVM_AVAILABLE_LIBS@)
>> >
>> > +set(LLVM_LINK_LLVM_DYLIB @LLVM_LINK_LLVM_DYLIB@)
>> > +
>> >  set(LLVM_DYLIB_COMPONENTS @LLVM_DYLIB_COMPONENTS@)
>> >
>> >  set(LLVM_ALL_TARGETS @LLVM_ALL_TARGETS@)
>> >
>> >
>> > _______________________________________________
>> > llvm-commits mailing list
>> > llvm-commits at lists.llvm.org
>> > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>