> On 03 Nov 2016, at 21:22, Johannes Sixt <j...@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 <j...@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 &&

Reply via email to