Re: [PATCH 11/12] t0610: fix non-portable variable assignment

[Patrick Steinhardt <ps@pks.im>]

[-- Attachment #1: Type: text/plain, Size: 2813 bytes --]

On Fri, Apr 05, 2024 at 05:14:34AM -0400, Eric Sunshine wrote:
> On Fri, Apr 5, 2024 at 3:51 AM Patrick Steinhardt <ps@pks.im> wrote:
> > In `test_expect_perms()` we assign the output of a command to a variable
> > declared via `local`. To assert that the command is actually successful
> > we also chain it with `&&`. This construct is seemingly not portable and
> > may fail with "local: 1: bad variable name".
> >
> > Split up the variable declaration and assignment to fix this.
> 
> Under what configuration, circumstances, conditions did you encounter
> this problem? I ask because this isn't the only such case in the test
> suite, as shown by:
> 
>     git grep -nP 'local ..*=\$\(..*&&' -- t
> 
> What makes this case distinct from the others? Including such
> information would improve the commit message and help future readers.

I only ever saw this in some subset of GitHub CI jobs. See e.g. [1],
where the logs [2] show the following error:

```
ok 6 - init: reinitializing reftable with files backend fails
+ test_when_finished rm -rf repo
+ test 0 = 0
+ test_cleanup={ rm -rf repo
                } && (exit "$eval_ret"); eval_ret=$?; :
+ umask 002
+ git init --shared=true repo
Initialized empty shared Git repository in /home/runner/work/git/git/t/trash directory.t0610-reftable-basics/repo/.git/
+ git -C repo config core.sharedrepository
+ test 1 = 1
+ test_expect_perms -rw-rw-r-- repo/.git/reftable/tables.list
+ local perms=-rw-rw-r--
+ local file=repo/.git/reftable/tables.list
+ ls -l repo/.git/reftable/tables.list
+ local actual=-rw-rw-r-- 1 runner docker 43 Apr 4 11:55 repo/.git/reftable/tables.list
t0610-reftable-basics.sh: 83: local: 1: bad variable name
+ die
+ code=2
+ test_atexit_handler
+ test : != :
+ return 0
+ test -n
+ echo FATAL: Unexpected exit with code 2
FATAL: Unexpected exit with code 2
```

I was a bit surprised by this, too, and cannot reproduce it with any of
my systems. But I would bet this is dependent on the actual version of
your shell  because it only happened with the Ubuntu 20.04 and 16.04
jobs.

I didn't dig much deeper though as the fix is easy enough, even though
it is surprising.

[1]: https://github.com/git/git/actions/runs/8554157462/job/23438877643
[2]: https://github.com/git/git/actions/runs/8554157462/artifacts/1384620962

Patrick

> > Signed-off-by: Patrick Steinhardt <ps@pks.im>
> > ---
> > diff --git a/t/t0610-reftable-basics.sh b/t/t0610-reftable-basics.sh
> > @@ -80,8 +80,9 @@ test_expect_success 'init: reinitializing reftable with files backend fails' '
> >  test_expect_perms () {
> >         local perms="$1"
> >         local file="$2"
> > -       local actual=$(ls -l "$file") &&
> > +       local actual
> >
> > +       actual=$(ls -l "$file") &&

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]