Re: [PATCH 4/4] native.bbclass: Use fixed DISTRO_FEATURES

[Richard Purdie <richard.purdie@linuxfoundation.org>]

On Fri, 2017-04-07 at 14:59 +0300, Jussi Kukkonen wrote:
> On 7 April 2017 at 11:28, Richard Purdie <richard.purdie@linuxfoundat
> ion.org> wrote:
> > On Fri, 2017-04-07 at 09:09 +0300, Jussi Kukkonen wrote:
> > > There seems to be no advantage to letting distro features affect
> > > native builds. There is a significant disadvantage: a change to
> > > DISTRO_FEATURES will trigger a lot of unnecessary native tasks.
> > In a
> > > test like this:
> > >   $ bitbake core-image-minimal
> > >   # append " systemd" to DISTRO_FEATURES
> > >   $ bitbake core-image-minimal
> > > The latter build takes 44 minutes (28%) of cpu-time less with
> > this
> > > patch (skipping 135 native tasks). Sadly wall clock time was not
> > > affected as glibc remains the bottleneck.
> > >
> > > Set DISTRO_FEATURES to a fixed value for native recipes to avoid
> > the
> > > unnecessary tasks: currently the default value is empty.
> > >
> > > Do the variable setting in native_virtclass_handler() because
> > > otherwise
> > > it could still be overridden by appends and the feature
> > backfilling.
> > > Shuffle the early returns so DISTRO_FEATURES gets set as long as
> > > the packagename ends with "-native".
> > >
> > > Signed-off-by: Jussi Kukkonen <jussi.kukkonen@intel.com>
> > > ---
> > >  meta/classes/native.bbclass | 12 ++++++++----
> > >  meta/conf/bitbake.conf      |  2 ++
> > >  2 files changed, 10 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/meta/classes/native.bbclass
> > > b/meta/classes/native.bbclass
> > > index 1919fbc..fbca4c6 100644
> > > --- a/meta/classes/native.bbclass
> > > +++ b/meta/classes/native.bbclass
> > > @@ -121,14 +121,18 @@ PATH_prepend = "${COREBASE}/scripts/native-
> > > intercept:"
> > >  SSTATE_SCAN_CMD ?= "${SSTATE_SCAN_CMD_NATIVE}"
> > >  
> > >  python native_virtclass_handler () {
> > > -    classextend = e.data.getVar('BBCLASSEXTEND') or ""
> > > -    if "native" not in classextend:
> > > -        return
> > > -
> > >      pn = e.data.getVar("PN")
> > >      if not pn.endswith("-native"):
> > >          return
> > >  
> > > +    # Set features here to prevent appends and distro features
> > > backfill
> > > +    # from modifying native distro features
> > > +    d.setVar("DISTRO_FEATURES", "${NATIVE_DISTRO_FEATURES}")
> > > +
> > > +    classextend = e.data.getVar('BBCLASSEXTEND') or ""
> > > +    if "native" not in classextend:
> > > +        return
> > > +
> > >      def map_dependencies(varname, d, suffix = ""):
> > >          if suffix:
> > >              varname = varname + "_" + suffix
> > > diff --git a/meta/conf/bitbake.conf b/meta/conf/bitbake.conf
> > > index 5e98d45..78a3470 100644
> > > --- a/meta/conf/bitbake.conf
> > > +++ b/meta/conf/bitbake.conf
> > > @@ -789,6 +789,8 @@ MACHINE_ESSENTIAL_EXTRA_RRECOMMENDS ?= ""
> > >  EXTRA_IMAGE_FEATURES ??= ""
> > >  IMAGE_FEATURES += "${EXTRA_IMAGE_FEATURES}"
> > >  
> > > +NATIVE_DISTRO_FEATURES ?= ""
> > > +
> > >  DISTRO_FEATURES_BACKFILL = "pulseaudio sysvinit bluez5 gobject-
> > > introspection-data ldconfig"
> > >  MACHINE_FEATURES_BACKFILL = "rtc qemu-usermode"
> > 
> > Thanks for working on this, looks good.
> > 
> > Most native variables are called XXX_NATIVE, not NATIVE_XXX so we
> > might
> > want to make this match?
> Will do.
>  
> > Also, one question. Does this have any effect on qemu-native? Does
> > qemu-native need/use x11 today?
> qemu itself does not use anything x11 since last year and the default
> native packageconfig is already not
> dependent on distro features.
> 
> Now that you mentioned I had a closer look at qemu & SDL:
>  * qemu-native doesn't build with "sdl" by default, but someone might
> change that of course 
>  * If that happens (and libsdl-native is not in ASSUME_PROVIDED),
> libsdl is built and by default depends on libx11
> I completely expected this to fail now (without x11 in distro
> features) but it actually builds just fine. It turns out that a
> clever Burton has tweaked 
> REQUIRED_DISTRO_FEATURES_class-native for some X libraries for
> exactly this purpose already.

Right, we actually have a commented section for sdl and qemu in our
local.conf.sample so we do need to test and figure this piece out.

Would we want to put x11 in the default native distro flags to avoid
some of the other issues above?

Cheers,

Richard