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

Patrick Ohly
2017-06-08T19:31:59Z

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.

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

--
Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.