On Thu, Dec 09, 2021 at 11:53:47AM -0500, Eric Sunshine wrote: > On Thu, Dec 9, 2021 at 11:44 AM Elijah Newren <newren@xxxxxxxxx> wrote: > > On Wed, Dec 8, 2021 at 11:39 PM Eric Sunshine <sunshine@xxxxxxxxxxxxxx> 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.) That makes sense to me. I wondered why it was even worth changing the earlier semi-colon in that case, then, but... > 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 `$?`. ...your unseen magic script explains it. :) All of the changes here look reasonable. We'd either want to know about failure (e.g., "cd") or don't expect it to fail (e.g., "echo"). These "trap" calls are probably fine. I can't imagine why they'd fail, but being a weird shell builtin I wonder if it's possible for them to fail in odd circumstances. I'm happy to leave that as a hypothetical until we see it in practice. -Peff
