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

Joshua Watt
2017-06-09T13:50:23Z

On Thu, 2017-06-08 at 21:31 +0200, Patrick Ohly wrote:
> On Thu, 2017-06-08 at 10:28 -0500, Joshua Watt wrote:
> > Sure. I wouldn't suggest using an if statement for "just anything",
> > you
> > can surely do terrible things that way. It would (by convention) be
> > restricted to the same sorts of things that the conditional
> > includes
> > allow now. On a similar token, you can do the same sorts of
> > terrible
> > things with conditional includes as currently proposed because it
> > has
> > the same enforcement policy (i.e. "by convention").
>
> I'm starting to wonder whether this "convention" can be strengthened
> with additional warnings. The code which currently evaluates the
> include
> parameter could record in the datastore the original expression and
> what
> it evaluated to, then later when the recipe gets finalized, an event
> handler can check whether evaluating the expression still gives the
> same
> result.
>
> This would also be useful for "inherit". I remember struggling to
> understand why certain image type classes kept getting inherited
> despite
> changing IMAGE_FSTYPES - it turned out, that change had to be made
> earlier.
>
> That's neither an argument for nor against the "if" check - the same
> could be done for that. Just something that occurred to me.
>
> > On the other hand, perhaps the range of terrible things that can be
> > done extends to more than just how you conditionally include
> > something.
> > *What* is conditionally included might also require some scrutiny.
> > As
> > you have alluded to, overrides are probably the best option for
> > variables, so putting them in a conditional include file is
> > probably
> > not ideal. Forcing people to move the things that have to be
> > conditional to a separate file might actually be detrimental in a
> > number of ways:
> >  1) It might encourage recipe writers to do more in the include
> > file
> > than they maybe should so that they don't have to make a plethora
> > of
> > files.
> >  2) It might make it harder to verify that what the recipe writers
> > did
> > is correct since the context of what they are doing is removed from
> > the
> > parent recipe.
> >
> > IIRC the conditional syntax (if or conditional include) is really
> > mostly needed for the parts of bitbake that don't allow overrides
> > (addtask and such). If that is the desired restriction, it would
> > not be
> > difficult to have bitbake enforce that by only allowing the subset
> > of
> > things that don't support overrides to be in the body of a if
> > statement. This would be more difficult with conditional includes
> > unless some other bitbake syntax was added.
>
> There's some truth to that IMHO, but I'm uncertain whether it
> warrants
> introducing entirely new syntax. In refkit, I only ran into one
> particular case were an include file was necessary.

I'd be curious to see that. How big was the .inc file?

>
> > If that's the consensus, than I'm fine with that. From my
> > perspective,
> > conditional includes are just another (more difficult to use) form
> > of
> > an "if" statement, and making it difficult to do things
> > conditionally
> > doesn't necessarily make it better for anyone.
>
> Making it hard sends the message that it shouldn't be used lightly.
> Documentation will have to make clear that conditional includes are
> the
> last resort when everything else isn't usable.
>