Fix linking to libssh 0.9.1
AbandonedPublic

Authored by arojas on Nov 5 2019, 3:33 PM.

Details

Reviewers
apol
sitter
fvogt
Summary

libssh 0.9.1 no longer defines LIBSSH_LIBRARIES in its cmake config, and exports a ssh_shared target instead.

Test Plan

Builds against libssh 0.8 and 0.9.1

Diff Detail

Repository
R320 KIO Extras
Lint
Lint Skipped
Unit
Unit Tests Skipped
arojas created this revision.Nov 5 2019, 3:33 PM
Restricted Application added projects: Dolphin, Frameworks. · View Herald TranscriptNov 5 2019, 3:33 PM
Restricted Application added subscribers: kfm-devel, kde-frameworks-devel. · View Herald Transcript
arojas requested review of this revision.Nov 5 2019, 3:33 PM
arojas retitled this revision from Fix linking to libssh 0.9 to Fix linking to libssh 0.9.1.Nov 5 2019, 3:43 PM
arojas edited the summary of this revision. (Show Details)
arojas edited the test plan for this revision. (Show Details)
apol accepted this revision.Nov 5 2019, 3:48 PM
apol added a subscriber: apol.

Fixes the build for me too, patch looks good.

Thanks!

This revision is now accepted and ready to land.Nov 5 2019, 3:48 PM
sitter accepted this revision.Nov 5 2019, 4:26 PM
sitter added subscribers: asn, sitter.

I think there's a smarter way of dealing with this somewhere in our finder, not blocking though. I'll take a look when I find a minute.

@asn it seems to me the cmake config broke compat between 0.9.0 and 0.9.1 :|

asn added a comment.Nov 5 2019, 4:36 PM

We moved from a manually generated libssh-config.cmake to install(EXPORTS libssh-config) and it does things completely different.

I'm currently trying to fix it. However better don't apply this as ssh_shared will vanish as a target.

I wonder how I can define LIBSSH_LIBRARIES again with EXPORTS.

sitter added a comment.Nov 5 2019, 4:49 PM
In D25159#559016, @asn wrote:

We moved from a manually generated libssh-config.cmake to install(EXPORTS libssh-config) and it does things completely different.

I'm currently trying to fix it. However better don't apply this as ssh_shared will vanish as a target.

I wonder how I can define LIBSSH_LIBRARIES again with EXPORTS.

I am not super certain, but I don't think you can as that is pretty much exactly where configure_package_config_file would be used. We certainly do use it that way all over KDE frameworks.

Btw about the ssh_shared target. It may make sense to settle on a target name and use that moving forward and advertise it as the recommended way of using libssh. IMPORTED targets are vastly preferred over the _LIBRARES/_INCLUDE_DIRS variables from a cmake POV because the targets can inject include dirs, flags and the likes without the library user having to worry about anything. So they are nicer to use in cmake. Just something to think about perhaps.

asn added a comment.Nov 5 2019, 4:52 PM

The plan is to have ssh as the target name and nothing else and support BUILD_SHARED_LIBS.

fvogt requested changes to this revision.Nov 5 2019, 5:58 PM
fvogt added a subscriber: fvogt.

The libssh maintainer is likely reverting the change, so this should not be necessary.

This revision now requires changes to proceed.Nov 5 2019, 5:58 PM
arojas abandoned this revision.Nov 5 2019, 5:59 PM
asn added a comment.Nov 6 2019, 8:56 AM

I've created https://gitlab.com/libssh/libssh-mirror/merge_requests/71

This means the right code would be then.

if (TARGET ssh) # libssh 0.9+
    target_link_libraries(kio_sftp ssh)
endif()
nisavid added a subscriber: nisavid.Nov 6 2019, 9:47 AM
sitter added a comment.Nov 6 2019, 1:00 PM

D25170 for when that lands