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

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Mar 10, 2019, 10:13 UTC
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:

Show 16 quoted lines

> 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.