bb-matrix.sh: check for the existence of time
PATCH 1/1 bb-matrix.sh: check for the existence of time
What an existential subject. The patch is quite self-explanatory.
Thanks,
Jeff
From 62972e71cd952a890068a1f6b6e2234608747a1c Mon Sep 17 00:00:00 2001
From: Jeff Mitchell <[email protected]>
Date: Tue, 26 Jul 2011 17:45:58 -0400
Subject: [PATCH] Check that the time binary is installed in /usr/bin/time when running
bb-matrix.sh. Also fix a typo.
--- scripts/contrib/bb-perf/bb-matrix.sh | 9 +++++++-- 1 files changed, 7 insertions(+), 2 deletions(-)
scripts/contrib/bb-perf/bb-matrix.sh +7 -2
@@ -18,7 +18,7 @@ # Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA. # # DESCRIPTION -# This script runs BB_CMD (typically building core-image-sato) for all +# This script runs BB_CMD (typically building core-image-minimal) for all # combincations of BB_RANGE and PM_RANGE values. It saves off all the console # logs, the buildstats directories, and creates a bb-pm-runtime.dat file which # can be used to postprocess the results with a plotting tool, spreadsheet, etc.
@@ -32,6 +32,11 @@ # Darren Hart <[email protected]> # +if [ ! -f /usr/bin/time ]; then + echo "The \"time\" binary was not found at /usr/bin/time -- please install it!" + exit 1 +fi + # The following ranges are appropriate for a 4 core system with 8 logical units BB_RANGE="04 05 06 07 08 09 10 11 12 13 14 15 16" PM_RANGE="04 05 06 07 08 09 10 11 12 13 14 15 16"
@@ -48,7 +53,7 @@ TIME_STR="%e %S %U %P %c %w %R %F %M %x" mkdir $DATADIR if [ $? -ne 0 ]; then echo "Failed to create $DATADIR." - exit 1 + exit 2 fi # Add a simple header
Signature · 1 line
1.7.3.4
Quoted 4 lines · Jeff Mitchell
What an existential subject. The patch is quite self-explanatory.
Thanks,
Jeff
This version has a Signed-off-by line...sorry about that.
Thanks,
Jeff
From 62972e71cd952a890068a1f6b6e2234608747a1c Mon Sep 17 00:00:00 2001
From: Jeff Mitchell <[email protected]>
Date: Tue, 26 Jul 2011 17:45:58 -0400
Subject: [PATCH] Check that the time binary is installed in /usr/bin/time when running
bb-matrix.sh. Also fix a typo.
--- scripts/contrib/bb-perf/bb-matrix.sh | 9 +++++++-- 1 files changed, 7 insertions(+), 2 deletions(-)
scripts/contrib/bb-perf/bb-matrix.sh +7 -2
@@ -18,7 +18,7 @@ # Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA. # # DESCRIPTION -# This script runs BB_CMD (typically building core-image-sato) for all +# This script runs BB_CMD (typically building core-image-minimal) for all # combincations of BB_RANGE and PM_RANGE values. It saves off all the console # logs, the buildstats directories, and creates a bb-pm-runtime.dat file which # can be used to postprocess the results with a plotting tool, spreadsheet, etc.
@@ -32,6 +32,11 @@ # Darren Hart <[email protected]> # +if [ ! -f /usr/bin/time ]; then + echo "The \"time\" binary was not found at /usr/bin/time -- please install it!" + exit 1 +fi + # The following ranges are appropriate for a 4 core system with 8 logical units BB_RANGE="04 05 06 07 08 09 10 11 12 13 14 15 16" PM_RANGE="04 05 06 07 08 09 10 11 12 13 14 15 16"
@@ -48,7 +53,7 @@ TIME_STR="%e %S %U %P %c %w %R %F %M %x" mkdir $DATADIR if [ $? -ne 0 ]; then echo "Failed to create $DATADIR." - exit 1 + exit 2 fi # Add a simple header
Signature · 1 line
1.7.3.4
Hi Jeff,
Thanks for the patch. A couple points of feedback below:
Quoted 1 line · Jeff Mitchell
What an existential subject. The patch is quite self-explanatory.
Well, there is almost always a need for commit log beyond just the subject. The only exception might be "whitespace fixes only" or "spelling fixes only".
Something like:
"If time is missing, bb-matrix will fail to run and report misleading
errors about tmp/buildstats not existing. Check and abort with an appropriate error message."
Also, while it is tempting to squeeze in typo fixes and such with a functional patch, unless they are in the code you are changing anyway, they should be sent as separate patches. If it were necessary to revert the functional patch, we'd rather not have to also lose the typo or whitespace cleanup, for example.
scripts/contrib/bb-perf/bb-matrix.sh line 22
Quoted 3 lines
scripts/contrib/bb-perf/bb-matrix.sh +1
-1-# This script runs BB_CMD (typically building core-image-sato) for all +# This script runs BB_CMD (typically building core-image-minimal) for all # combincations of BB_RANGE and PM_RANGE values. It saves off all the console
^ heh, another typo! bad dvhart.
scripts/contrib/bb-perf/bb-matrix.sh line 35
Quoted 7 lines
# logs, the buildstats directories, and creates a bb-pm-runtime.dat file which
# can be used to postprocess the results with a plotting tool, spreadsheet, etc.scripts/contrib/bb-perf/bb-matrix.sh +1
-0@@ -32,6 +32,11 @@ # Darren Hart <[email protected]> # +if [ ! -f /usr/bin/time ]; then
This should probably be a bit more generic:
TIME_CMD=$(which time)
if [ -z "$TIME_CMD" ]; then
echo 'The "time" binary was not found in your path, \
please install it.'
exit 1fi
Then replace /usr/bin/time below with TIME_CMD.
Signature · 3 lines
Darren Hart Intel Open Source Technology Center Yocto Project - Linux Kernel
Quoted 11 lines · Darren Hart
Hi Jeff,
Thanks for the patch. A couple points of feedback below:
Quoted 1 line · Jeff Mitchell
What an existential subject. The patch is quite self-explanatory.
Well, there is almost always a need for commit log beyond just the subject. The only exception might be "whitespace fixes only" or "spelling fixes only".
I did write a commit log, but git-format-patch turned it into the subject of the attached patch. No, I haven't used git-format-patch before :-) (I've been fortunate to have been able to avoid it for many years now.)
Quoted 3 lines
Also, while it is tempting to squeeze in typo fixes and such with a functional patch, unless they are in the code you are changing anyway, they should be sent as separate patches.
Given the triviality of both changes I didn't think this would be an issue.
--Jeff