threads / patch / 57467

patch, 2 partsSubject: [PATCH 0/2] [GSoC][PATCH 0/2] t0000-t0050: avoid pipes with Git on LHS

Subject: [PATCH 0/2] Subject: [PATCH 0/2] [GSoC][PATCH 0/2] t0000-t0050: avoid pipes with Git on LHS

## tl;dr

19 messages between Feb 24, 2022 and Mar 12, 2022. Diffs are folded; open one to read it.

replies: 18people: 4as markdown or json

Shubham Mishra· Feb 24, 2022, 05:47 UTC · lore

As a microproject, I remove all pipes with GIT on LHS from files t0000 to t0050.

Shubham Mishra (2):
  t0001-t0028: avoid pipes with Git on LHS
  t0030-t0050: avoid pipes with Git on LHS
 t/t0000-basic.sh            | 10 +++--
 t/t0022-crlf-rename.sh      |  4 +-
 t/t0025-crlf-renormalize.sh |  4 +-
 t/t0027-auto-crlf.sh        | 12 +++---
 t/t0030-stripspace.sh       | 81 ++++++++++++++++++++++++-------------
 t/t0050-filesystem.sh       |  3 +-
 6 files changed, 72 insertions(+), 42 deletions(-)
base-commit: e6ebfd0e8cbbd10878070c8a356b5ad1b3ca464e
-- 
2.25.1
Shubham Mishra· Feb 24, 2022, 05:47 UTC · re: Shubham Mishra · lore

[PATCH 1/2] t0001-t0028: avoid pipes with Git on LHS

Pipes ignore error codes of LHS command and thus we should not use them with Git in tests. As an alternative, use a 'tmp' file to write the Git output so we can test the exit code.

Signed-off-by: Shubham Mishra <shivam828787@gmail.com>
---
 t/t0000-basic.sh            | 10 ++++++----
 t/t0022-crlf-rename.sh      |  4 ++--
 t/t0025-crlf-renormalize.sh |  4 ++--
 t/t0027-auto-crlf.sh        | 12 ++++++------
 4 files changed, 16 insertions(+), 14 deletions(-)
Show changes to 4 files +16 −14

t/t0000-basic.sh, t/t0022-crlf-rename.sh, t/t0025-crlf-renormalize.sh, t/t0027-auto-crlf.sh

diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
index b007f0efef..b5fa76059b 100755
--- a/t/t0000-basic.sh
+++ b/t/t0000-basic.sh
@@ -1089,7 +1089,8 @@ test_expect_success 'update-index D/F conflict' '
 	mv path2 path0 &&
 	mv tmp path2 &&
 	git update-index --add --replace path2 path0/file2 &&
-	numpath0=$(git ls-files path0 | wc -l) &&
+	git ls-files path0 >tmp &&
+	numpath0=$(wc -l <tmp) &&
 	test $numpath0 = 1
 '
 
@@ -1104,12 +1105,13 @@ test_expect_success 'very long name in the index handled sanely' '
 	>path4 &&
 	git update-index --add path4 &&
 	(
-		git ls-files -s path4 |
-		sed -e "s/	.*/	/" |
+		git ls-files -s path4 >tmp &&
+		sed -e "s/	.*/	/" tmp |
 		tr -d "\012" &&
 		echo "$a"
 	) | git update-index --index-info &&
-	len=$(git ls-files "a*" | wc -c) &&
+	git ls-files "a*" >tmp &&
+	len=$(wc -c <tmp) &&
 	test $len = 4098
 '
 
diff --git a/t/t0022-crlf-rename.sh b/t/t0022-crlf-rename.sh
index c1a331e9e9..9fe9891251 100755
--- a/t/t0022-crlf-rename.sh
+++ b/t/t0022-crlf-rename.sh
@@ -24,8 +24,8 @@ test_expect_success setup '
 
 test_expect_success 'diff -M' '
 
-	git diff-tree -M -r --name-status HEAD^ HEAD |
-	sed -e "s/R[0-9]*/RNUM/" >actual &&
+	git diff-tree -M -r --name-status HEAD^ HEAD >tmp &&
+	sed -e "s/R[0-9]*/RNUM/" tmp >actual &&
 	echo "RNUM	sample	elpmas" >expect &&
 	test_cmp expect actual
 
diff --git a/t/t0025-crlf-renormalize.sh b/t/t0025-crlf-renormalize.sh
index 81447978b7..f7202c192e 100755
--- a/t/t0025-crlf-renormalize.sh
+++ b/t/t0025-crlf-renormalize.sh
@@ -22,8 +22,8 @@ test_expect_success 'renormalize CRLF in repo' '
 	i/lf w/lf attr/text=auto LF.txt
 	i/lf w/mixed attr/text=auto CRLF_mix_LF.txt
 	EOF
-	git ls-files --eol |
-	sed -e "s/	/ /g" -e "s/  */ /g" |
+	git ls-files --eol >tmp &&
+	sed -e "s/	/ /g" -e "s/  */ /g" tmp |
 	sort >actual &&
 	test_cmp expect actual
 '
diff --git a/t/t0027-auto-crlf.sh b/t/t0027-auto-crlf.sh
index c5f7ac63b0..0feb41a23f 100755
--- a/t/t0027-auto-crlf.sh
+++ b/t/t0027-auto-crlf.sh
@@ -311,8 +311,8 @@ checkout_files () {
 		i/-text w/$(stats_ascii $crlfnul) attr/$(attr_ascii $attr $aeol) crlf_false_attr__CRLF_nul.txt
 		i/-text w/$(stats_ascii $crlfnul) attr/$(attr_ascii $attr $aeol) crlf_false_attr__LF_nul.txt
 		EOF
-		git ls-files --eol crlf_false_attr__* |
-		sed -e "s/	/ /g" -e "s/  */ /g" |
+		git ls-files --eol crlf_false_attr__* >tmp &&
+		sed -e "s/	/ /g" -e "s/  */ /g" tmp |
 		sort >actual &&
 		test_cmp expect actual
 	'
@@ -359,12 +359,12 @@ test_expect_success 'ls-files --eol -o Text/Binary' '
 	i/ w/crlf TeBi_126_CL
 	i/ w/-text TeBi_126_CLC
 	EOF
-	git ls-files --eol -o |
+	git ls-files --eol -o >tmp &&
 	sed -n -e "/TeBi_/{s!attr/[	]*!!g
 	s!	! !g
 	s!  *! !g
 	p
-	}" | sort >actual &&
+	}" tmp | sort >actual &&
 	test_cmp expect actual
 '
 
@@ -617,8 +617,8 @@ test_expect_success 'ls-files --eol -d -z' '
 	i/lf w/ crlf_false_attr__LF.txt
 	i/mixed w/ crlf_false_attr__CRLF_mix_LF.txt
 	EOF
-	git ls-files --eol -d |
-	sed -e "s!attr/[^	]*!!g" -e "s/	/ /g" -e "s/  */ /g" |
+	git ls-files --eol -d >tmp &&
+	sed -e "s!attr/[^	]*!!g" -e "s/	/ /g" -e "s/  */ /g" tmp |
 	sort >actual &&
 	test_cmp expect actual
 '

-- 
2.25.1
Shubham Mishra· Feb 24, 2022, 05:47 UTC · re: Shubham Mishra · lore

[PATCH 2/2] t0030-t0050: avoid pipes with Git on LHS

Pipes ignore error codes of LHS command and thus we should not use them with Git in tests. As an alternative, use a 'tmp' file to write the Git output so we can test the exit code.

Signed-off-by: Shubham Mishra <shivam828787@gmail.com>
---
 t/t0030-stripspace.sh | 81 ++++++++++++++++++++++++++++---------------
 t/t0050-filesystem.sh |  3 +-
 2 files changed, 56 insertions(+), 28 deletions(-)
Show changes to 2 files +56 −28

t/t0030-stripspace.sh, t/t0050-filesystem.sh

diff --git a/t/t0030-stripspace.sh b/t/t0030-stripspace.sh
index ae1ca380c1..4d0af9bf98 100755
--- a/t/t0030-stripspace.sh
+++ b/t/t0030-stripspace.sh
@@ -225,32 +225,48 @@ test_expect_success \
 
 test_expect_success \
     'text without newline at end should end with newline' '
-    test $(printf "$ttt" | git stripspace | wc -l) -gt 0 &&
-    test $(printf "$ttt$ttt" | git stripspace | wc -l) -gt 0 &&
-    test $(printf "$ttt$ttt$ttt" | git stripspace | wc -l) -gt 0 &&
-    test $(printf "$ttt$ttt$ttt$ttt" | git stripspace | wc -l) -gt 0
+    printf "$ttt" | git stripspace >tmp &&
+    test $(wc -l <tmp) -gt 0 &&
+    printf "$ttt$ttt" | git stripspace >tmp &&
+    test $(wc -l <tmp) -gt 0 &&
+    printf "$ttt$ttt$ttt" | git stripspace >tmp &&
+    test $(wc -l <tmp) -gt 0 &&
+    printf "$ttt$ttt$ttt$ttt" | git stripspace >tmp &&
+    test $(wc -l <tmp) -gt 0
 '
 
 # text plus spaces at the end:
 
 test_expect_success \
     'text plus spaces without newline at end should end with newline' '
-    test $(printf "$ttt$sss" | git stripspace | wc -l) -gt 0 &&
-    test $(printf "$ttt$ttt$sss" | git stripspace | wc -l) -gt 0 &&
-    test $(printf "$ttt$ttt$ttt$sss" | git stripspace | wc -l) -gt 0 &&
-    test $(printf "$ttt$sss$sss" | git stripspace | wc -l) -gt 0 &&
-    test $(printf "$ttt$ttt$sss$sss" | git stripspace | wc -l) -gt 0 &&
-    test $(printf "$ttt$sss$sss$sss" | git stripspace | wc -l) -gt 0
+    printf "$ttt$sss" | git stripspace >tmp &&
+    test $(wc -l <tmp) -gt 0 &&
+    printf "$ttt$ttt$sss" | git stripspace >tmp &&
+    test $(wc -l <tmp) -gt 0 &&
+    printf "$ttt$ttt$ttt$sss" | git stripspace >tmp &&
+    test $(wc -l <tmp) -gt 0 &&
+    printf "$ttt$sss$sss" | git stripspace >tmp &&
+    test $(wc -l <tmp) -gt 0 &&
+    printf "$ttt$ttt$sss$sss" | git stripspace >tmp &&
+    test $(wc -l <tmp) -gt 0 &&
+    printf "$ttt$sss$sss$sss" | git stripspace >tmp &&
+    test $(wc -l <tmp) -gt 0
 '
 
 test_expect_success \
     'text plus spaces without newline at end should not show spaces' '
-    ! (printf "$ttt$sss" | git stripspace | grep "  " >/dev/null) &&
-    ! (printf "$ttt$ttt$sss" | git stripspace | grep "  " >/dev/null) &&
-    ! (printf "$ttt$ttt$ttt$sss" | git stripspace | grep "  " >/dev/null) &&
-    ! (printf "$ttt$sss$sss" | git stripspace | grep "  " >/dev/null) &&
-    ! (printf "$ttt$ttt$sss$sss" | git stripspace | grep "  " >/dev/null) &&
-    ! (printf "$ttt$sss$sss$sss" | git stripspace | grep "  " >/dev/null)
+    printf "$ttt$sss" | git stripspace >tmp &&
+    ! (grep "  " tmp >/dev/null) &&
+    printf "$ttt$ttt$sss" | git stripspace &&
+    ! (grep "  " tmp >/dev/null) &&
+    printf "$ttt$ttt$ttt$sss" | git stripspace &&
+    ! (grep "  " tmp >/dev/nul) &&
+    printf "$ttt$sss$sss" | git stripspace &&
+    ! (grep "  " tmp >/dev/null) &&
+    printf "$ttt$ttt$sss$sss" | git stripspace &&
+    ! (grep "  " tmp >/dev/null) &&
+    printf "$ttt$sss$sss$sss" | git stripspace &&
+    ! (grep "  " tmp >/dev/null)
 '
 
 test_expect_success \
@@ -282,12 +298,18 @@ test_expect_success \
 
 test_expect_success \
     'text plus spaces at end should not show spaces' '
-    ! (echo "$ttt$sss" | git stripspace | grep "  " >/dev/null) &&
-    ! (echo "$ttt$ttt$sss" | git stripspace | grep "  " >/dev/null) &&
-    ! (echo "$ttt$ttt$ttt$sss" | git stripspace | grep "  " >/dev/null) &&
-    ! (echo "$ttt$sss$sss" | git stripspace | grep "  " >/dev/null) &&
-    ! (echo "$ttt$ttt$sss$sss" | git stripspace | grep "  " >/dev/null) &&
-    ! (echo "$ttt$sss$sss$sss" | git stripspace | grep "  " >/dev/null)
+    echo "$ttt$sss" | git stripspace >tmp &&
+    ! (grep "  " tmp >/dev/null) &&
+    echo "$ttt$ttt$sss" | git stripspace >tmp &&
+    ! (grep "  " tmp>/dev/null) &&
+    echo "$ttt$ttt$ttt$sss" &&
+    ! (grep "  " tmp >/dev/null) &&
+    echo "$ttt$sss$sss" | git stripspace >tmp &&
+    ! (grep "  " tmp >/dev/null) &&
+    echo "$ttt$ttt$sss$sss" | git stripspace >tmp &&
+    ! (grep "  " tmp >/dev/null) &&
+    echo "$ttt$sss$sss$sss" | git stripspace >tmp &&
+    ! (grep "  " tmp >/dev/null)
 '
 
 test_expect_success \
@@ -339,11 +361,16 @@ test_expect_success \
 
 test_expect_success \
     'spaces without newline at end should not show spaces' '
-    ! (printf "" | git stripspace | grep " " >/dev/null) &&
-    ! (printf "$sss" | git stripspace | grep " " >/dev/null) &&
-    ! (printf "$sss$sss" | git stripspace | grep " " >/dev/null) &&
-    ! (printf "$sss$sss$sss" | git stripspace | grep " " >/dev/null) &&
-    ! (printf "$sss$sss$sss$sss" | git stripspace | grep " " >/dev/null)
+    printf "" | git stripspace >tmp &&
+    ! ( grep " " tmp >/dev/null) &&
+    printf "$sss" | git stripspace >tmp &&
+    ! ( grep " " tmp >/dev/null) &&
+    printf "$sss$sss" | git stripspace >tmp &&
+    ! (grep " " tmp >/dev/null) &&
+    printf "$sss$sss$sss" | git stripspace >tmp &&
+    ! (grep " " tmp >/dev/null) &&
+    printf "$sss$sss$sss$sss" | git stripspace >tmp &&
+    ! (grep " " tmp >/dev/null)
 '
 
 test_expect_success \
diff --git a/t/t0050-filesystem.sh b/t/t0050-filesystem.sh
index afc343cf9b..5c9dc90d0b 100755
--- a/t/t0050-filesystem.sh
+++ b/t/t0050-filesystem.sh
@@ -104,7 +104,8 @@ test_expect_failure CASE_INSENSITIVE_FS 'add (with different case)' '
 	rm camelcase &&
 	echo 1 >CamelCase &&
 	git add CamelCase &&
-	camel=$(git ls-files | grep -i camelcase) &&
+	git ls-files >tmp &&
+	camel=$(grep -i camelcase tmp) &&
 	test $(echo "$camel" | wc -l) = 1 &&
 	test "z$(git cat-file blob :$camel)" = z1
 '

-- 
2.25.1
Ævar Arnfjörð Bjarmason· Feb 24, 2022, 08:59 UTC · re: Shubham Mishra · lore

Re: [PATCH 2/2] t0030-t0050: avoid pipes with Git on LHS

On Thu, Feb 24 2022, Shubham Mishra wrote:
Show 31 quoted lines
> Pipes ignore error codes of LHS command and thus we should not use
> them with Git in tests. As an alternative, use a 'tmp' file to write
> the Git output so we can test the exit code.
>
> Signed-off-by: Shubham Mishra <shivam828787@gmail.com>
> ---
>  t/t0030-stripspace.sh | 81 ++++++++++++++++++++++++++++---------------
>  t/t0050-filesystem.sh |  3 +-
>  2 files changed, 56 insertions(+), 28 deletions(-)
>
> diff --git a/t/t0030-stripspace.sh b/t/t0030-stripspace.sh
> index ae1ca380c1..4d0af9bf98 100755
> --- a/t/t0030-stripspace.sh
> +++ b/t/t0030-stripspace.sh
> @@ -225,32 +225,48 @@ test_expect_success \
>  
>  test_expect_success \
>      'text without newline at end should end with newline' '
> -    test $(printf "$ttt" | git stripspace | wc -l) -gt 0 &&
> -    test $(printf "$ttt$ttt" | git stripspace | wc -l) -gt 0 &&
> -    test $(printf "$ttt$ttt$ttt" | git stripspace | wc -l) -gt 0 &&
> -    test $(printf "$ttt$ttt$ttt$ttt" | git stripspace | wc -l) -gt 0
> +    printf "$ttt" | git stripspace >tmp &&
> +    test $(wc -l <tmp) -gt 0 &&
> +    printf "$ttt$ttt" | git stripspace >tmp &&
> +    test $(wc -l <tmp) -gt 0 &&
> +    printf "$ttt$ttt$ttt" | git stripspace >tmp &&
> +    test $(wc -l <tmp) -gt 0 &&
> +    printf "$ttt$ttt$ttt$ttt" | git stripspace >tmp &&
> +    test $(wc -l <tmp) -gt 0
>  '

You're modifying some tests here that are using some old coding style, so maybe it's better to adjust it while we're at it?

Also I think this would be a lot nicer with test_stdout_line_count and a
helper to deal with that pritnf, e.g.:
	
	diff --git a/t/t0030-stripspace.sh b/t/t0030-stripspace.sh
	index ae1ca380c1a..d48a3579511 100755
	--- a/t/t0030-stripspace.sh
	+++ b/t/t0030-stripspace.sh
	@@ -223,12 +223,15 @@ test_expect_success \
	     test_cmp expect actual
	 '
	 
	-test_expect_success \
	-    'text without newline at end should end with newline' '
	-    test $(printf "$ttt" | git stripspace | wc -l) -gt 0 &&
	-    test $(printf "$ttt$ttt" | git stripspace | wc -l) -gt 0 &&
	-    test $(printf "$ttt$ttt$ttt" | git stripspace | wc -l) -gt 0 &&
	-    test $(printf "$ttt$ttt$ttt$ttt" | git stripspace | wc -l) -gt 0
	+printf_git_stripspace () {
	+	printf "$1" | git stripspace
	+}
	+
	+test_expect_success 'text without newline at end should end with newline' '
	+	test_stdout_line_count -gt 0 printf_git_stripspace "$ttt" &&
	+	test_stdout_line_count -gt 0 printf_git_stripspace "$ttt$ttt" &&
	+	test_stdout_line_count -gt 0 printf_git_stripspace "$ttt$ttt$ttt" &&
	+	test_stdout_line_count -gt 0 printf_git_stripspace "$ttt$ttt$ttt$ttt"
	 '
	 
	 # text plus spaces at the end:
Show 23 quoted lines
>  # text plus spaces at the end:
>  
>  test_expect_success \
>      'text plus spaces without newline at end should end with newline' '
> -    test $(printf "$ttt$sss" | git stripspace | wc -l) -gt 0 &&
> -    test $(printf "$ttt$ttt$sss" | git stripspace | wc -l) -gt 0 &&
> -    test $(printf "$ttt$ttt$ttt$sss" | git stripspace | wc -l) -gt 0 &&
> -    test $(printf "$ttt$sss$sss" | git stripspace | wc -l) -gt 0 &&
> -    test $(printf "$ttt$ttt$sss$sss" | git stripspace | wc -l) -gt 0 &&
> -    test $(printf "$ttt$sss$sss$sss" | git stripspace | wc -l) -gt 0
> +    printf "$ttt$sss" | git stripspace >tmp &&
> +    test $(wc -l <tmp) -gt 0 &&
> +    printf "$ttt$ttt$sss" | git stripspace >tmp &&
> +    test $(wc -l <tmp) -gt 0 &&
> +    printf "$ttt$ttt$ttt$sss" | git stripspace >tmp &&
> +    test $(wc -l <tmp) -gt 0 &&
> +    printf "$ttt$sss$sss" | git stripspace >tmp &&
> +    test $(wc -l <tmp) -gt 0 &&
> +    printf "$ttt$ttt$sss$sss" | git stripspace >tmp &&
> +    test $(wc -l <tmp) -gt 0 &&
> +    printf "$ttt$sss$sss$sss" | git stripspace >tmp &&
> +    test $(wc -l <tmp) -gt 0
>  '
ditto.
Show 21 quoted lines
>  test_expect_success \
>      'text plus spaces without newline at end should not show spaces' '
> -    ! (printf "$ttt$sss" | git stripspace | grep "  " >/dev/null) &&
> -    ! (printf "$ttt$ttt$sss" | git stripspace | grep "  " >/dev/null) &&
> -    ! (printf "$ttt$ttt$ttt$sss" | git stripspace | grep "  " >/dev/null) &&
> -    ! (printf "$ttt$sss$sss" | git stripspace | grep "  " >/dev/null) &&
> -    ! (printf "$ttt$ttt$sss$sss" | git stripspace | grep "  " >/dev/null) &&
> -    ! (printf "$ttt$sss$sss$sss" | git stripspace | grep "  " >/dev/null)
> +    printf "$ttt$sss" | git stripspace >tmp &&
> +    ! (grep "  " tmp >/dev/null) &&
> +    printf "$ttt$ttt$sss" | git stripspace &&
> +    ! (grep "  " tmp >/dev/null) &&
> +    printf "$ttt$ttt$ttt$sss" | git stripspace &&
> +    ! (grep "  " tmp >/dev/nul) &&
> +    printf "$ttt$sss$sss" | git stripspace &&
> +    ! (grep "  " tmp >/dev/null) &&
> +    printf "$ttt$ttt$sss$sss" | git stripspace &&
> +    ! (grep "  " tmp >/dev/null) &&
> +    printf "$ttt$sss$sss$sss" | git stripspace &&
> +    ! (grep "  " tmp >/dev/null)
>  '

This is not on you, but generally we don't pipe "grep" to >/dev/null, and just let the --verbose option do its work.

With/without that change you no longer need the () subshell here.
Show 46 quoted lines
>  test_expect_success \
> @@ -282,12 +298,18 @@ test_expect_success \
>  
>  test_expect_success \
>      'text plus spaces at end should not show spaces' '
> -    ! (echo "$ttt$sss" | git stripspace | grep "  " >/dev/null) &&
> -    ! (echo "$ttt$ttt$sss" | git stripspace | grep "  " >/dev/null) &&
> -    ! (echo "$ttt$ttt$ttt$sss" | git stripspace | grep "  " >/dev/null) &&
> -    ! (echo "$ttt$sss$sss" | git stripspace | grep "  " >/dev/null) &&
> -    ! (echo "$ttt$ttt$sss$sss" | git stripspace | grep "  " >/dev/null) &&
> -    ! (echo "$ttt$sss$sss$sss" | git stripspace | grep "  " >/dev/null)
> +    echo "$ttt$sss" | git stripspace >tmp &&
> +    ! (grep "  " tmp >/dev/null) &&
> +    echo "$ttt$ttt$sss" | git stripspace >tmp &&
> +    ! (grep "  " tmp>/dev/null) &&
> +    echo "$ttt$ttt$ttt$sss" &&
> +    ! (grep "  " tmp >/dev/null) &&
> +    echo "$ttt$sss$sss" | git stripspace >tmp &&
> +    ! (grep "  " tmp >/dev/null) &&
> +    echo "$ttt$ttt$sss$sss" | git stripspace >tmp &&
> +    ! (grep "  " tmp >/dev/null) &&
> +    echo "$ttt$sss$sss$sss" | git stripspace >tmp &&
> +    ! (grep "  " tmp >/dev/null)
>  '
>  
>  test_expect_success \
> @@ -339,11 +361,16 @@ test_expect_success \
>  
>  test_expect_success \
>      'spaces without newline at end should not show spaces' '
> -    ! (printf "" | git stripspace | grep " " >/dev/null) &&
> -    ! (printf "$sss" | git stripspace | grep " " >/dev/null) &&
> -    ! (printf "$sss$sss" | git stripspace | grep " " >/dev/null) &&
> -    ! (printf "$sss$sss$sss" | git stripspace | grep " " >/dev/null) &&
> -    ! (printf "$sss$sss$sss$sss" | git stripspace | grep " " >/dev/null)
> +    printf "" | git stripspace >tmp &&
> +    ! ( grep " " tmp >/dev/null) &&
> +    printf "$sss" | git stripspace >tmp &&
> +    ! ( grep " " tmp >/dev/null) &&
> +    printf "$sss$sss" | git stripspace >tmp &&
> +    ! (grep " " tmp >/dev/null) &&
> +    printf "$sss$sss$sss" | git stripspace >tmp &&
> +    ! (grep " " tmp >/dev/null) &&
> +    printf "$sss$sss$sss$sss" | git stripspace >tmp &&
> +    ! (grep " " tmp >/dev/null)
>  '
ditto for these.
Show 15 quoted lines
>  test_expect_success \
> diff --git a/t/t0050-filesystem.sh b/t/t0050-filesystem.sh
> index afc343cf9b..5c9dc90d0b 100755
> --- a/t/t0050-filesystem.sh
> +++ b/t/t0050-filesystem.sh
> @@ -104,7 +104,8 @@ test_expect_failure CASE_INSENSITIVE_FS 'add (with different case)' '
>  	rm camelcase &&
>  	echo 1 >CamelCase &&
>  	git add CamelCase &&
> -	camel=$(git ls-files | grep -i camelcase) &&
> +	git ls-files >tmp &&
> +	camel=$(grep -i camelcase tmp) &&
>  	test $(echo "$camel" | wc -l) = 1 &&
>  	test "z$(git cat-file blob :$camel)" = z1
>  '
Shubham Mishra· Feb 26, 2022, 12:09 UTC · re: Ævar Arnfjörð Bjarmason · lore

Re: [PATCH 2/2] t0030-t0050: avoid pipes with Git on LHS

Hi,
Thanks for Reviewing.
Show 33 quoted lines
> You're modifying some tests here that are using some old coding style,
> so maybe it's better to adjust it while we're at it?
>
> Also I think this would be a lot nicer with test_stdout_line_count and a
> helper to deal with that pritnf, e.g.:
>
>         diff --git a/t/t0030-stripspace.sh b/t/t0030-stripspace.sh
>         index ae1ca380c1a..d48a3579511 100755
>         --- a/t/t0030-stripspace.sh
>         +++ b/t/t0030-stripspace.sh
>         @@ -223,12 +223,15 @@ test_expect_success \
>              test_cmp expect actual
>          '
>
>         -test_expect_success \
>         -    'text without newline at end should end with newline' '
>         -    test $(printf "$ttt" | git stripspace | wc -l) -gt 0 &&
>         -    test $(printf "$ttt$ttt" | git stripspace | wc -l) -gt 0 &&
>         -    test $(printf "$ttt$ttt$ttt" | git stripspace | wc -l) -gt 0 &&
>         -    test $(printf "$ttt$ttt$ttt$ttt" | git stripspace | wc -l) -gt 0
>         +printf_git_stripspace () {
>         +       printf "$1" | git stripspace
>         +}
>         +
>         +test_expect_success 'text without newline at end should end with newline' '
>         +       test_stdout_line_count -gt 0 printf_git_stripspace "$ttt" &&
>         +       test_stdout_line_count -gt 0 printf_git_stripspace "$ttt$ttt" &&
>         +       test_stdout_line_count -gt 0 printf_git_stripspace "$ttt$ttt$ttt" &&
>         +       test_stdout_line_count -gt 0 printf_git_stripspace "$ttt$ttt$ttt$ttt"
>          '
>
>          # text plus spaces at the end:
>

it makes sense to improve this code as we are touching it, this way looks much better. I will implement it.

> This is not on you, but generally we don't pipe "grep" to >/dev/null,
> and just let the --verbose option do its work.

I don't think I understood this, I guess you are talking about the "-v" flag that stands for invert match? I didn't find "--verbose" with grep. Please correct me if I am wrong.

> With/without that change you no longer need the () subshell here.
sure.

Thanks, Shubham

Taylor Blau· Feb 26, 2022, 17:32 UTC · re: Ævar Arnfjörð Bjarmason · lore

Re: [PATCH 2/2] t0030-t0050: avoid pipes with Git on LHS

On Thu, Feb 24, 2022 at 09:59:45AM +0100, Ævar Arnfjörð Bjarmason wrote:
Show 5 quoted lines
> You're modifying some tests here that are using some old coding style,
> so maybe it's better to adjust it while we're at it?
>
> Also I think this would be a lot nicer with test_stdout_line_count and a
> helper to deal with that pritnf, e.g.:
Thanks, Ævar for the quality review. I have nothing to add here.

Thanks, Taylor

Shubham Mishra· Feb 27, 2022, 12:24 UTC · re: Ævar Arnfjörð Bjarmason · lore

[GSoC] [PATCH v2 0/2] avoid pipes with Git on LHS

changes since v1:
* replaced "wc -l" with test_stdout_line_count 
* removed unnecessary "()" subshells
Shubham Mishra (2):
  t0001-t0028: avoid pipes with Git on LHS
  t0030-t0050: avoid pipes with Git on LHS
 t/t0000-basic.sh            | 10 +++--
 t/t0022-crlf-rename.sh      |  4 +-
 t/t0025-crlf-renormalize.sh |  4 +-
 t/t0027-auto-crlf.sh        | 12 +++---
 t/t0030-stripspace.sh       | 75 ++++++++++++++++++++++++-------------
 t/t0050-filesystem.sh       |  3 +-
 6 files changed, 66 insertions(+), 42 deletions(-)
Range-diff against v1:
1:  2a219ace42 = 1:  2a219ace42 t0001-t0028: avoid pipes with Git on LHS
2:  d08c144476 ! 2:  c90fc271d9 t0030-t0050: avoid pipes with Git on LHS
    @@ Commit message
         Signed-off-by: Shubham Mishra <shivam828787@gmail.com>
     
      ## t/t0030-stripspace.sh ##
    +@@ t/t0030-stripspace.sh: s40='                                        '
    + sss="$s40$s40$s40$s40$s40$s40$s40$s40$s40$s40" # 400
    + ttt="$t40$t40$t40$t40$t40$t40$t40$t40$t40$t40" # 400
    + 
    ++printf_git_stripspace () {
    ++    printf "$1" | git stripspace
    ++}
    ++
    + test_expect_success \
    +     'long lines without spaces should be unchanged' '
    +     echo "$ttt" >expect &&
     @@ t/t0030-stripspace.sh: test_expect_success \
      
      test_expect_success \
    @@ t/t0030-stripspace.sh: test_expect_success \
     -    test $(printf "$ttt$ttt" | git stripspace | wc -l) -gt 0 &&
     -    test $(printf "$ttt$ttt$ttt" | git stripspace | wc -l) -gt 0 &&
     -    test $(printf "$ttt$ttt$ttt$ttt" | git stripspace | wc -l) -gt 0
    -+    printf "$ttt" | git stripspace >tmp &&
    -+    test $(wc -l <tmp) -gt 0 &&
    -+    printf "$ttt$ttt" | git stripspace >tmp &&
    -+    test $(wc -l <tmp) -gt 0 &&
    -+    printf "$ttt$ttt$ttt" | git stripspace >tmp &&
    -+    test $(wc -l <tmp) -gt 0 &&
    -+    printf "$ttt$ttt$ttt$ttt" | git stripspace >tmp &&
    -+    test $(wc -l <tmp) -gt 0
    ++    test_stdout_line_count -gt 0 printf_git_stripspace "$ttt" &&
    ++    test_stdout_line_count -gt 0 printf_git_stripspace "$ttt$ttt" &&
    ++    test_stdout_line_count -gt 0 printf_git_stripspace "$ttt$ttt$ttt" &&
    ++    test_stdout_line_count -gt 0 printf_git_stripspace "$ttt$ttt$ttt$ttt"
      '
      
      # text plus spaces at the end:
    @@ t/t0030-stripspace.sh: test_expect_success \
     -    test $(printf "$ttt$sss$sss" | git stripspace | wc -l) -gt 0 &&
     -    test $(printf "$ttt$ttt$sss$sss" | git stripspace | wc -l) -gt 0 &&
     -    test $(printf "$ttt$sss$sss$sss" | git stripspace | wc -l) -gt 0
    -+    printf "$ttt$sss" | git stripspace >tmp &&
    -+    test $(wc -l <tmp) -gt 0 &&
    -+    printf "$ttt$ttt$sss" | git stripspace >tmp &&
    -+    test $(wc -l <tmp) -gt 0 &&
    -+    printf "$ttt$ttt$ttt$sss" | git stripspace >tmp &&
    -+    test $(wc -l <tmp) -gt 0 &&
    -+    printf "$ttt$sss$sss" | git stripspace >tmp &&
    -+    test $(wc -l <tmp) -gt 0 &&
    -+    printf "$ttt$ttt$sss$sss" | git stripspace >tmp &&
    -+    test $(wc -l <tmp) -gt 0 &&
    -+    printf "$ttt$sss$sss$sss" | git stripspace >tmp &&
    -+    test $(wc -l <tmp) -gt 0
    ++    test_stdout_line_count -gt 0 printf_git_stripspace "$ttt$sss" &&
    ++    test_stdout_line_count -gt 0 printf_git_stripspace "$ttt$ttt$sss" &&
    ++    test_stdout_line_count -gt 0 printf_git_stripspace "$ttt$ttt$ttt$sss" &&
    ++    test_stdout_line_count -gt 0 printf_git_stripspace "$ttt$sss$sss" &&
    ++    test_stdout_line_count -gt 0 printf_git_stripspace "$ttt$ttt$sss$sss" &&
    ++    test_stdout_line_count -gt 0 printf_git_stripspace "$ttt$sss$sss$sss"
      '
      
      test_expect_success \
    @@ t/t0030-stripspace.sh: test_expect_success \
     -    ! (printf "$ttt$ttt$sss$sss" | git stripspace | grep "  " >/dev/null) &&
     -    ! (printf "$ttt$sss$sss$sss" | git stripspace | grep "  " >/dev/null)
     +    printf "$ttt$sss" | git stripspace >tmp &&
    -+    ! (grep "  " tmp >/dev/null) &&
    ++    ! grep "  " tmp >/dev/null &&
     +    printf "$ttt$ttt$sss" | git stripspace &&
    -+    ! (grep "  " tmp >/dev/null) &&
    ++    ! grep "  " tmp >/dev/null &&
     +    printf "$ttt$ttt$ttt$sss" | git stripspace &&
    -+    ! (grep "  " tmp >/dev/nul) &&
    ++    ! grep "  " tmp >/dev/null &&
     +    printf "$ttt$sss$sss" | git stripspace &&
    -+    ! (grep "  " tmp >/dev/null) &&
    ++    ! grep "  " tmp >/dev/null &&
     +    printf "$ttt$ttt$sss$sss" | git stripspace &&
    -+    ! (grep "  " tmp >/dev/null) &&
    ++    ! grep "  " tmp >/dev/null &&
     +    printf "$ttt$sss$sss$sss" | git stripspace &&
    -+    ! (grep "  " tmp >/dev/null)
    ++    ! grep "  " tmp >/dev/null
      '
      
      test_expect_success \
    @@ t/t0030-stripspace.sh: test_expect_success \
     -    ! (echo "$ttt$ttt$sss$sss" | git stripspace | grep "  " >/dev/null) &&
     -    ! (echo "$ttt$sss$sss$sss" | git stripspace | grep "  " >/dev/null)
     +    echo "$ttt$sss" | git stripspace >tmp &&
    -+    ! (grep "  " tmp >/dev/null) &&
    ++    ! grep "  " tmp >/dev/null &&
     +    echo "$ttt$ttt$sss" | git stripspace >tmp &&
    -+    ! (grep "  " tmp>/dev/null) &&
    ++    ! grep "  " tmp>/dev/null &&
     +    echo "$ttt$ttt$ttt$sss" &&
    -+    ! (grep "  " tmp >/dev/null) &&
    ++    ! grep "  " tmp >/dev/null &&
     +    echo "$ttt$sss$sss" | git stripspace >tmp &&
    -+    ! (grep "  " tmp >/dev/null) &&
    ++    ! grep "  " tmp >/dev/null &&
     +    echo "$ttt$ttt$sss$sss" | git stripspace >tmp &&
    -+    ! (grep "  " tmp >/dev/null) &&
    ++    ! grep "  " tmp >/dev/null &&
     +    echo "$ttt$sss$sss$sss" | git stripspace >tmp &&
    -+    ! (grep "  " tmp >/dev/null)
    ++    ! grep "  " tmp >/dev/null
      '
      
      test_expect_success \
    @@ t/t0030-stripspace.sh: test_expect_success \
     -    ! (printf "$sss$sss$sss" | git stripspace | grep " " >/dev/null) &&
     -    ! (printf "$sss$sss$sss$sss" | git stripspace | grep " " >/dev/null)
     +    printf "" | git stripspace >tmp &&
    -+    ! ( grep " " tmp >/dev/null) &&
    ++    ! grep " " tmp >/dev/null &&
     +    printf "$sss" | git stripspace >tmp &&
    -+    ! ( grep " " tmp >/dev/null) &&
    ++    ! grep " " tmp >/dev/null &&
     +    printf "$sss$sss" | git stripspace >tmp &&
    -+    ! (grep " " tmp >/dev/null) &&
    ++    ! grep " " tmp >/dev/null &&
     +    printf "$sss$sss$sss" | git stripspace >tmp &&
    -+    ! (grep " " tmp >/dev/null) &&
    ++    ! grep " " tmp >/dev/null &&
     +    printf "$sss$sss$sss$sss" | git stripspace >tmp &&
    -+    ! (grep " " tmp >/dev/null)
    ++    ! grep " " tmp >/dev/null
      '
      
      test_expect_success \
-- 
2.25.1
Shubham Mishra· Feb 27, 2022, 12:24 UTC · re: Shubham Mishra · lore

[PATCH v2 1/2] t0001-t0028: avoid pipes with Git on LHS

Pipes ignore error codes of LHS command and thus we should not use them with Git in tests. As an alternative, use a 'tmp' file to write the Git output so we can test the exit code.

Signed-off-by: Shubham Mishra <shivam828787@gmail.com>
---
 t/t0000-basic.sh            | 10 ++++++----
 t/t0022-crlf-rename.sh      |  4 ++--
 t/t0025-crlf-renormalize.sh |  4 ++--
 t/t0027-auto-crlf.sh        | 12 ++++++------
 4 files changed, 16 insertions(+), 14 deletions(-)
Show changes to 4 files +16 −14

t/t0000-basic.sh, t/t0022-crlf-rename.sh, t/t0025-crlf-renormalize.sh, t/t0027-auto-crlf.sh

diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
index b007f0efef..b5fa76059b 100755
--- a/t/t0000-basic.sh
+++ b/t/t0000-basic.sh
@@ -1089,7 +1089,8 @@ test_expect_success 'update-index D/F conflict' '
 	mv path2 path0 &&
 	mv tmp path2 &&
 	git update-index --add --replace path2 path0/file2 &&
-	numpath0=$(git ls-files path0 | wc -l) &&
+	git ls-files path0 >tmp &&
+	numpath0=$(wc -l <tmp) &&
 	test $numpath0 = 1
 '
 
@@ -1104,12 +1105,13 @@ test_expect_success 'very long name in the index handled sanely' '
 	>path4 &&
 	git update-index --add path4 &&
 	(
-		git ls-files -s path4 |
-		sed -e "s/	.*/	/" |
+		git ls-files -s path4 >tmp &&
+		sed -e "s/	.*/	/" tmp |
 		tr -d "\012" &&
 		echo "$a"
 	) | git update-index --index-info &&
-	len=$(git ls-files "a*" | wc -c) &&
+	git ls-files "a*" >tmp &&
+	len=$(wc -c <tmp) &&
 	test $len = 4098
 '
 
diff --git a/t/t0022-crlf-rename.sh b/t/t0022-crlf-rename.sh
index c1a331e9e9..9fe9891251 100755
--- a/t/t0022-crlf-rename.sh
+++ b/t/t0022-crlf-rename.sh
@@ -24,8 +24,8 @@ test_expect_success setup '
 
 test_expect_success 'diff -M' '
 
-	git diff-tree -M -r --name-status HEAD^ HEAD |
-	sed -e "s/R[0-9]*/RNUM/" >actual &&
+	git diff-tree -M -r --name-status HEAD^ HEAD >tmp &&
+	sed -e "s/R[0-9]*/RNUM/" tmp >actual &&
 	echo "RNUM	sample	elpmas" >expect &&
 	test_cmp expect actual
 
diff --git a/t/t0025-crlf-renormalize.sh b/t/t0025-crlf-renormalize.sh
index 81447978b7..f7202c192e 100755
--- a/t/t0025-crlf-renormalize.sh
+++ b/t/t0025-crlf-renormalize.sh
@@ -22,8 +22,8 @@ test_expect_success 'renormalize CRLF in repo' '
 	i/lf w/lf attr/text=auto LF.txt
 	i/lf w/mixed attr/text=auto CRLF_mix_LF.txt
 	EOF
-	git ls-files --eol |
-	sed -e "s/	/ /g" -e "s/  */ /g" |
+	git ls-files --eol >tmp &&
+	sed -e "s/	/ /g" -e "s/  */ /g" tmp |
 	sort >actual &&
 	test_cmp expect actual
 '
diff --git a/t/t0027-auto-crlf.sh b/t/t0027-auto-crlf.sh
index c5f7ac63b0..0feb41a23f 100755
--- a/t/t0027-auto-crlf.sh
+++ b/t/t0027-auto-crlf.sh
@@ -311,8 +311,8 @@ checkout_files () {
 		i/-text w/$(stats_ascii $crlfnul) attr/$(attr_ascii $attr $aeol) crlf_false_attr__CRLF_nul.txt
 		i/-text w/$(stats_ascii $crlfnul) attr/$(attr_ascii $attr $aeol) crlf_false_attr__LF_nul.txt
 		EOF
-		git ls-files --eol crlf_false_attr__* |
-		sed -e "s/	/ /g" -e "s/  */ /g" |
+		git ls-files --eol crlf_false_attr__* >tmp &&
+		sed -e "s/	/ /g" -e "s/  */ /g" tmp |
 		sort >actual &&
 		test_cmp expect actual
 	'
@@ -359,12 +359,12 @@ test_expect_success 'ls-files --eol -o Text/Binary' '
 	i/ w/crlf TeBi_126_CL
 	i/ w/-text TeBi_126_CLC
 	EOF
-	git ls-files --eol -o |
+	git ls-files --eol -o >tmp &&
 	sed -n -e "/TeBi_/{s!attr/[	]*!!g
 	s!	! !g
 	s!  *! !g
 	p
-	}" | sort >actual &&
+	}" tmp | sort >actual &&
 	test_cmp expect actual
 '
 
@@ -617,8 +617,8 @@ test_expect_success 'ls-files --eol -d -z' '
 	i/lf w/ crlf_false_attr__LF.txt
 	i/mixed w/ crlf_false_attr__CRLF_mix_LF.txt
 	EOF
-	git ls-files --eol -d |
-	sed -e "s!attr/[^	]*!!g" -e "s/	/ /g" -e "s/  */ /g" |
+	git ls-files --eol -d >tmp &&
+	sed -e "s!attr/[^	]*!!g" -e "s/	/ /g" -e "s/  */ /g" tmp |
 	sort >actual &&
 	test_cmp expect actual
 '

-- 
2.25.1
Johannes Sixt· Feb 27, 2022, 17:43 UTC · re: Shubham Mishra · lore

Re: [PATCH v2 1/2] t0001-t0028: avoid pipes with Git on LHS

Am 27.02.22 um 13:24 schrieb Shubham Mishra:
> diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
> index b007f0efef..b5fa76059b 100755
> --- a/t/t0000-basic.sh
> +++ b/t/t0000-basic.sh
Show 11 quoted lines
> @@ -1104,12 +1105,13 @@ test_expect_success 'very long name in the index handled sanely' '
>  	>path4 &&
>  	git update-index --add path4 &&
>  	(
> -		git ls-files -s path4 |
> -		sed -e "s/	.*/	/" |
> +		git ls-files -s path4 >tmp &&
> +		sed -e "s/	.*/	/" tmp |
>  		tr -d "\012" &&
>  		echo "$a"
>  	) | git update-index --index-info &&

We see a pipe here, and in the upstream of that pipe is a git invocation. That should be fixed, too. After the rewrite that you already did here, it should be sufficient to move the git invocation before the parentheses.

Show 5 quoted lines
> -	len=$(git ls-files "a*" | wc -c) &&
> +	git ls-files "a*" >tmp &&
> +	len=$(wc -c <tmp) &&
>  	test $len = 4098
>  '
-- Hannes
Shubham Mishra· Mar 3, 2022, 12:48 UTC · re: Johannes Sixt · lore

Re: [PATCH v2 1/2] t0001-t0028: avoid pipes with Git on LHS

On Sun, Feb 27, 2022 at 11:13 PM Johannes Sixt <j6t@kdbg.org> wrote:
> We see a pipe here, and in the upstream of that pipe is a git
> invocation. That should be fixed, too. After the rewrite that you
> already did here, it should be sufficient to move the git invocation
> before the parentheses.
I missed that pipe, Thanks for pointing it out and reviewing.

Sorry for responding late, I am in the middle of my exams, so I am hardly checking mails.

Thanks, Shubham

Shubham Mishra· Feb 27, 2022, 12:24 UTC · re: Shubham Mishra · lore

[PATCH v2 2/2] t0030-t0050: avoid pipes with Git on LHS

Pipes ignore error codes of LHS command and thus we should not use them with Git in tests. As an alternative, use a 'tmp' file to write the Git output so we can test the exit code.

Signed-off-by: Shubham Mishra <shivam828787@gmail.com>
---
 t/t0030-stripspace.sh | 75 +++++++++++++++++++++++++++----------------
 t/t0050-filesystem.sh |  3 +-
 2 files changed, 50 insertions(+), 28 deletions(-)
Show changes to 2 files +50 −28

t/t0030-stripspace.sh, t/t0050-filesystem.sh

diff --git a/t/t0030-stripspace.sh b/t/t0030-stripspace.sh
index ae1ca380c1..7bc3dcd07c 100755
--- a/t/t0030-stripspace.sh
+++ b/t/t0030-stripspace.sh
@@ -13,6 +13,10 @@ s40='                                        '
 sss="$s40$s40$s40$s40$s40$s40$s40$s40$s40$s40" # 400
 ttt="$t40$t40$t40$t40$t40$t40$t40$t40$t40$t40" # 400
 
+printf_git_stripspace () {
+    printf "$1" | git stripspace
+}
+
 test_expect_success \
     'long lines without spaces should be unchanged' '
     echo "$ttt" >expect &&
@@ -225,32 +229,38 @@ test_expect_success \
 
 test_expect_success \
     'text without newline at end should end with newline' '
-    test $(printf "$ttt" | git stripspace | wc -l) -gt 0 &&
-    test $(printf "$ttt$ttt" | git stripspace | wc -l) -gt 0 &&
-    test $(printf "$ttt$ttt$ttt" | git stripspace | wc -l) -gt 0 &&
-    test $(printf "$ttt$ttt$ttt$ttt" | git stripspace | wc -l) -gt 0
+    test_stdout_line_count -gt 0 printf_git_stripspace "$ttt" &&
+    test_stdout_line_count -gt 0 printf_git_stripspace "$ttt$ttt" &&
+    test_stdout_line_count -gt 0 printf_git_stripspace "$ttt$ttt$ttt" &&
+    test_stdout_line_count -gt 0 printf_git_stripspace "$ttt$ttt$ttt$ttt"
 '
 
 # text plus spaces at the end:
 
 test_expect_success \
     'text plus spaces without newline at end should end with newline' '
-    test $(printf "$ttt$sss" | git stripspace | wc -l) -gt 0 &&
-    test $(printf "$ttt$ttt$sss" | git stripspace | wc -l) -gt 0 &&
-    test $(printf "$ttt$ttt$ttt$sss" | git stripspace | wc -l) -gt 0 &&
-    test $(printf "$ttt$sss$sss" | git stripspace | wc -l) -gt 0 &&
-    test $(printf "$ttt$ttt$sss$sss" | git stripspace | wc -l) -gt 0 &&
-    test $(printf "$ttt$sss$sss$sss" | git stripspace | wc -l) -gt 0
+    test_stdout_line_count -gt 0 printf_git_stripspace "$ttt$sss" &&
+    test_stdout_line_count -gt 0 printf_git_stripspace "$ttt$ttt$sss" &&
+    test_stdout_line_count -gt 0 printf_git_stripspace "$ttt$ttt$ttt$sss" &&
+    test_stdout_line_count -gt 0 printf_git_stripspace "$ttt$sss$sss" &&
+    test_stdout_line_count -gt 0 printf_git_stripspace "$ttt$ttt$sss$sss" &&
+    test_stdout_line_count -gt 0 printf_git_stripspace "$ttt$sss$sss$sss"
 '
 
 test_expect_success \
     'text plus spaces without newline at end should not show spaces' '
-    ! (printf "$ttt$sss" | git stripspace | grep "  " >/dev/null) &&
-    ! (printf "$ttt$ttt$sss" | git stripspace | grep "  " >/dev/null) &&
-    ! (printf "$ttt$ttt$ttt$sss" | git stripspace | grep "  " >/dev/null) &&
-    ! (printf "$ttt$sss$sss" | git stripspace | grep "  " >/dev/null) &&
-    ! (printf "$ttt$ttt$sss$sss" | git stripspace | grep "  " >/dev/null) &&
-    ! (printf "$ttt$sss$sss$sss" | git stripspace | grep "  " >/dev/null)
+    printf "$ttt$sss" | git stripspace >tmp &&
+    ! grep "  " tmp >/dev/null &&
+    printf "$ttt$ttt$sss" | git stripspace &&
+    ! grep "  " tmp >/dev/null &&
+    printf "$ttt$ttt$ttt$sss" | git stripspace &&
+    ! grep "  " tmp >/dev/null &&
+    printf "$ttt$sss$sss" | git stripspace &&
+    ! grep "  " tmp >/dev/null &&
+    printf "$ttt$ttt$sss$sss" | git stripspace &&
+    ! grep "  " tmp >/dev/null &&
+    printf "$ttt$sss$sss$sss" | git stripspace &&
+    ! grep "  " tmp >/dev/null
 '
 
 test_expect_success \
@@ -282,12 +292,18 @@ test_expect_success \
 
 test_expect_success \
     'text plus spaces at end should not show spaces' '
-    ! (echo "$ttt$sss" | git stripspace | grep "  " >/dev/null) &&
-    ! (echo "$ttt$ttt$sss" | git stripspace | grep "  " >/dev/null) &&
-    ! (echo "$ttt$ttt$ttt$sss" | git stripspace | grep "  " >/dev/null) &&
-    ! (echo "$ttt$sss$sss" | git stripspace | grep "  " >/dev/null) &&
-    ! (echo "$ttt$ttt$sss$sss" | git stripspace | grep "  " >/dev/null) &&
-    ! (echo "$ttt$sss$sss$sss" | git stripspace | grep "  " >/dev/null)
+    echo "$ttt$sss" | git stripspace >tmp &&
+    ! grep "  " tmp >/dev/null &&
+    echo "$ttt$ttt$sss" | git stripspace >tmp &&
+    ! grep "  " tmp>/dev/null &&
+    echo "$ttt$ttt$ttt$sss" &&
+    ! grep "  " tmp >/dev/null &&
+    echo "$ttt$sss$sss" | git stripspace >tmp &&
+    ! grep "  " tmp >/dev/null &&
+    echo "$ttt$ttt$sss$sss" | git stripspace >tmp &&
+    ! grep "  " tmp >/dev/null &&
+    echo "$ttt$sss$sss$sss" | git stripspace >tmp &&
+    ! grep "  " tmp >/dev/null
 '
 
 test_expect_success \
@@ -339,11 +355,16 @@ test_expect_success \
 
 test_expect_success \
     'spaces without newline at end should not show spaces' '
-    ! (printf "" | git stripspace | grep " " >/dev/null) &&
-    ! (printf "$sss" | git stripspace | grep " " >/dev/null) &&
-    ! (printf "$sss$sss" | git stripspace | grep " " >/dev/null) &&
-    ! (printf "$sss$sss$sss" | git stripspace | grep " " >/dev/null) &&
-    ! (printf "$sss$sss$sss$sss" | git stripspace | grep " " >/dev/null)
+    printf "" | git stripspace >tmp &&
+    ! grep " " tmp >/dev/null &&
+    printf "$sss" | git stripspace >tmp &&
+    ! grep " " tmp >/dev/null &&
+    printf "$sss$sss" | git stripspace >tmp &&
+    ! grep " " tmp >/dev/null &&
+    printf "$sss$sss$sss" | git stripspace >tmp &&
+    ! grep " " tmp >/dev/null &&
+    printf "$sss$sss$sss$sss" | git stripspace >tmp &&
+    ! grep " " tmp >/dev/null
 '
 
 test_expect_success \
diff --git a/t/t0050-filesystem.sh b/t/t0050-filesystem.sh
index afc343cf9b..5c9dc90d0b 100755
--- a/t/t0050-filesystem.sh
+++ b/t/t0050-filesystem.sh
@@ -104,7 +104,8 @@ test_expect_failure CASE_INSENSITIVE_FS 'add (with different case)' '
 	rm camelcase &&
 	echo 1 >CamelCase &&
 	git add CamelCase &&
-	camel=$(git ls-files | grep -i camelcase) &&
+	git ls-files >tmp &&
+	camel=$(grep -i camelcase tmp) &&
 	test $(echo "$camel" | wc -l) = 1 &&
 	test "z$(git cat-file blob :$camel)" = z1
 '

-- 
2.25.1
Johannes Sixt· Feb 27, 2022, 17:52 UTC · re: Shubham Mishra · lore

Re: [PATCH v2 2/2] t0030-t0050: avoid pipes with Git on LHS

Am 27.02.22 um 13:24 schrieb Shubham Mishra:
Show 21 quoted lines
>  test_expect_success \
>      'text plus spaces without newline at end should not show spaces' '
> -    ! (printf "$ttt$sss" | git stripspace | grep "  " >/dev/null) &&
> -    ! (printf "$ttt$ttt$sss" | git stripspace | grep "  " >/dev/null) &&
> -    ! (printf "$ttt$ttt$ttt$sss" | git stripspace | grep "  " >/dev/null) &&
> -    ! (printf "$ttt$sss$sss" | git stripspace | grep "  " >/dev/null) &&
> -    ! (printf "$ttt$ttt$sss$sss" | git stripspace | grep "  " >/dev/null) &&
> -    ! (printf "$ttt$sss$sss$sss" | git stripspace | grep "  " >/dev/null)
> +    printf "$ttt$sss" | git stripspace >tmp &&
> +    ! grep "  " tmp >/dev/null &&
> +    printf "$ttt$ttt$sss" | git stripspace &&
> +    ! grep "  " tmp >/dev/null &&
> +    printf "$ttt$ttt$ttt$sss" | git stripspace &&
> +    ! grep "  " tmp >/dev/null &&
> +    printf "$ttt$sss$sss" | git stripspace &&
> +    ! grep "  " tmp >/dev/null &&
> +    printf "$ttt$ttt$sss$sss" | git stripspace &&
> +    ! grep "  " tmp >/dev/null &&
> +    printf "$ttt$sss$sss$sss" | git stripspace &&
> +    ! grep "  " tmp >/dev/null
>  '
You forgot to redirect all but the first `git stripspace`.
Show 25 quoted lines
>  
>  test_expect_success \
> @@ -282,12 +292,18 @@ test_expect_success \
>  
>  test_expect_success \
>      'text plus spaces at end should not show spaces' '
> -    ! (echo "$ttt$sss" | git stripspace | grep "  " >/dev/null) &&
> -    ! (echo "$ttt$ttt$sss" | git stripspace | grep "  " >/dev/null) &&
> -    ! (echo "$ttt$ttt$ttt$sss" | git stripspace | grep "  " >/dev/null) &&
> -    ! (echo "$ttt$sss$sss" | git stripspace | grep "  " >/dev/null) &&
> -    ! (echo "$ttt$ttt$sss$sss" | git stripspace | grep "  " >/dev/null) &&
> -    ! (echo "$ttt$sss$sss$sss" | git stripspace | grep "  " >/dev/null)
> +    echo "$ttt$sss" | git stripspace >tmp &&
> +    ! grep "  " tmp >/dev/null &&
> +    echo "$ttt$ttt$sss" | git stripspace >tmp &&
> +    ! grep "  " tmp>/dev/null &&
> +    echo "$ttt$ttt$ttt$sss" &&
> +    ! grep "  " tmp >/dev/null &&
> +    echo "$ttt$sss$sss" | git stripspace >tmp &&
> +    ! grep "  " tmp >/dev/null &&
> +    echo "$ttt$ttt$sss$sss" | git stripspace >tmp &&
> +    ! grep "  " tmp >/dev/null &&
> +    echo "$ttt$sss$sss$sss" | git stripspace >tmp &&
> +    ! grep "  " tmp >/dev/null
>  '

A superficial glance at the visual pattern formed by the new lines reveals immediately that there is something foul. And indeed, one of the rewritten cases misses `| git stripspace >tmp`.

-- Hannes
Shubham Mishra· Feb 24, 2022, 05:51 UTC · re: Shubham Mishra · lore

Re: [PATCH 0/2] Subject: [PATCH 0/2] [GSoC][PATCH 0/2] t0000-t0050: avoid pipes with Git on LHS

for further reference, you can follow the discussion on my last patchset - https://lore.kernel.org/git/20220222114313.14921-1-shivam828787@gmail.com/

On Thu, Feb 24, 2022 at 11:17 AM Shubham Mishra <shivam828787@gmail.com> wrote:
Show 21 quoted lines
>
> As a microproject, I remove all pipes with GIT on LHS from
> files t0000 to t0050.
>
> Shubham Mishra (2):
>   t0001-t0028: avoid pipes with Git on LHS
>   t0030-t0050: avoid pipes with Git on LHS
>
>  t/t0000-basic.sh            | 10 +++--
>  t/t0022-crlf-rename.sh      |  4 +-
>  t/t0025-crlf-renormalize.sh |  4 +-
>  t/t0027-auto-crlf.sh        | 12 +++---
>  t/t0030-stripspace.sh       | 81 ++++++++++++++++++++++++-------------
>  t/t0050-filesystem.sh       |  3 +-
>  6 files changed, 72 insertions(+), 42 deletions(-)
>
>
> base-commit: e6ebfd0e8cbbd10878070c8a356b5ad1b3ca464e
> --
> 2.25.1
>
Taylor Blau· Feb 26, 2022, 17:30 UTC · re: Shubham Mishra · lore

Re: [PATCH 0/2] Subject: [PATCH 0/2] [GSoC][PATCH 0/2] t0000-t0050: avoid pipes with Git on LHS

On Thu, Feb 24, 2022 at 11:21:05AM +0530, Shubham Mishra wrote:
> for further reference, you can follow the discussion on my last
> patchset - https://lore.kernel.org/git/20220222114313.14921-1-shivam828787@gmail.com/

In the future, you can use `--in-reply-to` to make sure that subsequent versions are attached to the previous version's thread.

Thanks, Taylor

Shubham Mishra· Mar 12, 2022, 06:21 UTC · re: Shubham Mishra · lore

[GSoC] [PATCH v3 0/2] avoid pipes with Git on LHS

changes since v2:
* t0000: remove git command out of subshell
* t0030: fix missing redirections to "tmp" file
Shubham Mishra (2):
  t0001-t0028: avoid pipes with Git on LHS
  t0030-t0050: avoid pipes with Git on LHS
 t/t0000-basic.sh            | 10 +++--
 t/t0022-crlf-rename.sh      |  4 +-
 t/t0025-crlf-renormalize.sh |  4 +-
 t/t0027-auto-crlf.sh        | 12 +++---
 t/t0030-stripspace.sh       | 75 ++++++++++++++++++++++++-------------
 t/t0050-filesystem.sh       |  3 +-
 6 files changed, 66 insertions(+), 42 deletions(-)
Range-diff against v2:
1:  2a219ace42 ! 1:  5e122c0acf t0001-t0028: avoid pipes with Git on LHS
    @@ t/t0000-basic.sh: test_expect_success 'update-index D/F conflict' '
      '
      
     @@ t/t0000-basic.sh: test_expect_success 'very long name in the index handled sanely' '
    + 
      	>path4 &&
      	git update-index --add path4 &&
    ++	git ls-files -s path4 >tmp &&
      	(
     -		git ls-files -s path4 |
     -		sed -e "s/	.*/	/" |
    -+		git ls-files -s path4 >tmp &&
     +		sed -e "s/	.*/	/" tmp |
      		tr -d "\012" &&
      		echo "$a"
2:  c90fc271d9 ! 2:  c412380af3 t0030-t0050: avoid pipes with Git on LHS
    @@ t/t0030-stripspace.sh: test_expect_success \
     -    ! (printf "$ttt$sss$sss$sss" | git stripspace | grep "  " >/dev/null)
     +    printf "$ttt$sss" | git stripspace >tmp &&
     +    ! grep "  " tmp >/dev/null &&
    -+    printf "$ttt$ttt$sss" | git stripspace &&
    ++    printf "$ttt$ttt$sss" | git stripspace >tmp &&
     +    ! grep "  " tmp >/dev/null &&
    -+    printf "$ttt$ttt$ttt$sss" | git stripspace &&
    ++    printf "$ttt$ttt$ttt$sss" | git stripspace >tmp &&
     +    ! grep "  " tmp >/dev/null &&
    -+    printf "$ttt$sss$sss" | git stripspace &&
    ++    printf "$ttt$sss$sss" | git stripspace >tmp &&
     +    ! grep "  " tmp >/dev/null &&
    -+    printf "$ttt$ttt$sss$sss" | git stripspace &&
    ++    printf "$ttt$ttt$sss$sss" | git stripspace >tmp &&
     +    ! grep "  " tmp >/dev/null &&
    -+    printf "$ttt$sss$sss$sss" | git stripspace &&
    ++    printf "$ttt$sss$sss$sss" | git stripspace >tmp &&
     +    ! grep "  " tmp >/dev/null
      '
      
    @@ t/t0030-stripspace.sh: test_expect_success \
     +    echo "$ttt$sss" | git stripspace >tmp &&
     +    ! grep "  " tmp >/dev/null &&
     +    echo "$ttt$ttt$sss" | git stripspace >tmp &&
    -+    ! grep "  " tmp>/dev/null &&
    -+    echo "$ttt$ttt$ttt$sss" &&
    ++    ! grep "  " tmp >/dev/null &&
    ++    echo "$ttt$ttt$ttt$sss" | git stripspace >tmp &&
     +    ! grep "  " tmp >/dev/null &&
     +    echo "$ttt$sss$sss" | git stripspace >tmp &&
     +    ! grep "  " tmp >/dev/null &&
-- 
2.25.1
Shubham Mishra· Mar 12, 2022, 06:21 UTC · re: Shubham Mishra · lore

[PATCH v3 1/2] t0001-t0028: avoid pipes with Git on LHS

Pipes ignore error codes of LHS command and thus we should not use them with Git in tests. As an alternative, use a 'tmp' file to write the Git output so we can test the exit code.

Signed-off-by: Shubham Mishra <shivam828787@gmail.com>
---
 t/t0000-basic.sh            | 10 ++++++----
 t/t0022-crlf-rename.sh      |  4 ++--
 t/t0025-crlf-renormalize.sh |  4 ++--
 t/t0027-auto-crlf.sh        | 12 ++++++------
 4 files changed, 16 insertions(+), 14 deletions(-)
Show changes to 4 files +16 −14

t/t0000-basic.sh, t/t0022-crlf-rename.sh, t/t0025-crlf-renormalize.sh, t/t0027-auto-crlf.sh

diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
index b007f0efef..9dcbf518a7 100755
--- a/t/t0000-basic.sh
+++ b/t/t0000-basic.sh
@@ -1089,7 +1089,8 @@ test_expect_success 'update-index D/F conflict' '
 	mv path2 path0 &&
 	mv tmp path2 &&
 	git update-index --add --replace path2 path0/file2 &&
-	numpath0=$(git ls-files path0 | wc -l) &&
+	git ls-files path0 >tmp &&
+	numpath0=$(wc -l <tmp) &&
 	test $numpath0 = 1
 '
 
@@ -1103,13 +1104,14 @@ test_expect_success 'very long name in the index handled sanely' '
 
 	>path4 &&
 	git update-index --add path4 &&
+	git ls-files -s path4 >tmp &&
 	(
-		git ls-files -s path4 |
-		sed -e "s/	.*/	/" |
+		sed -e "s/	.*/	/" tmp |
 		tr -d "\012" &&
 		echo "$a"
 	) | git update-index --index-info &&
-	len=$(git ls-files "a*" | wc -c) &&
+	git ls-files "a*" >tmp &&
+	len=$(wc -c <tmp) &&
 	test $len = 4098
 '
 
diff --git a/t/t0022-crlf-rename.sh b/t/t0022-crlf-rename.sh
index c1a331e9e9..9fe9891251 100755
--- a/t/t0022-crlf-rename.sh
+++ b/t/t0022-crlf-rename.sh
@@ -24,8 +24,8 @@ test_expect_success setup '
 
 test_expect_success 'diff -M' '
 
-	git diff-tree -M -r --name-status HEAD^ HEAD |
-	sed -e "s/R[0-9]*/RNUM/" >actual &&
+	git diff-tree -M -r --name-status HEAD^ HEAD >tmp &&
+	sed -e "s/R[0-9]*/RNUM/" tmp >actual &&
 	echo "RNUM	sample	elpmas" >expect &&
 	test_cmp expect actual
 
diff --git a/t/t0025-crlf-renormalize.sh b/t/t0025-crlf-renormalize.sh
index 81447978b7..f7202c192e 100755
--- a/t/t0025-crlf-renormalize.sh
+++ b/t/t0025-crlf-renormalize.sh
@@ -22,8 +22,8 @@ test_expect_success 'renormalize CRLF in repo' '
 	i/lf w/lf attr/text=auto LF.txt
 	i/lf w/mixed attr/text=auto CRLF_mix_LF.txt
 	EOF
-	git ls-files --eol |
-	sed -e "s/	/ /g" -e "s/  */ /g" |
+	git ls-files --eol >tmp &&
+	sed -e "s/	/ /g" -e "s/  */ /g" tmp |
 	sort >actual &&
 	test_cmp expect actual
 '
diff --git a/t/t0027-auto-crlf.sh b/t/t0027-auto-crlf.sh
index c5f7ac63b0..0feb41a23f 100755
--- a/t/t0027-auto-crlf.sh
+++ b/t/t0027-auto-crlf.sh
@@ -311,8 +311,8 @@ checkout_files () {
 		i/-text w/$(stats_ascii $crlfnul) attr/$(attr_ascii $attr $aeol) crlf_false_attr__CRLF_nul.txt
 		i/-text w/$(stats_ascii $crlfnul) attr/$(attr_ascii $attr $aeol) crlf_false_attr__LF_nul.txt
 		EOF
-		git ls-files --eol crlf_false_attr__* |
-		sed -e "s/	/ /g" -e "s/  */ /g" |
+		git ls-files --eol crlf_false_attr__* >tmp &&
+		sed -e "s/	/ /g" -e "s/  */ /g" tmp |
 		sort >actual &&
 		test_cmp expect actual
 	'
@@ -359,12 +359,12 @@ test_expect_success 'ls-files --eol -o Text/Binary' '
 	i/ w/crlf TeBi_126_CL
 	i/ w/-text TeBi_126_CLC
 	EOF
-	git ls-files --eol -o |
+	git ls-files --eol -o >tmp &&
 	sed -n -e "/TeBi_/{s!attr/[	]*!!g
 	s!	! !g
 	s!  *! !g
 	p
-	}" | sort >actual &&
+	}" tmp | sort >actual &&
 	test_cmp expect actual
 '
 
@@ -617,8 +617,8 @@ test_expect_success 'ls-files --eol -d -z' '
 	i/lf w/ crlf_false_attr__LF.txt
 	i/mixed w/ crlf_false_attr__CRLF_mix_LF.txt
 	EOF
-	git ls-files --eol -d |
-	sed -e "s!attr/[^	]*!!g" -e "s/	/ /g" -e "s/  */ /g" |
+	git ls-files --eol -d >tmp &&
+	sed -e "s!attr/[^	]*!!g" -e "s/	/ /g" -e "s/  */ /g" tmp |
 	sort >actual &&
 	test_cmp expect actual
 '

-- 
2.25.1
Shubham Mishra· Mar 12, 2022, 06:21 UTC · re: Shubham Mishra · lore

[PATCH v3 2/2] t0030-t0050: avoid pipes with Git on LHS

Pipes ignore error codes of LHS command and thus we should not use them with Git in tests. As an alternative, use a 'tmp' file to write the Git output so we can test the exit code.

Signed-off-by: Shubham Mishra <shivam828787@gmail.com>
---
 t/t0030-stripspace.sh | 75 +++++++++++++++++++++++++++----------------
 t/t0050-filesystem.sh |  3 +-
 2 files changed, 50 insertions(+), 28 deletions(-)
Show changes to 2 files +50 −28

t/t0030-stripspace.sh, t/t0050-filesystem.sh

diff --git a/t/t0030-stripspace.sh b/t/t0030-stripspace.sh
index ae1ca380c1..0a5713c524 100755
--- a/t/t0030-stripspace.sh
+++ b/t/t0030-stripspace.sh
@@ -13,6 +13,10 @@ s40='                                        '
 sss="$s40$s40$s40$s40$s40$s40$s40$s40$s40$s40" # 400
 ttt="$t40$t40$t40$t40$t40$t40$t40$t40$t40$t40" # 400
 
+printf_git_stripspace () {
+    printf "$1" | git stripspace
+}
+
 test_expect_success \
     'long lines without spaces should be unchanged' '
     echo "$ttt" >expect &&
@@ -225,32 +229,38 @@ test_expect_success \
 
 test_expect_success \
     'text without newline at end should end with newline' '
-    test $(printf "$ttt" | git stripspace | wc -l) -gt 0 &&
-    test $(printf "$ttt$ttt" | git stripspace | wc -l) -gt 0 &&
-    test $(printf "$ttt$ttt$ttt" | git stripspace | wc -l) -gt 0 &&
-    test $(printf "$ttt$ttt$ttt$ttt" | git stripspace | wc -l) -gt 0
+    test_stdout_line_count -gt 0 printf_git_stripspace "$ttt" &&
+    test_stdout_line_count -gt 0 printf_git_stripspace "$ttt$ttt" &&
+    test_stdout_line_count -gt 0 printf_git_stripspace "$ttt$ttt$ttt" &&
+    test_stdout_line_count -gt 0 printf_git_stripspace "$ttt$ttt$ttt$ttt"
 '
 
 # text plus spaces at the end:
 
 test_expect_success \
     'text plus spaces without newline at end should end with newline' '
-    test $(printf "$ttt$sss" | git stripspace | wc -l) -gt 0 &&
-    test $(printf "$ttt$ttt$sss" | git stripspace | wc -l) -gt 0 &&
-    test $(printf "$ttt$ttt$ttt$sss" | git stripspace | wc -l) -gt 0 &&
-    test $(printf "$ttt$sss$sss" | git stripspace | wc -l) -gt 0 &&
-    test $(printf "$ttt$ttt$sss$sss" | git stripspace | wc -l) -gt 0 &&
-    test $(printf "$ttt$sss$sss$sss" | git stripspace | wc -l) -gt 0
+    test_stdout_line_count -gt 0 printf_git_stripspace "$ttt$sss" &&
+    test_stdout_line_count -gt 0 printf_git_stripspace "$ttt$ttt$sss" &&
+    test_stdout_line_count -gt 0 printf_git_stripspace "$ttt$ttt$ttt$sss" &&
+    test_stdout_line_count -gt 0 printf_git_stripspace "$ttt$sss$sss" &&
+    test_stdout_line_count -gt 0 printf_git_stripspace "$ttt$ttt$sss$sss" &&
+    test_stdout_line_count -gt 0 printf_git_stripspace "$ttt$sss$sss$sss"
 '
 
 test_expect_success \
     'text plus spaces without newline at end should not show spaces' '
-    ! (printf "$ttt$sss" | git stripspace | grep "  " >/dev/null) &&
-    ! (printf "$ttt$ttt$sss" | git stripspace | grep "  " >/dev/null) &&
-    ! (printf "$ttt$ttt$ttt$sss" | git stripspace | grep "  " >/dev/null) &&
-    ! (printf "$ttt$sss$sss" | git stripspace | grep "  " >/dev/null) &&
-    ! (printf "$ttt$ttt$sss$sss" | git stripspace | grep "  " >/dev/null) &&
-    ! (printf "$ttt$sss$sss$sss" | git stripspace | grep "  " >/dev/null)
+    printf "$ttt$sss" | git stripspace >tmp &&
+    ! grep "  " tmp >/dev/null &&
+    printf "$ttt$ttt$sss" | git stripspace >tmp &&
+    ! grep "  " tmp >/dev/null &&
+    printf "$ttt$ttt$ttt$sss" | git stripspace >tmp &&
+    ! grep "  " tmp >/dev/null &&
+    printf "$ttt$sss$sss" | git stripspace >tmp &&
+    ! grep "  " tmp >/dev/null &&
+    printf "$ttt$ttt$sss$sss" | git stripspace >tmp &&
+    ! grep "  " tmp >/dev/null &&
+    printf "$ttt$sss$sss$sss" | git stripspace >tmp &&
+    ! grep "  " tmp >/dev/null
 '
 
 test_expect_success \
@@ -282,12 +292,18 @@ test_expect_success \
 
 test_expect_success \
     'text plus spaces at end should not show spaces' '
-    ! (echo "$ttt$sss" | git stripspace | grep "  " >/dev/null) &&
-    ! (echo "$ttt$ttt$sss" | git stripspace | grep "  " >/dev/null) &&
-    ! (echo "$ttt$ttt$ttt$sss" | git stripspace | grep "  " >/dev/null) &&
-    ! (echo "$ttt$sss$sss" | git stripspace | grep "  " >/dev/null) &&
-    ! (echo "$ttt$ttt$sss$sss" | git stripspace | grep "  " >/dev/null) &&
-    ! (echo "$ttt$sss$sss$sss" | git stripspace | grep "  " >/dev/null)
+    echo "$ttt$sss" | git stripspace >tmp &&
+    ! grep "  " tmp >/dev/null &&
+    echo "$ttt$ttt$sss" | git stripspace >tmp &&
+    ! grep "  " tmp >/dev/null &&
+    echo "$ttt$ttt$ttt$sss" | git stripspace >tmp &&
+    ! grep "  " tmp >/dev/null &&
+    echo "$ttt$sss$sss" | git stripspace >tmp &&
+    ! grep "  " tmp >/dev/null &&
+    echo "$ttt$ttt$sss$sss" | git stripspace >tmp &&
+    ! grep "  " tmp >/dev/null &&
+    echo "$ttt$sss$sss$sss" | git stripspace >tmp &&
+    ! grep "  " tmp >/dev/null
 '
 
 test_expect_success \
@@ -339,11 +355,16 @@ test_expect_success \
 
 test_expect_success \
     'spaces without newline at end should not show spaces' '
-    ! (printf "" | git stripspace | grep " " >/dev/null) &&
-    ! (printf "$sss" | git stripspace | grep " " >/dev/null) &&
-    ! (printf "$sss$sss" | git stripspace | grep " " >/dev/null) &&
-    ! (printf "$sss$sss$sss" | git stripspace | grep " " >/dev/null) &&
-    ! (printf "$sss$sss$sss$sss" | git stripspace | grep " " >/dev/null)
+    printf "" | git stripspace >tmp &&
+    ! grep " " tmp >/dev/null &&
+    printf "$sss" | git stripspace >tmp &&
+    ! grep " " tmp >/dev/null &&
+    printf "$sss$sss" | git stripspace >tmp &&
+    ! grep " " tmp >/dev/null &&
+    printf "$sss$sss$sss" | git stripspace >tmp &&
+    ! grep " " tmp >/dev/null &&
+    printf "$sss$sss$sss$sss" | git stripspace >tmp &&
+    ! grep " " tmp >/dev/null
 '
 
 test_expect_success \
diff --git a/t/t0050-filesystem.sh b/t/t0050-filesystem.sh
index afc343cf9b..5c9dc90d0b 100755
--- a/t/t0050-filesystem.sh
+++ b/t/t0050-filesystem.sh
@@ -104,7 +104,8 @@ test_expect_failure CASE_INSENSITIVE_FS 'add (with different case)' '
 	rm camelcase &&
 	echo 1 >CamelCase &&
 	git add CamelCase &&
-	camel=$(git ls-files | grep -i camelcase) &&
+	git ls-files >tmp &&
+	camel=$(grep -i camelcase tmp) &&
 	test $(echo "$camel" | wc -l) = 1 &&
 	test "z$(git cat-file blob :$camel)" = z1
 '

-- 
2.25.1
Johannes Sixt· Mar 12, 2022, 09:05 UTC · re: Shubham Mishra · lore

Re: [GSoC] [PATCH v3 0/2] avoid pipes with Git on LHS

Am 12.03.22 um 07:21 schrieb Shubham Mishra:
Show 65 quoted lines
> changes since v2:
> * t0000: remove git command out of subshell
> * t0030: fix missing redirections to "tmp" file
> Shubham Mishra (2):
>   t0001-t0028: avoid pipes with Git on LHS
>   t0030-t0050: avoid pipes with Git on LHS
> 
>  t/t0000-basic.sh            | 10 +++--
>  t/t0022-crlf-rename.sh      |  4 +-
>  t/t0025-crlf-renormalize.sh |  4 +-
>  t/t0027-auto-crlf.sh        | 12 +++---
>  t/t0030-stripspace.sh       | 75 ++++++++++++++++++++++++-------------
>  t/t0050-filesystem.sh       |  3 +-
>  6 files changed, 66 insertions(+), 42 deletions(-)
> 
> Range-diff against v2:
> 1:  2a219ace42 ! 1:  5e122c0acf t0001-t0028: avoid pipes with Git on LHS
>     @@ t/t0000-basic.sh: test_expect_success 'update-index D/F conflict' '
>       '
>       
>      @@ t/t0000-basic.sh: test_expect_success 'very long name in the index handled sanely' '
>     + 
>       	>path4 &&
>       	git update-index --add path4 &&
>     ++	git ls-files -s path4 >tmp &&
>       	(
>      -		git ls-files -s path4 |
>      -		sed -e "s/	.*/	/" |
>     -+		git ls-files -s path4 >tmp &&
>      +		sed -e "s/	.*/	/" tmp |
>       		tr -d "\012" &&
>       		echo "$a"
> 2:  c90fc271d9 ! 2:  c412380af3 t0030-t0050: avoid pipes with Git on LHS
>     @@ t/t0030-stripspace.sh: test_expect_success \
>      -    ! (printf "$ttt$sss$sss$sss" | git stripspace | grep "  " >/dev/null)
>      +    printf "$ttt$sss" | git stripspace >tmp &&
>      +    ! grep "  " tmp >/dev/null &&
>     -+    printf "$ttt$ttt$sss" | git stripspace &&
>     ++    printf "$ttt$ttt$sss" | git stripspace >tmp &&
>      +    ! grep "  " tmp >/dev/null &&
>     -+    printf "$ttt$ttt$ttt$sss" | git stripspace &&
>     ++    printf "$ttt$ttt$ttt$sss" | git stripspace >tmp &&
>      +    ! grep "  " tmp >/dev/null &&
>     -+    printf "$ttt$sss$sss" | git stripspace &&
>     ++    printf "$ttt$sss$sss" | git stripspace >tmp &&
>      +    ! grep "  " tmp >/dev/null &&
>     -+    printf "$ttt$ttt$sss$sss" | git stripspace &&
>     ++    printf "$ttt$ttt$sss$sss" | git stripspace >tmp &&
>      +    ! grep "  " tmp >/dev/null &&
>     -+    printf "$ttt$sss$sss$sss" | git stripspace &&
>     ++    printf "$ttt$sss$sss$sss" | git stripspace >tmp &&
>      +    ! grep "  " tmp >/dev/null
>       '
>       
>     @@ t/t0030-stripspace.sh: test_expect_success \
>      +    echo "$ttt$sss" | git stripspace >tmp &&
>      +    ! grep "  " tmp >/dev/null &&
>      +    echo "$ttt$ttt$sss" | git stripspace >tmp &&
>     -+    ! grep "  " tmp>/dev/null &&
>     -+    echo "$ttt$ttt$ttt$sss" &&
>     ++    ! grep "  " tmp >/dev/null &&
>     ++    echo "$ttt$ttt$ttt$sss" | git stripspace >tmp &&
>      +    ! grep "  " tmp >/dev/null &&
>      +    echo "$ttt$sss$sss" | git stripspace >tmp &&
>      +    ! grep "  " tmp >/dev/null &&
This round addresses all my comments. Thanks!
-- Hannes
Shubham Mishra· Mar 12, 2022, 10:27 UTC · re: Johannes Sixt · lore

Re: [GSoC] [PATCH v3 0/2] avoid pipes with Git on LHS

> This round addresses all my comments. Thanks!
>

Thanks, this series really helped me learn how code reviewing works here :). looking forward to picking other tasks.

Thanks, Shubham

← back to recent threads