[OE-core] [PATCH] base.bbclass: Add COMPATIBLE_DISTRO_FEATURES support

[Otavio Salvador]

On Tue, May 28, 2013 at 12:12 PM, Phil Blundell <pb at pbcl.net> wrote:

> On Tue, 2013-05-28 at 12:04 -0300, Otavio Salvador wrote:
> > This add support to list compatible distro feature for a recipe; this
> > is important for example when you have two different recipes which
> > should be choosen depending on the distro features set.
>
> The terminology used here seems to be wrong/misleading/confusing:
>
> > +        need_distro_features = d.getVar('COMPATIBLE_DISTRO_FEATURES',
> True)
> > +        if need_distro_features:
> > +            need_distro_features = need_distro_features.split()
> > +            distro_features = (d.getVar('DISTRO_FEATURES', True) or
> "").split()
> > +            for f in need_distro_features:
> > +                if f in distro_features:
> > +                    bb.note("here")
> > +                    break
> > +            else:
> > +                raise bb.parse.SkipPackage("incompatible with distro
> features %s (not in DISTRO_FEATURES)" % need_distro_features)
>
> Calling the variable "COMPATIBLE_DISTRO_FEATURES" implies that the
> recipe won't work with any features not in the list.  But the actual
> implementation shown above is that the recipe will be skipped unless all
> of the features listed are present in DISTRO_FEATURES.  If those are the
> intended semantics then it seems as though the variable ought to be
> named REQUIRED_DISTRO_FEATURES or something.
>

Done.


> Similarly, the error message "incompatible with distro features %s (not
> in DISTRO_FEATURES)" is, at best, confusing.  This message seems to be
> staying that the recipe won't work if the named features are set, but it
> then goes on to say that they are not in fact set.  Plus, it lists the
> entire contents of COMPATIBLE_DISTRO_FEATURES rather than just the items
> that are causing a problem.
>

Done.


> Also, you seem to have a spurious bb.note() call in there.
>

Arrg, done.


> Also also, we've managed without this functionality in oe-core for some
> time which makes me wonder how widely useful it would actually be.  If
> it's only going to be used by a few recipes then it could go in a class,
> or in in the recipes themselves, rather than adding parse time and
> memory footprint to every recipe.
>

Right; let's first agree in the code so we can discuss where to put it:

+        need_distro_features = d.getVar('REQUIRED_DISTRO_FEATURES', True)
+        if need_distro_features:
+            need_distro_features = need_distro_features.split()
+            distro_features = (d.getVar('DISTRO_FEATURES', True) or
"").split()
+            for f in need_distro_features:
+                if f in distro_features:
+                    break
+            else:
+                raise bb.parse.SkipPackage("missing required distro
feature %s (not in DISTRO_FEATURES)" % need_distro_features)



-- 
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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openembedded.org/pipermail/openembedded-core/attachments/20130528/bcd8b510/attachment-0002.html>