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

[Jim Meyering]

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
...
> 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?

Good catch.  That also affects dash-0.5.6-2.fc14.x86_64
Probably worth reporting.

> 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 tested all retry_delay_ using tests with dash like:
> (cd tests && make check TESTS=misc/stat-birthtime VERBOSE=yes SHELL=dash)
> Note that required fixing the 'local saved_IFS=$IFS' in tests/init.cfg also.
> Are we going to disallow dash because of this "local" issue, or are

For now, I'll remove that test, to re-allow dash.
Longer term, I want to clean up.

> we going to change all uses of local name=$val to local name; name=$val ?
> Note by using the single float patch above you don't need to change
> any of the local delay="$1" lines in the tests ($1 is quoted in any case).
> In fact I can't see any uses of "local" under tests that need to change,
> but we may want to do so for style/consistency reasons.

Yes, your patch is better.  You're welcome to push it.

I'm thinking of adding a syntax check to detect these:

  $ git grep -E '^ *local .*[a-zA-Z0-9_]+=\$'
  lib/t-chdir-long:  local root=${TMPDIR=/tmp}
  tests/init.cfg:    local pid=$!
  tests/init.cfg:  local test_func=$1
  tests/init.cfg:  local init_delay=$2
  tests/init.cfg:  local max_n_tries=$3
  tests/init.cfg:  local num_sleeps=$attempt
  tests/misc/sort-float:  local ldbl_whole=$1 ldbl_frac=$2 ldbl_exp=$3
  tests/misc/sort-float:  local dbl_whole=$1 dbl_frac=$2 dbl_exp=$3