Patchwork [1/2] bb-matrix: Clean before, rather than after, building

login
register
mail settings
Submitter Peter Kjellerstedt
Date Sept. 6, 2013, 4:12 p.m.
Message ID <8455f1f49363caff9ceb89a81d9b9a4650dd8ece.1378483732.git.pkj@axis.com>
Download mbox | patch
Permalink /patch/57577/
State Accepted
Commit f8f86ac88aa1bba99ba28762cfbd97d3721da7d9
Headers show

Comments

Peter Kjellerstedt - Sept. 6, 2013, 4:12 p.m.
This makes sure the the first build starts from a clean state. Otherwise
one could have the first build affected by any leftover state from
a previous build.

This also leaves a working state behind after the final build.

Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
---
 scripts/contrib/bb-perf/bb-matrix.sh | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)
Darren Hart - Sept. 10, 2013, 3:30 p.m.
On Fri, 2013-09-06 at 18:12 +0200, Peter Kjellerstedt wrote:
> This makes sure the the first build starts from a clean state. Otherwise
> one could have the first build affected by any leftover state from
> a previous build.
> 
> This also leaves a working state behind after the final build.
> 
> Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt@axis.com>

Hi Peter,

Thanks for taking the time to send in a fix!

> ---
>  scripts/contrib/bb-perf/bb-matrix.sh | 12 +++++-------
>  1 file changed, 5 insertions(+), 7 deletions(-)
> 
> diff --git a/scripts/contrib/bb-perf/bb-matrix.sh b/scripts/contrib/bb-perf/bb-matrix.sh
> index 37721fe..1064565 100755
> --- a/scripts/contrib/bb-perf/bb-matrix.sh
> +++ b/scripts/contrib/bb-perf/bb-matrix.sh
> @@ -63,6 +63,10 @@ for BB in $BB_RANGE; do
>  		date
>  		echo "BB=$BB PM=$PM Logging to $BB_LOG"
>  
> +		echo -n "  Preparing the work directory... "
> +		rm -rf pseudodone tmp sstate-cache tmp-eglibc &> /dev/null
> +		echo "done"
> +

Makes sense to me, although there is one point worth discussing. The
tmp-eglibc directory could change depending on the DISTRO setting iiuc.
All of tmp, sstate-cache, and tmp-eglibc could be dealt with using
cleansstate I believe:

bitbake <target> -c cleansstate

Richard, should we consider using this instead?

Either way, the above is an improvement, so:

Acked-by: Darren Hart <dvhart@linux.intel.com>

Finally, In the future Peter, please have a look at the git log for the
file(s) you are patching and if there is an obvious author/maintainer
from the log, please include them on Cc. This will help ensure your
patch gets reviewed and merged in a timely fashion.

Thanks again for the patch!

>  		# Export the variables under test and run the bitbake command
>  		# Strip any leading zeroes before passing to bitbake
>  		export BB_NUMBER_THREADS=$(echo $BB | sed 's/^0*//')
> @@ -70,12 +74,6 @@ for BB in $BB_RANGE; do
>  		/usr/bin/time -f "$BB $PM $TIME_STR" -a -o $RUNTIME_LOG $BB_CMD &> $BB_LOG
>  
>  		echo "  $(tail -n1 $RUNTIME_LOG)"
> -		echo -n "  Cleaning up..."
> -		mv tmp/buildstats $RUNDIR/$BB-$PM-buildstats
> -		rm -f pseudodone &> /dev/null
> -		rm -rf tmp &> /dev/null
> -		rm -rf sstate-cache &> /dev/null
> -		rm -rf tmp-eglibc &> /dev/null
> -		echo "done"
> +		cp -a tmp/buildstats $RUNDIR/$BB-$PM-buildstats
>  	done
>  done
Richard Purdie - Sept. 10, 2013, 3:37 p.m.
On Tue, 2013-09-10 at 08:30 -0700, Darren Hart wrote:
> On Fri, 2013-09-06 at 18:12 +0200, Peter Kjellerstedt wrote:
> > This makes sure the the first build starts from a clean state. Otherwise
> > one could have the first build affected by any leftover state from
> > a previous build.
> > 
> > This also leaves a working state behind after the final build.
> > 
> > Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
> 
> Hi Peter,
> 
> Thanks for taking the time to send in a fix!
> 
> > ---
> >  scripts/contrib/bb-perf/bb-matrix.sh | 12 +++++-------
> >  1 file changed, 5 insertions(+), 7 deletions(-)
> > 
> > diff --git a/scripts/contrib/bb-perf/bb-matrix.sh b/scripts/contrib/bb-perf/bb-matrix.sh
> > index 37721fe..1064565 100755
> > --- a/scripts/contrib/bb-perf/bb-matrix.sh
> > +++ b/scripts/contrib/bb-perf/bb-matrix.sh
> > @@ -63,6 +63,10 @@ for BB in $BB_RANGE; do
> >  		date
> >  		echo "BB=$BB PM=$PM Logging to $BB_LOG"
> >  
> > +		echo -n "  Preparing the work directory... "
> > +		rm -rf pseudodone tmp sstate-cache tmp-eglibc &> /dev/null
> > +		echo "done"
> > +
> 
> Makes sense to me, although there is one point worth discussing. The
> tmp-eglibc directory could change depending on the DISTRO setting iiuc.
> All of tmp, sstate-cache, and tmp-eglibc could be dealt with using
> cleansstate I believe:
> 
> bitbake <target> -c cleansstate
> 
> Richard, should we consider using this instead?

Sadly this isn't recursive, it applies to <target> and not any
dependencies of it. I'd be tempted to simplify this to rm tmp* and
depend on a convention of calling them tmp*...

Cheers,

Richard
Darren Hart - Sept. 10, 2013, 3:40 p.m.
On Tue, 2013-09-10 at 16:37 +0100, Richard Purdie wrote:
> On Tue, 2013-09-10 at 08:30 -0700, Darren Hart wrote:
> > On Fri, 2013-09-06 at 18:12 +0200, Peter Kjellerstedt wrote:
> > > This makes sure the the first build starts from a clean state. Otherwise
> > > one could have the first build affected by any leftover state from
> > > a previous build.
> > > 
> > > This also leaves a working state behind after the final build.
> > > 
> > > Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
> > 
> > Hi Peter,
> > 
> > Thanks for taking the time to send in a fix!
> > 
> > > ---
> > >  scripts/contrib/bb-perf/bb-matrix.sh | 12 +++++-------
> > >  1 file changed, 5 insertions(+), 7 deletions(-)
> > > 
> > > diff --git a/scripts/contrib/bb-perf/bb-matrix.sh b/scripts/contrib/bb-perf/bb-matrix.sh
> > > index 37721fe..1064565 100755
> > > --- a/scripts/contrib/bb-perf/bb-matrix.sh
> > > +++ b/scripts/contrib/bb-perf/bb-matrix.sh
> > > @@ -63,6 +63,10 @@ for BB in $BB_RANGE; do
> > >  		date
> > >  		echo "BB=$BB PM=$PM Logging to $BB_LOG"
> > >  
> > > +		echo -n "  Preparing the work directory... "
> > > +		rm -rf pseudodone tmp sstate-cache tmp-eglibc &> /dev/null
> > > +		echo "done"
> > > +
> > 
> > Makes sense to me, although there is one point worth discussing. The
> > tmp-eglibc directory could change depending on the DISTRO setting iiuc.
> > All of tmp, sstate-cache, and tmp-eglibc could be dealt with using
> > cleansstate I believe:
> > 
> > bitbake <target> -c cleansstate
> > 
> > Richard, should we consider using this instead?
> 
> Sadly this isn't recursive, it applies to <target> and not any
> dependencies of it. I'd be tempted to simplify this to rm tmp* and
> depend on a convention of calling them tmp*...
> 

yes.... but.... any kind of a wildcard in an rm -rf statement makes me
see purple....
Ross Burton - Sept. 10, 2013, 3:40 p.m.
On 10 September 2013 16:37, Richard Purdie
<richard.purdie@linuxfoundation.org> wrote:
> Sadly this isn't recursive, it applies to <target> and not any
> dependencies of it. I'd be tempted to simplify this to rm tmp* and
> depend on a convention of calling them tmp*...

You could invoke bitbake -e and get the right directories to remove
(see wipe-sysroot).

Ross

Patch

diff --git a/scripts/contrib/bb-perf/bb-matrix.sh b/scripts/contrib/bb-perf/bb-matrix.sh
index 37721fe..1064565 100755
--- a/scripts/contrib/bb-perf/bb-matrix.sh
+++ b/scripts/contrib/bb-perf/bb-matrix.sh
@@ -63,6 +63,10 @@  for BB in $BB_RANGE; do
 		date
 		echo "BB=$BB PM=$PM Logging to $BB_LOG"
 
+		echo -n "  Preparing the work directory... "
+		rm -rf pseudodone tmp sstate-cache tmp-eglibc &> /dev/null
+		echo "done"
+
 		# Export the variables under test and run the bitbake command
 		# Strip any leading zeroes before passing to bitbake
 		export BB_NUMBER_THREADS=$(echo $BB | sed 's/^0*//')
@@ -70,12 +74,6 @@  for BB in $BB_RANGE; do
 		/usr/bin/time -f "$BB $PM $TIME_STR" -a -o $RUNTIME_LOG $BB_CMD &> $BB_LOG
 
 		echo "  $(tail -n1 $RUNTIME_LOG)"
-		echo -n "  Cleaning up..."
-		mv tmp/buildstats $RUNDIR/$BB-$PM-buildstats
-		rm -f pseudodone &> /dev/null
-		rm -rf tmp &> /dev/null
-		rm -rf sstate-cache &> /dev/null
-		rm -rf tmp-eglibc &> /dev/null
-		echo "done"
+		cp -a tmp/buildstats $RUNDIR/$BB-$PM-buildstats
 	done
 done