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

[Darren Hart]

On Wed, 2013-09-11 at 16:49 +0200, Peter Kjellerstedt wrote:
> > -----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.
> 

All sound like good approaches to me. I especially like checking the
actual runtime value rather than scanning conf files.

-- 
Darren Hart
Intel Open Source Technology Center
Yocto Project - Linux Kernel