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

Richard Purdie
2017-06-08T08:57:01Z

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).

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.

> 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.

Cheers,

Richard