| Message ID | 20240118215407.8609-1-shyamthakkar001@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/2] t0024: avoid losing exit status to pipes | expand |
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. > 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
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
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. Also refactor an existing use of '>' to avoid having a space after '>', according to Documentation/CodingGuidelines. Signed-off-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com> --- t/t0024-crlf-archive.sh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)