Re: [PATCH 0/2] Yocto Compatible 2.0 support code

Richard Purdie
2017-06-08T14:33:26Z

On Thu, 2017-06-08 at 08:55 -0500, Joshua Watt wrote:
> On Thu, 2017-06-08 at 09:56 +0100, Richard Purdie wrote:
> >
> > On Wed, 2017-06-07 at 10:43 -0500, Joshua Watt wrote:
> > >
> > > On Wed, 2017-06-07 at 17:31 +0200, Patrick Ohly wrote:
> > > >
> > > >
> > > > As discussed in the "[Openembedded-architecture] Yocto
> > > > Compatible
> > > > 2.0
> > > > + signature changes" mail thread, changes in a .bbappend cannot
> > > > be
> > > > done unconditionally. Making _append and _remove depend on
> > > > overrides
> > > > which get set based on DISTRO_FEATURES is one way of achieving
> > > > this.
> > > >
> > > > The oe.utils.optional_includes() helper function has not been
> > > > discussed before. It's an attempt to address concerns by
> > > > developers
> > > > that having to write code for (potentially complex) condition
> > > > checking
> > > > is error prone and hard to read.
> > > I promise I'm not trying to start a flame war here, and perhaps
> > > there
> > > is history behind this that I'm not aware of but...
> > >
> > > Why doesn't bitbake support some sort of "if" statement? It seems
> > > like most of what we are trying to do could be accomplished with
> > > much
> > > less fuss if one could simply do this in the bb file:
> > >
> > >  if bb.utils.contains('DISTRO_FEATURES', 'my-feature', d):
> > >     include foo.inc
> > This wouldn't actually solve as much of the problem as you think it
> > might at first glance and probably causes others, at least as I
> > understand it (as someone who's worked on bitbake's override code).
> >
> > For example, at what point does this get evaluated? Most bitbake
> > variables are expanded at usage time, not parse time but here, the
> > way
> > the parser works today, it would have to do an immediate expansion
> > of
> > DISTRO_FEATURES to decide whether to include this file (or code
> > block).
> Doesn't this same argument apply to doing a conditional include of a
> file? When bitbake goes to resolve the file name while evaluating the
> AST, it has to evaluate DISTRO_FEATURES which might not be complete.
> If
> the conditional in an "if" statement were also evaluated when
> evaluating the AST, I believe the following snipet:
>
>  require ${@ oe.utils.optional_includes(d, 'foo-feature:bar.inc') }
>
> Would be (functionally) identical to something (sort of) like:
>
>  if oe.utils.optional_includes(d, 'foo-feature:True'):
>      <All of the content of bar.inc>
>
> Without requiring splitting the recipe content up into multiple
> files.

I did say the problem applied to the require syntax, yes.

Put another way, my big worry is that the if syntax will make people
start to want if syntax for things other than include style operations
and try and do other things other than "inclusion" type work with it.
We'll also need to then start dealing with nesting and most likely
other complications as well as pushing us to having to deal with
immediate expansion problems.

One of the strengths of the current syntax we have to day is that it
makes most things possible but does try and encourage you to do things
"the right way". In adding an if syntax like this I suspect we're on a
path which won't lead to a good place. I appreciate this isn't an exact
science answer :/.

To recap on how we get here, there is a problem of selective content
inclusion in distros/layers. Right now you tend to have to buy into
everything in a layer or nothing. This is bad for usability and
adoption of components in layers. Sometimes its not practical to
separate everything into isolated layers.

We've therefore tried to come up with a way of handling this adding
minimal changes but allowing the configuration we need.

We do need to try and limit the scope of the usage of this as there is
a fundamental issue, namely immediate expansion. I know most users will
not realise there is even a problem with this.

*If* we limit the scope to DISTRO_FEATURES, we stand a reasonable
chance of being able to limit the occasions a user runs into this.

On the other hand, if we add a generic if syntax, encourage usage of
any variable and so on I think we're setting ourselves up for failure.

Patrick for example mentioned IMAGE_FEATURES. This one is fraught with
problems since:

a) Its a recipe level setting so using it in a base configuration
context would end badly

b) Users change this in a variety of places some of which would be
bitten by the immediate expansion problem even just in recipe context

So no, I really don't like the idea of the if syntax, attractive as it
may look at first.

Cheers,

Richard