[OE-core] [PATCH 2/2] bb-matrix: Make sure local.conf does not interfere

[Peter Kjellerstedt]

> -----Original Message-----
> From: Richard Purdie [mailto:richard.purdie at linuxfoundation.org]
> Sent: den 10 september 2013 17:48
> To: Darren Hart
> Cc: Peter Kjellerstedt; openembedded-core at lists.openembedded.org
> Subject: Re: [OE-core] [PATCH 2/2] bb-matrix: Make sure local.conf does
> not interfere
> 
> On Tue, 2013-09-10 at 08:42 -0700, Darren Hart wrote:
> > On Tue, 2013-09-10 at 16:37 +0100, Richard Purdie wrote:
> > > On Tue, 2013-09-10 at 08:33 -0700, Darren Hart wrote:
> > > > On Fri, 2013-09-06 at 18:12 +0200, Peter Kjellerstedt wrote:
> > > > > If any of BB_NUMBER_THREADS, PARALLEL_MAKE or SSTATE_DIR
> happened to be
> > > > > set in local.conf then the bb-matrix script would not perform
> as
> > > > > intended.
> > > > >
> > > > > Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt at axis.com>
> > > > > ---
> > > > >  scripts/contrib/bb-perf/bb-matrix.sh | 4 ++++
> > > > >  1 file changed, 4 insertions(+)
> > > > >
> > > > > diff --git a/scripts/contrib/bb-perf/bb-matrix.sh
> b/scripts/contrib/bb-perf/bb-matrix.sh
> > > > > index 1064565..d5127e7 100755
> > > > > --- a/scripts/contrib/bb-perf/bb-matrix.sh
> > > > > +++ b/scripts/contrib/bb-perf/bb-matrix.sh
> > > > > @@ -53,6 +53,10 @@ if [ $? -ne 0 ]; then
> > > > >  	exit 1
> > > > >  fi
> > > > >
> > > > > +# Make sure neither of BB_NUMBER_THREADS, PARALLEL_MAKE and
> SSTATE_DIR are set
> > > > > +# in local.conf
> > > > > +sed -ri
> 's/^([[:space:]]*(BB_NUMBER_THREADS|PARALLEL_MAKE|SSTATE_DIR)[[:space:]
> ]*\??=.*)/#\1/' conf/local.conf
> > > > > +
> > > >
> > > > Unless I'm mistaken, you are modifying the users local.conf?
> > > >
> > > > We definitely should *not* be doing that. I would support
> documenting
> > > > this as a requirement and even printing a warning if any of a set
> of
> > > > variables are found in the bitbake environment.
> > > >
> > > > Note that local.conf is not the only place where these could be
> set.
> > > >
> > > > Richard, shouldn't the env setting override anything in
> local.conf?
> > >
> > > No, it will override a ?= or ??= but not a =.
> >
> > OK, well, a warning if found in local.conf (or site.conf?) is
> probably
> > the best we can do for now. We can't check bitbake -e since that will
> > report the default which we wouldn't be able to distinguish from an
> > explicit setting.... unless we checked for an explicit assignment?
> Might
> > be doable.... but a warning should be sufficient for a script in
> > contrib, yes?
> 
> Yes, although I think in the comments, bitbake -e will tell you which
> kind of operation was used :)
> 
> Cheers,
> 
> Richard

My suggestion is to test that the values can be set by the script 
before starting the test run. Something like 
'BB_NUMBER_THREADS=1234 PARALLEL_MAKE="-j 1234" bitbake -e core-image-minimal'
and then grep for the expected values. If the correct values are 
not found then just abort with an error message as the script will 
not be able to do what it is supposed to do.

I would also suggest aborting if SSTATE_DIR does not match 
"$(pwd)/sstate-cache" since the script would clear a directory 
other than the one that is actually used.

Finally, if SSTATE_MIRRORS is non-empty I would generate a warning. 
I would not abort in this case as it can be a useful setup (e.g., 
I used this to build with a sstate cache mirror that contained the 
native packages as this halved the build time and I thus could test 
more combinations over a weekend).

I'll update my patch as per above unless you have other suggestions.

//Peter