Fix some complaints from rpmlint

[nephros]

Aims to remove some common complaints from rpmlint.

NOT sure if compiling PIC/PIE is smart for the daemon, but I have tested running it that way, seems to work.

[Olf0]

While full-heartedly agree to "don't own the whole /etc/dbus-1/system.d/ directory" in commit 5860111, I wonder if the other changes do make sense:

1. Just for reference / if I understood correctly:
   %{_sysconfdir}/dbus-1/system.d/org.SfietKonstantin.patchmanager.conf is the default configuration file deployed for Patchmanager, hence as soon a user alters a setting, the altered file is worth being backed up instead of being overwritten by updates.
   Thus you chose to add the %config prefix, which saves the version altered by the user as <filename>.rpmsave when this file is being updated, while deploying the updated version with its original file name.
   I wonder why this has not been like that in the first place, but also wonder why I do not remember Patchmanager's setting being reset to their default values after every update. 🤨

2. But the change %{_sysconfdir}/firejail/whitelist-common-%{name}.local → %config(noreplace) %{_sysconfdir}/firejail/whitelist-common-%{name}.local and the extant %config(noreplace) %{_sysconfdir}/%{name}/manglelist.conf made me wonder, if …

   • we really expect these two files to be altered by users in a regular usage scenario.
   • changes made by users really must not be overwritten by updates.
     Especially this point is dangerous, because we are not able to update these files by Patchmanager updates (without changing the spec file again), any more.

   => IMO %config(noreplace) shall only be used with thorough consideration and great care.
   Hence I suggest (on first sight, needs more pondering on my side: 🤔) to downgrade these two from %config(noreplace) to %config.

[Olf0]

I also wondered (nitpicking, unrelated to aforementioned commit, but triggered by reviewing the spec file for it), why …

3. … I do not see the /usr/share/patchmanager/dbus-1 directory on-device (quick-checked with PM3.0.65 on an XperiaX@SFOS3.2.1), which is there from PM's inception.
   And why does this directory carry an appended slash ("/") in the spec file, but all other directories do not (at least after your %{_sysconfdir}/dbus-1/system.d/→%config %{_sysconfdir}/dbus-1/system.d/org.SfietKonstantin.patchmanager.conf? See e.g., %{_datadir}/%{name}/data, %dir %{_datadir}/%{name}/patches, %{_datadir}/%{name}/tools.
   I am asking, because sometimes I miss something crucial. Maybe D-BUS' dynamic nature has to do with it?!?

4. … you introduced lines 146 to 150 three months ago, although Jolla advises against deploying documentation (READMEs, man-pages etc.) on SailfishOS. While I regard Jolla's guidance far from being normative (for me / us), most SailfishOS users are used to look up all documentation in the Web (due to a lack of choice), so I do not expect this to be useful; specifically not the Patch examples.
   Hence I suggest to delete lines 146 to 150 of the spec file and to generally cease deploying PM-related documentation files which are not displayed at the GUI on SailfishOS devices.

[Olf0]

I also learnt something from commit 569bfd4 (although I am not sure, if this is really relevant, but that is on behalf of rpmlint). Thanks!

And commit aa74c57 is interesting to read for me and sure is "modern"; again I cannot judge if this is really relevant, but it is looking cool. 😉

I could acknowledge these two commits, but would prefer @b100dian to do that, because I lack a thorough understanding of the effects and relevance of these things in practice.

[nephros]

1. Just for reference / if I understood correctly:
   %{_sysconfdir}/dbus-1/system.d/org.SfietKonstantin.patchmanager.conf is the default configuration file deployed for Patchmanager, hence as soon a user alters a setting, the altered file is worth being backed up instead of being overwritten by updates.
   [...]
   I wonder why this has not been like that in the first place, but also wonder why I do not remember Patchmanager's setting being reset to their default values after every update. 🤨

If you look at the file, it's a configuration/policy for the dbus service only. No user-changeable settings are stored there. Hence your modifications to PM settings were never written to that file, so no surprise they persited over updates.

It is now marked %config because rpmlint wants that for everything under /etc.

It's very unlikely a user would change anything there, but in case they do, we override with backup.

[nephros]

2. But the change %{_sysconfdir}/firejail/whitelist-common-%{name}.local → %config(noreplace) %{_sysconfdir}/firejail/whitelist-common-%{name}.local and the extant %config(noreplace) %{_sysconfdir}/%{name}/manglelist.conf made me wonder, if …

   • we really expect these two files to be altered by users in a regular usage scenario.
   • changes made by users really must not be overwritten by updates.
     Especially this point is dangerous, because we are not able to update these files by Patchmanager updates (without changing the spec file again), any more.

   => IMO %config(noreplace) shall only be used with thorough consideration and great care.
   Hence I suggest (on first sight, needs more pondering on my side: 🤔) to downgrade these two from %config(noreplace) to %config.

I absolutely expect users to change manglelist.conf if they discover a path needs mangling we haven't included yet. Yes, they should make a bug report about it and wait for an update, but if they don't want to for some reason editing that file is a quick fix.

Less so with the sailjail whitelist, but still it's imaginable that (e.g. due to security considerations), a user may want to disable patches for sailjailed applications, or make the whitellist more fine-grained. If they do, IMO those changes should survive an update.

IMO if you opt to touch those files as a user, you are then responsible for maintaining them, and get to keep the pieces when an update breaks because of them.

[nephros]

3. … I do not see the /usr/share/patchmanager/dbus-1 directory on-device (quick-checked with PM3.0.65 on an XperiaX@SFOS3.2.1), which is there from PM's inception.
And why does this directory carry an appended slash ("/") in the spec file, but all other directories do not (at least after your %{_sysconfdir}/dbus-1/system.d/→%config %{_sysconfdir}/dbus-1/system.d/org.SfietKonstantin.patchmanager.conf? See e.g., %{_datadir}/%{name}/data, %dir %{_datadir}/%{name}/patches, %{_datadir}/%{name}/tools.

It's /usr/share/dbus-1 but you're right, we shouldn't own that either. Nice catch! Fixed in 3bf8e2b

[nephros]

4. … you introduced lines 146 to 150 three months ago, although Jolla advises against deploying documentation (READMEs, man-pages etc.)

Agreed, removed in 21c16bb

[Olf0]

IMO if you opt to touch those files as a user, you are then responsible for maintaining them, and get to keep the pieces when an update breaks because of them.

I do agree, but from experience I can tell that many users will not (… take responsibility).
But as long as we do not advertise this "feature" IMO nobody will use it.
While I prefer a fool proof approach, I can live with this.

[Olf0]

It's /usr/share/dbus-1

Oops, thanks: Now I see what is there.
Well, I am glad that it still was a correct hint.

[Olf0]

Looking good.

[b100dian]

👍