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

[ChenQi <Qi.Chen@windriver.com>]

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