[PATCH 0/4] Avoid command-line length limit combining test results

[PATCH 0/4] Avoid command-line length limit combining test results

[Pedro Alves]

On Windows, "make check-parallel" dies when combining the per-test
gdb.sum / gdb.log files into the final combined pair:

 $ find outputs/ -name gdb.sum | wc -l
 2657
 .../dg-extract-results.sh: line 39: /ucrt64/bin/python3: Argument list too long
 make: *** [Makefile:274: check-parallel] Error 2

The testsuite Makefile passes the whole file list to
contrib/dg-extract-results.sh on the command line, expanded from a
find(1) backtick.  GDB's parallel harness writes one gdb.sum (and one
gdb.log) per test script, so on a full run that list holds thousands
of paths, which overflows the Windows command-line length limit
(~32 KB).

This series teaches dg-extract-results to read the file list from
stdin (or a file) instead, and has the Makefile pipe the find output
straight in, so the length limit no longer applies regardless of the
host.

The first patch touches contrib/dg-extract-results.{sh,py}, which are
shared with GCC, so I plan to submit that one to GCC as well.

The two middle patches (factoring out the combine recipe, and adding
the extract-results target) are pure GDB and can go in at any time.

Only the last patch depends on the new -f option added by the first
patch.

Pedro Alves (4):
  dg-extract-results.{sh,py}: Optionally read file list from stdin/file
  gdb/testsuite: Factor out dg-extract-results.sh calls
  gdb/testsuite: Add extract-results make target
  gdb/testsuite: Pipe the dg-extract-results file list via stdin

 contrib/dg-extract-results.py | 30 +++++++++++++++++++---
 contrib/dg-extract-results.sh | 17 +++++++++++--
 gdb/testsuite/Makefile.in     | 47 ++++++++++++++++++++++++-----------
 3 files changed, 74 insertions(+), 20 deletions(-)


base-commit: 39fc0cc4aa6f82a73b9eac6b8739895484ab269f
-- 
2.54.0

[PATCH 1/4] dg-extract-results.{sh, py}: Optionally read file list from stdin/file

[Pedro Alves]

dg-extract-results.sh and dg-extract-results.py take the summary/log
files to combine as command-line arguments, one argument per file.

That is fine for GCC, whose parallel harness produces one .sum per
target variant -- a handful of files.

GDB uses these scripts too, but its parallel harness writes one .sum
(and one .log) per test script, of which there are 2657 at the moment.

On Windows, with native Windows python, passing that many sum or log
files to dg-extract-results overflows the command-line length limit
(~32 KB), and the combine step dies:

 $ make check -j$(nproc)
 ...
 make[1]: Leaving directory '/c/msys2/home/alves/gdb/build-testsuite-windows'
 /c/gdb/src/gdb/testsuite/../../contrib/dg-extract-results.sh: line 39: /ucrt64/bin/python3: Argument list too long
 /c/gdb/src/gdb/testsuite/../../contrib/dg-extract-results.sh: line 39: /ucrt64/bin/python3: Argument list too long
 make: *** [Makefile:274: check-parallel] Error 2
 ...
 $ find outputs/ -name gdb.sum | wc -l
 2657

The .sh wrapper re-execs the faster Python implementation, forwarding
the file list, so the overflow can also surface at the re-exec.

Fix it by teaching both dg-extract-results.sh and
dg-extract-results.py a new -f option that reads the list of files to
process, one per line, from a named file, or from standard input when
the argument is "-".  The option may be given more than once and mixed
with file arguments.

A caller can then pipe the file list straight in via "-f -" instead of
expanding it onto the command line, so the length limit no longer
applies regardless of the host or Python flavor.

Existing callers can continue working as they were if they want, as
passing the file list as one file per argument is still supported.

Change-Id: Iaced5ba228972ce267f355de65fe5ad8a1e38c68
---
 contrib/dg-extract-results.py | 30 ++++++++++++++++++++++++++----
 contrib/dg-extract-results.sh | 17 +++++++++++++++--
 2 files changed, 41 insertions(+), 6 deletions(-)

diff --git a/contrib/dg-extract-results.py b/contrib/dg-extract-results.py
index 98b0f4989c9..b5d890f6406 100644
--- a/contrib/dg-extract-results.py
+++ b/contrib/dg-extract-results.py
@@ -164,7 +164,7 @@ class Prog:
     def usage (self):
         name = sys.argv[0]
         sys.stderr.write ('Usage: ' + name
-                          + ''' [-t tool] [-l variant-list] [-L] log-or-sum-file ...
+                          + ''' [-t tool] [-l variant-list] [-L] [-f list-file] log-or-sum-file ...
 
     tool           The tool (e.g. g++, libffi) for which to create a
                    new test summary file.  If not specified then output
@@ -174,6 +174,12 @@ class Prog:
                    variants in the files for <tool>.
     sum-file       A test summary file with the format of those
                    created by runtest from DejaGnu.
+    list-file      A file listing the log-or-sum files to process, one
+                   per line.  Use "-" to read the list from standard
+                   input.  This avoids the command-line length limit
+                   when combining very many files.  May be given more
+                   than once, and may be mixed with log-or-sum-file
+                   arguments.
     If -L is used, merge *.log files instead of *.sum.  In this
     mode the exact order of lines may not be preserved, just different
     Running *.exp chunks should be in correct order.
@@ -189,19 +195,35 @@ class Prog:
     # Parse the command-line arguments.
     def parse_cmdline (self):
         try:
-            (options, self.files) = getopt.getopt (sys.argv[1:], 'l:t:L')
-            if len (self.files) == 0:
-                self.usage()
+            (options, self.files) = getopt.getopt (sys.argv[1:], 'l:t:Lf:')
             for (option, value) in options:
                 if option == '-l':
                     self.variations.append (value)
                 elif option == '-t':
                     self.tools.append (value)
+                elif option == '-f':
+                    self.read_file_list (value)
                 else:
                     self.do_sum = False
+            if len (self.files) == 0:
+                self.usage()
         except getopt.GetoptError as e:
             self.fatal (None, e.msg)
 
+    # Append the files listed in FILENAME, one per line, to self.files.
+    # FILENAME of "-" means read the list from standard input.  Blank
+    # lines are ignored.
+    def read_file_list (self, filename):
+        f = sys.stdin if filename == '-' else open (filename, 'r')
+        try:
+            for line in f:
+                line = line.strip()
+                if line:
+                    self.files.append (line)
+        finally:
+            if f is not sys.stdin:
+                f.close()
+
     # Try to parse time string TIME, returning an arbitrary time on failure.
     # Getting this right is just a nice-to-have so failures should be silent.
     def parse_time (self, time):
diff --git a/contrib/dg-extract-results.sh b/contrib/dg-extract-results.sh
index d64ba255838..d03e1a9b4e1 100755
--- a/contrib/dg-extract-results.sh
+++ b/contrib/dg-extract-results.sh
@@ -42,7 +42,7 @@ done
 
 usage() {
   cat <<EOF >&2
-Usage: $PROGNAME [-t tool] [-l variant-list] [-L] sum-file ...
+Usage: $PROGNAME [-t tool] [-l variant-list] [-L] [-f list-file] sum-file ...
 
     tool           The tool (e.g. g++, libffi) for which to create a
                    new test summary file.  If not specified then all
@@ -52,6 +52,11 @@ Usage: $PROGNAME [-t tool] [-l variant-list] [-L] sum-file ...
                    variants in the files for <tool>.
     sum-file       A test summary file with the format of those
                    created by runtest from DejaGnu.
+    list-file      A file listing the sum-files to process, one per line.
+                   Use "-" to read the list from standard input.  This
+                   avoids the command-line length limit when combining
+                   very many files.  May be given more than once, and
+                   may be mixed with sum-file arguments.
     If -L is used, merge *.log files instead of *.sum.  In this
     mode the exact order of lines may not be preserved, just different
     Running *.exp chunks should be in correct order.
@@ -69,18 +74,26 @@ msg() {
 VARIANTS=""
 TOOL=""
 MODE="sum"
+LIST_FILES=""
 
-while getopts "l:t:L" ARG; do
+while getopts "l:t:Lf:" ARG; do
   case $ARG in
   l)  VARIANTS="${VARIANTS} ${OPTARG}";;
   t)  test -z "$TOOL" || (msg "${PROGNAME}: only one tool can be specified"; exit 1);
       TOOL="${OPTARG}";;
   L)  MODE="log";;
+  f)  if test "${OPTARG}" = "-" ; then
+        LIST_FILES="${LIST_FILES} `cat`"
+      else
+        LIST_FILES="${LIST_FILES} `cat "${OPTARG}"`"
+      fi;;
   \?) usage; exit 0;;
   esac
 done
 shift `expr ${OPTIND} - 1`
 
+set -- ${LIST_FILES} "$@"
+
 if test $# -lt 1 ; then
   usage
   exit 1
-- 
2.54.0

[PATCH 2/4] gdb/testsuite: Factor out dg-extract-results.sh calls

[Pedro Alves]

The parallel-testing targets in the testsuite Makefile each combine
the per-test gdb.sum / gdb.log files produced by a run into a single
combined pair, using contrib/dg-extract-results.sh.  The same pair of
invocations (once for gdb.sum, once with -L for gdb.log) is copied in
check-parallel, check-parallel-racy, and check-all-boards, each only
differing in the input and output directories.

Factor those invocations into a DG_EXTRACT_RESULTS canned recipe
parameterized by the input and output directories, and use it from all
three targets.

Change-Id: Ia3417c9410b87a415cb9b104efa60d4bfb9ffbc5
---
 gdb/testsuite/Makefile.in | 27 +++++++++++++--------------
 1 file changed, 13 insertions(+), 14 deletions(-)

diff --git a/gdb/testsuite/Makefile.in b/gdb/testsuite/Makefile.in
index f103d7ddd65..fb312583b0e 100644
--- a/gdb/testsuite/Makefile.in
+++ b/gdb/testsuite/Makefile.in
@@ -254,16 +254,23 @@ check-single-racy:
 	  `ls racy_outputs/*/gdb.sum` > racy.sum; \
 	sed -n '/=== gdb Summary ===/,$$ p' racy.sum
 
+# Combine the individual gdb.sum / gdb.log files found anywhere under
+# directory $(1) into a single gdb.sum / gdb.log pair in directory
+# $(2), using contrib/dg-extract-results.sh.
+define DG_EXTRACT_RESULTS
+	$(SHELL) $(srcdir)/../../contrib/dg-extract-results.sh \
+	  `find $(1) -name gdb.sum -print` > $(2)/gdb.sum; \
+	$(SHELL) $(srcdir)/../../contrib/dg-extract-results.sh -L \
+	  `find $(1) -name gdb.log -print` > $(2)/gdb.log
+endef
+
 check-parallel:
 	-rm -f *core*
 	-rm -rf cache outputs temp
 	$(MAKE) -k do-check-parallel; \
 	result=$$?; \
 	if test -d outputs; then \
-	  $(SHELL) $(srcdir)/../../contrib/dg-extract-results.sh \
-	    `find outputs -name gdb.sum -print` > gdb.sum; \
-	  $(SHELL) $(srcdir)/../../contrib/dg-extract-results.sh -L \
-	    `find outputs -name gdb.log -print` > gdb.log; \
+	  $(call DG_EXTRACT_RESULTS,outputs,.); \
 	  $(SHELL) $(srcdir)/lib/dg-add-core-file-count.sh; \
 	  sed -n '/=== gdb Summary ===/,$$ p' gdb.sum; \
 	fi; \
@@ -282,12 +289,7 @@ check-parallel-racy:
 	for n in `seq $$racyiter` ; do \
 	  $(MAKE) -k do-check-parallel-racy \
 	    RACY_OUTPUT_N=$$n; \
-	  $(SHELL) $(srcdir)/../../contrib/dg-extract-results.sh \
-	    `find racy_outputs/$$n -name gdb.sum -print` > \
-	    racy_outputs/$$n/gdb.sum; \
-	  $(SHELL) $(srcdir)/../../contrib/dg-extract-results.sh -L \
-	    `find racy_outputs/$$n -name gdb.log -print` > \
-	    racy_outputs/$$n/gdb.log; \
+	  $(call DG_EXTRACT_RESULTS,racy_outputs/$$n,racy_outputs/$$n); \
 	  sed -n '/=== gdb Summary ===/,$$ p' racy_outputs/$$n/gdb.sum; \
 	done; \
 	$(srcdir)/analyze-racy-logs.py \
@@ -389,10 +391,7 @@ check-all-boards: all $(abs_builddir)/site.exp
 		"$(TESTS)" \
 	result=$$?; \
 	if test -d check-all; then \
-	  $(SHELL) $(srcdir)/../../contrib/dg-extract-results.sh \
-	    `find check-all -name gdb.sum -print` > check-all/gdb.sum; \
-	  $(SHELL) $(srcdir)/../../contrib/dg-extract-results.sh -L \
-	    `find check-all -name gdb.log -print` > check-all/gdb.log; \
+	  $(call DG_EXTRACT_RESULTS,check-all,check-all); \
 	  sed -n '/=== gdb Summary ===/,$$ p' check-all/gdb.sum; \
 	fi; \
 	exit $$result
-- 
2.54.0

[PATCH 3/4] gdb/testsuite: Add extract-results make target

[Pedro Alves]

Add a new "make extract-results" target that regenerates the combined
gdb.sum/gdb.log from an existing outputs/ directory, without
re-running any tests.

This is handy after a parallel "make check" run to rebuild the
combined results by hand -- for example after tweaking
contrib/dg-extract-results.sh, to see the effect on the combined files
without a full testsuite run.

Change-Id: I67cac742343dd2c6091cf0013343af86cb86a1e7
---
 gdb/testsuite/Makefile.in | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/gdb/testsuite/Makefile.in b/gdb/testsuite/Makefile.in
index fb312583b0e..f671e90e125 100644
--- a/gdb/testsuite/Makefile.in
+++ b/gdb/testsuite/Makefile.in
@@ -276,6 +276,19 @@ check-parallel:
 	fi; \
 	exit $$result
 
+# Regenerate the combined gdb.sum / gdb.log from an existing outputs/
+# directory, without re-running any tests.  Handy after a "make check"
+# parallel run to rebuild the combined results by hand -- for example
+# after tweaking contrib/dg-extract-results.sh.
+extract-results:
+	@if test ! -d outputs; then \
+	  echo "No outputs/ directory found; run a parallel \"make check\" first."; \
+	  exit 1; \
+	fi
+	$(call DG_EXTRACT_RESULTS,outputs,.)
+	$(SHELL) $(srcdir)/lib/dg-add-core-file-count.sh
+	sed -n '/=== gdb Summary ===/,$$ p' gdb.sum
+
 check-parallel-racy:
 	-rm -rf cache racy_outputs temp
 	racyiter="$(RACY_ITER)"; \
-- 
2.54.0

[PATCH 4/4] gdb/testsuite: Pipe the dg-extract-results file list via stdin

[Pedro Alves]

The parallel-testing targets combine the per-test result files into a
single gdb.sum / gdb.log pair with the DG_EXTRACT_RESULTS macro, which
passes the whole list to contrib/dg-extract-results.sh on the command
line, one positional argument per file:

 $(SHELL) .../dg-extract-results.sh \
   `find $(1) -name gdb.sum -print` > $(2)/gdb.sum

A parallel run writes one gdb.sum (and one gdb.log) per .exp, so with
GDB's testsuite the whole list is thousands of paths.  On Windows with
native Windows Python that overflows the command-line length limit
(~32 KB), and the run dies while combining the results:

 $ make check-parallel
 ...
 .../dg-extract-results.sh: line 39: /ucrt64/bin/python3: Argument list too long
 make: *** [Makefile:274: check-parallel] Error 2
 ...
 $ find outputs/ -name gdb.sum | wc -l
 2657

An earlier commit taught dg-extract-results.sh/dg-extract-results.py a
-f option that reads the file list from a file, or from standard input
when the argument is "-", so we can now fix this by piping the find
output into dg-extract-results.sh via stdin instead, so the command
line length limit no longer applies.

Change-Id: I5d8c94332eda9bf49c9b0144dabb846de1102f57
---
 gdb/testsuite/Makefile.in | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/gdb/testsuite/Makefile.in b/gdb/testsuite/Makefile.in
index f671e90e125..11782a860db 100644
--- a/gdb/testsuite/Makefile.in
+++ b/gdb/testsuite/Makefile.in
@@ -257,11 +257,18 @@ check-single-racy:
 # Combine the individual gdb.sum / gdb.log files found anywhere under
 # directory $(1) into a single gdb.sum / gdb.log pair in directory
 # $(2), using contrib/dg-extract-results.sh.
+#
+# The file list is piped in via "-f -" rather than expanded onto the
+# command line: a parallel run writes one gdb.sum/gdb.log per .exp, so
+# the list can hold thousands of paths and overflow the command-line
+# length limit (notably on Windows, where the limit is ~32 KB).
 define DG_EXTRACT_RESULTS
-	$(SHELL) $(srcdir)/../../contrib/dg-extract-results.sh \
-	  `find $(1) -name gdb.sum -print` > $(2)/gdb.sum; \
-	$(SHELL) $(srcdir)/../../contrib/dg-extract-results.sh -L \
-	  `find $(1) -name gdb.log -print` > $(2)/gdb.log
+	find $(1) -name gdb.sum -print \
+	  | $(SHELL) $(srcdir)/../../contrib/dg-extract-results.sh -f - \
+	  > $(2)/gdb.sum; \
+	find $(1) -name gdb.log -print \
+	  | $(SHELL) $(srcdir)/../../contrib/dg-extract-results.sh -L -f - \
+	  > $(2)/gdb.log
 endef
 
 check-parallel:
-- 
2.54.0