Re: [OE-core] [PATCH] meta: simplify conditional operations with bb.utils.filter

[Quentin Schulz <quentin.schulz@cherry.de>]

Hi João,

On 4/24/26 5:17 PM, Joao Marcos Costa via lists.openembedded.org wrote:
> Some recipes and configuration files use bb.utils.contains to check for
> a string inside a variable, and return the exact same string if true.
> 
> This can be simplified by a call to bb.utils.filter, since the result is
> the same, and the inline is shorter.
> 
> Replace "bb.utils.contains(A, 'a', 'a', '', d)" by "bb.utils.filter(A, 'a', d)".
> 
> Signed-off-by: João Marcos Costa <joaomarcos.costa@bootlin.com>
> ---
>   meta/classes-global/insane.bbclass                        | 2 +-
>   meta/conf/machine/include/arm/feature-arm-neon.inc        | 8 ++++----
>   meta/conf/machine/include/arm/feature-arm-vfp.inc         | 2 +-
>   meta/recipes-connectivity/connman/connman_2.0.bb          | 2 +-
>   meta/recipes-core/coreutils/coreutils_9.10.bb             | 2 +-
>   meta/recipes-core/ovmf/ovmf_git.bb                        | 2 +-
>   meta/recipes-extended/at/at_3.2.5.bb                      | 2 +-
>   meta/recipes-graphics/libglvnd/libglvnd_1.7.0.bb          | 2 +-
>   meta/recipes-graphics/waffle/waffle_1.8.1.bb              | 2 +-
>   .../gstreamer/gstreamer1.0-plugins-bad_1.28.2.bb          | 2 +-
>   10 files changed, 13 insertions(+), 13 deletions(-)
> 
> diff --git a/meta/classes-global/insane.bbclass b/meta/classes-global/insane.bbclass
> index feddfe0335..04700be71c 100644
> --- a/meta/classes-global/insane.bbclass
> +++ b/meta/classes-global/insane.bbclass
> @@ -46,7 +46,7 @@ ERROR_QA ?= "\
>       ${CHECKLAYER_REQUIRED_TESTS}"
>   
>   # Add usrmerge QA check based on distro feature
> -ERROR_QA:append = "${@bb.utils.contains('DISTRO_FEATURES', 'usrmerge', ' usrmerge', '', d)}"
> +ERROR_QA:append = " ${@bb.utils.filter('DISTRO_FEATURES', 'usrmerge', d)}"
>   WARN_QA:append:layer-core = " missing-metadata missing-maintainer"
>   
>   FAKEROOT_QA = "host-user-contaminated"
> diff --git a/meta/conf/machine/include/arm/feature-arm-neon.inc b/meta/conf/machine/include/arm/feature-arm-neon.inc
> index 174b9b9f2a..2ec354bfeb 100644
> --- a/meta/conf/machine/include/arm/feature-arm-neon.inc
> +++ b/meta/conf/machine/include/arm/feature-arm-neon.inc
> @@ -5,16 +5,16 @@
>   # 'vfp', -mfloat-abi parameter and 'hf' suffix is implemented in feature-arm-vfp.inc
>   
>   TUNEVALID[neon] = "Enable Neon SIMD accelerator unit."
> -TUNE_CCARGS_MFPU .= "${@bb.utils.contains('TUNE_FEATURES', 'neon', ' neon', '', d)}"
> +TUNE_CCARGS_MFPU .= " ${@bb.utils.filter('TUNE_FEATURES', 'neon', d)}"
>   
>   TUNEVALID[vfpv3d16] = "Enable Vector Floating Point Version 3 with 16 registers (vfpv3-d16) unit."
> -TUNE_CCARGS_MFPU .= "${@bb.utils.contains('TUNE_FEATURES', 'vfpv3d16', ' vfpv3-d16', '', d)}"
> +TUNE_CCARGS_MFPU .= " ${@bb.utils.contains('TUNE_FEATURES', 'vfpv3d16', ' vfpv3-d16', '', d)}"
>   
>   TUNEVALID[vfpv3] = "Enable Vector Floating Point Version 3 with 32 registers (vfpv3) unit."
> -TUNE_CCARGS_MFPU .= "${@bb.utils.contains('TUNE_FEATURES', 'vfpv3', ' vfpv3', '' , d)}"
> +TUNE_CCARGS_MFPU .= " ${@bb.utils.filter('TUNE_FEATURES', 'vfpv3', d)}"
>   
>   TUNEVALID[vfpv4] = "Enable Vector Floating Point Version 4 (vfpv4) unit."
> -TUNE_CCARGS_MFPU .= "${@bb.utils.contains('TUNE_FEATURES', 'vfpv4', ' vfpv4', '', d)}"
> +TUNE_CCARGS_MFPU .= " ${@bb.utils.filter('TUNE_FEATURES', 'vfpv4', d)}"
>   TUNE_CCARGS_MFPU .= "${@bb.utils.contains('TUNE_FEATURES', [ 'vfpv4', 'neon' ], ' neon-vfpv4', '', d)}"
>   
>   TUNEVALID[vfpv4d16] = "Enable Vector Floating Point Version 4 with 16 registers (vfpv4-d16) unit."
> diff --git a/meta/conf/machine/include/arm/feature-arm-vfp.inc b/meta/conf/machine/include/arm/feature-arm-vfp.inc
> index d020100daa..be0c19bd03 100644
> --- a/meta/conf/machine/include/arm/feature-arm-vfp.inc
> +++ b/meta/conf/machine/include/arm/feature-arm-vfp.inc
> @@ -3,7 +3,7 @@
>   # and this .inc file is included from armv5
>   
>   TUNEVALID[vfp] = "Enable Vector Floating Point (vfp) unit."
> -TUNE_CCARGS_MFPU .= "${@bb.utils.contains('TUNE_FEATURES', 'vfp', ' vfp', '', d)}"
> +TUNE_CCARGS_MFPU .= " ${@bb.utils.filter('TUNE_FEATURES', 'vfp', d)}"
>   
>   # simd is special, we don't pass this to the -mfpu, it's implied
>   TUNE_CCARGS  .= "${@ (' -mfpu=%s' % d.getVar('TUNE_CCARGS_MFPU').split()[-1]) if (d.getVar('TUNE_CCARGS_MFPU') != '') else ''}"

I'm pretty sure your changes in the conf files won't work, but maybe I'm 
wrong?

Before, we only added a space if there was a new tune to add. Now, it's 
always added.

Considering TUNE_CCARGS_MFPU can now possibly contain only spaces, the 
check if TUNE_CCARGS_MFPU is not the empty string to access the last 
element in the list returned by split(), which will now be the empty 
list, will raise an IndexError exception.

You need to adapt anything that's using this variable to check if it 
only contains spaces or at least remove spurious ones, possibly with 
str.strip() for example. Unfortunately, if TUNE_CCARGS_MFPU doesn't 
exist, d.getVar() will return None and you cannot strip() on None. Of 
course, considering TUNE_CCARGS_MFPU is declared just above in the file, 
it'll always be defined, so maybe that's fine to not check for None.

If the space didn't matter, we probably would have used += instead of 
.=, so I'm wary of this change (the rest looks fine).

Cheers,
Quentin