search

koenkooi / meta-kodi

The official OpenEmbedded/Yocto Project layer for Kodi
11 stars 27 forks source link

layer: make systemd more optional, additional variable to turn off kodi autostart. #15

Closed dev-0x7C6 closed 4 years ago

dev-0x7C6 commented 4 years ago

With this PR:

Updated documentation can be quickly viewed here: https://github.com/dev-0x7C6/meta-kodi/blob/zeus/README.md

koenkooi commented 4 years ago

I'm highly sceptical of making systemd optional, what's the exact use case for that?

dev-0x7C6 commented 4 years ago

I'm highly sceptical of making systemd optional, what's the exact use case for that?

Well maybe I spend to much time in using Gentoo. I like idea that make components more optional. I really dislike if I have minimal configuration an something pulling x11, gtk and other stuff. I was wondering if someone just pulling busybox and kodi and some simple bash script as init. Not sure if every distro need to end-up with systemd as init.

The systemd optional part is this line: ${@bb.utils.contains('DISTRO_FEATURES', 'systemd', 'kodi-systemd-service', '', d)} \

I also splited service installation to separate recipe, is this ok?

My intent is not make everything optional, just bigger parts.

koenkooi commented 4 years ago

I've personally given up trying to support non-systemd configurations, since it's more of a complete OS than an init system :) Especially for kodi, there was a memory leak in the python subsystem, it was really easy to add a memory cap in system and make it restart on OOM.

I don't want to set the wrong expectation when someone files a bug with "kodi won't work when using sysvinit" and expecting us to make it work. The change you made is fine and can go in, just be aware of the possible maintenance burden such changes can cause.

dev-0x7C6 commented 4 years ago

Systemd is pretty convenient that for sure. Upcoming version will be released with systemd-reparted which can expand rootfs partition after first boot.

So if we only want to support systemd configuration, maybe we should add systemd to REQUIRED_DISTRO_FEATURES="systemd" in kodi-systemd-service recipe and replace ${@bb.utils.contains('DISTRO_FEATURES', 'systemd', 'kodi-systemd-service', '', d)} with simply kodi-systemd-service, and that will results with mandatory systemd requirement for kodi recipe.

By keeping kodi-systemd-service as standalone recipe there will be possible to do kodi_%.bbappend with DEPENDS_remove = "kodi-systemd-service" if someone really want get rid of systemd requirement.

What do you think?

g-scott-murray commented 4 years ago

I'd prefer to just leave it as it is, with the DISTRO_FEATURES check for systemd in PACKAGECONFIG. That's what most other recipes do, and it still allows people to futz with sysvinit if they want to. I consider REQUIRED_DISTRO_FEATURES something of a last resort. Just make a note in README that sysvinit support is untested, done.

Scott

On Fri, Feb 28, 2020 at 12:45 PM Bartłomiej Burdukiewicz < notifications@github.com> wrote:

Systemd is pretty convenient that for sure. Upcoming version will be released with systemd-reparted which can expand rootfs partition after first boot.

So if we only want to support systemd configuration, maybe we should add systemd to REQUIRED_DISTRO_FEATURES in kodi-systemd-service recipe and replace ${@bb.utils.contains('DISTRO_FEATURES', 'systemd', 'kodi-systemd-service', '', d)} with simply kodi-systemd-service, and that will results with mandatory systemd requirement for kodi recipe.

By keeping kodi-systemd-service as standalone recipe there will be possible to do kodi_%.bbappend with DEPENDS_remove = "kodi-systemd-service" if someone really want get rid of systemd requirement.

What do you think?

— You are receiving this because your review was requested. Reply to this email directly, view it on GitHub https://github.com/koenkooi/meta-kodi/pull/15?email_source=notifications&email_token=ABZZFQJF4XUT346RK4H7F33RFFETXA5CNFSM4K5QM7WKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOENJRBOY#issuecomment-592646331, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABZZFQNH64F4RL6E3DYT7CTRFFETXANCNFSM4K5QM7WA .

dev-0x7C6 commented 4 years ago

I updated current approach to use PACKAGECONFIG as @g-scott-murray mentioned. Now we have it line by line with pulseaudio. This make more sense as PACKAGECONFIG is often customized by users.

dev-0x7C6 commented 4 years ago

Any objections to merge this to development branch? We always can correct things later on or change concepts. I don't want to end-up with to much clever recipes and too much parameterized variables. I'll iterate over changes that I made to check how this play out in real world.

I updated readme.md with disclaimer that sysvinit is untested and systemd should be used. So lets progress with switchable systemd services at least for now ;-)

Probably tomorrow I'll start to work on broken hardware acceleration. I also need to test some raspberry build as I mention before. Good-night :-)

koenkooi commented 4 years ago

Can you squash the commits onto 2 commits ? one for the readme and one for the recipe?