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

Patrick Ohly
2017-06-09T14:11:58Z

On Fri, 2017-06-09 at 08:47 -0500, Joshua Watt wrote:
> On Fri, 2017-06-09 at 10:12 +0200, Patrick Ohly wrote:
> > 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.
> >
> > Below is a quick-and-dirty proof of concept. Example bmap-tools_
> > %.bbappend:
> >
> > FOO = "bmap-tools-deploy.inc"
> > require ${FOO}
> > FOO = ""
> >
> > Example warning:
> >
> > WARNING: .../bmap-tools/bmap-tools_3.2.bb: .../bmap-
> > tools_%.bbappend:2: include/require/inherit "${FOO}" resulted in
> > including "bmap-tools-deploy.inc" while parsing. Variables effecting
> > the parameter changed later such that nothing would have been
> > included at the end of the recipe.
>
> I like it. The error message could maybe use a little bit of work...
> Maybe it needs some sort of short and unique key words in it, like
> "conditional invariance violation" to help direct people who copy the
> error into google when looking for an answer (or slightly better, grep
> the Mega Manual).

When doing it as sanity.bbclass check it would have a short ID string.

> > Should this warning be entirely in bitbake or should it become part
> > of
> > OE's sanity.bbclass? The latter would have the advantage that it
> > could
> > be configured as fatal error. The downside is that bitbake needs to
> > export data, which implies adding a semi-stable API.
>
> A fatal error would be nice.... anything that was 2.0 compatible should
> be fatal, but I don't know if you would want it to make it fatal for
> non-2.0 compatible (i.e. existing) layers, as it might break some
> existing uses (or maybe we don't care about that?).

It's independent of Yocto Compatible 2.0. The image type inheritance
issue I mentioned is a basic usability problem, and this TARGET_SYS
dependency looks like a genuine bug to me.

> > Do we want some suppression mechanism? Something like:
> >
> > require ${PARSE_TIME_EVALUATION}${FOO}
> >
> > where PARSE_TIME_EVALUATION is unset, but can be checked for in the
> > code
> > that normally would print the warning?
>
> Is there a currently known valid use for such a thing?

Using LAYERDIR is one, although that particular example is probably
better caught by looking for that particular variable instead of
annotating all places where it gets used.

I'm unsure whether there are valid uses. I'm just worried that by the
time someone shows up with one, it will be too late to add a suppression
mechanism (as in "I've just updated from 2.3 to 2.4, how do I disable
this warning?").

> I'd just as soon
> add it once we know it is needed (otherwise, the top search result for
> "conditional invariance violation" will end up being some blog that
> says "Just prefix it with ${PARSE_TIME_EVALUATION} and everything will
> be fine" :)

I would add such an explanation to the error message itself and warn
about using it, then hopefully no-one writes such stupid blog posts ;-}

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