Re: [PATCH 1/2] t0024: avoid losing exit status to pipes
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jan 18, 2024, 23:04 UTC
- Message-ID
- <xmqqmst2nszq.fsf@gitster.g>
- In-Reply-To
- <20240118215407.8609-1-shyamthakkar001@gmail.com>
Ghanshyam Thakkar <shyamthakkar001@gmail.com> writes:
> Replace pipe with redirection operator '>' to store the output > to a temporary file after 'git archive' command since the pipe > will swallow the command's exit code and a crash won't > necessarily be noticed.
OK. I think this case what the patch does is the right thing. If we were creating a huge tar archive and have the downstream only consuming for a small disk footprint (e.g., "git archive | tar tf -"), use of pipe (and loss of exit status) may be justified, but I do not think that is the case here.
> Also refactor an existing use of '>' to avoid having a space after > '>', according to Documentation/CodingGuidelines.
It may be just me, but I wouldn't call that "refactor", which we often use to refer to changing the way an existing functional unit works internally, by splitting the innard of a function into separate helper functions, by replacing the open-coded duplicate with calls to such helper functions, etc. Correcting the coding style violation is merely a "fix".
Also fix an unwanted space after '>' redirection to match the style in CodingGuidelines.
In any case, the patch text looks good.
Thanks.
Show 28 quoted lines
> Signed-off-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com> > --- > t/t0024-crlf-archive.sh | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/t/t0024-crlf-archive.sh b/t/t0024-crlf-archive.sh > index a34de56420..fa4da7c2b3 100755 > --- a/t/t0024-crlf-archive.sh > +++ b/t/t0024-crlf-archive.sh > @@ -9,7 +9,7 @@ test_expect_success setup ' > > git config core.autocrlf true && > > - printf "CRLF line ending\r\nAnd another\r\n" > sample && > + printf "CRLF line ending\r\nAnd another\r\n" >sample && > git add sample && > > test_tick && > @@ -19,8 +19,8 @@ test_expect_success setup ' > > test_expect_success 'tar archive' ' > > - git archive --format=tar HEAD | > - ( mkdir untarred && cd untarred && "$TAR" -xf - ) && > + git archive --format=tar HEAD >test.tar && > + ( mkdir untarred && cd untarred && "$TAR" -xf ../test.tar ) && > > test_cmp sample untarred/sample