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

ChenQi
2026-04-27T10:06:09Z

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!
>