Re: [bitbake-devel] [PATCH 1/2] bitbake: data_smart: fix ??= operator for getVarFlags

[Louis Rannou <louis.rannou@syslinbit.com>]

Hello,

Ok for the solution which is indeed prettier. I'll add a test with 
internalflags.

I also agree the test is not exactly at the right place as it does not 
test parsing but the result of getVarFlags. Perhaps it would fit better 
in tests/data.py ? (if that's ok to make a call to bb.parse there)

Thanks,
Louis

On 12/02/2025 15:13, Richard Purdie wrote:
> On Wed, 2025-02-12 at 13:16 +0100, Louis Rannou via lists.openembedded.org wrote:
>> From: Louis Rannou <louis.rannou@non.se.com>
>>
>> Variable flags have been fixed in commit
>> 0329a7e3ac694737f2d2c1861f65492551360663 which introduces the
>> "_defaultval_flag_" prefix for default values. This must not be ignored as
>> others "_"-prefixed flags names.
>>
>> Split the processing of default values to be sure they are overwritted by the
>> others operators disregarding of their order.
>>
>> Fixes [YOCTO #15685]
>>
>> Signed-off-by: Louis Rannou <louis.rannou@non.se.com>
>> ---
>>   lib/bb/data_smart.py | 16 +++++++++++++---
>>   1 file changed, 13 insertions(+), 3 deletions(-)
>>
>> diff --git a/lib/bb/data_smart.py b/lib/bb/data_smart.py
>> index 897ceeb32c7ce0acb8ed44c25e1bf2f2f28aa9dc..7663f28c5071a77c8452c1016e5ee0d969c84bec 100644
>> --- a/lib/bb/data_smart.py
>> +++ b/lib/bb/data_smart.py
>> @@ -952,12 +952,22 @@ class DataSmart(MutableMapping):
>>       def getVarFlags(self, var, expand = False, internalflags=False):
>>           local_var = self._findVar(var)
>>           flags = {}
>> +        flags_set = {}
>>   
>>           if local_var:
>> -            for i in local_var:
>> -                if i.startswith(("_", ":")) and not internalflags:
>> +            for i, val in local_var.items():
>> +                if i.startswith("_defaultval_flag_"):
>> +                    i = i[len("_defaultval_flag_"):]
>> +                    flags[i] = val
>> +                elif i.startswith(("_", ":")) and not internalflags:
>>                       continue
>> -                flags[i] = local_var[i]
>> +                else:
>> +                    flags_set[i] = val
>> +
>> +            # Flags sets take over defaults
>> +            flags.update(flags_set)
>> +
>> +            for i in flags:
>>                   if expand and i in expand:
>>                       flags[i] = self.expand(flags[i], var + "[" + i + "]")
>>           if len(flags) == 0:
> 
> I did spend more time looking at this and I had misunderstood the
> problem. I think this can still be simplified though and I also suspect
> the internalflags handling above isn't correct. If internalflags is
> set, I think it probably should return _defaultval_flag_ values
> unchanged. The code would therefore look more like:
> 
> 
> for i, val in local_var.items():
>      if i.startswith("_defaultval_flag_") and not internalflags:
>          i = i[len("_defaultval_flag_"):]
>          if i not in local_var:
>              flags[i] = val
>      elif i.startswith(("_", ":")) and not internalflags:
>          continue
>      else:
>          flags_set[i] = val
> 
> I'd also add a comment to the test case explaining that we're testing
> weak default make it to the getVarFlags output.
> 
> Cheers,
> 
> Richard