Make them optional dependencies.

What is KF5::Service used for? You don't seem to be #ifdef it. Maybe it's not needed at all?

Make them optional dependencies.

What do you mean by this?

What is KF5::Service used for? You don't seem to be #ifdef it. Maybe it's not needed at all?

I don't know. I didn't investigate that.....I thought, it's some kind of "self-initializing" component and it is enough to link the library.

In D5521#103535, @palokisa wrote:

Make them optional dependencies.

What do you mean by this?

find_package(KF5Crash) #note there's no REQUIRED
if (KF5Crash_FOUND)
    somethingsomething()
endif()

What is KF5::Service used for? You don't seem to be #ifdef it. Maybe it's not needed at all?

I don't know. I didn't investigate that.....I thought, it's some kind of "self-initializing" component and it is enough to link the library.

I don't think it's the case for KService (it is for KCrash though). Please investigate (or wait for feedback).

In D5521#103536, @apol wrote:

In D5521#103535, @palokisa wrote:

Make them optional dependencies.

What do you mean by this?

find_package(KF5Crash) #note there's no REQUIRED
if (KF5Crash_FOUND)
    somethingsomething()
endif()

Why not give the packager/builder full control of what she/he wants to do and leave the decision to presence of dev package during the build process? IMO this is error prone.

What is KF5::Service used for? You don't seem to be #ifdef it. Maybe it's not needed at all?

I don't know. I didn't investigate that.....I thought, it's some kind of "self-initializing" component and it is enough to link the library.

I don't think it's the case for KService (it is for KCrash though). Please investigate (or wait for feedback).

Then I believe, it's only some remnant, because it's not used anywhere in the code.

I don't really understand what this change is supposed to fix. Distributions will ship only one variant of kglobalaccel and that will most likely be the one which is wanted by KDE. If kglobalaccel is shipped without KCrash support I would consider this as a serious problem and report that to the distributions. Also given my experience about breakage in weird situations I'm against such build flexibility. KCrash is an important component for kglobalaccel and I'm not interested in having to spend time on bug reports because a distro mis-configured kglobalaccel.

Distributions will ship only one variant of kglobalaccel and that will most likely be the one which is wanted by KDE

Why is that? Why can't we have e.g. one foo-kde and other foo-lxqt package, both providing virtual foo (these will be in conflict as they will provide the same file(s)). Then KDE can depend strictly on foo-kde.

In D5521#103661, @palokisa wrote:

Distributions will ship only one variant of kglobalaccel and that will most likely be the one which is wanted by KDE

Why is that? Why can't we have e.g. one foo-kde and other foo-lxqt package, both providing virtual foo (these will be in conflict as they will provide the same file(s)). Then KDE can depend strictly on foo-kde.

Talk to distros. My experience is that this will be an absolute no-go for most distros. You might find one or two, but I doubt any of the major distributions will do that. The obvious reason is that it would be impossible to install frameworks and lxqt at the same time. Note that this is about frameworks and not even Plasma. Plasma has no play in kglobalaccel.

The obvious reason is that it would be impossible to install frameworks and lxqt at the same time.

Why? With the aforementioned package names example: frameworks will provide foo by foo-kde and foo-lxqt. LXQt will require foo and KDE will (strictly) require foo-kde. In this way the user can install both KDE and LXQt at the same time (only the foo-kde will be installed and it also satisfies LXQt).

... but looking at the fingerprints of packages (in debian), the libkf5crash5 is very light. So not real disadvantage to pull this dependency. So the KCoreAddons can be seen as more problematic dependency (from size point of view).

What is KF5::Service used for? You don't seem to be #ifdef it. Maybe it's not needed at all?

I don't know. I didn't investigate that.....I thought, it's some kind of "self-initializing" component and it is enough to link the library.

I don't think it's the case for KService (it is for KCrash though). Please investigate (or wait for feedback).

Then I believe, it's only some remnant, because it's not used anywhere in the code.

... but looking at the fingerprints of packages (in debian), the libkf5crash5 is very light. So not real disadvantage to pull this dependency. So the KCoreAddons can be seen as more problematic dependency (from size point of view).

Should I close this request and create a new one for the KF5::Service removal?

I can confirm that kglobalaccel does not need the KF5Service dependency (neither the library does, nor the runtime). While it is listed in the link dependencies, no symbol is pulled from the library, so dependency can just be removed.

Btw, libKF5GlobalAccel is actually Tier1, so applications needing global shortcuts will not have any (additional) KF5 dependencies, only the runtime has.

In D5521#103775, @cfeck wrote:

Btw, libKF5GlobalAccel is actually Tier1, so applications needing global shortcuts will not have any (additional) KF5 dependencies, only the runtime has.

But without the runtime the application using libKF5GlobalAccel will not get any shortcut signals delivered, not?

In D5521#103778, @palokisa wrote:

In D5521#103775, @cfeck wrote:

Btw, libKF5GlobalAccel is actually Tier1, so applications needing global shortcuts will not have any (additional) KF5 dependencies, only the runtime has.

But without the runtime the application using libKF5GlobalAccel will not get any shortcut signals delivered, not?

yes and no. Someone needs to provide the dbus interface. Currently we have two ways to provide the DBus interface: kglobalacceld5 and kwin_wayland with the latter reusing almost everything of the former. So from a technical point of view the runtime is not needed, from a practical point of view it is needed.

That's also a reason why we moved the runtime into kglobalaccel as it's kind of pointless to have the framework without the runtime.

oh and yes for removing service dependency please open a separate review. The other dependencies: well KCoreAddons is needed for KCrash only IIRC. That is setting the KAboutData. So a possibility could be to move the binary out and replace it by a binary which does not use KCrash. And we could add the good old kglobalacceld5 with KCrash into Plasma. That could solve all the problems we have.

In D5521#103810, @graesslin wrote:

The other dependencies: well KCoreAddons is needed for KCrash only IIRC. That is setting the KAboutData.

I've had a look... and the plugins are also handled by KPluginLoader, KPluginMetaData & co.

In D5521#103818, @palokisa wrote:

In D5521#103810, @graesslin wrote:

The other dependencies: well KCoreAddons is needed for KCrash only IIRC. That is setting the KAboutData.

I've had a look... and the plugins are also handled by KPluginLoader, KPluginMetaData & co.

ah right, so it probably needs to have KCoreAddons.