Re: [coreutils] [PATCH] tests: avoid portability problem with dash vs. local v=$splittable_val

[Pádraig Brady]

On 08/11/10 10:30, Jim Meyering wrote:
> Pádraig Brady wrote:
>> On 06/11/10 17:08, Jim Meyering wrote:
>>> Hi Pádraig,
>>>
>>> When running make check SHELL=dash, I got one failure.
>>> It was in misc/stat-birthtime, due to its use retry_delay_:
>>>
>>>   + num_sleeps=16
>>>   + test 5 -le 5
>>>   + gawk -v n=16 -v s=.07 BEGIN { for (i=0;i<n;i++) t = s" "t; print t }
>>>   + local delay=.07 .07 .07 .07 .07 .07 .07 .07 .07 .07 .07 .07 .07 .07 .07 
>>> .07
>>>   local: 1: .07: bad variable name
>>>
>>>
>>> >From bda912213e8605121226ce2e07951f1b682ef595 Mon Sep 17 00:00:00 2001
>>> From: Jim Meyering <address@hidden>
>>> Date: Sat, 6 Nov 2010 18:04:45 +0100
>>> Subject: [PATCH] tests: avoid portability problem with dash vs. local 
>>> v=$splittable_val
>>>
>>> * tests/init.cfg (retry_delay_): Put "local var" in a statement before
>>> "var=...", to avoid portability problems with dash, when the RHS value
>>> contains white space.
>>> ---
>>>  tests/init.cfg |    5 +++--
>>>  1 files changed, 3 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/tests/init.cfg b/tests/init.cfg
>>> index 7786257..fa1f0e8 100644
>>> --- a/tests/init.cfg
>>> +++ b/tests/init.cfg
>>> @@ -390,8 +390,9 @@ retry_delay_()
>>>    local num_sleeps=$attempt
>>>    local time_fail
>>>    while test $attempt -le $max_n_tries; do
>>> -    local delay=$($AWK -v n=$num_sleeps -v s="$init_delay" \
>>> -                  'BEGIN { for (i=0;i<n;i++) t = s" "t; print t }')
>>> +    local delay
>>> +    delay=$($AWK -v n=$num_sleeps -v s="$init_delay" \
>>> +            'BEGIN { for (i=0;i<n;i++) t = s" "t; print t }')
>>>      "$test_func" "$delay" && { time_fail=0; break; } || time_fail=1
>>>      attempt=$(expr $attempt + 1)
>>>      num_sleeps=$(expr $num_sleeps '*' 2)
>>> --
>>> 1.7.3.2.193.g78bbb
>>
>> I see that issue with my dash-0.5.5.1-2.fc11 also:
>>
>> $ dash -c 's="1 1"; ok=$s; local l=$s'
>> local: 1: 1: bad variable name
>>
>> BTW, the above should give another error because "local"
>> is used outside a function. It does when within a script
>> that's executed, but not when the script (file name)
>> is passed as a parameter to dash?
>>
>> I also tested the following (generate a single float)
>> which seems a bit cleaner to me:
>>
>> diff --git a/tests/init.cfg b/tests/init.cfg
>> index 1048cf3..d704f34 100644
>> --- a/tests/init.cfg
>> +++ b/tests/init.cfg
>> @@ -388,8 +388,8 @@ retry_delay_()
>>    local num_sleeps=$attempt
>>    local time_fail
>>    while test $attempt -le $max_n_tries; do
>> -    local delay=$($AWK -v n=$num_sleeps -v s="$init_delay" \
>> -                  'BEGIN { for (i=0;i<n;i++) t = s" "t; print t }')
>> +    local delay=$($AWK -v n=$num_sleeps -v s=$init_delay \
>> +                  'BEGIN { print s*n }')
>>      "$test_func" "$delay" && { time_fail=0; break; } || time_fail=1
>>      attempt=$(expr $attempt + 1)
>>      num_sleeps=$(expr $num_sleeps '*' 2)
> 
> I went for a slightly smaller change in your name.
> If you'd like to remove the quotes around "$init_delay"
> in a later change, please add a test like this to ensure
> that it is a valid token (i.e., no white space):
> 
>   case $init_delay in
>   *[^+.0-9]*) skip_ "invalid delay: $init_delay; internal error, please 
> report"
>   esac
> 
> I'll push this shortly.

Well it's a very minor issue, but I removed the quotes
so that we'd get an error if multiple params passed.

d="1m 2s"
awk -v n=4 -v s="$d" 'BEGIN { print s * n }'
awk -v n=4 -v s=$d 'BEGIN { print s * n }'

To be robust we'd need to restrict to a single seconds value.
I'll fix that up after the release if I get bored.

cheers,
Pádraig.