[PATCH] systemd: check 'efi' in MACHINE_FEATURES

[PATCH] systemd: check 'efi' in MACHINE_FEATURES

[Mikko Ylinen]

The recipe checks 'efi' in DISTRO_FEATURES but it's a machine feature.

Fix the check to use MACHINE_FEATURES to determine whether to
enable/disable 'efi' PACKAGECONFIG.

Signed-off-by: Mikko Ylinen <mikko.ylinen@linux.intel.com>
---
 meta/recipes-core/systemd/systemd_232.bb | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/meta/recipes-core/systemd/systemd_232.bb b/meta/recipes-core/systemd/systemd_232.bb
index fa6a6a817f..150bb9ea65 100644
--- a/meta/recipes-core/systemd/systemd_232.bb
+++ b/meta/recipes-core/systemd/systemd_232.bb
@@ -40,7 +40,8 @@ SRC_URI_append_libc-uclibc = "\
 SRC_URI_append_qemuall = " file://0001-core-device.c-Change-the-default-device-timeout-to-2.patch"
 
 PACKAGECONFIG ??= "xz \
-                   ${@bb.utils.filter('DISTRO_FEATURES', 'efi pam selinux ldconfig', d)} \
+                   ${@bb.utils.filter('DISTRO_FEATURES', 'pam selinux ldconfig', d)} \
+                   ${@bb.utils.filter('MACHINE_FEATURES', 'efi', d)} \
                    ${@bb.utils.contains('DISTRO_FEATURES', 'x11', 'xkbcommon', '', d)} \
                    ${@bb.utils.contains('DISTRO_FEATURES', 'wifi', 'rfkill', '', d)} \
                    binfmt \
-- 
2.11.0

Re: [PATCH] systemd: check 'efi' in MACHINE_FEATURES

[Burton, Ross]

[-- Attachment #1: Type: text/plain, Size: 593 bytes --]

On 29 March 2017 at 10:26, Mikko Ylinen <mikko.ylinen@linux.intel.com>
wrote:

> The recipe checks 'efi' in DISTRO_FEATURES but it's a machine feature.
>
> Fix the check to use MACHINE_FEATURES to determine whether to
> enable/disable 'efi' PACKAGECONFIG.
>

This makes systemd a MACHINE-specific recipe, and as it contains libudev
causes rebuilds for everything linking to that.  What's the actual impact
of enabling/disabling EFI?

There is a whitelist for recipes which are machine-specific but should be
treated as tune, but the barrier to get on that list is high.

Ross

[-- Attachment #2: Type: text/html, Size: 1098 bytes --]

Re: [PATCH] systemd: check 'efi' in MACHINE_FEATURES

[Peter Kjellerstedt]

[-- Attachment #1: Type: text/plain, Size: 1207 bytes --]

For the record, this is just correcting a regression that was introduced with my change to use bb.utils.filter() where I apparently missed that not all the features were DISTRO_FEATURES…

That said, I do agree that it would be preferable to get rid of the machine specific dependency if possible.

//Peter

From: openembedded-core-bounces@lists.openembedded.org [mailto:openembedded-core-bounces@lists.openembedded.org] On Behalf Of Burton, Ross
Sent: den 29 mars 2017 18:01
To: Mikko Ylinen
Cc: OE-core
Subject: Re: [OE-core] [PATCH] systemd: check 'efi' in MACHINE_FEATURES


On 29 March 2017 at 10:26, Mikko Ylinen <mikko.ylinen@linux.intel.com<mailto:mikko.ylinen@linux.intel.com>> wrote:
The recipe checks 'efi' in DISTRO_FEATURES but it's a machine feature.

Fix the check to use MACHINE_FEATURES to determine whether to
enable/disable 'efi' PACKAGECONFIG.

This makes systemd a MACHINE-specific recipe, and as it contains libudev causes rebuilds for everything linking to that.  What's the actual impact of enabling/disabling EFI?

There is a whitelist for recipes which are machine-specific but should be treated as tune, but the barrier to get on that list is high.

Ross

[-- Attachment #2: Type: text/html, Size: 4823 bytes --]

Re: [PATCH] systemd: check 'efi' in MACHINE_FEATURES

[Mikko Ylinen]

On 30/03/17 02:36, Peter Kjellerstedt wrote:
>
> For the record, this is just correcting a regression that was 
> introduced with my change to use bb.utils.filter() where I apparently 
> missed that not all the features were DISTRO_FEATURES…
>

Yes, this was the case. I got a report that 'bootctl' is no longer 
installed part of systemd.

> That said, I do agree that it would be preferable to get rid of the 
> machine specific dependency if possible.
>
>

I'm fine. Perhaps 'efi' should not be a PACKAGECONFIG at all (and 
unconditionally disabled here) and
the functionality (just bootctl?) is moved in systemd-boot. Thoughts?

-- Mikko

Re: [PATCH] systemd: check 'efi' in MACHINE_FEATURES

[Burton, Ross]

[-- Attachment #1: Type: text/plain, Size: 375 bytes --]

On 30 March 2017 at 07:59, Mikko Ylinen <mikko.ylinen@linux.intel.com>
wrote:

> I'm fine. Perhaps 'efi' should not be a PACKAGECONFIG at all (and
> unconditionally disabled here) and
> the functionality (just bootctl?) is moved in systemd-boot. Thoughts?
>

I'm not fully aware of the details here but this seems like a good idea.
Can you prepare a patch?

Ross

[-- Attachment #2: Type: text/html, Size: 787 bytes --]

Re: [PATCH] systemd: check 'efi' in MACHINE_FEATURES

[Mikko Ylinen]

On 30/03/17 19:15, Burton, Ross wrote:
>
> On 30 March 2017 at 07:59, Mikko Ylinen <mikko.ylinen@linux.intel.com 
> <mailto:mikko.ylinen@linux.intel.com>> wrote:
>
>     I'm fine. Perhaps 'efi' should not be a PACKAGECONFIG at all (and
>     unconditionally disabled here) and
>     the functionality (just bootctl?) is moved in systemd-boot. Thoughts?
>
>
> I'm not fully aware of the details here but this seems like a good 
> idea.  Can you prepare a patch?
>

Looks like it's not very easy to build bootctl part of systemd-boot 
(without re-building the whole
systemd there too).

Therefore, I'd just:

-                   ${@bb.utils.filter('DISTRO_FEATURES', 'efi pam 
selinux ldconfig', d)} \
+                   ${@bb.utils.filter('DISTRO_FEATURES', 'pam selinux 
ldconfig', d)} \

and perhaps:

-PACKAGECONFIG[efi] = "--enable-efi,--disable-efi"
+PACKAGECONFIG[bootctl] = "--enable-efi,--disable-efi"

efi/bootctl can then be added in PACKAGECONFIG by those who need bootctl.

-- Mikko