search

RPi-Distro / repo

Issue tracking for the archive.raspberrypi.com repo
37 stars 1 forks source link

Kodi postinst script fails if polkit-1 dir does not exist #153

Open MichaIng opened 5 years ago

MichaIng commented 5 years ago

Sorry if this is to wrong place to report this, but I got confused and couldn't find out which "xmbc" fork finally serves as source the the RPi repo package.

However, the package postinst scripts contains this:

if [ ! -f "/etc/polkit-1/localauthority/50-local.d/kodi.pkla" ]; then
tee -a /etc/polkit-1/localauthority/50-local.d/kodi.pkla > /dev/null 2>&1 <<_EOF_
...

Since policykit1 is not a dependency of the kodi package, the required /etc/polkit-1/localauthority/50-local.d dir does not always exist, hence the install fails.

Three possible solutions:

Reference: https://github.com/MichaIng/DietPi/issues/3031#issuecomment-540477241

derdershat commented 5 years ago

I think the deepest source is https://github.com/popcornmix/xbmc so an @popcornmix could help

XECDesign commented 5 years ago

The package is kindly provided by Rascas of pipplware - I've already left a message here https://www.raspberrypi.org/forums/viewtopic.php?f=66&t=251645&start=125#p1552570

diogomsantos commented 5 years ago

Hi, I am rascas, the Kodi packages maintainer.

The Kodi 18 Leia source code for the RPi 0/1/2/3 is available here: https://github.com/PIPplware/xbmc/tree/leia_backports And Kodi 18 Leia source code for the RPi 4 is available here: https://github.com/PIPplware/xbmc/tree/leia_pi4

More info here: https://www.raspberrypi.org/forums/viewtopic.php?f=66&t=251645

So correct me if I am wrong: DietPi for the RPi is based on Raspbian but not on the pre made oficial images. I am asking this because policykit-1 is installed on the pre made images.

Anyway, what the polkit rule does is to allow the user who will run Kodi, to be able to reboot and shutdown the Pi and to mount/unmount devices inside Kodi. Maybe the best is to just add policykit-1 as a dependency, but I can do whatever you think is best,

MichaIng commented 5 years ago

@diogomsantos Jep DietPi RPi images are based on Raspbian, but come with very few pre-installed packages, instead installs them ondemand via software install or config scripts.

We solve Kodi permissions via udev rules (generally for all devices, not just RPi), however policykit-1 of course is another nice solution, if installed. But I would keep dependencies as low as possible.

Wouldn't it be easiest to make /etc/polkit-1/localauthority/50-local.d/kodi.pkla a regular deb package config file, so the dir and file are created automatically by dpkg, if not yet existent, else user is asked by dpkg to keep old file, if it has changed, or use maintainer updated one? Otherwise I personally would prefer simply mkdir -p /etc/polkit-1/localauthority/50-local.d before the cat. In both ways the policy becomes effective as fast as policykit-1 is installed, but the choice is left to user.

XECDesign commented 5 years ago

If you go with the mkdir route, the nice thing to do in postrm purge would be to remove the pkla file and all created directories if they're empty.

MichaIng commented 5 years ago

E.g.

[[ ! -f '/etc/polkit-1/localauthority/50-local.d/kodi.pkla' ]] || rm /etc/polkit-1/localauthority/50-local.d/kodi.pkla
rmdir -p --ignore-fail-on-non-empty /etc/polkit-1/localauthority/50-local.d
XECDesign commented 5 years ago

If I was doing this I'd probably ship it as a normal file in /var/lib/polkit-1/localauthority/50-local.d

diogomsantos commented 5 years ago

Putting the rule on a conffile was my first idea, but I use cmake cpack deb for a while now, as it is easier and supported in upstream Kodi, and adding a conffile there didn't look easy, so I ended up using postinst/postrm.

PanderMusubi commented 4 years ago

Are there any updates for this issue?

MichaIng commented 3 years ago

To re-activate this. Isn't it possible to simply add a conffiles file here: https://github.com/PIPplware/xbmc/tree/leia_pi4/cmake/cpack/deb With content:

/etc/polkit-1/localauthority/50-local.d/kodi.pkla

Now I don't see where the file itself would need to be added, but since postinst and postrm seem to be used by cmake cpack like that, conffiles should be used the same way.

As alternative I'd actually second @XECDesign's idea to make it a regular file. I lost track what the reason was to not hardcode it into the package, if there was any? Shipping it statically as /var/lib/polkit-1/localauthority/50-local.d/kodi.pkla still allows admins to override it, when wanted, by creating an own /etc/polkit-1/localauthority/50-local.d/kodi.pkla. It has the benefit that the original file stays intact, custom changes can be easily reverted (by removing the custom file from /etc) and also maintainer-wise new rules/defaults can be shipped without requiring the admin to confirm overwriting an old config file on upgrade (when using the above conffiles method).

diogomsantos commented 3 years ago

If you tell me how to do that with cmake cpak deb, I will gladly do it.

MichaIng commented 3 years ago

I see you added policykit-1 as dependency already: https://github.com/PIPplware/xbmc/commit/1320da287853b4dc63d6b99c98f4e41786a1b1e2 So the issue should be fixed by this (although this has not yet been uploaded into the repo). Changing the way how these files are added would still have some benefits, e.g. also making the udev rules a static file /lib/udev/rules.d/99-kodi.rules or /usr/lib/udev/rules.d/99-kodi.rules to leave /etc/udev/rules.d for local admin additions/overrides only, as intended.

Out of interest, I was digging a little further. Simply placing the conffiles file into https://github.com/PIPplware/xbmc/tree/leia_pi4/cmake/cpack/deb is not enough, but it needs to be added here as well: https://github.com/PIPplware/xbmc/blob/leia_pi4/cmake/cpack/CPackConfigDEB.cmake#L314-L326 This can be used to add further files to the package, but since DESTINATION share/ is used, it seems to place everything below /usr/ and hence I don't know how to add one below /lib/ or /var/ with this install function. Cleanest seems to be to extend the variable CPACK_INSTALLED_DIRECTORIES by a new directory structure to bundle additional files:

When I find some more spare time and joy to tinker with this, I'll open a new issue and PR at your repo as a testing and discussion basis. But lacking cpack experience, it would be trial and failure and currently IMO not worth the effort. It's a new issue anyway, not to fix the OP, so I mark this as closed 🙂.

MichaIng commented 1 year ago

The issue re-appeared with Bookworm, since the policykit-1 package there does not ship /etc/polkit-1/localauthority.conf.d anymore. Compare the file list of the package on Bullseye: https://packages.debian.org/bullseye/arm64/policykit-1/filelist With Bookworm:

Checking changelogs, the directory has been obsoleted: https://metadata.ftp-master.debian.org/changelogs//main/p/policykit-1/policykit-1_122-3_changelog

  • postinst: Only clean up config directories if not owned. If we only have polkitd installed, then we want to clean up the obsolete directory /etc/polkit-1/localauthority.conf.d on upgrade, but if we have polkitd-pkla installed, then it owns that directory and we should not remove it. (Closes: #1026425)

There is a legacy package which should pre-create it instead: https://packages.debian.org/bookworm/polkitd-pkla But I guess it makes sense to use the modern methods instead. I have no deep insights, but looks like /etc/polkit-1/rules.d would be used nowadays.

EDIT: Related notes from Debian: https://www.debian.org/releases/bookworm/arm64/release-notes/ch-information.en.html#changes-to-polkit-configuration

EDIT2: Now it is @popcornmix: https://github.com/popcornmix/xbmc/blob/gbm/cmake/cpack/deb/postinst#L22-L38 Shall I open a new issue over there? I can also open a PR when legacy polkitd-pkla shall be used, but I do not know (yet) how to translate it into modern /etc/polkit-1/rules.d JavaScript rules.