Hi Eric, Peter, and thanks to both for the super-quick diagnosis and fix.
On 01/10/2012 12:52 AM, Peter Rosin wrote:
> Eric Blake skrev 2012-01-10 00:34:
>> On 01/09/2012 03:42 PM, Peter Rosin wrote:
>>> FWIW, this "fixes" it, but I don't actually know why a subshell would
>>> make a difference?
>>>
>>> $ sh --version
>>> GNU bash, version 3.1.17(1)-release (i686-pc-msys)
>>
>>>
>>> -{
>>> +(
>>> (
>>> # Ignore common signals (in this subshell only!), to avoid potential
>>> # problems with Korn shells. Some Korn shells are known to propagate
>>> @@ -634,7 +634,7 @@ exit 0
>>> '
>>>
>>> # TODO: document that we consume the file descriptor 3 :-(
>>> -} 3>"$log_file"
>>> +) 3>"$log_file"
>>
>> Ah - the classic bash bug documented in the autoconf manual:
>>
>> https://www.gnu.org/software/autoconf/manual/autoconf.html#Limitations-of-Builtins
>>
>> Bash 3.2 (and earlier versions) sometimes does not properly set ‘$?’
>> when failing to write redirected output of a compound command. This
>> problem is most commonly observed with ‘{...}’; it does not occur with
>> ‘(...)’. For example:
>>
>> $ bash -c '{ echo foo; } >/bad; echo $?'
>> bash: line 1: /bad: Permission denied
>> 0
>> $ bash -c 'while :; do echo; done >/bad; echo $?'
>> bash: line 1: /bad: Permission denied
>> 0
>>
>> To work around the bug, prepend ‘:;’:
>>
>> $ bash -c ':;{ echo foo; } >/bad; echo $?'
>> bash: line 1: /bad: Permission denied
>> 1
>>
>
> That works too, and is cheaper! Thanks for pointing to that workaround!
>
> So, better version:
>
The patch is good, but I have a couple of nits below. Addressing them is
not a requirement for an ACK, though (even if I'd prefer to have them
addressed obviously). Feel free to push when you are ready.
Thanks!
> From 38c4a7e2fcfe17a27bd16744f12d53e0968d1e52 Mon Sep 17 00:00:00 2001
> From: Peter Rosin <p...@lysator.liu.se>
> Date: Tue, 10 Jan 2012 00:50:22 +0100
> Subject: [PATCH] tap/awk: add workaround for bash 3.2 and earlier
>
s/add workaround for/fix redirection issues with/ perhaps?
> Fixes automake bug#10465.
>
> * lib/tap-driver.sh: Add workaround for bash 3.2 and earlier which has
> problems with compound statements, failing to redirect and exit status,
> as documented in the Autoconf manual.
>
What about stealing the wording of the autoconf manual, to make this entry
clearer? Something like this:
Add workaround for bash 3.2 and earlier, which sometimes does not
properly set '$?' when failing to write redirected output of a compound
command. See the Autoconf manual for more details.
> The workaround was pointed out by Eric Blake.
> ---
> lib/tap-driver.sh | 4 +++-
> 1 files changed, 3 insertions(+), 1 deletions(-)
>
> diff --git a/lib/tap-driver.sh b/lib/tap-driver.sh
> index c011298..39de9c4 100755
> --- a/lib/tap-driver.sh
> +++ b/lib/tap-driver.sh
> @@ -115,7 +115,9 @@ else
> init_colors=''
> fi
>
> -{
> +# :; is there to work around a bug in bash 3.2 (and earlier) which
> +# does not always set $? properly on redirection failure.
>
For the sake of completeness (and at cost of being repetitive), I'd add a
reference to the Autoconf manual here as well, as: "See the Autoconf manual
for more details."
> +:;{
> (
> # Ignore common signals (in this subshell only!), to avoid potential
> # problems with Korn shells. Some Korn shells are known to propagate
>
Regards,
Stefano
