diff mbox

dg-extract-results.sh tolerate spaces in variant names.

Message ID 51B71442.80605@arm.com
State New
Headers show

Commit Message

Marcus Shawcroft June 11, 2013, 12:12 p.m. UTC
Hi,

The dg-extract-result.sh script is not tolerant of spaces in test variants.

Consider the following site.exp fragment:

====
set target_list [
   list "arm-eabi-aem{-marm -march=armv7-a -mfpu=vfpv3-d16 
-mfloat-abi=softfp}" \
        "arm-eabi-aem{-marm -march=armv8-a -mfpu=crypto-neon-fp-armv8 
-mfloat-abi=hard}" \
   ]
====

which results in a .sum file of the form:

====
		=== gcc tests ===

Schedule of variations:
     arm-eabi-aem/-marm -march=armv7-a -mfpu=vfpv3-d16 -mfloat-abi=softfp
     arm-eabi-aem/-marm -march=armv8-a -mfpu=crypto-neon-fp-armv8 
-mfloat-abi=hard

Running target arm-eabi-aem/-marm -march=armv7-a -mfpu=vfpv3-d16 
-mfloat-abi=softfp
====

When the check target is executed in parallel (make -j) and RUNTESTFLAGS 
is empty the Makefile arranges to split the tests into multiple groups 
each run in parallel, the resulting .sum.sep and .log.sep files are then 
stitched back into a single .sum and .log file using dg-extract-results.sh

The existing implementation of dg-extract-results.sh does not take 
account of the spaces within a variation.  Therefore in the above 
example all bar the first option are stripped.  Since the prefix of the 
variation names are identical dg-extract-results.sh loses one set of 
results.

Given the site.exp fragment above the output of make check is therefore 
different depending on whether or not -j is specified.

I can find no definitive documentation that explicitly permits nor 
prevents the use of space separated options within test variants. 
However it seems like a reasonable use case and (bar the use of -j) it 
currently works.

This patch adjusts dg-extract-results.sh to permit spaces within variant 
names.  The changes are rather ugly.  Suggestions welcome.

I experimented with adjusting IFS to allow the shell scripts for loops 
that iterate over lists of variants to ignore spaces, but this solution 
was fragile (and ugly).  The use of bash arrays would simplify the 
change, however I believe this script was written with portability in 
mind so I've attempted to stick to posix shell.

OK?


/Marcus

2013-06-11  Marcus Shawcroft  <marcus.shawcroft@arm.com>

	* dg-extract-results.sh (a_init, a_length, a_append, a_lookup): New.
	* dg-extract-results.sh: Use array for VARIANTS.
	Strip whitespace from temporary filenames.  GAWK fragments to tolerate
	whitespace embedded in variant names.  Match variants in gawk
	scripts using match().

Comments

Jakub Jelinek June 11, 2013, 12:55 p.m. UTC | #1
On Tue, Jun 11, 2013 at 01:12:50PM +0100, Marcus Shawcroft wrote:
> Hi,
> 
> The dg-extract-result.sh script is not tolerant of spaces in test variants.
> 
> Consider the following site.exp fragment:
> 
> ====
> set target_list [
>   list "arm-eabi-aem{-marm -march=armv7-a -mfpu=vfpv3-d16
> -mfloat-abi=softfp}" \
>        "arm-eabi-aem{-marm -march=armv8-a -mfpu=crypto-neon-fp-armv8
> -mfloat-abi=hard}" \
>   ]

Why don't you separate the variants with commas instead of spaces?

	Jakub
Marcus Shawcroft June 11, 2013, 1:07 p.m. UTC | #2
On 11/06/13 13:55, Jakub Jelinek wrote:
> On Tue, Jun 11, 2013 at 01:12:50PM +0100, Marcus Shawcroft wrote:
>> Hi,
>>
>> The dg-extract-result.sh script is not tolerant of spaces in test variants.
>>
>> Consider the following site.exp fragment:
>>
>> ====
>> set target_list [
>>    list "arm-eabi-aem{-marm -march=armv7-a -mfpu=vfpv3-d16
>> -mfloat-abi=softfp}" \
>>         "arm-eabi-aem{-marm -march=armv8-a -mfpu=crypto-neon-fp-armv8
>> -mfloat-abi=hard}" \
>>    ]
>
> Why don't you separate the variants with commas instead of spaces?
>
> 	Jakub
>


That would test each of the listed options in isolation, one test 
variant for each comma separated option, eight variants in total.  The 
example above tests just two variants, each with the set of options as 
listed.

/Marcus
Jakub Jelinek June 11, 2013, 1:12 p.m. UTC | #3
On Tue, Jun 11, 2013 at 02:07:50PM +0100, Marcus Shawcroft wrote:
> On 11/06/13 13:55, Jakub Jelinek wrote:
> >On Tue, Jun 11, 2013 at 01:12:50PM +0100, Marcus Shawcroft wrote:
> >>Hi,
> >>
> >>The dg-extract-result.sh script is not tolerant of spaces in test variants.
> >>
> >>Consider the following site.exp fragment:
> >>
> >>====
> >>set target_list [
> >>   list "arm-eabi-aem{-marm -march=armv7-a -mfpu=vfpv3-d16
> >>-mfloat-abi=softfp}" \
> >>        "arm-eabi-aem{-marm -march=armv8-a -mfpu=crypto-neon-fp-armv8
> >>-mfloat-abi=hard}" \
> >>   ]
> >
> >Why don't you separate the variants with commas instead of spaces?
> >
> >	Jakub
> >
> 
> 
> That would test each of the listed options in isolation, one test
> variant for each comma separated option, eight variants in total.
> The example above tests just two variants, each with the set of
> options as listed.

For that I think the syntax is slashes separating the options.
I.e.
RUNTESTFLAGS='--target_board=arm-eabi-aem\{-marm/-march=armv7-a/-mfpu=vfpv3-d16/-mfloat-abi=softfp,-marm/-march=armv8-a/-mfpu=crypto-neon-fp-armv8/-mfloat-abi=hard\}'

	Jakub
Marcus Shawcroft June 11, 2013, 1:24 p.m. UTC | #4
On 11/06/13 14:12, Jakub Jelinek wrote:
> On Tue, Jun 11, 2013 at 02:07:50PM +0100, Marcus Shawcroft wrote:
>> On 11/06/13 13:55, Jakub Jelinek wrote:
>>> On Tue, Jun 11, 2013 at 01:12:50PM +0100, Marcus Shawcroft wrote:
>>>> Hi,
>>>>
>>>> The dg-extract-result.sh script is not tolerant of spaces in test variants.
>>>>
>>>> Consider the following site.exp fragment:
>>>>
>>>> ====
>>>> set target_list [
>>>>    list "arm-eabi-aem{-marm -march=armv7-a -mfpu=vfpv3-d16
>>>> -mfloat-abi=softfp}" \
>>>>         "arm-eabi-aem{-marm -march=armv8-a -mfpu=crypto-neon-fp-armv8
>>>> -mfloat-abi=hard}" \
>>>>    ]
>>>
>>> Why don't you separate the variants with commas instead of spaces?
>>>
>>> 	Jakub
>>>
>>
>>
>> That would test each of the listed options in isolation, one test
>> variant for each comma separated option, eight variants in total.
>> The example above tests just two variants, each with the set of
>> options as listed.
>
> For that I think the syntax is slashes separating the options.
> I.e.
> RUNTESTFLAGS='--target_board=arm-eabi-aem\{-marm/-march=armv7-a/-mfpu=vfpv3-d16/-mfloat-abi=softfp,-marm/-march=armv8-a/-mfpu=crypto-neon-fp-armv8/-mfloat-abi=hard\}'
>
> 	Jakub
>

Aha, that works fine.  Thank you!

/Marcus
diff mbox

Patch

diff --git a/contrib/dg-extract-results.sh b/contrib/dg-extract-results.sh
index 7029281..3253c51 100755
--- a/contrib/dg-extract-results.sh
+++ b/contrib/dg-extract-results.sh
@@ -52,15 +52,51 @@  msg() {
   echo "$@" >&2
 }
 
+# Provide the illusion of arrays.  These arrays can store values that
+# can white space.  We use an additional layer of indirection, each
+# value is stored in a shell variable with a name of the form
+# <array-name>_<array-element>.  The array variable itself is just a
+# list of the underlying variable names.
+
+# Initialise an array to empty.
+a_init() {
+  local a="$1"
+  eval ${a}_n=0
+  eval ${a}=""
+}
+
+# Get the length of an array.
+a_length() {
+  local a="$1"
+  local nv=${a}_n
+  eval echo \$$nv
+}
+
+a_append() {
+  local a="$1"
+  local v="$2"
+  local nv=${a}_n
+  eval local count=\$$nv
+  eval ${a}_$count=\"$v\"
+  eval ${a}=\"\$${a} ${a}_$count\"
+  count=`expr $count + 1`
+  eval $nv=$count
+}
+
+a_lookup()
+{
+  eval echo \$$1
+}
+
 # Parse the command-line options.
 
-VARIANTS=""
+a_init OPTVARIANTS
 TOOL=""
 MODE="sum"
 
 while getopts "l:t:L" ARG; do
   case $ARG in
-  l)  VARIANTS="${VARIANTS} ${OPTARG}";;
+  l)  a_append OPTVARIANTS "${OPTARG}";;
   t)  test -z "$TOOL" || (msg "${PROGNAME}: only one tool can be specified"; exit 1);
       TOOL="${OPTARG}";;
   L)  MODE="log";;
@@ -201,41 +237,39 @@  EOF
   exit 0
 fi
 
+a_init VARIANTS
+
 # If no variants were specified, find all variants in the remaining
 # summary files.  Otherwise, ignore specified variants that aren't in
 # any of those summary files.
-
-if test -z "$VARIANTS" ; then
+if test -z "$OPTVARIANTS" ; then
   VAR_AWK=${TMP}/variants.awk
   cat <<EOF > $VAR_AWK
 /^Schedule of variations:/      { in_vars=1; next }
 /^$/                            { in_vars=0 }
 /^Running target/               { exit }
-{ if (in_vars==1) print \$1; else next }
+{ if (in_vars==1) print \$0; else next }
 EOF
 
-  touch ${TMP}/varlist
   for FILE in $SUM_FILES; do
-    $AWK -f $VAR_AWK $FILE >> ${TMP}/varlist
-  done
-  VARIANTS="`sort -u ${TMP}/varlist`"
+    $AWK -f $VAR_AWK $FILE
+  done | sed 's/^[ ]*//' | sort -u > ${TMP}/varlist
+  while read line; do
+    a_append VARIANTS "$line"
+  done < ${TMP}/varlist
 else
-  VARS="$VARIANTS"
-  VARIANTS=""
-  for VAR in $VARS
+  # Check the command line provided list of variants exists.
+  for v in $OPTVARIANTS
   do
-    grep "Running target $VAR" $SUM_FILES > /dev/null && VARIANTS="$VARIANTS $VAR"
+    VAR=`a_lookup $v`
+    if grep "Running target $VAR" $SUM_FILES > /dev/null; then
+      a_append VARIANTS "$VAR"
+    fi
   done
 fi
-
 # Find out if we have more than one variant, or any at all.
 
-VARIANT_COUNT=0
-for VAR in $VARIANTS
-do
-  VARIANT_COUNT=`expr $VARIANT_COUNT + 1`
-done
-
+VARIANT_COUNT=`a_length VARIANTS`
 if test $VARIANT_COUNT -eq 0 ; then
   msg "${PROGNAME}: no file for $TOOL has results for the specified variants"
   exit 1
@@ -252,8 +286,9 @@  echo
 echo "		=== $TOOL tests ==="
 echo
 echo "Schedule of variations:"
-for VAR in $VARIANTS
+for v in $VARIANTS
 do
+  VAR=`a_lookup $v`
   echo "    $VAR"
 done
 echo
@@ -263,8 +298,9 @@  echo
 # initialize VAR and TOOL with the script, rather than assuming that the
 # available version of awk can pass variables from the command line.
 
-for VAR in $VARIANTS
+for v in $VARIANTS
 do
+  VAR=`a_lookup $v`
   GUTS_AWK=${TMP}/guts.awk
   cat << EOF > $GUTS_AWK
 BEGIN {
@@ -281,16 +317,16 @@  BEGIN {
   expfileno = expfileno + 1
 }
 /^Running target / {
-  curvar = \$3
-  if (variant == curvar && firstvar == 1) { print; print_using=1; firstvar = 0 }
+  matchvar=index(\$0, variant)
+  if (matchvar > 0 && firstvar == 1) { print; print_using=1; firstvar = 0 }
   next
 }
 /^Using / {
-  if (variant == curvar && print_using) { print; next }
+  if (matchvar > 0 && print_using) { print; next }
 }
 /^Running .*\\.exp \\.\\.\\./ {
   print_using=0
-  if (variant == curvar) {
+  if (matchvar > 0) {
     if (need_close) close(curfile)
     curfile="${TMP}/list"expfilesr[\$2]
     expfileseen[\$2]=expfileseen[\$2] + 1
@@ -299,7 +335,7 @@  BEGIN {
     next
   }
 }
-/^\t\t=== .* ===$/ { curvar = ""; next }
+/^\t\t=== .* ===$/ { matchvar = 0; next }
 /^(PASS|XPASS|FAIL|XFAIL|UNRESOLVED|WARNING|ERROR|UNSUPPORTED|UNTESTED|KFAIL):/ {
   testname=\$2
   # Ugly hack for gfortran.dg/dg.exp
@@ -307,7 +343,7 @@  BEGIN {
     testname="h"testname
 }
 /^$/ { if ("$MODE" == "sum") next }
-{ if (variant == curvar && curfile) {
+{ if (matchvar > 0 && curfile) {
     if ("$MODE" == "sum") {
       printf "%s %08d|", testname, cnt >> curfile
       cnt = cnt + 1
@@ -346,10 +382,10 @@  BEGIN {
   variant="$VAR"
   tool="$TOOL"
   passcnt=0; failcnt=0; untstcnt=0; xpasscnt=0; xfailcnt=0; kpasscnt=0; kfailcnt=0; unsupcnt=0; unrescnt=0;
-  curvar=""; insummary=0
+  matchvar=0; insummary=0
 }
-/^Running target /		{ curvar = \$3; next }
-/^# of /			{ if (variant == curvar) insummary = 1 }
+/^Running target /		{ matchvar=index(\$0, variant); next }
+/^# of /			{ if (matchvar > 0) insummary = 1 }
 /^# of expected passes/		{ if (insummary == 1) passcnt += \$5; next; }
 /^# of unexpected successes/	{ if (insummary == 1) xpasscnt += \$5; next; }
 /^# of unexpected failures/	{ if (insummary == 1) failcnt += \$5; next; }
@@ -360,7 +396,7 @@  BEGIN {
 /^# of unresolved testcases/	{ if (insummary == 1) unrescnt += \$5; next; }
 /^# of unsupported tests/	{ if (insummary == 1) unsupcnt += \$5; next; }
 /^$/				{ if (insummary == 1)
-				    { insummary = 0; curvar = "" }
+				    { insummary = 0; matchvar = 0 }
 				  next
 				}
 { next }
@@ -377,8 +413,7 @@  END {
   if (unsupcnt != 0) printf ("# of unsupported tests\t\t%d\n", unsupcnt)
 }
 EOF
-
-  PVAR=`echo $VAR | sed 's,/,.,g'`
+  PVAR=`echo $VAR | sed 's,[/ ],.,g'`
   TMPFILE=${TMP}/var-$PVAR
   rm -f $TMPFILE
   rm -f ${TMP}/list*