Why just not make xsettingsd required?

In D28360#636403, @gikari wrote:

Why just not make xsettingsd required?

It is set to REQUIRED (and ON by default), but can be disabled, mainly because I would like to have this in 5.18, and it seems safer that way.

In D28360#636404, @eszlari wrote:

I would like to have this in 5.18

We cannot add mandatory dependencies to the stable branches/releases. Currently it's an optional dependency. If you want to make it mandatory, only 5.19 and later are possible options.

In D28360#636406, @gikari wrote:

In D28360#636404, @eszlari wrote:

I would like to have this in 5.18

We cannot add mandatory dependencies to the stable branches/releases. Currently it's an optional dependency. If you want to make it mandatory, only 5.19 and later are possible options.

That's why I made it an option, so that it fails explicitly but can still be disabled.

But the option is enabled by default and needs to be explicitly disabled. This seems to be an overengineered solution. It basically says, that XSettingsd is an optional dependency, which already is.

In D28360#636418, @gikari wrote:

But the option is enabled by default and needs to be explicitly disabled. This seems to be an overengineered solution. It basically says, that XSettingsd is an optional dependency, which already is.

But it make the packager aware, that this dependency exists, instead of at runtime, the end user wondering, why his/her Plasma desktop behaves differently from others.

Anecdote: obs-studio checks for pulseaudio at compile time, but fails silently(!) when it can't find it, leading users to wonder, why they can't record audio.

In D28360#636435, @eszlari wrote:

In D28360#636418, @gikari wrote:

But the option is enabled by default and needs to be explicitly disabled. This seems to be an overengineered solution. It basically says, that XSettingsd is an optional dependency, which already is.

But it make the packager aware, that this dependency exists, instead of at runtime, the end user wondering, why his/her Plasma desktop behaves differently from others.

Anecdote: obs-studio checks for pulseaudio at compile time, but fails silently(!) when it can't find it, leading users to wonder, why they can't record audio.

Again, why not just make XSettingsd required? It's simpler.

+1 for just making it a required dependency

Can you tell which packagers missed that this is a required runtime dependency?

Noting runtime dependencies as done so far here, by not having it required and tagging it as "RUNTIME" is a pattern everywhere else in KDE code, so the proposed patch or just setting it to required would be not following existing practices, thus better avoided.

Edit: The proposed solution to make it simply required only forces packagers to add that and all what it pulls in as build-time dependency to their package creation, even if not needed for the package creation itself at all, which is not something packagers like (wastes resources on package build servers).

In D28360#636448, @gikari wrote:

Again, why not just make XSettingsd required? It's simpler.

To quote yourself: "We cannot add mandatory dependencies to the stable branches/releases."

I think it's important to get it in 5.18 LTS.

In D28360#636610, @kossebau wrote:

Can you tell which packagers missed that this is a required runtime dependency?

https://packages.ubuntu.com/focal/kde-config-gtk-style
https://src.fedoraproject.org/rpms/kde-gtk-config/blob/master/f/kde-gtk-config.spec
https://gitweb.gentoo.org/repo/gentoo.git/tree/kde-plasma/kde-gtk-config/kde-gtk-config-5.18.3.ebuild

Edit: The proposed solution to make it simply required only forces packagers to add that and all what it pulls in as build-time dependency to their package creation, even if not needed for the package creation itself at all, which is not something packagers like (wastes resources on package build servers).

But they don't have to! They can just compile with

cmake -DENABLE_XSETTINGSD=OFF ...

In D28360#636615, @eszlari wrote:

In D28360#636610, @kossebau wrote:

Can you tell which packagers missed that this is a required runtime dependency?

https://packages.ubuntu.com/focal/kde-config-gtk-style
https://src.fedoraproject.org/rpms/kde-gtk-config/blob/master/f/kde-gtk-config.spec
https://gitweb.gentoo.org/repo/gentoo.git/tree/kde-plasma/kde-gtk-config/kde-gtk-config-5.18.3.ebuild

Okay, that is quite a few. Though, I wonder why they missed it. The current addition to RUNTIME dependency was only added post 5.18, right? And no-one told the packagers explicitly otherwise by the usual ways (like in release announcement). They do read the cmake log though, and usually ask for that listing as RUNTIME dependency.

So the current state with those packages might not be a reason for the proposed noisy approach.

Edit: The proposed solution to make it simply required only forces packagers to add that and all what it pulls in as build-time dependency to their package creation, even if not needed for the package creation itself at all, which is not something packagers like (wastes resources on package build servers).

But they don't have to! They can just compile with

cmake -DENABLE_XSETTINGSD=OFF ...

They can, but ENABLE_XSETTINGSD needs extra work, makes things more complex, and the term is also misleading (does it enable support for the demon?). One has to know detais.

Edit: Besides, that part was a reply to the proposal to simply make xsettingsd a required option in all cases, without the option.

All in all, this is an unusual approach. Best talk to packagers what they prefer and expect, instead of more complex code based on assumption of how packagers operate. Done that before myself, and pure listing as RUNTIME dep always worked out :)

See how it is done across KDE repos here:
https://lxr.kde.org/search?_filestring=CMakeLists.txt&_string=TYPE+RUNTIME&_casesensitive=1

There should be no reason the same works here not as well.

In D28360#636615, @eszlari wrote:

https://packages.ubuntu.com/focal/kde-config-gtk-style
https://src.fedoraproject.org/rpms/kde-gtk-config/blob/master/f/kde-gtk-config.spec
https://gitweb.gentoo.org/repo/gentoo.git/tree/kde-plasma/kde-gtk-config/kde-gtk-config-5.18.3.ebuild

In D28360#636617, @kossebau wrote:

Okay, that is quite a few. Though, I wonder why they missed it. The current addition to RUNTIME dependency was only added post 5.18, right? And no-one told the packagers explicitly otherwise by the usual ways (like in release announcement). They do read the cmake log though, and usually ask for that listing as RUNTIME dependency.

To be specific, this has been added as a runtime check in the 5.18 branch AFTER release of the 5.18.3 tars, so it would only be someone who is doing stable git builds (CI or otherwise) who would have had build logs flag this up.

As far as K/Ubuntu is concerned, this has also been added at a point what we are well past feature freeze for 20.04. Adding a runtime depends/recommends that draws a new package into the default install AND on the ISO needs a feature freeze exception application to be made, and since this is not just that but a persistent runtime daemon, that is doubly so.

Given that this is ENTIRELY optional, and designed to fix a 'problem' that I honestly cannot recall any users raising (at all, or) as a significant problem, then I certainly won't be looking at it until after the 20.04 beta next week, as there are many more important things to deal with for now.

In D28360#636610, @kossebau wrote:

Edit: The proposed solution to make it simply required only forces packagers to add that and all what it pulls in as build-time dependency to their package creation, even if not needed for the package creation itself at all, which is not something packagers like (wastes resources on package build servers).

The thing is that xsettigsd is a daemon, which allows some settings, which were set to GTK applications, to be applied to the current running GTK applications without restarting them. Without it settings are still applied, but GTK applications must be restarted in order to use new settings. Daemon absence is the user experience sacrifice and IMO we should use this daemon by default. So, that's why I proposed making the daemon dependency mandatory.

Sadly Cmake does not have separate "RUNTIME-OPTIONAL" & "RUNTIME-REQUIRED" so far. Previously that was solved by noting the required vs. optional in the purpose text, so the info still arrived with packagers.
Just "REQUIRED" means, it is required for the build. Could be even a build-only requirement.

So "RUNTIME" is the right type here, with the hint to be a required dependency done in the "PURPOSE" text:

set_package_properties(XSettingsd PROPERTIES
    DESCRIPTION "XSettingsd daemon"
    TYPE RUNTIME
    PURPOSE "Required to have GTK Config kded module to apply settings to GTK applications on the fly"
)

Edit: and yes, I agree CMake has awful flaws here when it comes to our needs to properly communicate dependencies to fellow developers & packagers. I am trying to give you best practices done so far elsewhere in KDE software to deal with this, to ensure consistent patterns in the approach to the same problem :)

In D28360#636687, @kossebau wrote:

Edit: and yes, I agree CMake has awful flaws here when it comes to our needs to properly communicate dependencies to fellow developers & packagers. I am trying to give you best practices done so far elsewhere in KDE software to deal with this, to ensure consistent patterns in the approach to the same problem :)

Did I understand correctly, that to make the dependency required for distribution packagers you need to substitute existing code with the following (i.e. only change the PURPOSE text, but keep find_package as it was):

find_package(XSettingsd)
set_package_properties(XSettingsd PROPERTIES
    DESCRIPTION "XSettingsd daemon"
    TYPE RUNTIME
    PURPOSE "Required to have GTK Config kded module to apply settings to GTK applications on the fly"
)

Yes, I would just do a patch for the 'PURPOSE' text, mentioning "Required" or "Optional" (whatever you decide on), based on previously discussions with packagers on similar cases.

How can I close this? (phabricator UI really sucks, I hope the move to Gitlab is completed soon)