[PATCH] bb-matrix.sh: check for the existence of time

[PATCH] bb-matrix.sh: check for the existence of time

[Jeff Mitchell]

[-- Attachment #1: Type: text/plain, Size: 84 bytes --]

What an existential subject. The patch is quite self-explanatory.

Thanks,
Jeff

[-- Attachment #2: 0001-Check-that-the-time-binary-is-installed-in-usr-bin-t.patch --]
[-- Type: text/x-patch, Size: 1709 bytes --]

From 62972e71cd952a890068a1f6b6e2234608747a1c Mon Sep 17 00:00:00 2001
From: Jeff Mitchell <jmitchell@ll.mit.edu>
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(-)

diff --git a/scripts/contrib/bb-perf/bb-matrix.sh b/scripts/contrib/bb-perf/bb-matrix.sh
index b9edd5f..66bd567 100755
--- a/scripts/contrib/bb-perf/bb-matrix.sh
+++ b/scripts/contrib/bb-perf/bb-matrix.sh
@@ -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 <dvhart@linux.intel.com>
 #
 
+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
-- 
1.7.3.4

Re: [PATCH] bb-matrix.sh: check for the existence of time

[Jeff Mitchell]

[-- Attachment #1: Type: text/plain, Size: 215 bytes --]

On 07/26/2011 05:57 PM, Jeff Mitchell wrote:
> 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

[-- Attachment #2: 0001-Check-that-the-time-binary-is-installed-in-usr-bin-t.patch --]
[-- Type: text/x-patch, Size: 1764 bytes --]

From 62972e71cd952a890068a1f6b6e2234608747a1c Mon Sep 17 00:00:00 2001
From: Jeff Mitchell <jmitchell@ll.mit.edu>
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.


Signed-off-by: Jeff Mitchell <jmitchell@ll.mit.edu>
---
 scripts/contrib/bb-perf/bb-matrix.sh |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/scripts/contrib/bb-perf/bb-matrix.sh b/scripts/contrib/bb-perf/bb-matrix.sh
index b9edd5f..66bd567 100755
--- a/scripts/contrib/bb-perf/bb-matrix.sh
+++ b/scripts/contrib/bb-perf/bb-matrix.sh
@@ -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 <dvhart@linux.intel.com>
 #
 
+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
-- 
1.7.3.4

Re: [PATCH] bb-matrix.sh: check for the existence of time

[Darren Hart]

Hi Jeff,

Thanks for the patch. A couple points of feedback below:

On 07/26/2011 02:57 PM, Jeff Mitchell wrote:
> 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.

> -# 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.

>  # 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 <dvhart@linux.intel.com>
>  #
>  
> +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 1
fi

Then replace /usr/bin/time below with TIME_CMD.

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

Re: [PATCH] bb-matrix.sh: check for the existence of time

[Jeff Mitchell]

On 07/26/2011 06:41 PM, Darren Hart wrote:
>
> Hi Jeff,
>
> Thanks for the patch. A couple points of feedback below:
>
> On 07/26/2011 02:57 PM, Jeff Mitchell wrote:
>> 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.)

> 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