| Message ID | 20260427130846.96013-4-joaomarcos.costa@bootlin.com |
|---|---|
| State | Rejected |
| Headers | show |
| Series | Simplify conditional operations with bb.utils.filter | expand |
Hi João, On 4/27/26 3:08 PM, João Marcos Costa wrote: > Some 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)". > > bb.utils.filter() does not return the string with a leading space, and > this is handled by a leading space outside of the helper. This > workaround, however, has its limitations: the leading space is always > added. To avoid any potential issues, use .strip() when dereferencing > TUNE_CCARGS_MFPU in the if statements. > I'm not sure this is worth it as this pattern means next time we add a new tunefeatures we'll likely follow the same pattern. This means that even though the existing machines won't set this new tunefeature, they'll likely rebuild a few things because TUNE_CCARGS_MFPU will have an additional space in it (but maybe BitBake parsing is smart enough to realize that only TUNE_CCARGS, ARMPKGSFX_FPU and TUNE_CCARGS_MFLOAT use it and they won't change if there's only one space difference, so: Reviewed-by: Quentin Schulz <quentin.schulz@cherry.de> Thanks! Quentin
On Mon, 2026-04-27 at 15:08 +0200, Joao Marcos Costa via lists.openembedded.org wrote: > Some 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)". > > bb.utils.filter() does not return the string with a leading space, and > this is handled by a leading space outside of the helper. This > workaround, however, has its limitations: the leading space is always > added. To avoid any potential issues, use .strip() when dereferencing > TUNE_CCARGS_MFPU in the if statements. > > Signed-off-by: João Marcos Costa <joaomarcos.costa@bootlin.com> > --- > meta/conf/machine/include/arm/feature-arm-neon.inc | 8 ++++---- > meta/conf/machine/include/arm/feature-arm-vfp.inc | 8 ++++---- > 2 files changed, 8 insertions(+), 8 deletions(-) We've taken the other patches but not this one since we don't think this one is a net gain in readability or usability and there is no pressing reason we have to change. Cheers, Richard
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..22cd5e1767 100644 --- a/meta/conf/machine/include/arm/feature-arm-vfp.inc +++ b/meta/conf/machine/include/arm/feature-arm-vfp.inc @@ -3,14 +3,14 @@ # 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 ''}" +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 ''}" TUNEVALID[callconvention-hard] = "Enable EABI hard float call convention, requires VFP." -TUNE_CCARGS_MFLOAT = "${@ bb.utils.contains('TUNE_FEATURES', 'callconvention-hard', 'hard', 'softfp', d) if (d.getVar('TUNE_CCARGS_MFPU') != '' or bb.utils.contains('TUNE_FEATURES', 'simd', True, False, d)) else '' }" +TUNE_CCARGS_MFLOAT = "${@ bb.utils.contains('TUNE_FEATURES', 'callconvention-hard', 'hard', 'softfp', d) if (d.getVar('TUNE_CCARGS_MFPU').strip() != '' or bb.utils.contains('TUNE_FEATURES', 'simd', True, False, d)) else '' }" TUNE_CCARGS .= "${@ ' -mfloat-abi=${TUNE_CCARGS_MFLOAT}' if (d.getVar('TUNE_CCARGS_MFLOAT') != '') else ''}" ARMPKGSFX_EABI = "${@ 'hf' if (d.getVar('TUNE_CCARGS_MFLOAT') == 'hard') else ''}"
Some 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)". bb.utils.filter() does not return the string with a leading space, and this is handled by a leading space outside of the helper. This workaround, however, has its limitations: the leading space is always added. To avoid any potential issues, use .strip() when dereferencing TUNE_CCARGS_MFPU in the if statements. Signed-off-by: João Marcos Costa <joaomarcos.costa@bootlin.com> --- meta/conf/machine/include/arm/feature-arm-neon.inc | 8 ++++---- meta/conf/machine/include/arm/feature-arm-vfp.inc | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-)