| Message ID | 20240118215407.8609-2-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: > Refactor t0024 to avoid having multiple chaining commands on a single > line, according to current styling norms. > > e.g turn > ( mkdir testdir && cd testdir && echo "in testdir" ) > into: > mkdir testdir && > ( > cd testdir && > echo "in testdir" > ) > > This is also described in the Documentation/CodingGuidelines file. Sure. Subject: t0024: style fix t0024 has multiple command invocations on a single line, which goes against the style given by CodingGuidelines. would be sufficient. > - ( mkdir untarred && cd untarred && "$TAR" -xf ../test.tar ) && > + mkdir untarred && > + ( > + cd untarred && > + "$TAR" -xf ../test.tar > + ) && I think we assume "$TAR" is modern enough to know about the "C" option (see t/t5004-archive-corner-cases.sh), so mkdir untarred && "$TAR" Cxf untarred test.tar without even a subshell may be sufficient. > @@ -30,7 +34,11 @@ test_expect_success UNZIP 'zip archive' ' > > git archive --format=zip HEAD >test.zip && > > - ( mkdir unzipped && cd unzipped && "$GIT_UNZIP" ../test.zip ) && > + mkdir unzipped && > + ( > + cd unzipped && > + "$GIT_UNZIP" ../test.zip > + ) && I do not think we assume "$GIT_UNZIP" to always know about the equivalent of "C" (is that "-d exdir"?), so what you wrote is the best we can do. Thanks.
On Fri Jan 19, 2024 at 4:48 AM IST, Junio C Hamano wrote: > > - ( mkdir untarred && cd untarred && "$TAR" -xf ../test.tar ) && > > + mkdir untarred && > > + ( > > + cd untarred && > > + "$TAR" -xf ../test.tar > > + ) && > > I think we assume "$TAR" is modern enough to know about the "C" > option (see t/t5004-archive-corner-cases.sh), so > > mkdir untarred && > "$TAR" Cxf untarred test.tar > > without even a subshell may be sufficient. Will update it in v2. > > > @@ -30,7 +34,11 @@ test_expect_success UNZIP 'zip archive' ' > > > > git archive --format=zip HEAD >test.zip && > > > > - ( mkdir unzipped && cd unzipped && "$GIT_UNZIP" ../test.zip ) && > > + mkdir unzipped && > > + ( > > + cd unzipped && > > + "$GIT_UNZIP" ../test.zip > > + ) && > > I do not think we assume "$GIT_UNZIP" to always know about the > equivalent of "C" (is that "-d exdir"?), so what you wrote is the > best we can do. Yeah, "-d exdir" would be the equivalent, but there's no mention of it in the testcases in t5003 or t5004. So, keeping it as is in v2. Thanks.
On Fri Jan 19, 2024 at 4:48 AM IST, Junio C Hamano wrote: > Ghanshyam Thakkar <shyamthakkar001@gmail.com> writes: > > - ( mkdir untarred && cd untarred && "$TAR" -xf ../test.tar ) && > > + mkdir untarred && > > + ( > > + cd untarred && > > + "$TAR" -xf ../test.tar > > + ) && > > I think we assume "$TAR" is modern enough to know about the "C" > option (see t/t5004-archive-corner-cases.sh), so > > mkdir untarred && > "$TAR" Cxf untarred test.tar > > without even a subshell may be sufficient. I suppose '"$TAR" Cxf untarred test.tar' is not a valid syntax on alpine, since it was breaking CI. Instead changed it to '"$TAR" xf test.tar -C untarred' in v3, which is how it's written in t/t5004-archive-corner-cases.sh, which passes CI.
diff --git a/t/t0024-crlf-archive.sh b/t/t0024-crlf-archive.sh index fa4da7c2b3..ff4efeaaee 100755 --- a/t/t0024-crlf-archive.sh +++ b/t/t0024-crlf-archive.sh @@ -20,7 +20,11 @@ test_expect_success setup ' test_expect_success 'tar archive' ' git archive --format=tar HEAD >test.tar && - ( mkdir untarred && cd untarred && "$TAR" -xf ../test.tar ) && + mkdir untarred && + ( + cd untarred && + "$TAR" -xf ../test.tar + ) && test_cmp sample untarred/sample @@ -30,7 +34,11 @@ test_expect_success UNZIP 'zip archive' ' git archive --format=zip HEAD >test.zip && - ( mkdir unzipped && cd unzipped && "$GIT_UNZIP" ../test.zip ) && + mkdir unzipped && + ( + cd unzipped && + "$GIT_UNZIP" ../test.zip + ) && test_cmp sample unzipped/sample
Refactor t0024 to avoid having multiple chaining commands on a single line, according to current styling norms. e.g turn ( mkdir testdir && cd testdir && echo "in testdir" ) into: mkdir testdir && ( cd testdir && echo "in testdir" ) This is also described in the Documentation/CodingGuidelines file. Signed-off-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com> --- t/t0024-crlf-archive.sh | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)