Fix kio-extras build on Windows
ClosedPublic

Authored by brute4s99 on Tue, May 5, 10:37 PM.

Diff Detail

Repository
R320 KIO Extras
Branch
arcpatch-D29461
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 27077
Build 27095: arc lint + arc unit
brute4s99 created this revision.Tue, May 5, 10:37 PM
Restricted Application added projects: Dolphin, Frameworks. · View Herald TranscriptTue, May 5, 10:37 PM
Restricted Application added subscribers: kfm-devel, kde-frameworks-devel. · View Herald Transcript
brute4s99 requested review of this revision.Tue, May 5, 10:37 PM
brute4s99 updated this revision to Diff 82039.EditedTue, May 5, 10:43 PM

needs testing on windows, will update in a while

UPDATE: All good on Windows as well as Linux ( tested on Manjaro with latest packages from repos).

meven requested changes to this revision.Wed, May 6, 6:34 AM
meven added a subscriber: meven.

We have :
kio-extras/cmake/Findlibssh.cmake

To do that. I don't think copy/pasting code to sftp/CMakeLists.txt is necessary.

This revision now requires changes to proceed.Wed, May 6, 6:34 AM
brute4s99 updated this revision to Diff 82069.Wed, May 6, 8:52 AM

updated the diff

@sitter could you please review this change as well?

sitter added a comment.Wed, May 6, 9:21 AM

Pid changes look fine, though perhaps we should just throw those two lines away? With Qt5 logging the pid is fairly pointless because one can simply set QT_MESSAGE_PATTERN to include the pid when necessary https://doc.qt.io/qt-5/qtglobal.html#qSetMessagePattern

sftp/CMakeLists.txt
25

Hm, I am a bit hazy on the details but I think this changes makes no sense. libssh (upstream) introduced an imported target ssh. For backwards compatibility we also inject this target when building with older libssh's than the latest (to be honest though, with libssh you basically always want the latest or you'll have an incredibly subpar experience).

brute4s99 added inline comments.Wed, May 6, 9:34 AM
sftp/CMakeLists.txt
25

I just built libssh 0.9.4, and kio-extras builds fine without this line change. Maybe we should try getting latest libssh onboard. Can I help in that somehow @vonreth ?

vonreth added inline comments.Wed, May 6, 10:15 AM
sftp/CMakeLists.txt
25

yes pls update libssh

brute4s99 updated this revision to Diff 83159.Tue, May 26, 7:06 PM
brute4s99 marked 3 inline comments as done.Tue, May 26, 7:10 PM
brute4s99 added inline comments.
sftp/CMakeLists.txt
25

Hi! Sorry, I got around to this patch quite late. I added a new patch for 0.9.4 release as parent revision to this revision, please take a look. ^_^

brute4s99 marked an inline comment as done.EditedTue, May 26, 10:18 PM

Thank you for reviewing the rev, Hannah! I've merged the parent rev and hence updated libssh to use version 0.9.4 by default in Craft. If we can merge this we would get 🟢 for kio-extras builds. \o/

meven accepted this revision.Wed, May 27, 5:34 AM
This revision is now accepted and ready to land.Wed, May 27, 5:34 AM
brute4s99 closed this revision.Wed, May 27, 4:30 PM