[PATCH v3] distro_features_check.bbclass: Allow checking of required/conflicting features

[PATCH v3] distro_features_check.bbclass: Allow checking of required/conflicting features

[Otavio Salvador]

This add support to list required/confliting distro features for a
recipe; this avoids user mistake when building recipes/images which
would not work depending on DISTRO_FEATURES option set.

Adding:

,----[ Use example ]
| inherit distro_features_check
|
| REQUIRED_DISTRO_FEATURES = "x11"
| CONFLICT_DISTRO_FEATURES_mx6 = "wayland"
`----

In the image recipe allow us to make clear to user that this image
needs X11 and /cannot/ be build with Wayland support in i.MX6
platforms, for example.

Signed-off-by: Otavio Salvador <otavio@ossystems.com.br>
---
Changes in v3:
- Move code from base.bbclass to distro_features_check.bbclass
- Rework code
- Rework commit log

Changes in v2:
- Rename to REQUIRED_DISTRO_FEATURES;
- Remove spurious bb.note;
- Rewrote error message;

 meta/classes/distro_features_check.bbclass | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)
 create mode 100644 meta/classes/distro_features_check.bbclass

diff --git a/meta/classes/distro_features_check.bbclass b/meta/classes/distro_features_check.bbclass
new file mode 100644
index 0000000..61b11b7
--- /dev/null
+++ b/meta/classes/distro_features_check.bbclass
@@ -0,0 +1,28 @@
+# Allow checking of required and conflicting DISTRO_FEATURES
+#
+# REQUIRED_DISTRO_FEATURES: ensure every item on this list is included
+#                           in DISTRO_FEATURES.
+# CONFLICT_DISTRO_FEATURES: ensure no item in this list is included in
+#                           DISTRO_FEATURES.
+#
+# Copyright 2013 (C) O.S. Systems Software LTDA.
+
+python () {
+    required_distro_features = d.getVar('REQUIRED_DISTRO_FEATURES', True)
+    if required_distro_features:
+        required_distro_features = required_distro_features.split()
+        distro_features = (d.getVar('DISTRO_FEATURES', True) or "").split()
+        for f in required_distro_features:
+            if f in distro_features:
+                break
+        else:
+            raise bb.parse.SkipPackage("missing required distro feature %s (not in DISTRO_FEATURES)" % required_distro_features)
+
+    conflict_distro_features = d.getVar('CONFLICT_DISTRO_FEATURES', True)
+    if conflict_distro_features:
+        conflict_distro_features = conflict_distro_features.split()
+        distro_features = (d.getVar('DISTRO_FEATURES', True) or "").split()
+        for f in conflict_distro_features:
+            if f in distro_features:
+                raise bb.parse.SkipPackage("conflicting distro feature %s (in DISTRO_FEATURES)" % conflict_distro_features)
+}
-- 
1.8.3.2

Re: [PATCH v3] distro_features_check.bbclass: Allow checking of required/conflicting features

[Chris Larson]

[-- Attachment #1: Type: text/plain, Size: 864 bytes --]

On Thu, Aug 1, 2013 at 3:13 PM, Otavio Salvador <otavio@ossystems.com.br>wrote:

> This add support to list required/confliting distro features for a
> recipe; this avoids user mistake when building recipes/images which
> would not work depending on DISTRO_FEATURES option set.
>
> Adding:
>
> ,----[ Use example ]
> | inherit distro_features_check
> |
> | REQUIRED_DISTRO_FEATURES = "x11"
> | CONFLICT_DISTRO_FEATURES_mx6 = "wayland"
>

I wonder if it would be worth doing this like PNBLACKLIST to let the recipe
provide its reasoning as a message, or if that would add too much overhead.
I don't know enough about this use case, but something to consider if it
hasn't been already.
-- 
Christopher Larson
clarson at kergoth dot com
Founder - BitBake, OpenEmbedded, OpenZaurus
Maintainer - Tslib
Senior Software Engineer, Mentor Graphics

[-- Attachment #2: Type: text/html, Size: 1278 bytes --]

Re: [PATCH v3] distro_features_check.bbclass: Allow checking of required/conflicting features

[Otavio Salvador]

On Fri, Aug 2, 2013 at 6:16 PM, Chris Larson <clarson@kergoth.com> wrote:
> On Thu, Aug 1, 2013 at 3:13 PM, Otavio Salvador <otavio@ossystems.com.br>
> wrote:
>>
>> This add support to list required/confliting distro features for a
>> recipe; this avoids user mistake when building recipes/images which
>> would not work depending on DISTRO_FEATURES option set.
>>
>> Adding:
>>
>> ,----[ Use example ]
>> | inherit distro_features_check
>> |
>> | REQUIRED_DISTRO_FEATURES = "x11"
>> | CONFLICT_DISTRO_FEATURES_mx6 = "wayland"
>
>
> I wonder if it would be worth doing this like PNBLACKLIST to let the recipe
> provide its reasoning as a message, or if that would add too much overhead.
> I don't know enough about this use case, but something to consider if it
> hasn't been already.

I did a look now on PNBLACKLIST and while I see the value of a custom
message I didn't come up with a way to do it using overrides as well.
So, as the example in the commit log, it'd be harder to make a mx6
specific conflict.

CONFLICT_DISTRO_FEATURES[wayland]_mx6 = "Vivante GPU driver for
Wayland is incompatible with X11"

Would the above work with current BitBake code?

-- 
Otavio Salvador                             O.S. Systems
http://www.ossystems.com.br        http://projetos.ossystems.com.br
Mobile: +55 (53) 9981-7854            Mobile: +1 (347) 903-9750

Re: [PATCH v3] distro_features_check.bbclass: Allow checking of required/conflicting features

[Chris Larson]

[-- Attachment #1: Type: text/plain, Size: 1821 bytes --]

On Sat, Aug 3, 2013 at 9:55 AM, Otavio Salvador <otavio@ossystems.com.br>wrote:

> On Fri, Aug 2, 2013 at 6:16 PM, Chris Larson <clarson@kergoth.com> wrote:
> > On Thu, Aug 1, 2013 at 3:13 PM, Otavio Salvador <otavio@ossystems.com.br
> >
> > wrote:
> >>
> >> This add support to list required/confliting distro features for a
> >> recipe; this avoids user mistake when building recipes/images which
> >> would not work depending on DISTRO_FEATURES option set.
> >>
> >> Adding:
> >>
> >> ,----[ Use example ]
> >> | inherit distro_features_check
> >> |
> >> | REQUIRED_DISTRO_FEATURES = "x11"
> >> | CONFLICT_DISTRO_FEATURES_mx6 = "wayland"
> >
> >
> > I wonder if it would be worth doing this like PNBLACKLIST to let the
> recipe
> > provide its reasoning as a message, or if that would add too much
> overhead.
> > I don't know enough about this use case, but something to consider if it
> > hasn't been already.
>
> I did a look now on PNBLACKLIST and while I see the value of a custom
> message I didn't come up with a way to do it using overrides as well.
> So, as the example in the commit log, it'd be harder to make a mx6
> specific conflict.
>
> CONFLICT_DISTRO_FEATURES[wayland]_mx6 = "Vivante GPU driver for
> Wayland is incompatible with X11"
>
> Would the above work with current BitBake code?


That's a good point, unfortunately I don't think flags propagate for
overrides today :(. I think it would be something like
CONFLICT_DISTRO_FEATURES_mx6[wayland], but I don't think that folds into
the main variable when the overrides are applied (the semantics of that may
be unclear0. Oh well, nevermind :)
-- 
Christopher Larson
clarson at kergoth dot com
Founder - BitBake, OpenEmbedded, OpenZaurus
Maintainer - Tslib
Senior Software Engineer, Mentor Graphics

[-- Attachment #2: Type: text/html, Size: 2459 bytes --]

Re: [PATCH v3] distro_features_check.bbclass: Allow checking of required/conflicting features

[Otavio Salvador]

On Sat, Aug 3, 2013 at 3:26 PM, Chris Larson <clarson@kergoth.com> wrote:
>
> On Sat, Aug 3, 2013 at 9:55 AM, Otavio Salvador <otavio@ossystems.com.br>
> wrote:
>>
>> On Fri, Aug 2, 2013 at 6:16 PM, Chris Larson <clarson@kergoth.com> wrote:
>> > On Thu, Aug 1, 2013 at 3:13 PM, Otavio Salvador
>> > <otavio@ossystems.com.br>
>> > wrote:
>> >>
>> >> This add support to list required/confliting distro features for a
>> >> recipe; this avoids user mistake when building recipes/images which
>> >> would not work depending on DISTRO_FEATURES option set.
>> >>
>> >> Adding:
>> >>
>> >> ,----[ Use example ]
>> >> | inherit distro_features_check
>> >> |
>> >> | REQUIRED_DISTRO_FEATURES = "x11"
>> >> | CONFLICT_DISTRO_FEATURES_mx6 = "wayland"
>> >
>> >
>> > I wonder if it would be worth doing this like PNBLACKLIST to let the
>> > recipe
>> > provide its reasoning as a message, or if that would add too much
>> > overhead.
>> > I don't know enough about this use case, but something to consider if it
>> > hasn't been already.
>>
>> I did a look now on PNBLACKLIST and while I see the value of a custom
>> message I didn't come up with a way to do it using overrides as well.
>> So, as the example in the commit log, it'd be harder to make a mx6
>> specific conflict.
>>
>> CONFLICT_DISTRO_FEATURES[wayland]_mx6 = "Vivante GPU driver for
>> Wayland is incompatible with X11"
>>
>> Would the above work with current BitBake code?
>
>
> That's a good point, unfortunately I don't think flags propagate for
> overrides today :(. I think it would be something like
> CONFLICT_DISTRO_FEATURES_mx6[wayland], but I don't think that folds into the
> main variable when the overrides are applied (the semantics of that may be
> unclear0. Oh well, nevermind :)

Yes; or done using code to set it only for specific cases. So I think
it is better to let this as is and put a comment on the code where
using the class /why/ it conflicts/requires it.

-- 
Otavio Salvador                             O.S. Systems
http://www.ossystems.com.br        http://projetos.ossystems.com.br
Mobile: +55 (53) 9981-7854            Mobile: +1 (347) 903-9750

Re: [PATCH v3] distro_features_check.bbclass: Allow checking of required/conflicting features

[Kamble, Nitin A]

> -----Original Message-----
> From: openembedded-core-bounces@lists.openembedded.org
> [mailto:openembedded-core-bounces@lists.openembedded.org] On Behalf
> Of Otavio Salvador
> Sent: Thursday, August 01, 2013 3:14 PM
> To: OpenEmbedded Core Mailing List
> Cc: Evan Kotara; Lauren Post; Otavio Salvador; Daiane Angolini
> Subject: [OE-core] [PATCH v3] distro_features_check.bbclass: Allow checking
> of required/conflicting features
> 
> This add support to list required/confliting distro features for a recipe; this
> avoids user mistake when building recipes/images which would not work
> depending on DISTRO_FEATURES option set.
> 
> Adding:
> 
> ,----[ Use example ]
> | inherit distro_features_check
> |
> | REQUIRED_DISTRO_FEATURES = "x11"
> | CONFLICT_DISTRO_FEATURES_mx6 = "wayland"
> `----
> 
> In the image recipe allow us to make clear to user that this image needs X11
> and /cannot/ be build with Wayland support in i.MX6 platforms, for example.
> 
> Signed-off-by: Otavio Salvador <otavio@ossystems.com.br>
> ---
> Changes in v3:
> - Move code from base.bbclass to distro_features_check.bbclass
> - Rework code
> - Rework commit log
> 
> Changes in v2:
> - Rename to REQUIRED_DISTRO_FEATURES;
> - Remove spurious bb.note;
> - Rewrote error message;
> 
>  meta/classes/distro_features_check.bbclass | 28
> ++++++++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
>  create mode 100644 meta/classes/distro_features_check.bbclass
> 
> diff --git a/meta/classes/distro_features_check.bbclass
> b/meta/classes/distro_features_check.bbclass
> new file mode 100644
> index 0000000..61b11b7
> --- /dev/null
> +++ b/meta/classes/distro_features_check.bbclass
> @@ -0,0 +1,28 @@
> +# Allow checking of required and conflicting DISTRO_FEATURES # #
> +REQUIRED_DISTRO_FEATURES: ensure every item on this list is included
> +#                           in DISTRO_FEATURES.
> +# CONFLICT_DISTRO_FEATURES: ensure no item in this list is included in
> +#                           DISTRO_FEATURES.
> +#
> +# Copyright 2013 (C) O.S. Systems Software LTDA.
> +
> +python () {
> +    required_distro_features = d.getVar('REQUIRED_DISTRO_FEATURES',
> True)
> +    if required_distro_features:
> +        required_distro_features = required_distro_features.split()
> +        distro_features = (d.getVar('DISTRO_FEATURES', True) or "").split()
> +        for f in required_distro_features:
> +            if f in distro_features:
> +                break
> +        else:
> +            raise bb.parse.SkipPackage("missing required distro feature
> +%s (not in DISTRO_FEATURES)" % required_distro_features)


Hi Otavio,

In my observations, these SkipPackage exceptions do not show up on the screen, leaving the user wondering what happened.
Also the above logic can be simplified as

 # check all the required DISTRO_FEATURES are enabled
 distro_features_split = (d.getVar('DISTRO_FEATURES', True) or "").split()
 required_distro_features_split = (d.getVar('REQUIRED_DISTRO_FEATURES', True) or "").split()
 for rdf in required_distro_features_split:
      if rdf not in distro_features_split:
           bb.error / raise exception...

Thanks,
Nitin

> +
> +    conflict_distro_features = d.getVar('CONFLICT_DISTRO_FEATURES', True)
> +    if conflict_distro_features:
> +        conflict_distro_features = conflict_distro_features.split()
> +        distro_features = (d.getVar('DISTRO_FEATURES', True) or "").split()
> +        for f in conflict_distro_features:
> +            if f in distro_features:
> +                raise bb.parse.SkipPackage("conflicting distro feature
> +%s (in DISTRO_FEATURES)" % conflict_distro_features) }
> --
> 1.8.3.2
> 
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core

Re: [PATCH v3] distro_features_check.bbclass: Allow checking of required/conflicting features

[Kamble, Nitin A]

> -----Original Message-----
> From: Kamble, Nitin A
> Sent: Wednesday, August 21, 2013 10:14 AM
> To: 'Otavio Salvador'; OpenEmbedded Core Mailing List
> Cc: Evan Kotara; Lauren Post; Daiane Angolini
> Subject: RE: [OE-core] [PATCH v3] distro_features_check.bbclass: Allow
> checking of required/conflicting features
> 
> 
> 
> > -----Original Message-----
> > From: openembedded-core-bounces@lists.openembedded.org
> > [mailto:openembedded-core-bounces@lists.openembedded.org] On
> Behalf Of
> > Otavio Salvador
> > Sent: Thursday, August 01, 2013 3:14 PM
> > To: OpenEmbedded Core Mailing List
> > Cc: Evan Kotara; Lauren Post; Otavio Salvador; Daiane Angolini
> > Subject: [OE-core] [PATCH v3] distro_features_check.bbclass: Allow
> > checking of required/conflicting features
> >
> > This add support to list required/confliting distro features for a
> > recipe; this avoids user mistake when building recipes/images which
> > would not work depending on DISTRO_FEATURES option set.
> >
> > Adding:
> >
> > ,----[ Use example ]
> > | inherit distro_features_check
> > |
> > | REQUIRED_DISTRO_FEATURES = "x11"
> > | CONFLICT_DISTRO_FEATURES_mx6 = "wayland"
> > `----
> >
> > In the image recipe allow us to make clear to user that this image
> > needs X11 and /cannot/ be build with Wayland support in i.MX6 platforms,
> for example.
> >
> > Signed-off-by: Otavio Salvador <otavio@ossystems.com.br>
> > ---
> > Changes in v3:
> > - Move code from base.bbclass to distro_features_check.bbclass
> > - Rework code
> > - Rework commit log
> >
> > Changes in v2:
> > - Rename to REQUIRED_DISTRO_FEATURES;
> > - Remove spurious bb.note;
> > - Rewrote error message;
> >
> >  meta/classes/distro_features_check.bbclass | 28
> > ++++++++++++++++++++++++++++
> >  1 file changed, 28 insertions(+)
> >  create mode 100644 meta/classes/distro_features_check.bbclass
> >
> > diff --git a/meta/classes/distro_features_check.bbclass
> > b/meta/classes/distro_features_check.bbclass
> > new file mode 100644
> > index 0000000..61b11b7
> > --- /dev/null
> > +++ b/meta/classes/distro_features_check.bbclass
> > @@ -0,0 +1,28 @@
> > +# Allow checking of required and conflicting DISTRO_FEATURES # #
> > +REQUIRED_DISTRO_FEATURES: ensure every item on this list is included
> > +#                           in DISTRO_FEATURES.
> > +# CONFLICT_DISTRO_FEATURES: ensure no item in this list is included in
> > +#                           DISTRO_FEATURES.
> > +#
> > +# Copyright 2013 (C) O.S. Systems Software LTDA.
> > +
> > +python () {
> > +    required_distro_features = d.getVar('REQUIRED_DISTRO_FEATURES',
> > True)
> > +    if required_distro_features:
> > +        required_distro_features = required_distro_features.split()
> > +        distro_features = (d.getVar('DISTRO_FEATURES', True) or "").split()
> > +        for f in required_distro_features:
> > +            if f in distro_features:
> > +                break
> > +        else:
> > +            raise bb.parse.SkipPackage("missing required distro
> > +feature %s (not in DISTRO_FEATURES)" % required_distro_features)
> 
> 
> Hi Otavio,
> 
> In my observations, these SkipPackage exceptions do not show up on the
> screen, leaving the user wondering what happened.


With further debugging here, I found that after skipping the recipe it was picking
another version of the recipe. So if there is another recipe which can be used in place
of the skipped recipe, then no errors are reported on the screen. One way to solve this 
issue is to add REQUIRED_DISTRO_FEATURES to all these inter-replaceable recipes, which works
in my case, but it may not work for all kinds of recipes.

Thanks,
Nitin



> Also the above logic can be simplified as
> 
>  # check all the required DISTRO_FEATURES are enabled
> distro_features_split = (d.getVar('DISTRO_FEATURES', True) or "").split()
> required_distro_features_split = (d.getVar('REQUIRED_DISTRO_FEATURES',
> True) or "").split()  for rdf in required_distro_features_split:
>       if rdf not in distro_features_split:
>            bb.error / raise exception...
> 
> Thanks,
> Nitin
> 
> > +
> > +    conflict_distro_features = d.getVar('CONFLICT_DISTRO_FEATURES',
> True)
> > +    if conflict_distro_features:
> > +        conflict_distro_features = conflict_distro_features.split()
> > +        distro_features = (d.getVar('DISTRO_FEATURES', True) or "").split()
> > +        for f in conflict_distro_features:
> > +            if f in distro_features:
> > +                raise bb.parse.SkipPackage("conflicting distro
> > +feature %s (in DISTRO_FEATURES)" % conflict_distro_features) }
> > --
> > 1.8.3.2
> >
> > _______________________________________________
> > Openembedded-core mailing list
> > Openembedded-core@lists.openembedded.org
> > http://lists.openembedded.org/mailman/listinfo/openembedded-core