public inbox for openembedded-core@lists.openembedded.org
 help / color / mirror / Atom feed
* [PATCH] meta: simplify conditional operations with bb.utils.filter
@ 2026-04-24 15:17 João Marcos Costa
  2026-04-24 15:40 ` [OE-core] " Quentin Schulz
  2026-04-27  8:49 ` ChenQi
  0 siblings, 2 replies; 9+ messages in thread
From: João Marcos Costa @ 2026-04-24 15:17 UTC (permalink / raw)
  To: openembedded-core; +Cc: thomas.petazzoni, João Marcos Costa

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 ''}"
diff --git a/meta/recipes-connectivity/connman/connman_2.0.bb b/meta/recipes-connectivity/connman/connman_2.0.bb
index 6f7093301b..e9873f3163 100644
--- a/meta/recipes-connectivity/connman/connman_2.0.bb
+++ b/meta/recipes-connectivity/connman/connman_2.0.bb
@@ -169,7 +169,7 @@ FILES:${PN}-tools = "${bindir}/wispr"
 RDEPENDS:${PN}-tools = "${PN}"
 
 FILES:${PN}-tests = "${bindir}/*-test"
-RDEPENDS:${PN}-tests = "${@bb.utils.contains('PACKAGECONFIG', 'iptables', 'iptables', '', d)}"
+RDEPENDS:${PN}-tests = "${@bb.utils.filter('PACKAGECONFIG', 'iptables', d)}"
 
 FILES:${PN}-client = "${bindir}/connmanctl"
 RDEPENDS:${PN}-client = "${PN}"
diff --git a/meta/recipes-core/coreutils/coreutils_9.10.bb b/meta/recipes-core/coreutils/coreutils_9.10.bb
index 984c5b5292..744d930272 100644
--- a/meta/recipes-core/coreutils/coreutils_9.10.bb
+++ b/meta/recipes-core/coreutils/coreutils_9.10.bb
@@ -222,6 +222,6 @@ do_install_ptest:append:libc-musl () {
 }
 
 RDEPENDS:${PN}-ptest += "xz  \
-                         ${@bb.utils.contains('PACKAGECONFIG', 'acl', 'acl', '', d)} \
+                         ${@bb.utils.filter('PACKAGECONFIG', 'acl', d)} \
                          ${@bb.utils.contains('PACKAGECONFIG', 'xattr', 'attr', '', d)}"
 FILES:${PN}-ptest += "${bindir}/getlimits"
diff --git a/meta/recipes-core/ovmf/ovmf_git.bb b/meta/recipes-core/ovmf/ovmf_git.bb
index d731bca7f2..38d5d090b4 100644
--- a/meta/recipes-core/ovmf/ovmf_git.bb
+++ b/meta/recipes-core/ovmf/ovmf_git.bb
@@ -10,7 +10,7 @@ LIC_FILES_CHKSUM = "file://OvmfPkg/License.txt;md5=06357ddc23f46577c2aeaeaf7b776
 # compiling OVMF twice, so it is disabled by default. Distros
 # may change that default.
 PACKAGECONFIG ??= ""
-PACKAGECONFIG += "${@bb.utils.contains('MACHINE_FEATURES', 'tpm', 'tpm', '', d)}"
+PACKAGECONFIG += "${@bb.utils.filter('MACHINE_FEATURES', 'tpm', d)}"
 PACKAGECONFIG += "${@bb.utils.contains('MACHINE_FEATURES', 'tpm2', 'tpm', '', d)}"
 PACKAGECONFIG[debug] = ",,,"
 PACKAGECONFIG[secureboot] = ",,,"
diff --git a/meta/recipes-extended/at/at_3.2.5.bb b/meta/recipes-extended/at/at_3.2.5.bb
index 112d1c4adc..ee485f67ec 100644
--- a/meta/recipes-extended/at/at_3.2.5.bb
+++ b/meta/recipes-extended/at/at_3.2.5.bb
@@ -9,7 +9,7 @@ DEPENDS = "flex flex-native bison-native \
            ${@bb.utils.contains('DISTRO_FEATURES', 'pam', 'libpam', '', d)}"
 
 PACKAGECONFIG ?= "\
-    ${@bb.utils.contains('DISTRO_FEATURES', 'selinux', 'selinux', '', d)} \
+    ${@bb.utils.filter('DISTRO_FEATURES', 'selinux', d)} \
 "
 
 PACKAGECONFIG[selinux] = "--with-selinux,--without-selinux,libselinux,"
diff --git a/meta/recipes-graphics/libglvnd/libglvnd_1.7.0.bb b/meta/recipes-graphics/libglvnd/libglvnd_1.7.0.bb
index 18eeaa5523..c76763f811 100644
--- a/meta/recipes-graphics/libglvnd/libglvnd_1.7.0.bb
+++ b/meta/recipes-graphics/libglvnd/libglvnd_1.7.0.bb
@@ -14,7 +14,7 @@ REQUIRED_DISTRO_FEATURES = "opengl glvnd"
 inherit meson pkgconfig features_check
 
 PACKAGECONFIG ?= "\
-  ${@bb.utils.contains('DISTRO_FEATURES', 'x11', 'x11', '', d)} \
+  ${@bb.utils.filter('DISTRO_FEATURES', 'x11', d)} \
   ${@bb.utils.contains('DISTRO_FEATURES', 'opengl', 'egl gles1 gles2', '', d)} \
   ${@bb.utils.contains('DISTRO_FEATURES', 'opengl x11', 'glx', '', d)} \
   "
diff --git a/meta/recipes-graphics/waffle/waffle_1.8.1.bb b/meta/recipes-graphics/waffle/waffle_1.8.1.bb
index aefa0069cf..5d993cf254 100644
--- a/meta/recipes-graphics/waffle/waffle_1.8.1.bb
+++ b/meta/recipes-graphics/waffle/waffle_1.8.1.bb
@@ -21,7 +21,7 @@ DEPENDS:append = " python3"
 # This should be overridden per-machine to reflect the capabilities of the GL
 # stack.
 PACKAGECONFIG ??= "${@bb.utils.contains('DISTRO_FEATURES', 'x11', 'glx x11-egl', '', d)} \
-                   ${@bb.utils.contains('DISTRO_FEATURES', 'wayland', 'wayland', '', d)} \
+                   ${@bb.utils.filter('DISTRO_FEATURES', 'wayland', d)} \
                    ${@bb.utils.contains('DISTRO_FEATURES', 'opengl', 'gbm surfaceless-egl', '', d)} \
 "
 
diff --git a/meta/recipes-multimedia/gstreamer/gstreamer1.0-plugins-bad_1.28.2.bb b/meta/recipes-multimedia/gstreamer/gstreamer1.0-plugins-bad_1.28.2.bb
index cdf3a20dff..9945e79bf7 100644
--- a/meta/recipes-multimedia/gstreamer/gstreamer1.0-plugins-bad_1.28.2.bb
+++ b/meta/recipes-multimedia/gstreamer/gstreamer1.0-plugins-bad_1.28.2.bb
@@ -25,7 +25,7 @@ PACKAGECONFIG ??= " \
     ${GSTREAMER_ORC} \
     ${@bb.utils.contains('DISTRO_FEATURES', 'bluetooth', 'bluez', '', d)} \
     ${@bb.utils.filter('DISTRO_FEATURES', 'directfb vulkan x11', d)} \
-    ${@bb.utils.contains('DISTRO_FEATURES', 'wayland', 'wayland', '', d)} \
+    ${@bb.utils.filter('DISTRO_FEATURES', 'wayland', d)} \
     ${@bb.utils.contains('DISTRO_FEATURES', 'opengl', 'gl', '', d)} \
     bz2 closedcaption curl dash dtls hls openssl sbc smoothstreaming \
     sndfile ttml uvch264 webp analytics \
-- 
2.47.0



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [OE-core] [PATCH] meta: simplify conditional operations with bb.utils.filter
  2026-04-24 15:17 [PATCH] meta: simplify conditional operations with bb.utils.filter João Marcos Costa
@ 2026-04-24 15:40 ` Quentin Schulz
  2026-04-27  8:38   ` Joao Marcos Costa
  2026-04-27  8:49 ` ChenQi
  1 sibling, 1 reply; 9+ messages in thread
From: Quentin Schulz @ 2026-04-24 15:40 UTC (permalink / raw)
  To: joaomarcos.costa, openembedded-core; +Cc: thomas.petazzoni

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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [OE-core] [PATCH] meta: simplify conditional operations with bb.utils.filter
  2026-04-24 15:40 ` [OE-core] " Quentin Schulz
@ 2026-04-27  8:38   ` Joao Marcos Costa
  2026-04-27 10:45     ` Quentin Schulz
  0 siblings, 1 reply; 9+ messages in thread
From: Joao Marcos Costa @ 2026-04-27  8:38 UTC (permalink / raw)
  To: quentin.schulz, openembedded-core; +Cc: thomas.petazzoni

Hello, Quentin

On 4/24/26 17:40, Quentin Schulz via lists.openembedded.org wrote:
> 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>
>> ---

(...)

> 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

Thanks for your review. And yes, I'm looking at this part of the patch 
again, and I'm considering two things:

a) Add the strip() as you said, here, and keep my changes:

-TUNE_CCARGS  .= "${@ (' -mfpu=%s' % 
d.getVar('TUNE_CCARGS_MFPU').split()[-1]) if 
(d.getVar('TUNE_CCARGS_MFPU') != '') else ''}"
+TUNE_CCARGS  .= "${@ (' -mfpu=%s' % 
d.getVar('TUNE_CCARGS_MFPU').split()[-1]) if 
(d.getVar('TUNE_CCARGS_MFPU').strip() != '') else ''}"
  # The following deals with both vfpv3-d16 and vfpv4-d16
-ARMPKGSFX_FPU = "${@ ('-%s'       % 
d.getVar('TUNE_CCARGS_MFPU').split()[-1].replace('-d16', 'd16')) if 
(d.getVar('TUNE_CCARGS_MFPU') != '') else ''}"
+ARMPKGSFX_FPU = "${@ ('-%s'       % 
d.getVar('TUNE_CCARGS_MFPU').split()[-1].replace('-d16', 'd16')) if 
(d.getVar('TUNE_CCARGS_MFPU').strip() != '') else ''}"

b) Simply remove those changes, then send another patch keeping only the 
changes in recipes.

What do you think?

-- 
Best regards,
João Marcos Costa


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [OE-core] [PATCH] meta: simplify conditional operations with bb.utils.filter
  2026-04-24 15:17 [PATCH] meta: simplify conditional operations with bb.utils.filter João Marcos Costa
  2026-04-24 15:40 ` [OE-core] " Quentin Schulz
@ 2026-04-27  8:49 ` ChenQi
  2026-04-27  9:30   ` Joao Marcos Costa
  1 sibling, 1 reply; 9+ messages in thread
From: ChenQi @ 2026-04-27  8:49 UTC (permalink / raw)
  To: joaomarcos.costa, openembedded-core; +Cc: thomas.petazzoni

When using filter for two different things, isn't it a little strange?

Take the first change you made as an example:

-ERROR_QA:append ="${@bb.utils.contains('DISTRO_FEATURES', 'usrmerge', ' usrmerge', '', d)}"
+ERROR_QA:append = " ${@bb.utils.filter('DISTRO_FEATURES', 'usrmerge', d)}"

Previously, the line is very clear. It reads like: if DISTRO_FEATURES 
contains 'usrmerge', append ' usrmerge' to ERROR_QA.
After the change, it reads like: filter usrmerge from DISTRO_FEATRUES to 
ERROR_QA.
By doing this, you're connecting these two variables (DISTRO_FEATURES & 
ERROR_QA) conceptually.
And the leading space is also a tiny problem.

Regards,
Qi

On 4/24/26 23:17, 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 ''}"
> diff --git a/meta/recipes-connectivity/connman/connman_2.0.bb b/meta/recipes-connectivity/connman/connman_2.0.bb
> index 6f7093301b..e9873f3163 100644
> --- a/meta/recipes-connectivity/connman/connman_2.0.bb
> +++ b/meta/recipes-connectivity/connman/connman_2.0.bb
> @@ -169,7 +169,7 @@ FILES:${PN}-tools = "${bindir}/wispr"
>   RDEPENDS:${PN}-tools = "${PN}"
>   
>   FILES:${PN}-tests = "${bindir}/*-test"
> -RDEPENDS:${PN}-tests = "${@bb.utils.contains('PACKAGECONFIG', 'iptables', 'iptables', '', d)}"
> +RDEPENDS:${PN}-tests = "${@bb.utils.filter('PACKAGECONFIG', 'iptables', d)}"
>   
>   FILES:${PN}-client = "${bindir}/connmanctl"
>   RDEPENDS:${PN}-client = "${PN}"
> diff --git a/meta/recipes-core/coreutils/coreutils_9.10.bb b/meta/recipes-core/coreutils/coreutils_9.10.bb
> index 984c5b5292..744d930272 100644
> --- a/meta/recipes-core/coreutils/coreutils_9.10.bb
> +++ b/meta/recipes-core/coreutils/coreutils_9.10.bb
> @@ -222,6 +222,6 @@ do_install_ptest:append:libc-musl () {
>   }
>   
>   RDEPENDS:${PN}-ptest += "xz  \
> -                         ${@bb.utils.contains('PACKAGECONFIG', 'acl', 'acl', '', d)} \
> +                         ${@bb.utils.filter('PACKAGECONFIG', 'acl', d)} \
>                            ${@bb.utils.contains('PACKAGECONFIG', 'xattr', 'attr', '', d)}"
>   FILES:${PN}-ptest += "${bindir}/getlimits"
> diff --git a/meta/recipes-core/ovmf/ovmf_git.bb b/meta/recipes-core/ovmf/ovmf_git.bb
> index d731bca7f2..38d5d090b4 100644
> --- a/meta/recipes-core/ovmf/ovmf_git.bb
> +++ b/meta/recipes-core/ovmf/ovmf_git.bb
> @@ -10,7 +10,7 @@ LIC_FILES_CHKSUM = "file://OvmfPkg/License.txt;md5=06357ddc23f46577c2aeaeaf7b776
>   # compiling OVMF twice, so it is disabled by default. Distros
>   # may change that default.
>   PACKAGECONFIG ??= ""
> -PACKAGECONFIG += "${@bb.utils.contains('MACHINE_FEATURES', 'tpm', 'tpm', '', d)}"
> +PACKAGECONFIG += "${@bb.utils.filter('MACHINE_FEATURES', 'tpm', d)}"
>   PACKAGECONFIG += "${@bb.utils.contains('MACHINE_FEATURES', 'tpm2', 'tpm', '', d)}"
>   PACKAGECONFIG[debug] = ",,,"
>   PACKAGECONFIG[secureboot] = ",,,"
> diff --git a/meta/recipes-extended/at/at_3.2.5.bb b/meta/recipes-extended/at/at_3.2.5.bb
> index 112d1c4adc..ee485f67ec 100644
> --- a/meta/recipes-extended/at/at_3.2.5.bb
> +++ b/meta/recipes-extended/at/at_3.2.5.bb
> @@ -9,7 +9,7 @@ DEPENDS = "flex flex-native bison-native \
>              ${@bb.utils.contains('DISTRO_FEATURES', 'pam', 'libpam', '', d)}"
>   
>   PACKAGECONFIG ?= "\
> -    ${@bb.utils.contains('DISTRO_FEATURES', 'selinux', 'selinux', '', d)} \
> +    ${@bb.utils.filter('DISTRO_FEATURES', 'selinux', d)} \
>   "
>   
>   PACKAGECONFIG[selinux] = "--with-selinux,--without-selinux,libselinux,"
> diff --git a/meta/recipes-graphics/libglvnd/libglvnd_1.7.0.bb b/meta/recipes-graphics/libglvnd/libglvnd_1.7.0.bb
> index 18eeaa5523..c76763f811 100644
> --- a/meta/recipes-graphics/libglvnd/libglvnd_1.7.0.bb
> +++ b/meta/recipes-graphics/libglvnd/libglvnd_1.7.0.bb
> @@ -14,7 +14,7 @@ REQUIRED_DISTRO_FEATURES = "opengl glvnd"
>   inherit meson pkgconfig features_check
>   
>   PACKAGECONFIG ?= "\
> -  ${@bb.utils.contains('DISTRO_FEATURES', 'x11', 'x11', '', d)} \
> +  ${@bb.utils.filter('DISTRO_FEATURES', 'x11', d)} \
>     ${@bb.utils.contains('DISTRO_FEATURES', 'opengl', 'egl gles1 gles2', '', d)} \
>     ${@bb.utils.contains('DISTRO_FEATURES', 'opengl x11', 'glx', '', d)} \
>     "
> diff --git a/meta/recipes-graphics/waffle/waffle_1.8.1.bb b/meta/recipes-graphics/waffle/waffle_1.8.1.bb
> index aefa0069cf..5d993cf254 100644
> --- a/meta/recipes-graphics/waffle/waffle_1.8.1.bb
> +++ b/meta/recipes-graphics/waffle/waffle_1.8.1.bb
> @@ -21,7 +21,7 @@ DEPENDS:append = " python3"
>   # This should be overridden per-machine to reflect the capabilities of the GL
>   # stack.
>   PACKAGECONFIG ??= "${@bb.utils.contains('DISTRO_FEATURES', 'x11', 'glx x11-egl', '', d)} \
> -                   ${@bb.utils.contains('DISTRO_FEATURES', 'wayland', 'wayland', '', d)} \
> +                   ${@bb.utils.filter('DISTRO_FEATURES', 'wayland', d)} \
>                      ${@bb.utils.contains('DISTRO_FEATURES', 'opengl', 'gbm surfaceless-egl', '', d)} \
>   "
>   
> diff --git a/meta/recipes-multimedia/gstreamer/gstreamer1.0-plugins-bad_1.28.2.bb b/meta/recipes-multimedia/gstreamer/gstreamer1.0-plugins-bad_1.28.2.bb
> index cdf3a20dff..9945e79bf7 100644
> --- a/meta/recipes-multimedia/gstreamer/gstreamer1.0-plugins-bad_1.28.2.bb
> +++ b/meta/recipes-multimedia/gstreamer/gstreamer1.0-plugins-bad_1.28.2.bb
> @@ -25,7 +25,7 @@ PACKAGECONFIG ??= " \
>       ${GSTREAMER_ORC} \
>       ${@bb.utils.contains('DISTRO_FEATURES', 'bluetooth', 'bluez', '', d)} \
>       ${@bb.utils.filter('DISTRO_FEATURES', 'directfb vulkan x11', d)} \
> -    ${@bb.utils.contains('DISTRO_FEATURES', 'wayland', 'wayland', '', d)} \
> +    ${@bb.utils.filter('DISTRO_FEATURES', 'wayland', d)} \
>       ${@bb.utils.contains('DISTRO_FEATURES', 'opengl', 'gl', '', d)} \
>       bz2 closedcaption curl dash dtls hls openssl sbc smoothstreaming \
>       sndfile ttml uvch264 webp analytics \
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#235868): https://lists.openembedded.org/g/openembedded-core/message/235868
> Mute This Topic: https://lists.openembedded.org/mt/118989280/7304865
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [Qi.Chen@eng.windriver.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [OE-core] [PATCH] meta: simplify conditional operations with bb.utils.filter
  2026-04-27  8:49 ` ChenQi
@ 2026-04-27  9:30   ` Joao Marcos Costa
  2026-04-27 10:05     ` ChenQi
  0 siblings, 1 reply; 9+ messages in thread
From: Joao Marcos Costa @ 2026-04-27  9:30 UTC (permalink / raw)
  To: Qi.Chen, openembedded-core; +Cc: thomas.petazzoni

Hello, Chen

On 4/27/26 10:49, Chen Qi via lists.openembedded.org wrote:
> When using filter for two different things, isn't it a little strange?
> 
> Take the first change you made as an example:
> 
> -ERROR_QA:append ="${@bb.utils.contains('DISTRO_FEATURES', 'usrmerge', ' 
> usrmerge', '', d)}"
> +ERROR_QA:append = " ${@bb.utils.filter('DISTRO_FEATURES', 'usrmerge', d)}"
> 
> Previously, the line is very clear. It reads like: if DISTRO_FEATURES 
> contains 'usrmerge', append ' usrmerge' to ERROR_QA.

So far, this applies to bb.utils.filter() as a whole. Unless your point 
is about the space in ' usrmerge'.

> After the change, it reads like: filter usrmerge from DISTRO_FEATRUES to 
> ERROR_QA.
> By doing this, you're connecting these two variables (DISTRO_FEATURES & 
> ERROR_QA) conceptually.

I'm not sure I interpret/read this line the same way you do. Once again, 
unless I misunderstood your point, the way you read bb.utils.filter() 
applies to every use of such helper.

How different is this ERROR_QA case from the other occurences of 
bb.utils.filter() in my patch (i.e., in the *.bb), or even in oe-core?

> And the leading space is also a tiny problem.

After Quentin's comment, I double-checked and ERROR_QA is not handled in 
the same way as TUNE_CCARGS_MFPU, so the extra leading space should be 
fine. I agree, however, that it's not very nice to add it unconditionally.

One more thing: could you please clarify what you mean by "two different 
things"? I'm not sure to follow, even with the example. The use case is 
still one and only: check if the string is in the variable, and if so, 
return it.

> Regards,
> Qi

Thanks!

-- 
Best regards,
João Marcos Costa


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [OE-core] [PATCH] meta: simplify conditional operations with bb.utils.filter
  2026-04-27  9:30   ` Joao Marcos Costa
@ 2026-04-27 10:05     ` ChenQi
  2026-04-27 11:01       ` Joao Marcos Costa
  0 siblings, 1 reply; 9+ messages in thread
From: ChenQi @ 2026-04-27 10:05 UTC (permalink / raw)
  To: Joao Marcos Costa, openembedded-core; +Cc: thomas.petazzoni

On 4/27/26 17:30, Joao Marcos Costa wrote:
> Hello, Chen
>
> On 4/27/26 10:49, Chen Qi via lists.openembedded.org wrote:
>> When using filter for two different things, isn't it a little strange?
>>
>> Take the first change you made as an example:
>>
>> -ERROR_QA:append ="${@bb.utils.contains('DISTRO_FEATURES', 
>> 'usrmerge', ' usrmerge', '', d)}"
>> +ERROR_QA:append = " ${@bb.utils.filter('DISTRO_FEATURES', 
>> 'usrmerge', d)}"
>>
>> Previously, the line is very clear. It reads like: if DISTRO_FEATURES 
>> contains 'usrmerge', append ' usrmerge' to ERROR_QA.
>
> So far, this applies to bb.utils.filter() as a whole. Unless your 
> point is about the space in ' usrmerge'.
>
>> After the change, it reads like: filter usrmerge from DISTRO_FEATRUES 
>> to ERROR_QA.
>> By doing this, you're connecting these two variables (DISTRO_FEATURES 
>> & ERROR_QA) conceptually.
>
> I'm not sure I interpret/read this line the same way you do. Once 
> again, unless I misunderstood your point, the way you read 
> bb.utils.filter() applies to every use of such helper.
>
> How different is this ERROR_QA case from the other occurences of 
> bb.utils.filter() in my patch (i.e., in the *.bb), or even in oe-core?
>
>> And the leading space is also a tiny problem.
>
> After Quentin's comment, I double-checked and ERROR_QA is not handled 
> in the same way as TUNE_CCARGS_MFPU, so the extra leading space should 
> be fine. I agree, however, that it's not very nice to add it 
> unconditionally.
>
> One more thing: could you please clarify what you mean by "two 
> different things"? I'm not sure to follow, even with the example. The 
> use case is still one and only: check if the string is in the 
> variable, and if so, return it.

VAR_1 = bb.utils.filter(VAR_2, ....)

When you do this, this gives people an impression that VAR1 and VAR2 are 
conceptually related.

For example, PACKAGECONFIG & DISTRO_FEATURES both control the way 
recipes are built, they are switches for recipes, and they are related. 
Many PACKAEGECONFIG items' names are selected to be the same with 
DISTRO_FEAETURES, e.g., pam, acl., because they are the most reasonable 
names.

But if you look at ERROR_QA, the items names really have no relations 
with DISTRO_FEATURES. The usrmerge QA check was actually checking 
filesystem hierarchy. If we consider the possible sbin -> bin merge in 
the future, the QA check name should be something like 
"filesystem-hierarchy-check".

To be clear, I'm not against *all* parts of this patch. For example, you 
have:

  PACKAGECONFIG ?= "\
-    ${@bb.utils.contains('DISTRO_FEATURES', 'selinux', 'selinux', '', d)} \
+    ${@bb.utils.filter('DISTRO_FEATURES', 'selinux', d)} \
  "

And I think it's a correct change.

I think the problematic ones are ERROR_QA and RDEPENDS.

Regards,
Qi

>
>> Regards,
>> Qi
>
> Thanks!
>



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [OE-core] [PATCH] meta: simplify conditional operations with bb.utils.filter
  2026-04-27  8:38   ` Joao Marcos Costa
@ 2026-04-27 10:45     ` Quentin Schulz
  2026-04-27 10:53       ` Joao Marcos Costa
  0 siblings, 1 reply; 9+ messages in thread
From: Quentin Schulz @ 2026-04-27 10:45 UTC (permalink / raw)
  To: Joao Marcos Costa, openembedded-core; +Cc: thomas.petazzoni

Hi João,

On 4/27/26 10:38 AM, Joao Marcos Costa wrote:
> Hello, Quentin
> 
> On 4/24/26 17:40, Quentin Schulz via lists.openembedded.org wrote:
>> 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>
>>> ---
> 
> (...)
> 
>> 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
> 
> Thanks for your review. And yes, I'm looking at this part of the patch 
> again, and I'm considering two things:
> 
> a) Add the strip() as you said, here, and keep my changes:
> 
> -TUNE_CCARGS  .= "${@ (' -mfpu=%s' % 
> d.getVar('TUNE_CCARGS_MFPU').split()[-1]) if 
> (d.getVar('TUNE_CCARGS_MFPU') != '') else ''}"
> +TUNE_CCARGS  .= "${@ (' -mfpu=%s' % 
> d.getVar('TUNE_CCARGS_MFPU').split()[-1]) if 
> (d.getVar('TUNE_CCARGS_MFPU').strip() != '') else ''}"
>   # The following deals with both vfpv3-d16 and vfpv4-d16
> -ARMPKGSFX_FPU = "${@ ('-%s'       % 
> d.getVar('TUNE_CCARGS_MFPU').split()[-1].replace('-d16', 'd16')) if 
> (d.getVar('TUNE_CCARGS_MFPU') != '') else ''}"
> +ARMPKGSFX_FPU = "${@ ('-%s'       % 
> d.getVar('TUNE_CCARGS_MFPU').split()[-1].replace('-d16', 'd16')) if 
> (d.getVar('TUNE_CCARGS_MFPU').strip() != '') else ''}"
> 

Yet another option is to do

(d.getVar('TUNE_CCARGS_MFPU') or '').strip() != ''

but it's a bit eww :)

> b) Simply remove those changes, then send another patch keeping only the 
> changes in recipes.
> 

Yeah that works too. It's the same change but the implications are 
different. So maybe three patches (one for machihes, one for QA, one for 
recipes (maybe even in two, one for PACKAGECONFIG, one for RDEPENDS 
since Chen Qi doesn't seem to like the latter too much :) ) so everybody 
can bikeshed as much as they want on one and still have the rest merged :)

Cheers,
Quentin


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [OE-core] [PATCH] meta: simplify conditional operations with bb.utils.filter
  2026-04-27 10:45     ` Quentin Schulz
@ 2026-04-27 10:53       ` Joao Marcos Costa
  0 siblings, 0 replies; 9+ messages in thread
From: Joao Marcos Costa @ 2026-04-27 10:53 UTC (permalink / raw)
  To: Quentin Schulz, openembedded-core; +Cc: thomas.petazzoni

Hello again,

(...)
> 
> Yet another option is to do
> 
> (d.getVar('TUNE_CCARGS_MFPU') or '').strip() != ''
> 
> but it's a bit eww :)

I'm pretty sure that's how ERROR_QA is handled, actually:

meta/lib/oe/qa.py:    if error_class in (d.getVar("ERROR_QA") or 
"").split():

Unless this is different in a way I simply cannot see :)

> 
>> b) Simply remove those changes, then send another patch keeping only 
>> the changes in recipes.
>>
> 
> Yeah that works too. It's the same change but the implications are 
> different. So maybe three patches (one for machihes, one for QA, one for 
> recipes (maybe even in two, one for PACKAGECONFIG, one for RDEPENDS 
> since Chen Qi doesn't seem to like the latter too much :) ) so everybody 
> can bikeshed as much as they want on one and still have the rest merged :)
> 
> Cheers,
> Quentin

I'll do that. Maybe that's what I should have done in the first place, 
but I didn't want to spam N different patches that essentially do the 
same replacement.

Thanks!

-- 
Best regards,
João Marcos Costa


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [OE-core] [PATCH] meta: simplify conditional operations with bb.utils.filter
  2026-04-27 10:05     ` ChenQi
@ 2026-04-27 11:01       ` Joao Marcos Costa
  0 siblings, 0 replies; 9+ messages in thread
From: Joao Marcos Costa @ 2026-04-27 11:01 UTC (permalink / raw)
  To: Qi.Chen, openembedded-core; +Cc: thomas.petazzoni, Quentin Schulz

Hello,

(...)

> 
> VAR_1 = bb.utils.filter(VAR_2, ....)
> 
> When you do this, this gives people an impression that VAR1 and VAR2 are 
> conceptually related.
> 
> For example, PACKAGECONFIG & DISTRO_FEATURES both control the way 
> recipes are built, they are switches for recipes, and they are related. 
> Many PACKAEGECONFIG items' names are selected to be the same with 
> DISTRO_FEAETURES, e.g., pam, acl., because they are the most reasonable 
> names.

I see what you mean now, thanks!

> But if you look at ERROR_QA, the items names really have no relations 
> with DISTRO_FEATURES. The usrmerge QA check was actually checking 
> filesystem hierarchy. If we consider the possible sbin -> bin merge in 
> the future, the QA check name should be something like 
> "filesystem-hierarchy-check".

Indeed.

> 
> To be clear, I'm not against *all* parts of this patch. For example, you 
> have:
> 
>   PACKAGECONFIG ?= "\
> -    ${@bb.utils.contains('DISTRO_FEATURES', 'selinux', 'selinux', '', 
> d)} \
> +    ${@bb.utils.filter('DISTRO_FEATURES', 'selinux', d)} \
>   "
> 
> And I think it's a correct change.
> 
> I think the problematic ones are ERROR_QA and RDEPENDS.

Well, PACKAGECONFIG is also related to runtime, not only build-time. 
What I mean is you can specify additional runtime dependencies in the 
PACKAGECONFIG declaration/definition [1], so one could argue that 
RDEPENDS and PACKAGECONFIG are not so conceptually distant as the 
ERROR_QA and DISTRO_FEATURES example.

> Regards,
> Qi

Thanks for the review, and for the clarification as well.

[1] 
https://docs.yoctoproject.org/ref-manual/variables.html#term-PACKAGECONFIG
-- 
Best regards,
João Marcos Costa


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2026-04-27 11:02 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-24 15:17 [PATCH] meta: simplify conditional operations with bb.utils.filter João Marcos Costa
2026-04-24 15:40 ` [OE-core] " Quentin Schulz
2026-04-27  8:38   ` Joao Marcos Costa
2026-04-27 10:45     ` Quentin Schulz
2026-04-27 10:53       ` Joao Marcos Costa
2026-04-27  8:49 ` ChenQi
2026-04-27  9:30   ` Joao Marcos Costa
2026-04-27 10:05     ` ChenQi
2026-04-27 11:01       ` Joao Marcos Costa

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox