Re: [GSoC][PATCH v2 3/5] t0003-attributes: avoid using pipes

[jonathan chang <ttjtftx@gmail.com>]

On Sun, Mar 10, 2019 at 6:13 PM Eric Sunshine <sunshine@sunshineco.com> wrote:
>
> On Sun, Mar 10, 2019 at 4:09 AM Jonathan Chang <ttjtftx@gmail.com> 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@gmail.com>
> >
> > 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.