Bug 401255

Summary:	Messaging app does not have sufficient dependencies
Product:	[Applications] kdeconnect	Reporter:	Simon Redman <simon>
Component:	messaging-application	Assignee:	Simon Redman <simon>
Status:	RESOLVED FIXED
Severity:	normal	CC:	aleixpol, nate
Priority:	NOR
Version First Reported In:	unspecified
Target Milestone:	---
Platform:	Other
OS:	Linux
Latest Commit:	https://invent.kde.org/kde/kdeconnect-kde/commit/3b92b0de1d57f409c14132960d06c9abecf13eda	Version Fixed/Implemented In:
Sentry Crash Report:

Description Simon Redman 2018-11-21 05:49:34 UTC

SUMMARY
On a system without ANY Qt Quick applications installed (In this test case, Ubuntu 18.04 with Gnome 3), the application is not able to launch and spews errors:
module "org.kde.kdirigami" is not installed
module "QtQuick" is not installed

After installing Kirigami, get error:
module "org.kde.people" is not installed

STEPS TO REPRODUCE
1. 
2. 
3. 

OBSERVED RESULT
App cannot launch because of missing dependencies

EXPECTED RESULT
SMS App should depend on Kirigami2, same as KCApp
Should also depend on KPeople's QML component

SOFTWARE/OS VERSIONS
Linux/KDE Plasma: N/A
KDE Plasma Version: N/A
KDE Frameworks Version: 5.44.0
Qt Version: 

ADDITIONAL INFORMATION
Ubuntu 18.04 does not appear to have a recent enough version of Kirigami (2.3 but we require 2.4)

Comment 1 Aleix Pol 2018-11-23 16:14:25 UTC

That's because it's actually not installed, no?

Comment 2 Simon Redman 2018-11-23 17:12:49 UTC

Sure, but we should list the runtime dependencies in the cmake file so that packagers know what they're getting in to

Comment 3 Aleix Pol 2018-11-23 17:18:41 UTC

Both of them are listed already, right?

Comment 4 Simon Redman 2018-11-23 17:22:04 UTC

(In reply to Aleix Pol from comment #3)
> Both of them are listed already, right?

Kirigami is listed under the app, so only required if the app is being built (It wasn't for me). The QML side of KPeople is not listed (but that might be a Ubuntu packaging weirdness)

Comment 5 Aleix Pol 2018-11-24 00:11:33 UTC

It is listed in cmake, distros may have not picked it up...

if(SMSAPP_ENABLED)
    find_package(KF5People REQUIRED)
    find_package(KF5PeopleVCard)
    set_package_properties(KF5PeopleVCard PROPERTIES
        PURPOSE "Read vcards from the file system"
        URL "https://phabricator.kde.org/source/kpeoplevcard/"
        TYPE RUNTIME
    )

    add_subdirectory(smsapp)
endif()

Comment 6 Simon Redman 2018-11-24 00:48:11 UTC

(In reply to Aleix Pol from comment #5)
> It is listed in cmake, distros may have not picked it up...
> 
> if(SMSAPP_ENABLED)
>     find_package(KF5People REQUIRED)
>     find_package(KF5PeopleVCard)
>     set_package_properties(KF5PeopleVCard PROPERTIES
>         PURPOSE "Read vcards from the file system"
>         URL "https://phabricator.kde.org/source/kpeoplevcard/"
>         TYPE RUNTIME
>     )
> 
>     add_subdirectory(smsapp)
> endif()

Oh. In this case, it's a Ubuntu packaging oddity:
They package the QML part separately from the rest of the library. So, in fact, there's no way we can handle this from cmake

Here is the list of files in the qml part of KPeople in Ubuntu. Notice that there is no cmake .config file, so we're hosed. Just hope the packager picks it up
https://packages.ubuntu.com/xenial/amd64/qml-module-org-kde-people/filelist

We still need the sms app to independently depend on Kirigami, I guess

Comment 7 Aleix Pol 2018-11-24 01:25:23 UTC

Again, we can't fix ubuntu.

Comment 8 Nate Graham 2019-01-24 17:41:33 UTC

Fixed by https://invent.kde.org/kde/kdeconnect-kde/commit/3b92b0de1d57f409c14132960d06c9abecf13eda

The correct syntax to automatically close the bug "BUG: 401255"

Comment 9 Simon Redman 2019-01-24 17:45:54 UTC

(In reply to Nate Graham from comment #8)
> Fixed by
> https://invent.kde.org/kde/kdeconnect-kde/commit/
> 3b92b0de1d57f409c14132960d06c9abecf13eda
> 
> The correct syntax to automatically close the bug "BUG: 401255"

Your reaction time is impressive! Thanks for the catch. The related patch is one which has been floating for a long time. Our new merge request template has the keyword baked in, so hopefully I will get it correct from now on!