[dolphin] make link with LLVM
AcceptedPublic

Authored by rjvbb on Jul 29 2019, 9:22 AM.

Details

Reviewers
elvisangelaccio
Summary

This patch fixes a link failure when building with the LLVM toolchain which does not discover the dependency on or pull in the private dolphin library when linking dolphin itself.

BUG: 410237

Diff Detail

Lint
Lint Skipped
Unit
Unit Tests Skipped
Build Status
Buildable 15644
Build 15662: arc lint + arc unit
rjvbb created this revision.Jul 29 2019, 9:22 AM
Restricted Application added a subscriber: kfm-devel. · View Herald TranscriptJul 29 2019, 9:22 AM
rjvbb requested review of this revision.Jul 29 2019, 9:22 AM

Can you try to build another kdeinit app (i.e. khelpcenter) to check if you get the same error?

src/CMakeLists.txt
303–305

Untested: have you tried to make dolphinprivate a PUBLIC dependency of kdeinit_dolphin?

rjvbb added a comment.Aug 14 2019, 7:38 AM
Can you try to build another kdeinit app (i.e. `khelpcenter`) to check if you get the same error?

No. But khelpcentre doesn't use a static library.

Untested: have you tried to make dolphinprivate a PUBLIC dependency of kdeinit_dolphin?

No, should I?

rjvbb added a comment.Aug 15 2019, 8:27 AM

Untested: have you tried to make dolphinprivate a PUBLIC dependency of kdeinit_dolphin?

Appears that this works too:

target_link_libraries(kdeinit_dolphin PUBLIC
    dolphinprivate
    PRIVATE
    dolphinstatic
    KF5::Crash
)

Untested: have you tried to make dolphinprivate a PUBLIC dependency of kdeinit_dolphin?

Appears that this works too:

target_link_libraries(kdeinit_dolphin PUBLIC
    dolphinprivate
    PRIVATE
    dolphinstatic
    KF5::Crash
)

Cool. I'd prefer this fix, so that we don't add another target_link_libraries call.

rjvbb updated this revision to Diff 64638.Mon, Aug 26, 7:52 AM

Updated to use the "cool solution" ;)

rjvbb set the repository for this revision to R318 Dolphin.Mon, Aug 26, 7:53 AM
elvisangelaccio accepted this revision.Sun, Sep 1, 11:46 AM
This revision is now accepted and ready to land.Sun, Sep 1, 11:46 AM