RE: [OE-core] [PATCH] initial configs: set initial {DISTRO|MACHINE|IMAGE}FEATUREs as weak as possible

[Peter Kjellerstedt <peter.kjellerstedt@axis.com>]

> -----Original Message-----
> From: openembedded-core@lists.openembedded.org <openembedded-core@lists.openembedded.org> On Behalf Of Christoph Lauer
> Sent: den 12 november 2022 17:31
> To: openembedded-core@lists.openembedded.org
> Cc: Christoph Lauer <christoph.lauer@xtronic.de>
> Subject: [OE-core] [PATCH] initial configs: set initial {DISTRO|MACHINE|IMAGE}FEATUREs as weak as possible
> 
> From: Christoph Lauer <christoph.lauer@xtronic.de>
> 
> This gives BSP creators the flexibility to change features with weak defaults
> 
> Signed-off-by: Christoph Lauer <christoph.lauer@xtronic.de>
> ---
>  meta-poky/conf/distro/poky.conf               |  2 +-
>  meta/conf/bitbake.conf                        | 28 +++++++++----------
>  .../distro/include/default-distrovars.inc     |  6 ++--
>  meta/conf/machine/include/qemu.inc            |  2 +-
>  meta/conf/machine/include/riscv/qemuriscv.inc |  2 +-
>  5 files changed, 20 insertions(+), 20 deletions(-)
> 
> diff --git a/meta-poky/conf/distro/poky.conf b/meta-poky/conf/distro/poky.conf
> index 3e90766349..3a453ef9ed 100644
> --- a/meta-poky/conf/distro/poky.conf
> +++ b/meta-poky/conf/distro/poky.conf
> @@ -18,7 +18,7 @@ POKY_DEFAULT_DISTRO_FEATURES = "largefile opengl ptest multiarch wayland vulkan"
>  POKY_DEFAULT_EXTRA_RDEPENDS = "packagegroup-core-boot"
>  POKY_DEFAULT_EXTRA_RRECOMMENDS = "kernel-module-af-packet"
> 
> -DISTRO_FEATURES ?= "${DISTRO_FEATURES_DEFAULT} ${POKY_DEFAULT_DISTRO_FEATURES}"
> +DISTRO_FEATURES ??= "${DISTRO_FEATURES_DEFAULT} ${POKY_DEFAULT_DISTRO_FEATURES}"
> 
>  PREFERRED_VERSION_linux-yocto ?= "5.19%"
>  PREFERRED_VERSION_linux-yocto-rt ?= "5.19%"
> diff --git a/meta/conf/bitbake.conf b/meta/conf/bitbake.conf
> index dea573986d..b2540bb3f5 100644
> --- a/meta/conf/bitbake.conf
> +++ b/meta/conf/bitbake.conf
> @@ -885,31 +885,31 @@ OES_BITBAKE_CONF = "1"
>  # Machine properties and packagegroup-base stuff
>  ##################################################################
> 
> -MACHINE_FEATURES ?= ""
> -DISTRO_FEATURES ?= ""
> +MACHINE_FEATURES ??= ""
> +DISTRO_FEATURES ??= ""
> 
> -DISTRO_EXTRA_RDEPENDS ?= ""
> -DISTRO_EXTRA_RRECOMMENDS ?= ""
> -MACHINE_EXTRA_RDEPENDS ?= ""
> -MACHINE_EXTRA_RRECOMMENDS ?= ""
> -MACHINE_ESSENTIAL_EXTRA_RDEPENDS ?= ""
> -MACHINE_ESSENTIAL_EXTRA_RRECOMMENDS ?= ""
> +DISTRO_EXTRA_RDEPENDS ??= ""
> +DISTRO_EXTRA_RRECOMMENDS ??= ""
> +MACHINE_EXTRA_RDEPENDS ??= ""
> +MACHINE_EXTRA_RRECOMMENDS ??= ""
> +MACHINE_ESSENTIAL_EXTRA_RDEPENDS ??= ""
> +MACHINE_ESSENTIAL_EXTRA_RRECOMMENDS ??= ""
> 
>  EXTRA_IMAGE_FEATURES ??= ""
>  IMAGE_FEATURES += "${EXTRA_IMAGE_FEATURES}"
> 
>  # Native distro features (will always be used for -native, even if they
>  # are not enabled for target)
> -DISTRO_FEATURES_NATIVE ?= "x11 ipv6 xattr"
> -DISTRO_FEATURES_NATIVESDK ?= "x11"
> +DISTRO_FEATURES_NATIVE ??= "x11 ipv6 xattr"
> +DISTRO_FEATURES_NATIVESDK ??= "x11"
> 
>  # Normally target distro features will not be applied to native builds:
>  # Native distro features on this list will use the target feature value
> -DISTRO_FEATURES_FILTER_NATIVE ?= "api-documentation debuginfod opengl wayland"
> -DISTRO_FEATURES_FILTER_NATIVESDK ?= "api-documentation debuginfod opengl wayland"
> +DISTRO_FEATURES_FILTER_NATIVE ??= "api-documentation debuginfod opengl wayland"
> +DISTRO_FEATURES_FILTER_NATIVESDK ??= "api-documentation debuginfod opengl wayland"
> 
> -DISTRO_FEATURES_BACKFILL = "pulseaudio sysvinit gobject-introspection-data ldconfig"
> -MACHINE_FEATURES_BACKFILL = "rtc qemu-usermode"
> +DISTRO_FEATURES_BACKFILL ??= "pulseaudio sysvinit gobject-introspection-data ldconfig"
> +MACHINE_FEATURES_BACKFILL ??= "rtc qemu-usermode"
> 
>  COMBINED_FEATURES = "${@oe.utils.set_intersect('DISTRO_FEATURES', 'MACHINE_FEATURES', d)}"
>  COMBINED_FEATURES[vardeps] += "DISTRO_FEATURES MACHINE_FEATURES"
> diff --git a/meta/conf/distro/include/default-distrovars.inc b/meta/conf/distro/include/default-distrovars.inc
> index abf48f79f0..3b3d439ad3 100644
> --- a/meta/conf/distro/include/default-distrovars.inc
> +++ b/meta/conf/distro/include/default-distrovars.inc
> @@ -19,9 +19,9 @@ DISTRO_FEATURES_DEFAULT:remove:arc = "seccomp"
>  # seccomp is not yet ported to microblaze
>  DISTRO_FEATURES_DEFAULT:remove:microblaze = "seccomp"
> 
> -DISTRO_FEATURES_DEFAULT ?= "acl alsa bluetooth debuginfod ext2 ipv4 ipv6 pcmcia usbgadget usbhost wifi xattr nfs zeroconf pci 3g nfc x11 vfat seccomp"
> -DISTRO_FEATURES ?= "${DISTRO_FEATURES_DEFAULT}"
> -IMAGE_FEATURES ?= ""
> +DISTRO_FEATURES_DEFAULT ??= "acl alsa bluetooth debuginfod ext2 ipv4 ipv6 pcmcia usbgadget usbhost wifi xattr nfs zeroconf pci 3g nfc x11 vfat seccomp"
> +DISTRO_FEATURES ??= "${DISTRO_FEATURES_DEFAULT}"
> +IMAGE_FEATURES ??= ""
> 
>  COMMERCIAL_AUDIO_PLUGINS ?= ""
>  # COMMERCIAL_AUDIO_PLUGINS ?= "gst-plugins-ugly-mad gst-plugins-ugly-mpegaudioparse"
> diff --git a/meta/conf/machine/include/qemu.inc
> b/meta/conf/machine/include/qemu.inc
> index d9bed7708d..04ffe55309 100644
> --- a/meta/conf/machine/include/qemu.inc
> +++ b/meta/conf/machine/include/qemu.inc
> @@ -10,7 +10,7 @@ XSERVER ?= "xserver-xorg \
>              xf86-video-fbdev \
>              "
> 
> -MACHINE_FEATURES = "alsa bluetooth usbgadget screen vfat"
> +MACHINE_FEATURES ??= "alsa bluetooth usbgadget screen vfat"
> 
>  MACHINEOVERRIDES =. "qemuall:"
> 
> diff --git a/meta/conf/machine/include/riscv/qemuriscv.inc b/meta/conf/machine/include/riscv/qemuriscv.inc
> index 0b399ac1f9..f5f9f89a9f 100644
> --- a/meta/conf/machine/include/riscv/qemuriscv.inc
> +++ b/meta/conf/machine/include/riscv/qemuriscv.inc
> @@ -3,7 +3,7 @@ PREFERRED_PROVIDER_virtual/bootloader ??= "u-boot"
>  require conf/machine/include/qemu.inc
>  require conf/machine/include/riscv/tune-riscv.inc
> 
> -MACHINE_FEATURES = "screen keyboard ext2 ext3 serial"
> +MACHINE_FEATURES ?= "screen keyboard ext2 ext3 serial"
> 
>  KERNEL_IMAGETYPE = "Image"
>  KERNEL_IMAGETYPES += "uImage"
> --
> 2.17.1

This is a breaking change, which can have a lot of not easily detected 
consequences, and I would advise against it. Changing ??= to ?= means 
that you can no longer do, e.g., DISTRO_FEATURES += " foo" in your 
configuration. For comparison, if you add the following to a recipe or 
your local.conf:

FOOBAR1 ??= "foo"
FOOBAR1 += "bar"
FOOBAR2 ?= "foo"
FOOBAR2 += "bar"

the result will be:

FOOBAR1=" bar"
FOOBAR2="foo bar"

If your patch is applied to Poky, this is the resulting change in the 
configuration:

--- a.txt       2022-11-13 14:34:17.882922391 +0100
+++ b.txt       2022-11-13 14:34:37.902531305 +0100
@@ -7,6 +7,6 @@
 DISTRO_FEATURES_NATIVE="x11 ipv6 xattr"
 DISTRO_FEATURES_NATIVESDK="x11"
 IMAGE_FEATURES=" debug-tweaks ptest-pkgs debug-tweaks tools-debug"
-MACHINE_FEATURES="alsa bluetooth usbgadget screen vfat x86 pci rtc qemu-usermode"
+MACHINE_FEATURES=" x86 pci rtc qemu-usermode"
 MACHINE_FEATURES_BACKFILL="rtc qemu-usermode"
 MACHINE_FEATURES_BACKFILL_CONSIDERED=" "

As can be seen, your change has modified the default configuration for 
qemux86-64 and you did not notice, even though you were aware that you 
were changing the variables. Imagine what it would do to others...

//Peter