From: Eric Sunshine <sunshine@sunshineco.com>
Date: Sun, 10 Mar 2019 10:13:19 GMT
Subject: Re: [GSoC][PATCH v2 3/5] t0003-attributes: avoid using pipes
Message-ID: <CAPig+cQP74MC6fJC4fNCz9Rz3tDux1K4TGB7N+cOC8zScxOoAA@mail.gmail.com>
In-Reply-To: <20190310080931.64134-1-ttjtftx@gmail.com>

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.