git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH (optional)] t0021: use arithmetic expansion to trim whitespace from wc -c output

From
Lars Schneider <larsxschneider@gmail.com>
Date
Nov 6, 2016, 15:45 UTC
Message-ID
<C61D1F80-7B7A-4111-9418-75D178AF4353@gmail.com>
In-Reply-To
<3b58b90d-5435-1503-d052-413a947a5ab5@kdbg.org>
Show 159 quoted lines
> On 03 Nov 2016, at 21:22, Johannes Sixt <j6t@kdbg.org> wrote:
> 
> Instead of a pipeline with sed and a useless use of cat, return the
> unmodified text of wc -c from function file_size, but substitute the
> result with arithmetic expansion to get rid of the leading whitespace
> that some version of wc -c print.
> 
> Signed-off-by: Johannes Sixt <j6t@kdbg.org>
> ---
> This is a pure optimization that reduces the number of forks, which
> helps a bit on Windows.
> 
> There would be a solution with perl that does not require trimming
> of whitespace, but perl startup times are unbearable on Windows.
> wc -c is better.
> 
> t/t0021-conversion.sh | 50 +++++++++++++++++++++++++-------------------------
> 1 file changed, 25 insertions(+), 25 deletions(-)
> 
> diff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh
> index db71acacb3..42b529f615 100755
> --- a/t/t0021-conversion.sh
> +++ b/t/t0021-conversion.sh
> @@ -22,7 +22,7 @@ generate_random_characters () {
> }
> 
> file_size () {
> -	cat "$1" | wc -c | sed "s/^[ ]*//"
> +	wc -c <"$1"
> }
> 
> filter_git () {
> @@ -369,10 +369,10 @@ test_expect_success PERL 'required process filter should filter data' '
> 		cat >expected.log <<-EOF &&
> 			START
> 			init handshake complete
> -			IN: clean test.r $S [OK] -- OUT: $S . [OK]
> -			IN: clean test2.r $S2 [OK] -- OUT: $S2 . [OK]
> +			IN: clean test.r $(($S)) [OK] -- OUT: $(($S)) . [OK]
> +			IN: clean test2.r $(($S2)) [OK] -- OUT: $(($S2)) . [OK]
> 			IN: clean test4-empty.r 0 [OK] -- OUT: 0  [OK]
> -			IN: clean testsubdir/test3 '\''sq'\'',\$x.r $S3 [OK] -- OUT: $S3 . [OK]
> +			IN: clean testsubdir/test3 '\''sq'\'',\$x.r $(($S3)) [OK] -- OUT: $(($S3)) . [OK]
> 			STOP
> 		EOF
> 		test_cmp_count expected.log rot13-filter.log &&
> @@ -381,14 +381,14 @@ test_expect_success PERL 'required process filter should filter data' '
> 		cat >expected.log <<-EOF &&
> 			START
> 			init handshake complete
> -			IN: clean test.r $S [OK] -- OUT: $S . [OK]
> -			IN: clean test2.r $S2 [OK] -- OUT: $S2 . [OK]
> +			IN: clean test.r $(($S)) [OK] -- OUT: $(($S)) . [OK]
> +			IN: clean test2.r $(($S2)) [OK] -- OUT: $(($S2)) . [OK]
> 			IN: clean test4-empty.r 0 [OK] -- OUT: 0  [OK]
> -			IN: clean testsubdir/test3 '\''sq'\'',\$x.r $S3 [OK] -- OUT: $S3 . [OK]
> -			IN: clean test.r $S [OK] -- OUT: $S . [OK]
> -			IN: clean test2.r $S2 [OK] -- OUT: $S2 . [OK]
> +			IN: clean testsubdir/test3 '\''sq'\'',\$x.r $(($S3)) [OK] -- OUT: $(($S3)) . [OK]
> +			IN: clean test.r $(($S)) [OK] -- OUT: $(($S)) . [OK]
> +			IN: clean test2.r $(($S2)) [OK] -- OUT: $(($S2)) . [OK]
> 			IN: clean test4-empty.r 0 [OK] -- OUT: 0  [OK]
> -			IN: clean testsubdir/test3 '\''sq'\'',\$x.r $S3 [OK] -- OUT: $S3 . [OK]
> +			IN: clean testsubdir/test3 '\''sq'\'',\$x.r $(($S3)) [OK] -- OUT: $(($S3)) . [OK]
> 			STOP
> 		EOF
> 		test_cmp_count expected.log rot13-filter.log &&
> @@ -399,8 +399,8 @@ test_expect_success PERL 'required process filter should filter data' '
> 		cat >expected.log <<-EOF &&
> 			START
> 			init handshake complete
> -			IN: smudge test2.r $S2 [OK] -- OUT: $S2 . [OK]
> -			IN: smudge testsubdir/test3 '\''sq'\'',\$x.r $S3 [OK] -- OUT: $S3 . [OK]
> +			IN: smudge test2.r $(($S2)) [OK] -- OUT: $(($S2)) . [OK]
> +			IN: smudge testsubdir/test3 '\''sq'\'',\$x.r $(($S3)) [OK] -- OUT: $(($S3)) . [OK]
> 			STOP
> 		EOF
> 		test_cmp_exclude_clean expected.log rot13-filter.log &&
> @@ -409,7 +409,7 @@ test_expect_success PERL 'required process filter should filter data' '
> 		cat >expected.log <<-EOF &&
> 			START
> 			init handshake complete
> -			IN: clean test.r $S [OK] -- OUT: $S . [OK]
> +			IN: clean test.r $(($S)) [OK] -- OUT: $(($S)) . [OK]
> 			STOP
> 		EOF
> 		test_cmp_exclude_clean expected.log rot13-filter.log &&
> @@ -418,10 +418,10 @@ test_expect_success PERL 'required process filter should filter data' '
> 		cat >expected.log <<-EOF &&
> 			START
> 			init handshake complete
> -			IN: smudge test.r $S [OK] -- OUT: $S . [OK]
> -			IN: smudge test2.r $S2 [OK] -- OUT: $S2 . [OK]
> +			IN: smudge test.r $(($S)) [OK] -- OUT: $(($S)) . [OK]
> +			IN: smudge test2.r $(($S2)) [OK] -- OUT: $(($S2)) . [OK]
> 			IN: smudge test4-empty.r 0 [OK] -- OUT: 0  [OK]
> -			IN: smudge testsubdir/test3 '\''sq'\'',\$x.r $S3 [OK] -- OUT: $S3 . [OK]
> +			IN: smudge testsubdir/test3 '\''sq'\'',\$x.r $(($S3)) [OK] -- OUT: $(($S3)) . [OK]
> 			STOP
> 		EOF
> 		test_cmp_exclude_clean expected.log rot13-filter.log &&
> @@ -451,7 +451,7 @@ test_expect_success PERL 'required process filter takes precedence' '
> 		cat >expected.log <<-EOF &&
> 			START
> 			init handshake complete
> -			IN: clean test.r $S [OK] -- OUT: $S . [OK]
> +			IN: clean test.r $(($S)) [OK] -- OUT: $(($S)) . [OK]
> 			STOP
> 		EOF
> 		test_cmp_count expected.log rot13-filter.log
> @@ -474,7 +474,7 @@ test_expect_success PERL 'required process filter should be used only for "clean
> 		cat >expected.log <<-EOF &&
> 			START
> 			init handshake complete
> -			IN: clean test.r $S [OK] -- OUT: $S . [OK]
> +			IN: clean test.r $(($S)) [OK] -- OUT: $(($S)) . [OK]
> 			STOP
> 		EOF
> 		test_cmp_count expected.log rot13-filter.log &&
> @@ -603,11 +603,11 @@ test_expect_success PERL 'process filter should restart after unexpected write f
> 		cat >expected.log <<-EOF &&
> 			START
> 			init handshake complete
> -			IN: smudge smudge-write-fail.r $SF [OK] -- OUT: $SF [WRITE FAIL]
> +			IN: smudge smudge-write-fail.r $(($SF)) [OK] -- OUT: $(($SF)) [WRITE FAIL]
> 			START
> 			init handshake complete
> -			IN: smudge test.r $S [OK] -- OUT: $S . [OK]
> -			IN: smudge test2.r $S2 [OK] -- OUT: $S2 . [OK]
> +			IN: smudge test.r $(($S)) [OK] -- OUT: $(($S)) . [OK]
> +			IN: smudge test2.r $(($S2)) [OK] -- OUT: $(($S2)) . [OK]
> 			STOP
> 		EOF
> 		test_cmp_exclude_clean expected.log rot13-filter.log &&
> @@ -649,9 +649,9 @@ test_expect_success PERL 'process filter should not be restarted if it signals a
> 		cat >expected.log <<-EOF &&
> 			START
> 			init handshake complete
> -			IN: smudge error.r $SE [OK] -- OUT: 0 [ERROR]
> -			IN: smudge test.r $S [OK] -- OUT: $S . [OK]
> -			IN: smudge test2.r $S2 [OK] -- OUT: $S2 . [OK]
> +			IN: smudge error.r $(($SE)) [OK] -- OUT: 0 [ERROR]
> +			IN: smudge test.r $(($S)) [OK] -- OUT: $(($S)) . [OK]
> +			IN: smudge test2.r $(($S2)) [OK] -- OUT: $(($S2)) . [OK]
> 			STOP
> 		EOF
> 		test_cmp_exclude_clean expected.log rot13-filter.log &&
> @@ -688,7 +688,7 @@ test_expect_success PERL 'process filter abort stops processing of all further f
> 		cat >expected.log <<-EOF &&
> 			START
> 			init handshake complete
> -			IN: smudge abort.r $SA [OK] -- OUT: 0 [ABORT]
> +			IN: smudge abort.r $(($SA)) [OK] -- OUT: 0 [ABORT]
> 			STOP
> 		EOF
> 		test_cmp_exclude_clean expected.log rot13-filter.log &&
> -- 
> 2.11.0.rc0.55.gd967357
>

This looks good to me! As I run the Windows tests once in a while, too, I am in full support to make them faster :-)

Since the file size function became very simple with your patch, shouldn't we get rid of it? If you agree, then we could squash the patch below into your patch.

I also would like to move the arithmetic expansion to the variable to make the `expected.log` better readable. That means I would like to have this:

S=$(($(wc -c <test.r))) &&
...
			IN: clean test.r $S [OK] -- OUT: $S . [OK]
Instead of this:
S=$(wc -c <test.r) &&
...
			IN: clean test.r $(($S)) [OK] -- OUT: $(($S)) . [OK]
Would you agree?

Thanks, Lars

diff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh
index 9968465..4454587 100755
--- a/t/t0021-conversion.sh
+++ b/t/t0021-conversion.sh
@@ -23,10 +23,6 @@ generate_random_characters () {
 		perl -pe "s/./chr((ord($&) % 26) + ord('a'))/sge" >"$TEST_ROOT/$NAME"
 }
 
-file_size () {
-	wc -c <"$1"
-}
-
 filter_git () {
 	rm -f rot13-filter.log &&
 	git "$@" 2>git-stderr.log &&
@@ -363,9 +359,9 @@ test_expect_success PERL 'required process filter should filter data' '
 		cp "$TEST_ROOT/test3 '\''sq'\'',\$x.o" "testsubdir/test3 '\''sq'\'',\$x.r" &&
 		>test4-empty.r &&
 
-		S=$(file_size test.r) &&
-		S2=$(file_size test2.r) &&
-		S3=$(file_size "testsubdir/test3 '\''sq'\'',\$x.r") &&
+		S=$(wc -c <test.r) &&
+		S2=$(wc -c <test2.r) &&
+		S3=$(wc -c <"testsubdir/test3 '\''sq'\'',\$x.r") &&
 
 		filter_git add . &&
 		cat >expected.log <<-EOF &&
@@ -446,7 +442,7 @@ test_expect_success PERL 'required process filter takes precedence' '
 
 		echo "*.r filter=protocol" >.gitattributes &&
 		cp "$TEST_ROOT/test.o" test.r &&
-		S=$(file_size test.r) &&
+		S=$(wc -c <test.r) &&
 
 		# Check that the process filter is invoked here
 		filter_git add . &&
@@ -470,7 +466,7 @@ test_expect_success PERL 'required process filter should be used only for "clean
 
 		echo "*.r filter=protocol" >.gitattributes &&
 		cp "$TEST_ROOT/test.o" test.r &&
-		S=$(file_size test.r) &&
+		S=$(wc -c <test.r) &&
 
 		filter_git add . &&
 		cat >expected.log <<-EOF &&
@@ -589,9 +585,9 @@ test_expect_success PERL 'process filter should restart after unexpected write f
 		echo "this is going to fail" >smudge-write-fail.o &&
 		cp smudge-write-fail.o smudge-write-fail.r &&
 
-		S=$(file_size test.r) &&
-		S2=$(file_size test2.r) &&
-		SF=$(file_size smudge-write-fail.r) &&
+		S=$(wc -c <test.r) &&
+		S2=$(wc -c <test2.r) &&
+		SF=$(wc -c <smudge-write-fail.r) &&
 
 		git add . &&
 		rm -f *.r &&
@@ -640,9 +636,9 @@ test_expect_success PERL 'process filter should not be restarted if it signals a
 		echo "this will cause an error" >error.o &&
 		cp error.o error.r &&
 
-		S=$(file_size test.r) &&
-		S2=$(file_size test2.r) &&
-		SE=$(file_size error.r) &&
+		S=$(wc -c <test.r) &&
+		S2=$(wc -c <test2.r) &&
+		SE=$(wc -c <error.r) &&
 
 		git add . &&
 		rm -f *.r &&
@@ -679,7 +675,7 @@ test_expect_success PERL 'process filter abort stops processing of all further f
 		echo "error this blob and all future blobs" >abort.o &&
 		cp abort.o abort.r &&
 
-		SA=$(file_size abort.r) &&
+		SA=$(wc -c <abort.r) &&
 
 		git add . &&
 		rm -f *.r &&
Previous: Johannes SixtNext: Johannes Sixt
Message 19 of 25 in “Git filter protocol”
  1. 00/14 Git filter protocollarsxschneider@gmail.com, Oct 16, 2016
  2. 01/14 convert: quote filter names in error messageslarsxschneider@gmail.com, Oct 16, 2016
  3. 02/14 convert: modernize testslarsxschneider@gmail.com, Oct 16, 2016
  4. 03/14 run-command: move check_pipe() from write_or_die to run_commandlarsxschneider@gmail.com, Oct 16, 2016
  5. 04/14 run-command: add clean_on_exit_handlerlarsxschneider@gmail.com, Oct 16, 2016
  6. 05/14 pkt-line: rename packet_write() to packet_write_fmt()larsxschneider@gmail.com, Oct 16, 2016
  7. 08/14 pkt-line: add packet_flush_gently()larsxschneider@gmail.com, Oct 16, 2016
  8. 07/14 pkt-line: add packet_write_fmt_gently()larsxschneider@gmail.com, Oct 16, 2016
  9. 06/14 pkt-line: extract set_packet_header()larsxschneider@gmail.com, Oct 16, 2016
  10. 09/14 pkt-line: add packet_write_gently()larsxschneider@gmail.com, Oct 16, 2016
  11. 10/14 pkt-line: add functions to read/write flush terminated packet streamslarsxschneider@gmail.com, Oct 16, 2016
  12. 11/14 convert: make apply_filter() adhere to standard Git error handlinglarsxschneider@gmail.com, Oct 16, 2016
  13. 12/14 convert: prepare filter.<driver>.process optionlarsxschneider@gmail.com, Oct 16, 2016
  14. 13/14 convert: add filter.<driver>.process optionlarsxschneider@gmail.com, Oct 16, 2016
  15. Johannes SixtNov 2, 2016
  16. Lars SchneiderNov 3, 2016
  17. t0021: expect more variations in the output of uniq -cJohannes Sixt, Nov 3, 2016
  18. t0021: use arithmetic expansion to trim whitespace from wc -c outputJohannes Sixt, Nov 3, 2016
  19. Lars SchneiderNov 6, 2016
  20. Johannes SixtNov 6, 2016
  21. Lars SchneiderNov 6, 2016
  22. Lars SchneiderNov 6, 2016
  23. Johannes SixtNov 6, 2016
  24. 14/14 contrib/long-running-filter: add long running filter examplelarsxschneider@gmail.com, Oct 16, 2016
  25. Junio C HamanoOct 17, 2016

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.