D5521: Allow the KGlobalAccel be a "Tier 2" framework, if needed
graesslin added a comment. In https://phabricator.kde.org/D5521#103818, @palokisa wrote: > In https://phabricator.kde.org/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. REPOSITORY R268 KGlobalAccel REVISION DETAIL https://phabricator.kde.org/D5521 To: palokisa, graesslin, mck182 Cc: cfeck, apol, #frameworks
D5521: Allow the KGlobalAccel be a "Tier 2" framework, if needed
palokisa added a comment. In https://phabricator.kde.org/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 there are more of them: `KPluginLoader`, `KPluginMetaData` & co. REPOSITORY R268 KGlobalAccel REVISION DETAIL https://phabricator.kde.org/D5521 To: palokisa, graesslin, mck182 Cc: cfeck, apol, #frameworks
D5521: Allow the KGlobalAccel be a "Tier 2" framework, if needed
graesslin added a comment. 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. REPOSITORY R268 KGlobalAccel REVISION DETAIL https://phabricator.kde.org/D5521 To: palokisa, graesslin, mck182 Cc: cfeck, apol, #frameworks
D5521: Allow the KGlobalAccel be a "Tier 2" framework, if needed
graesslin added a comment. In https://phabricator.kde.org/D5521#103778, @palokisa wrote: > In https://phabricator.kde.org/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. REPOSITORY R268 KGlobalAccel REVISION DETAIL https://phabricator.kde.org/D5521 To: palokisa, graesslin, mck182 Cc: cfeck, apol, #frameworks
D5521: Allow the KGlobalAccel be a "Tier 2" framework, if needed
palokisa added a comment. In https://phabricator.kde.org/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? REPOSITORY R268 KGlobalAccel REVISION DETAIL https://phabricator.kde.org/D5521 To: palokisa, graesslin, mck182 Cc: cfeck, apol, #frameworks
D5521: Allow the KGlobalAccel be a "Tier 2" framework, if needed
cfeck added a comment. 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. REPOSITORY R268 KGlobalAccel REVISION DETAIL https://phabricator.kde.org/D5521 To: palokisa, graesslin, mck182 Cc: cfeck, apol, #frameworks
D5521: Allow the KGlobalAccel be a "Tier 2" framework, if needed
palokisa added a comment. 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? REPOSITORY R268 KGlobalAccel REVISION DETAIL https://phabricator.kde.org/D5521 To: palokisa, graesslin, mck182 Cc: apol, #frameworks
D5521: Allow the KGlobalAccel be a "Tier 2" framework, if needed
palokisa added a comment. > 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). REPOSITORY R268 KGlobalAccel REVISION DETAIL https://phabricator.kde.org/D5521 To: palokisa, graesslin, mck182 Cc: apol, #frameworks
D5521: Allow the KGlobalAccel be a "Tier 2" framework, if needed
graesslin added a comment. In https://phabricator.kde.org/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. REPOSITORY R268 KGlobalAccel REVISION DETAIL https://phabricator.kde.org/D5521 To: palokisa, graesslin, mck182 Cc: apol, #frameworks
D5521: Allow the KGlobalAccel be a "Tier 2" framework, if needed
palokisa added a comment. > 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`. REPOSITORY R268 KGlobalAccel REVISION DETAIL https://phabricator.kde.org/D5521 To: palokisa, graesslin, mck182 Cc: apol, #frameworks
D5521: Allow the KGlobalAccel be a "Tier 2" framework, if needed
graesslin added a comment. 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. REPOSITORY R268 KGlobalAccel REVISION DETAIL https://phabricator.kde.org/D5521 To: palokisa, graesslin, mck182 Cc: apol, #frameworks
D5521: Allow the KGlobalAccel be a "Tier 2" framework, if needed
palokisa added a comment. In https://phabricator.kde.org/D5521#103536, @apol wrote: > In https://phabricator.kde.org/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. REPOSITORY R268 KGlobalAccel REVISION DETAIL https://phabricator.kde.org/D5521 To: palokisa, graesslin, mck182 Cc: apol, #frameworks
D5521: Allow the KGlobalAccel be a "Tier 2" framework, if needed
apol added a comment. In https://phabricator.kde.org/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). REPOSITORY R268 KGlobalAccel REVISION DETAIL https://phabricator.kde.org/D5521 To: palokisa, graesslin, mck182 Cc: apol, #frameworks
D5521: Allow the KGlobalAccel be a "Tier 2" framework, if needed
palokisa added a comment. > 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. REPOSITORY R268 KGlobalAccel REVISION DETAIL https://phabricator.kde.org/D5521 To: palokisa, graesslin, mck182 Cc: apol, #frameworks
D5521: Allow the KGlobalAccel be a "Tier 2" framework, if needed
apol added a comment. 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? REPOSITORY R268 KGlobalAccel REVISION DETAIL https://phabricator.kde.org/D5521 To: palokisa, graesslin, mck182 Cc: apol, #frameworks
D5521: Allow the KGlobalAccel be a "Tier 2" framework, if needed
palokisa created this revision. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: Frameworks. REVISION SUMMARY By using the MINIMAL_RUNTIME_DEPS we allow minimizing the run-time dependencies -> remove the KCrash & KService dependency/usage. With this we can make KGlobalAccel a "Tier 2" framework and allow downstream packagers make a lighter versions of packages (these of course must state conflicts with the "normal" packages). It is usefull for projects outside KDE, that are trying to reuse existing work with minimal dependencies. Note: making this proposal for LXQt, where we would like to drop our own shortcut daemon and (re)use your work. TEST PLAN Compiled, run, works... REPOSITORY R268 KGlobalAccel REVISION DETAIL https://phabricator.kde.org/D5521 AFFECTED FILES CMakeLists.txt src/runtime/CMakeLists.txt src/runtime/main.cpp To: palokisa, graesslin, mck182 Cc: #frameworks
