On Sun, Mar 10, 2019 at 6:13 PM Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote: > > On Sun, Mar 10, 2019 at 4:09 AM Jonathan Chang <ttjtftx@xxxxxxxxx> wrote: > > The exit code of the upstream in a pipe is ignored thus we should avoid > > using it. By writing out the output of the git command to a file, we can > > test the exit codes of both the commands. > > > > Signed-off-by: Jonathan Chang <ttjtftx@xxxxxxxxx> > > > > diff --git a/t/t0003-attributes.sh b/t/t0003-attributes.sh > > @@ -203,15 +203,18 @@ test_expect_success 'attribute test: read paths from stdin' ' > > test_expect_success 'attribute test: --all option' ' > > grep -v unspecified <expect-all | sort >specified-all && > > sed -e "s/:.*//" <expect-all | uniq >stdin-all && > > - git check-attr --stdin --all <stdin-all | sort >actual && > > + git check-attr --stdin --all <stdin-all >actual && > > + sort -o actual actual && > > test_cmp specified-all actual > > ' > > There is no existing use of "sort -o" anywhere in the Git test suite > (or, for that matter, anywhere else in Git), which should give one > pause before using it here. Although -o is allowed by POSIX, and POSIX > even says it's safe for the output file to have the same name as one > of the input files, there is no guarantee that "sort -o" will be > supported on all platforms, or that all platforms promise that the > output filename can match an input filename (in fact, neither the > MacOS nor FreeBSD man pages for 'sort' make this promise). > Consequently, it would be better to err on the side of safety and > avoid "sort -o", which is easily enough done by using another > temporary file: > > git check-attr --stdin --all <stdin-all >output && > sort output >actual && > > The same comment applies to the remaining changes. Noted. I did check POSIX, I should have also checked the use in Git. Thanks for the review.
