Re: [PATCH] RFC: features.bbclass,bitbake.conf: use features lists

["'Larson, Chris' via isar-users" <isar-users@googlegroups.com>]

On 11/14/2024 9:23 AM, 'Jan Kiszka' via isar-users wrote:
> On 11.11.24 17:51, Christopher Larson wrote:
>> Any thoughts on something like this? Too much?
>>
>> The intention is that downstreams would use standard bitbake functions
>> to check features, the same way they do in the Yocto Project, which
>> ensures that signatures and variable checksums never include the
>> entirety of the features variable, only the presence or absence of the
>> feature being checked.
>>
>> This also adds bits to allow one to disable a feature without having
>> to use the problematic :remove mechanism, which can't be undone by
>> downstream layers easily. In this case, MY_FEATURES += "feature-a" and
>> then MY_FEATURES += "-feature-a" would result in removing it. Order
>
> Is that "-feature" feature something upstream OE has as well? Or how do
> they handle that use case?

This isn't an upstream feature, though ideally it should be. Most just 
directly use :remove when necessary, but it does end up adding 
complications for downstreams unless they use intermediate variables, 
and even then you end up with workarounds like 
DISTRO_FEATURES_REMOVE:remove = "somefeature" to undo its removal, which 
is confusing :)

>> matters, so it can be re-added again later on without complication. It
>> also adds a minimal mechanism to let one feature pull in others
>> implicitly, which helps to avoid duplication when a feature is a
>> superset of another.
>
> Same question: Would that syntax or even the logic isar-specific or in
> line with OE?

This is also not present in OE at this time, though OE has similar 
functionality, implemented in similar ways, for example, the support for 
dependencies amongst image compression/conversion tools.

As these two are enhancements, not regressions, I don't think there 
would be any issue with someone migrating from OE to isar, only perhaps 
in the other direction. That said, I have no objection to pursuing their 
submission in OE-Core also.

> BTW, missing signed-off, but I that might be because of "RFC".

Indeed!

>> diff --git a/meta/classes/features.bbclass 
b/meta/classes/features.bbclass
>> new file mode 100644
>> index 00000000..32964c91
>> --- /dev/null
>> +++ b/meta/classes/features.bbclass
>> @@ -0,0 +1,91 @@
>
> Missing license/copyright header.

I'll fix that when I submit the non-RFC patch, thanks.

>> +# Functions to manipulate feature lists
>> +#
>> +# This class provides functions to manipulate feature lists. The 
functions
>> +# are intended to be used in :remove and :append handlers to remove 
or append
>> +# items to a variable. The remove will interpret items prefixed 
with a '-'
>> +# as items to be removed. The append function will rely on a mapping of
>> +# implied features to append the corresponding items. The intention 
is to
>> +# provide the ability for a feature to imply enabling of other 
features,
>> +# much like a dependency, but without the implication of order.
>> +#
>> +# Usage example:
>> +#
>> +#  IMAGE_FEATURES ??= ""
>> +#  IMAGE_FEATURES:remove = 
"${@remove_prefixed_features('IMAGE_FEATURES', d)}"
>> +#  IMAGE_FEATURES:prepend =
>> "${@add_implied_features('IMAGE_FEATURES', 'IMPLIED_IMAGE_FEATURES',
>> d)} "
>> +#  IMAGE_FEATURES_DISABLED = "${@' '.join(f for f in
>> remove_prefixed_features('IMAGE_FEATURES', d).split() if not
>> f.startswith('-'))}"
>
> What's the purpose of the last line? Or is it a stale example?

Remnant line, thanks.

> And then these three or four lines are kind of boilerplate for every
> feature group one would like to create? Would there be a way to template
> that with the feature variable name as parameter?

That's an excellent question, and one which I considered as well, but 
forgot to mention in the email and commit.

A big part of why I implemented these features as inline python in 
:remove and :append was to ensure that there are no issues with 
order-of-operations in the metadata, and to ensure that we don't break 
the bitbake checksumming for lists of items like features.

BitBake has special handling of the `contains` functions to ensure that 
a caller only depends on the presence or absence of the feature they 
care about, rather than depending on the entirety of the feature 
variable, as that would lead to very brittle checksums, and reduced 
shared state usage. If one was to provide wrapper functions to check for 
a feature and implement this logic there, it would bypass bitbake's 
handling in this regard.

The other option was to implement the removal and prepend at some 
specific point, for example, in an event handler (i.e. ConfigParsed, 
BuildStarted), or anonymous python. The problem with that has to due to 
with order of operations, as the behavior would be incorrect if one checks
for a feature before this code runs. Any code to 
automatically set the flags based on a list of supported features 
variables would run into the same issue. The bitbake file format is 
already rather prone to issues relating to when operations happen, so 
setting the flags immediately, and having them operate when the variable 
is checked, seemed like a reasonable approach.

There's an argument to be made that no one should be checking for 
features prior to ConfigParsed anyway, as we don't know if a later 
config file will add or remove a feature, so combining a contains() 
with, say, :=, would be error prone to begin with. This is a valid 
argument in favor of applying the templating at ConfigParsed-time, but I 
figured it was clearer to be explicit here, even if it led to a small 
amount of redundancy, rather than adding more order-dependent logic, 
particularly given the small number of supported features variables.

I'd certainly like to hear opinions on this, though. Thoughts?

> BTW, patch was mangled by your email client.

Ah, apparently using git imap-send to tweak the email before sending 
still ended up mangling it. Apologies. I'll use format-patch, tweak, and 
then send-email instead.

>> +#
>> +#  IMPLIED_IMAGE_FEATURES[alpha] = "beta"
>
> So, if I add "alpha" to IMAGE_FEATURES, I will also get "beta" as
> feature, right?

Exactly, yes.

> Looks like it could be useful. I would just like to ensure that we are
> not creating unneeded surprises for those coming from/going to OE Yocto.
> And bonus for a template-based setup.

Understood, thanks for the feedback. If anyone has any ideas on how to 
pull off the templating either without adding order-dependent 
operations, or feels strongly that it's worth doing so, I'd love to 
discuss that. Would you feel better if I pursue submission to oe-core first?
-- 
Christopher Larson
Siemens AG
www.siemens.com

-- 
You received this message because you are subscribed to the Google Groups "isar-users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to isar-users+unsubscribe@googlegroups.com.
To view this discussion visit https://groups.google.com/d/msgid/isar-users/BN0PR07MB837594CAD54725861274EC729F202%40BN0PR07MB8375.namprd07.prod.outlook.com.