public inbox for openembedded-core@lists.openembedded.org
 help / color / mirror / Atom feed
From: Patrick Ohly <patrick.ohly@intel.com>
To: Joshua Watt <jpewhacker@gmail.com>
Cc: openembedded-core@lists.openembedded.org
Subject: Re: [PATCH 0/2] Yocto Compatible 2.0 support code
Date: Fri, 09 Jun 2017 16:11:55 +0200	[thread overview]
Message-ID: <1497017515.30163.193.camel@intel.com> (raw)
In-Reply-To: <1497016077.3131.5.camel@gmail.com>

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.





  reply	other threads:[~2017-06-09 14:11 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-06-07 15:31 [PATCH 0/2] Yocto Compatible 2.0 support code Patrick Ohly
2017-06-07 15:31 ` [PATCH 1/2] bitbake.conf: DISTRO_FEATURES as overrides Patrick Ohly
2017-06-07 16:11   ` Peter Kjellerstedt
2017-06-08  6:04     ` Patrick Ohly
2017-06-08 10:45       ` Richard Purdie
2017-06-08 13:16         ` Peter Kjellerstedt
2017-06-08 14:38           ` Patrick Ohly
2017-06-07 15:31 ` [PATCH 2/2] utils.py: helper function for optional include files Patrick Ohly
2017-06-08  9:20   ` Richard Purdie
2017-06-08 14:36     ` Patrick Ohly
2017-06-09 10:02       ` Richard Purdie
2017-06-07 15:43 ` [PATCH 0/2] Yocto Compatible 2.0 support code Joshua Watt
2017-06-08  8:56   ` Richard Purdie
2017-06-08 13:55     ` Joshua Watt
2017-06-08 14:33       ` Richard Purdie
2017-06-08 14:48         ` Patrick Ohly
2017-06-08 15:28         ` Joshua Watt
2017-06-08 19:31           ` Patrick Ohly
2017-06-09  8:12             ` Patrick Ohly
2017-06-09 13:47               ` Joshua Watt
2017-06-09 14:11                 ` Patrick Ohly [this message]
2017-06-09 14:24                   ` Patrick Ohly
2017-08-24  9:27               ` Patrick Ohly
2017-06-09 13:50             ` Joshua Watt
2017-06-09 14:04               ` Patrick Ohly
2017-06-09 13:04 ` [PATCH v2 " Patrick Ohly
2017-06-09 13:04   ` [PATCH v2 1/2] bitbake.conf: DISTRO_FEATURES as overrides Patrick Ohly
2017-06-12 19:46     ` Denys Dmytriyenko
2017-06-12 21:05       ` Patrick Ohly
2017-06-12 23:23         ` Denys Dmytriyenko
2017-06-13  7:14           ` Patrick Ohly
2017-06-13  8:06             ` Richard Purdie
2017-06-13  8:31             ` Patrick Ohly
2017-06-14 10:32             ` Patrick Ohly
2017-06-14 10:33               ` [PATCH 1/2] Revert "bitbake.conf: DISTRO_FEATURES as overrides" Patrick Ohly
2017-06-14 10:33                 ` [PATCH 2/2] distrooverrides.bbclass: DISTRO_FEATURES as overrides Patrick Ohly
2017-06-09 13:04   ` [PATCH v2 2/2] utils.py: helper function for optional include files Patrick Ohly
2017-06-11 18:47   ` [PATCH v2 0/2] Yocto Compatible 2.0 support code Denys Dmytriyenko
2017-06-12  6:22     ` Patrick Ohly
2017-06-12 15:32       ` Denys Dmytriyenko
2017-06-14 11:01   ` ✗ patchtest: failure for "[v2] bitbake.conf: DISTRO_FEAT..." and 1 more (rev2) Patchwork
2017-06-14 11:01   ` ✗ patchtest: failure for "[v2] bitbake.conf: DISTRO_FEAT..." and 1 more (rev3) Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1497017515.30163.193.camel@intel.com \
    --to=patrick.ohly@intel.com \
    --cc=jpewhacker@gmail.com \
    --cc=openembedded-core@lists.openembedded.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox