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

login
register
mail settings
Submitter Otavio Salvador
Date Aug. 1, 2013, 10:13 p.m.
Message ID <1375395237-17840-1-git-send-email-otavio@ossystems.com.br>
Download mbox | patch
Permalink /patch/54937/
State Accepted
Commit 26e543cb2118ed6a668ad37e1fac149fa6db672e
Headers show

Comments

Otavio Salvador - Aug. 1, 2013, 10:13 p.m.
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
Chris Larson - Aug. 2, 2013, 9:16 p.m.
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.
Otavio Salvador - Aug. 3, 2013, 4:55 p.m.
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?
Chris Larson - Aug. 3, 2013, 6:26 p.m.
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 :)
Otavio Salvador - Aug. 3, 2013, 6:37 p.m.
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.
Nitin A Kamble - Aug. 21, 2013, 5:14 p.m.
> -----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
Nitin A Kamble - Aug. 21, 2013, 8:49 p.m.
> -----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

Patch

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)
+}