Re: bug#10427: bug#10436: New testsuite driver and extra trailing backslash in recipes

[Paul Eggert]

I pushed the following doc fix into Autoconf, so that these two portability
issues are documented there.  It turns out that the second issue
is actually due to an old Bash bug -- it's not Solaris-specific.

>From b1f0e147aa7aa259dea2c34c5a0ac7965d6efd7e Mon Sep 17 00:00:00 2001
From: Paul Eggert 
Date: Thu, 5 Jan 2012 11:00:45 -0800
Subject: [PATCH 1/2] doc: clarify sed buffer limit

* doc/autoconf.texi (Limitations of Usual Tools):
That 4000-byte limit applies to output and internal buffers, too.
---
 ChangeLog |6 ++
 doc/autoconf.texi |5 +++--
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 30312be..238c09f 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+2012-01-05  Paul Eggert  
+
+   doc: clarify sed buffer limit
+   * doc/autoconf.texi (Limitations of Usual Tools):
+   That 4000-byte limit applies to output and internal buffers, too.
+
 2012-01-03  Paul Eggert  
 
maint: update copyright year
diff --git a/doc/autoconf.texi b/doc/autoconf.texi
index 72ff7f8..ea2419b 100644
--- a/doc/autoconf.texi
+++ b/doc/autoconf.texi
@@ -19256,8 +19256,9 @@ $ @kbd{echo a | sed 's/x/x/;;s/x/x/'}
 sed: 1: "s/x/x/;;s/x/x/": invalid command code ;
 @end example
 
-Input should not have unreasonably long lines, since some @command{sed}
-implementations have an input buffer limited to 4000 bytes.  Likewise,
+Some @command{sed} implementations have a buffer limited to 4000 bytes,
+and this limits the size of input lines, output lines, and internal
+buffers that can be processed portably.  Likewise,
 not all @command{sed} implementations can handle embedded @code{NUL} or
 a missing trailing newline.
 
-- 
1.7.6.5


>From 0445b4ad121e6356f409833dec3678ae613e1d08 Mon Sep 17 00:00:00 2001
From: Paul Eggert 
Date: Thu, 5 Jan 2012 12:32:12 -0800
Subject: [PATCH 2/2] doc: mention Bash 2.03 bug with backslash-newline

* doc/autoconf.texi (Invoking the Shell): New section.
(Backslash-Newline-Empty): Rename from Backslash-Newline-Newline.
Mention problem with Bash 2.03.
---
 ChangeLog |5 
 doc/autoconf.texi |   56 +---
 2 files changed, 57 insertions(+), 4 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 238c09f..69df405 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,10 @@
 2012-01-05  Paul Eggert  
 
+   doc: mention Bash 2.03 bug with backslash-newline
+   * doc/autoconf.texi (Invoking the Shell): New section.
+   (Backslash-Newline-Empty): Rename from Backslash-Newline-Newline.
+   Mention problem with Bash 2.03.
+
doc: clarify sed buffer limit
* doc/autoconf.texi (Limitations of Usual Tools):
That 4000-byte limit applies to output and internal buffers, too.
diff --git a/doc/autoconf.texi b/doc/autoconf.texi
index ea2419b..5c3feef 100644
--- a/doc/autoconf.texi
+++ b/doc/autoconf.texi
@@ -501,6 +501,7 @@ Dependencies Between Macros
 Portable Shell Programming
 
 * Shellology::  A zoology of shells
+* Invoking the Shell::  Invoking the shell as a command
 * Here-Documents::  Quirks and tricks
 * File Descriptors::FDs and redirections
 * Signal Handling:: Shells, signals, and headaches
@@ -520,7 +521,7 @@ Portable Make Programming
 * $< in Ordinary Make Rules::   $< in ordinary rules
 * Failure in Make Rules::   Failing portably in rules
 * Special Chars in Names::  Special Characters in Macro Names
-* Backslash-Newline-Newline::   Empty last lines in macro definitions
+* Backslash-Newline-Empty:: Empty lines after backslash-newline
 * Backslash-Newline Comments::  Spanning comments across line boundaries
 * Long Lines in Makefiles:: Line length limitations
 * Macros and Submakes:: @code{make macro=value} and submakes
@@ -15067,6 +15068,7 @@ subset described above, is fairly portable nowadays.  
Also please see
 
 @menu
 * Shellology::  A zoology of shells
+* Invoking the Shell::  Invoking the shell as a command
 * Here-Documents::  Quirks and tricks
 * File Descriptors::FDs and redirections
 * Signal Handling:: Shells, signals, and headaches
@@ -15205,6 +15207,28 @@ The default Mac OS X @command{sh} was originally Zsh; 
it was changed to
 Bash in Mac OS X 10.2.
 @end table
 
+@node Invoking the Shell
+@section Invoking the Shell
+@cindex invoking the shell
+@cindex shell invocation
+
+Bash 2.03 has a bug when invoked with the @option{-c} option: if the
+option-argument ends in backslash-newline, Bash incorrectly reports a
+syntax error.  The problem does not occur if a character follows the
+backslash:
+
+@example
+$ @kbd{$ bash -c 'echo foo \}
+> @kbd{'}
+bash: -c: line 2: syntax error: unexpected end of file
+$ @kbd{bash -c 'echo foo \}
+> @kbd{ '}
+foo
+@end example
+
+@noindent
+@xref{Backslash-Newline-Empty}, for how this can cause problems in makefiles.
+
 @node Here-Documents
 @sec

Re: bug#10437: parallel-tests: `recheck' recipe can cause sed to be invoked with too long input lines

[Stefano Lattarini]

Reference:
  

On 01/05/2012 03:07 PM, Stefano Lattarini wrote:
>
> Patch coming up soon.
> 
And here it is.  I will push by this evening if there is no objection.

Regards,
  Stefano
>From e3b0e12400f5fa4220fc0aa79dd0989e56def9c6 Mon Sep 17 00:00:00 2001
Message-Id: 
From: Stefano Lattarini 
Date: Thu, 5 Jan 2012 15:13:30 +0100
Subject: [PATCH] parallel-tests: avoid issue with overly long lines in sed
 input

See automake bug#10437:
  
and coreutils bug#10427:
  

* lib/am/check.am (recheck, recheck-html): In order to strip
trailing whitespace from the definition of the `$list' variable,
we used to invoke sed in a way that could cause it to get passed
overly long input lines, causing spurious failures.  So rework
the logic of the recipe to avoid any sed invocation, relying on
simpler shell idioms instead.
(check-TESTS): Reorganize the recipe to be more similar to the
one of `recheck', for consistency and simplicity.
* NEWS: Update.

Report and analysis by Paul Eggert.
---
 NEWS|4 
 lib/am/check.am |   38 +-
 2 files changed, 25 insertions(+), 17 deletions(-)

diff --git a/NEWS b/NEWS
index 2e572e4..7e52d83 100644
--- a/NEWS
+++ b/NEWS
@@ -89,6 +89,10 @@ Bugs fixed in 1.11.0a:
 
 * Bugs introduced by 1.11:
 
+  - The parallel-tests harness doesn't trip anymore on sed implementations
+with stricter limits on the length of input lines (problem seen at
+least on Solaris 8).
+
   - The `parallel-tests' test driver works around a GNU make 3.80 bug with
 trailing white space in the test list (`TESTS = foo $(EMPTY)'), and
 does not report spurious successes when used with concurrent FreeBSD
diff --git a/lib/am/check.am b/lib/am/check.am
index 3d0188d..29faa38 100644
--- a/lib/am/check.am
+++ b/lib/am/check.am
@@ -245,15 +245,16 @@ check-TESTS:
 ## OTOH, this means that, in the rule for `$(TEST_SUITE_LOG)', we
 ## cannot use `$?' to compute the set of lazily rerun tests, lest
 ## we rely on .PHONY to work portably.
-##
+	@test -z "$(TEST_SUITE_LOG)" || rm -f $(TEST_SUITE_LOG)
+	@list='' list2='$(TEST_LOGS)'; for f in $$list2; do \
 ## Trailing whitespace in `TESTS = foo.test $(empty)' causes GNU make
 ## 3.80 to erroneously expand $(TESTS_LOGS) to `foo.log .log'.
 ## Work around this bug.
-	@test -z "$(TEST_SUITE_LOG)" || rm -f $(TEST_SUITE_LOG)
-	@list='$(TEST_LOGS)';		\
-	list=`for f in $$list; do	\
-	  test .log = $$f || echo $$f;	\
-	done | tr '\012\015' '  '`;	\
+	  test .log = $$f && continue; \
+## Be careful to avoid extra whitespace in the definition of $list.  See
+## comments in `recheck' below for why this might be useful.
+	  if test -z "$$list"; then list=$$f; else list="$$list $$f"; fi; \
+	done; \
 	$(MAKE) $(AM_MAKEFLAGS) $(TEST_SUITE_LOG) TEST_LOGS="$$list"
 
 AM_RECURSIVE_TARGETS += check
@@ -298,17 +299,20 @@ AM_RECURSIVE_TARGETS += check-html
 
 ## Rerun all FAILed or XPASSed tests.
 recheck recheck-html:
-	@target=`echo $@ | sed 's,^re,,'`;\
-	list='$(TEST_LOGS)';		\
-	list=`for f in $$list; do	\
-	test -f $$f || continue;\
-	if test -r $$f && read line < $$f; then			\
-	  case $$line in FAIL*|XPASS*) echo $$f;; esac;		\
-	else echo $$f; fi;	\
-	  done | tr '\012\015' '  '`;\
-## This apparently useless munging helps to avoid a nasty bug (a
-## segmentation fault!) on Solaris XPG4 make.
-	list=`echo "$$list" | sed 's/ *$$//'`;\
+	@target=`echo $@ | sed 's,^re,,'`; \
+	list='' list2='$(TEST_LOGS)'; for f in $$list2; do \
+	  test -f $$f || continue; \
+	  if test -r $$f && read line < $$f; then \
+	case $$line in FAIL*|XPASS*) : ;; *) continue;; esac; \
+	  fi; \
+## Be careful to avoid extra whitespace in the definition of $list, since
+## its value will be passed to the recursive make invocation below through
+## the TEST_LOGS macro, and leading/trailing white space in a make macro
+## definition can be problematic.  In this particular case, trailing white
+## space was known to cause a segmentation fault on Solaris 10 XPG4 make:
+## 
+	  if test -z "$$list"; then list=$$f; else list="$$list $$f"; fi; \
+	done; \
 	$(MAKE) $(AM_MAKEFLAGS) $$target AM_MAKEFLAGS='$(AM_MAKEFLAGS) TEST_LOGS="'"$$list"'"'
 
 .PHONY: recheck recheck-html
-- 
1.7.7.3

Re: bug#10436: New testsuite driver and extra trailing backslash in recipes

[Stefano Lattarini]

On 01/05/2012 01:47 PM, Stefano Lattarini wrote:
> [adding bug-automake in CC:]
> 
> Reference: 
> 
And here is the definitive version of the patch that I'll push by this
evening (to master) if there is no objection.

Thanks,
  Stefano
>From c891eff22b84027ea56cf67b4c5aac55b75390d9 Mon Sep 17 00:00:00 2001
Message-Id: 
From: Stefano Lattarini 
Date: Thu, 5 Jan 2012 13:41:13 +0100
Subject: [PATCH] parallel-tests: avoid trailing backslashes in make recipes

The new testsuite-harness could generate recipes with a trailing
backslash character (possibly followed by blank characters only),
in the very common case where the user hadn't defined the special
$(AM_TESTS_FD_REDIRECT) variable.  This caused spurious syntax
errors with at least older bash versions (e.g., bash 2.05b), and
could be potentially unportable to other weaker shells.

See automake bug#10436:
  
and coreutils bug#10427:
  

* lib/am/check2.am: Rework line breaks so that no backslash can
be at the end of a line.
* tests/parallel-tests-trailing-bslash.test: New test.
* tests/list-of-tests.mk: Add it.
---
 lib/am/check2.am  |   10 +-
 tests/list-of-tests.mk|1 +
 tests/parallel-tests-trailing-bslash.test |  114 +
 3 files changed, 120 insertions(+), 5 deletions(-)
 create mode 100755 tests/parallel-tests-trailing-bslash.test

diff --git a/lib/am/check2.am b/lib/am/check2.am
index ad0a4aa..9a0fe9d 100644
--- a/lib/am/check2.am
+++ b/lib/am/check2.am
@@ -1,5 +1,5 @@
 ## automake - create Makefile.in from Makefile.am
-## Copyright (C) 2008, 2009, 2011 Free Software Foundation, Inc.
+## Copyright (C) 2008, 2009, 2011, 2012 Free Software Foundation, Inc.
 
 ## This program is free software; you can redistribute it and/or modify
 ## it under the terms of the GNU General Public License as published by
@@ -19,8 +19,8 @@
 ?!GENERIC?%OBJ%: %SOURCE%
 	@p='%SOURCE%'; $(am__check_pre) %DRIVER% --test-name "$$f" \
 	--log-file '%BASE%.log' --trs-file '%BASE%.trs' \
-	$(am__common_driver_flags) %DRIVER_FLAGS% -- %COMPILE% "$$tst" \
-	$(AM_TESTS_FD_REDIRECT)
+	$(am__common_driver_flags) %DRIVER_FLAGS% -- %COMPILE% \
+	"$$tst" $(AM_TESTS_FD_REDIRECT)
 
 ## If no programs are built in this package, then this rule is removed
 ## at automake time.  Otherwise, %am__EXEEXT% expands to a configure time
@@ -30,6 +30,6 @@ if %am__EXEEXT%
 ?GENERIC?%EXT%$(EXEEXT).log:
 	@p='%SOURCE%'; $(am__check_pre) %DRIVER% --test-name "$$f" \
 	--log-file '%BASE%.log' --trs-file '%BASE%.trs' \
-	$(am__common_driver_flags) %DRIVER_FLAGS% -- %COMPILE% "$$tst" \
-	$(AM_TESTS_FD_REDIRECT)
+	$(am__common_driver_flags) %DRIVER_FLAGS% -- %COMPILE% \
+	"$$tst" $(AM_TESTS_FD_REDIRECT)
 endif %am__EXEEXT%
diff --git a/tests/list-of-tests.mk b/tests/list-of-tests.mk
index 8b4f2ca..dbadfdc 100644
--- a/tests/list-of-tests.mk
+++ b/tests/list-of-tests.mk
@@ -709,6 +709,7 @@ parallel-tests-no-spurious-summary.test \
 parallel-tests-exit-statuses.test \
 parallel-tests-console-output.test \
 parallel-tests-once.test \
+parallel-tests-trailing-bslash.test \
 tests-environment.test \
 am-tests-environment.test \
 tests-environment-backcompat.test \
diff --git a/tests/parallel-tests-trailing-bslash.test b/tests/parallel-tests-trailing-bslash.test
new file mode 100755
index 000..418b722
--- /dev/null
+++ b/tests/parallel-tests-trailing-bslash.test
@@ -0,0 +1,114 @@
+#! /bin/sh
+# Copyright (C) 2012 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2, or (at your option)
+# any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see .
+
+# Check that the new testsuite harness do not generate recipes that can
+# have a trailing `\', since that can cause spurious syntax errors with
+# older bash versions (e.g., bash 2.05b).
+# See automake bug#10436.
+
+am_parallel_tests=yes
+. ./defs || Exit 1
+
+echo AC_OUTPUT >> configure.in
+
+cat > Makefile.am <<'END'
+TESTS = foo.test
+EXTRA_DIST = $(TESTS)
+am__backslash = \\ # foo
+.PHONY: bad-recipe
+bad-recipe:
+	@printf '%s\n' $(am__backslash)
+END
+
+cat > foo.test <<'END'
+#!/bin/sh
+exit 0
+END
+chmod +x foo.test
+
+am__SHELL=$SHELL; export am__SHELL
+am__PERL=$PERL; export am__PERL
+
+cat > my-shell <<'END'
+#!/bin/sh -e
+set -u
+tab='	'
+nl='
+'
+am__shell_flags=
+am__shell_command=; unset am__shell_command

Re: [PATCH] amhello: help compilers not supporting -c and -o together

[Peter Rosin]

Stefano Lattarini skrev 2012-01-05 10:04:
> On 01/05/2012 01:46 AM, Peter Rosin wrote:
>> Hi!
>>
>> Ok for maint, or should the example be as simple as it can possible get
>> even if portability is sacrificed?
>>
> Are you sure portability is being sacrificed in our example?  I thought that
> the use of `-c' and `-o' together was only required when target-specific flags
> where used (as with e.g. "foo_CPPFLAGS = -DFOO=1").  Am I missing something?

No.  I simply forgot about that.  But while -o (w/o -c) does work for
cl *in this case*, -o has been deprecated for eons (IIRC it was deprecated
at least in MSVC 6, some 10-15 years ago, but perhaps it was deprecated even
before that?) and doesn't work as one would hope (you can't move the output
to some other directory, or something like that).

Anyway, this doesn't look too good (but it works):

$ cl -o hello.exe main.obj
Microsoft (R) 32-bit C/C++ Optimizing Compiler Version 16.00.40219.01 for 80x86
Copyright (C) Microsoft Corporation.  All rights reserved.

cl : Command line warning D9035 : option 'o' has been deprecated and will be 
removed in a future release
Microsoft (R) Incremental Linker Version 10.00.40219.01
Copyright (C) Microsoft Corporation.  All rights reserved.

/out:main.exe
/out:hello.exe
main.obj


While this looks so much better:

$ lib/compile cl -o hello.exe main.obj
Microsoft (R) 32-bit C/C++ Optimizing Compiler Version 16.00.40219.01 for 80x86
Copyright (C) Microsoft Corporation.  All rights reserved.

Microsoft (R) Incremental Linker Version 10.00.40219.01
Copyright (C) Microsoft Corporation.  All rights reserved.

/out:hello.exe
main.obj


But lets cross the bridge when MS actually removes that option.


> You might also want to look at the test `amhello-binpkg.test'; if it doesn't
> fail when using a compiler that cannot grasp `-c -o' together, then our 
> example
> should have no portability problem.

Right.

Wasted effort, sigh...

Cheers,
Peter

Re: [PATCH] drop "Win32" term

[Peter Rosin]

Stefano Lattarini skrev 2012-01-04 18:28:
> Hi Paul, thanks for the patch.
> 
> I've completed the git commit log as follows (mostly to abide to the
> GCS):
> 
>   * doc/automake.texi: Mention "Windows", not "Win32".
>   * lib/Automake/XFile.pm: Likewise.
>   * lib/ar-lib: Likewise.
>   * lib/compile: Likewise.
>   * tests/compile2.test: Likewise.
> 
> I've then squashed in the amendments below, and pushed in your name.

Hmm, I believe the msvc branch would have been a better fit for
this.  Now we have different versions of these scripts on master,
maint and branch-1.11, and that just can't be good.

Cheers,
Peter

Re: [PATCH] drop "Win32" term

[Stefano Lattarini]

Hi Bruno, sorry for the delay.

On 01/04/2012 03:44 PM, Bruno Haible wrote:
> Hi Stefano,
> 
>>> Here's a proposed patch for automake.
>>>
>> Thanks.  I have some minor nits and qualms with the patch, which I hope
>> you can address in a re-roll.
> 
> Was this meant as an explanation to your fellow Automake maintainers,
> or to me personally?
>
It is an explanation for all the potential contributors; it is not meant
*exclusively* for you.  Sorry for not being more clear.

Anyway, it's perfectly OK not to address most the nits I've reported, as
I can address them myself more easily and faster, as you have correctly
pointed out (and as I *should* have pointed out myself, instead of leaving
it implicit; sorry about that).

There is one exception though, and this is for you personally ...

>
> [SNIP various good points]
> 

>> First (and this is the only serious objection): could you please
>> re-send your patch formatting with "git format-patch"?  That will make
>> it far more easy to apply.
> 
> Patches that are sent without git specific formatting can be applied
> with "patch -p0 < mailfile" or "patch -p1 < mailfile".
>
True, and in fact that worked with your patch as well.  *But*, since you are
using git anyway, and are likely quite proficient with it, why not use the
"blessed", standard way to prepare patches, that is, "git format-patch"?

Regards,
  Stefano

Re: [PATCH] amhello: help compilers not supporting -c and -o together

[Stefano Lattarini]

On 01/05/2012 01:46 AM, Peter Rosin wrote:
> Hi!
> 
> Ok for maint, or should the example be as simple as it can possible get
> even if portability is sacrificed?
>
Are you sure portability is being sacrificed in our example?  I thought that
the use of `-c' and `-o' together was only required when target-specific flags
where used (as with e.g. "foo_CPPFLAGS = -DFOO=1").  Am I missing something?

You might also want to look at the test `amhello-binpkg.test'; if it doesn't
fail when using a compiler that cannot grasp `-c -o' together, then our example
should have no portability problem.

Thanks,
  Stefano