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

[Joshua Watt <jpewhacker@gmail.com>]

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.


> So ok, lets assume we change bitbake massively and defer the
> expansion
> somehow. What if foo.inc influences the contents of DISTRO_FEATURES?
> Should it then "unparse" foo.inc if my-feature was removed? or error?
> or silently ignore that?
> 
> bitbake's main conditional today is through overrides and these do
> allow a controlled delayed expansion of metadata in most cases. In
> some
> cases such as include and inherit statements there is still the
> immediate expansion issue above but at least there aren't huge
> changes
> to the parser required to make it work so its the best of both
> worlds.

I was curious as to what it would it would actually take to make "if"
statements like the one I described above work (and I wanted to learn
more about the bitbake internals), so I did a proof of concept on
GitHub:
https://github.com/JPEWdev/poky/commit/998a00f122154bb509d22b412fba0773
97f6e433

It's actually not particularly terrible IMHO, but I'm sure it could be
better.

I can repost it to the bitbake mailing list as an RFC if you think that
would be helpful.

> 
> > One could even eliminate the separate inc file and simply put its
> > contents under the conditional (as much fun as it seems to have to
> > open
> > a new file just to see what a recipe is doing with a distro
> > feature...)
> > 
> > It would also appear that this could make a lot of other things
> > simpler as well (and may even negate the need to backfill
> > DISTRO_FEATURES into overrides?)
> 
> See if the above gives food for thought on that...
> 
> The big problems are the corner cases. If we do add new syntax it
> needs
> to avoid these as we already have some pretty nasty ones, thankfully
> most people don't hit them though.

That's fine. I"m not particularly trying to say that an "if" statement
is the magic bullet for corner cases, but I think it is equivalent
functionality to conditional includes and more readable and
maintainable for people writing recipes. Maybe that means
DISTRO_FEATURES still need to become OVERRIDES, IDK.

> 
> Cheers,
> 
> Richard