Re: [GSoC][PATCH v2 3/5] t0003-attributes: avoid using pipes
- Date: Fri, 15 Mar 2019 09:56:33 +0800
- From: jonathan chang <ttjtftx@xxxxxxxxx>
- Subject: Re: [GSoC][PATCH v2 3/5] t0003-attributes: avoid using pipes
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.