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: Thu, 08 Jun 2017 21:31:56 +0200 [thread overview]
Message-ID: <1496950316.30163.152.camel@intel.com> (raw)
In-Reply-To: <1496935714.8427.7.camel@gmail.com>
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.
next prev parent reply other threads:[~2017-06-08 19:31 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 [this message]
2017-06-09 8:12 ` Patrick Ohly
2017-06-09 13:47 ` Joshua Watt
2017-06-09 14:11 ` Patrick Ohly
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=1496950316.30163.152.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