Re: [PATCH 11/19] tests: fix broken &&-chains in `$(...)` command substitutions

[Eric Sunshine <sunshine@sunshineco.com>]

On Thu, Dec 9, 2021 at 11:44 AM Elijah Newren <newren@gmail.com> wrote:
> On Wed, Dec 8, 2021 at 11:39 PM Eric Sunshine <sunshine@sunshineco.com> wrote:
> >  test_expect_success !MINGW 'a constipated git dies with SIGPIPE even if parent ignores it' '
> > -       OUT=$( ((trap "" PIPE; large_git; echo $? 1>&3) | :) 3>&1 ) &&
> > +       OUT=$( ((trap "" PIPE && large_git; echo $? 1>&3) | :) 3>&1 ) &&
>
> Shouldn't the second ';' be replaced with '&&' as well?

Thanks for reading so carefully. In this case, the answer is "no", the
semicolon is correct. This code legitimately wants to capture in the
OUT variable the numeric exit status of the command preceding `echo
$?`. If the semicolon is replaced with `&&`, then the echo won't be
executed if the exit status is non-zero, but we want `echo` to be
executed regardless of the exit status. So, the code is correct with
the semicolon, and would be incorrect with `&&`. (I hope I'm
explaining this well enough to make sense.)

It's this sort of special case which accounts for why the new linter
(as mentioned in the cover letter) has special understanding that a
broken &&-chain can be legitimate in certain circumstances, such as
explicit handling of `$?`.

> > -       OUT=$( ((trap "" PIPE; git rm -n "some-file-*"; echo $? 1>&3) | :) 3>&1 ) &&
> > +       OUT=$( ((trap "" PIPE && git rm -n "some-file-*"; echo $? 1>&3) | :) 3>&1 ) &&
>
> Same here; shouldn't the second ';' be replaced with '&&' as well?

Same answer as above.