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

[Joao Marcos Costa <joaomarcos.costa@bootlin.com>]

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