Re: [PATCH 2/2] utils.py: helper function for optional include files

[Patrick Ohly <patrick.ohly@intel.com>]

On Thu, 2017-06-08 at 10:20 +0100, Richard Purdie wrote:
> On Wed, 2017-06-07 at 17:31 +0200, Patrick Ohly wrote:
> > By using oe.utils.optional_includes(), developers can simplify the
> > code which selects which additional include files need to be included
> > in a .bbappend.
> > 
> > In the simple case (one distro feature and one include file) the code
> > is not shorter, but the intent is clearer than corresponding code
> > using bb.utils.contains():
> > 
> >    require ${@ oe.utils.optional_includes(d, 'foo-feature:bar.inc') }
> > 
> > More complex cases are also supported, in particular include files
> > that are required for one of several distro features or multiple
> > different include files.
> > 
> > To keep the common use case simple, DISTRO_FEATURES are checked by
> > default. Checking IMAGE_FEATURES might also be useful.
> > 
> > The DISTRO_FEATURES default and the intended usage make this more
> > suitable for OE-core than bitbake.
> 
> I'm honestly not sure this actually aids readability.

Fair enough.

> Taking your example:
> 
> require ${@oe.utils.optional_includes(d, "foo,bar:foo-or-bar.inc xyz:x.inc,y.inc,z.inc")}
> 
> I think I actually prefer:
> 
> require ${@oe.utils.optional_includes(d, "foo,bar:foo-or-bar.inc")}
> require ${@oe.utils.optional_includes(d, "xyz:x.inc,y.inc,z.inc")}
> 
> since its more explict. Spelling this out with the existing syntax
> isn't so bad either, I probably still think that this is clearer too:
> 
> require ${@bb.utils.contains(d, "DISTRO_FEATURES", "foo bar", "foo-or-bar.inc", "")}

That's the "foo and bar" case, not "foo or bar". It's an actual mistake
that people have made and that didn't get caught during code review.

> require ${@bb.utils.contains(d, "DISTRO_FEATURES", "xyz", "x.inc y.inc z.inc", "")}
> 
> How about simply:
> 
> require ${@oe.utils.distro_features(d, "foo bar", "foo-or-bar.inc")}
> require ${@bb.utils.distro_features(d, "xyz", "x.inc y.inc z.inc")}

That works for me, I just wonder about the exact semantic of listing
multiple features: "and" as in contains(), or "any"?

I'm leaning towards two functions where that is explicit:
  oe.utils.all_distro_features(d, "foo bar", "foo-and-bar.inc")
  oe.utils.any_distro_features(d, "foo bar", "foo-or-bar.inc")

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