* [PATCH 0/3] Reduce number of processes spawned by git-mergetool
@ 2019-06-10 8:58 Johannes Sixt
2019-06-10 8:58 ` [PATCH 1/3] t7610-mergetool: do not place pipelines headed by `yes` in subshells Johannes Sixt
` (3 more replies)
0 siblings, 4 replies; 18+ messages in thread
From: Johannes Sixt @ 2019-06-10 8:58 UTC (permalink / raw)
To: git; +Cc: Johannes Sixt
git-mergetool spawns an enormous amount of processes. For this reason,
the test script, t7610, is exceptionally slow, in particular, on
Windows. Most of the processes are invocations of git, but there are
also some that can be replaced with shell builtins.
I've measured the number of processes with
ps; ./t7610-mergetool.sh >/dev/null; ps
on a quiet system and the number of non-git commands invoked with
strace -e execve -f -o /tmp/git.strace ./t7610-mergetool.sh
grep 'execve(' /tmp/git.strace | grep -v 'execve(".home' | wc -l
baseline:
15628 pts/1 00:00:00 ps
29413 pts/1 00:00:00 ps
=> 13785 processes
=> 3082 non-git commands
t7610-mergetool: do not place pipelines headed by `yes` in subshells
12620 pts/1 00:00:00 ps
26348 pts/1 00:00:00 ps
=> 13728 processes
=> 3082 non-git commands
mergetool: dissect strings with shell variable magic instead of `expr`
8766 pts/1 00:00:00 ps
21913 pts/1 00:00:00 ps
=> 13147 processes
=> 2521 non-git commands
mergetool: use shell variable magic instead of `awk`
2081 pts/1 00:00:00 ps
14393 pts/1 00:00:00 ps
=> 12312 processes
=> 2007 non-git commands
The reduction of processes is not as dramatic as I hoped, but still
more than 10%.
Johannes Sixt (3):
t7610-mergetool: do not place pipelines headed by `yes` in subshells
mergetool: dissect strings with shell variable magic instead of `expr`
mergetool: use shell variable magic instead of `awk`
git-mergetool.sh | 45 ++++++++----
t/t7610-mergetool.sh | 170 +++++++++++++++++++++----------------------
2 files changed, 116 insertions(+), 99 deletions(-)
--
2.21.0.285.gc38d92e052
^ permalink raw reply [flat|nested] 18+ messages in thread* [PATCH 1/3] t7610-mergetool: do not place pipelines headed by `yes` in subshells 2019-06-10 8:58 [PATCH 0/3] Reduce number of processes spawned by git-mergetool Johannes Sixt @ 2019-06-10 8:58 ` Johannes Sixt 2019-06-10 9:59 ` SZEDER Gábor 2019-06-10 8:58 ` [PATCH 2/3] mergetool: dissect strings with shell variable magic instead of `expr` Johannes Sixt ` (2 subsequent siblings) 3 siblings, 1 reply; 18+ messages in thread From: Johannes Sixt @ 2019-06-10 8:58 UTC (permalink / raw) To: git; +Cc: Johannes Sixt Subshells for pipelines are not required. This can save a number of processes (if the shell does not optimize it away anyway). The patch was generated with the command sed -i 's/( *\(yes.*[^ ]\) *) *\&\&/\1 \&\&/' t7610-mergetool.sh with a manual fixup of the case having no && at the end. Signed-off-by: Johannes Sixt <j6t@kdbg.org> --- t/t7610-mergetool.sh | 170 +++++++++++++++++++++---------------------- 1 file changed, 85 insertions(+), 85 deletions(-) diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh index 5b61c10a9c..b67440882b 100755 --- a/t/t7610-mergetool.sh +++ b/t/t7610-mergetool.sh @@ -131,13 +131,13 @@ test_expect_success 'custom mergetool' ' git checkout -b test$test_count branch1 && git submodule update -N && test_must_fail git merge master && - ( yes "" | git mergetool both ) && - ( yes "" | git mergetool file1 file1 ) && - ( yes "" | git mergetool file2 "spaced name" ) && - ( yes "" | git mergetool subdir/file3 ) && - ( yes "d" | git mergetool file11 ) && - ( yes "d" | git mergetool file12 ) && - ( yes "l" | git mergetool submod ) && + yes "" | git mergetool both && + yes "" | git mergetool file1 file1 && + yes "" | git mergetool file2 "spaced name" && + yes "" | git mergetool subdir/file3 && + yes "d" | git mergetool file11 && + yes "d" | git mergetool file12 && + yes "l" | git mergetool submod && test "$(cat file1)" = "master updated" && test "$(cat file2)" = "master new" && test "$(cat subdir/file3)" = "master new sub" && @@ -153,13 +153,13 @@ test_expect_success 'gui mergetool' ' git checkout -b test$test_count branch1 && git submodule update -N && test_must_fail git merge master && - ( yes "" | git mergetool --gui both ) && - ( yes "" | git mergetool -g file1 file1 ) && - ( yes "" | git mergetool --gui file2 "spaced name" ) && - ( yes "" | git mergetool --gui subdir/file3 ) && - ( yes "d" | git mergetool --gui file11 ) && - ( yes "d" | git mergetool --gui file12 ) && - ( yes "l" | git mergetool --gui submod ) && + yes "" | git mergetool --gui both && + yes "" | git mergetool -g file1 file1 && + yes "" | git mergetool --gui file2 "spaced name" && + yes "" | git mergetool --gui subdir/file3 && + yes "d" | git mergetool --gui file11 && + yes "d" | git mergetool --gui file12 && + yes "l" | git mergetool --gui submod && test "$(cat file1)" = "gui master updated" && test "$(cat file2)" = "gui master new" && test "$(cat subdir/file3)" = "gui master new sub" && @@ -172,13 +172,13 @@ test_expect_success 'gui mergetool without merge.guitool set falls back to merge git checkout -b test$test_count branch1 && git submodule update -N && test_must_fail git merge master && - ( yes "" | git mergetool --gui both ) && - ( yes "" | git mergetool -g file1 file1 ) && - ( yes "" | git mergetool --gui file2 "spaced name" ) && - ( yes "" | git mergetool --gui subdir/file3 ) && - ( yes "d" | git mergetool --gui file11 ) && - ( yes "d" | git mergetool --gui file12 ) && - ( yes "l" | git mergetool --gui submod ) && + yes "" | git mergetool --gui both && + yes "" | git mergetool -g file1 file1 && + yes "" | git mergetool --gui file2 "spaced name" && + yes "" | git mergetool --gui subdir/file3 && + yes "d" | git mergetool --gui file11 && + yes "d" | git mergetool --gui file12 && + yes "l" | git mergetool --gui submod && test "$(cat file1)" = "master updated" && test "$(cat file2)" = "master new" && test "$(cat subdir/file3)" = "master new sub" && @@ -195,14 +195,14 @@ test_expect_success 'mergetool crlf' ' test_config core.autocrlf true && git checkout -b test$test_count branch1 && test_must_fail git merge master && - ( yes "" | git mergetool file1 ) && - ( yes "" | git mergetool file2 ) && - ( yes "" | git mergetool "spaced name" ) && - ( yes "" | git mergetool both ) && - ( yes "" | git mergetool subdir/file3 ) && - ( yes "d" | git mergetool file11 ) && - ( yes "d" | git mergetool file12 ) && - ( yes "r" | git mergetool submod ) && + yes "" | git mergetool file1 && + yes "" | git mergetool file2 && + yes "" | git mergetool "spaced name" && + yes "" | git mergetool both && + yes "" | git mergetool subdir/file3 && + yes "d" | git mergetool file11 && + yes "d" | git mergetool file12 && + yes "r" | git mergetool submod && test "$(printf x | cat file1 -)" = "$(printf "master updated\r\nx")" && test "$(printf x | cat file2 -)" = "$(printf "master new\r\nx")" && test "$(printf x | cat subdir/file3 -)" = "$(printf "master new sub\r\nx")" && @@ -218,7 +218,7 @@ test_expect_success 'mergetool in subdir' ' ( cd subdir && test_must_fail git merge master && - ( yes "" | git mergetool file3 ) && + yes "" | git mergetool file3 && test "$(cat file3)" = "master new sub" ) ' @@ -230,13 +230,13 @@ test_expect_success 'mergetool on file in parent dir' ' ( cd subdir && test_must_fail git merge master && - ( yes "" | git mergetool file3 ) && - ( yes "" | git mergetool ../file1 ) && - ( yes "" | git mergetool ../file2 ../spaced\ name ) && - ( yes "" | git mergetool ../both ) && - ( yes "d" | git mergetool ../file11 ) && - ( yes "d" | git mergetool ../file12 ) && - ( yes "l" | git mergetool ../submod ) && + yes "" | git mergetool file3 && + yes "" | git mergetool ../file1 && + yes "" | git mergetool ../file2 ../spaced\ name && + yes "" | git mergetool ../both && + yes "d" | git mergetool ../file11 && + yes "d" | git mergetool ../file12 && + yes "l" | git mergetool ../submod && test "$(cat ../file1)" = "master updated" && test "$(cat ../file2)" = "master new" && test "$(cat ../submod/bar)" = "branch1 submodule" && @@ -250,9 +250,9 @@ test_expect_success 'mergetool skips autoresolved' ' git submodule update -N && test_must_fail git merge master && test -n "$(git ls-files -u)" && - ( yes "d" | git mergetool file11 ) && - ( yes "d" | git mergetool file12 ) && - ( yes "l" | git mergetool submod ) && + yes "d" | git mergetool file11 && + yes "d" | git mergetool file12 && + yes "l" | git mergetool submod && output="$(git mergetool --no-prompt)" && test "$output" = "No files need merging" ' @@ -264,8 +264,8 @@ test_expect_success 'mergetool merges all from subdir (rerere disabled)' ' ( cd subdir && test_must_fail git merge master && - ( yes "r" | git mergetool ../submod ) && - ( yes "d" "d" | git mergetool --no-prompt ) && + yes "r" | git mergetool ../submod && + yes "d" "d" | git mergetool --no-prompt && test "$(cat ../file1)" = "master updated" && test "$(cat ../file2)" = "master new" && test "$(cat file3)" = "master new sub" && @@ -283,8 +283,8 @@ test_expect_success 'mergetool merges all from subdir (rerere enabled)' ' ( cd subdir && test_must_fail git merge master && - ( yes "r" | git mergetool ../submod ) && - ( yes "d" "d" | git mergetool --no-prompt ) && + yes "r" | git mergetool ../submod && + yes "d" "d" | git mergetool --no-prompt && test "$(cat ../file1)" = "master updated" && test "$(cat ../file2)" = "master new" && test "$(cat file3)" = "master new sub" && @@ -301,8 +301,8 @@ test_expect_success 'mergetool skips resolved paths when rerere is active' ' git checkout -b test$test_count branch1 && git submodule update -N && test_must_fail git merge master && - ( yes "l" | git mergetool --no-prompt submod ) && - ( yes "d" "d" | git mergetool --no-prompt ) && + yes "l" | git mergetool --no-prompt submod && + yes "d" "d" | git mergetool --no-prompt && git submodule update -N && output="$(yes "n" | git mergetool --no-prompt)" && test "$output" = "No files need merging" @@ -343,7 +343,7 @@ test_expect_success 'mergetool takes partial path' ' git submodule update -N && test_must_fail git merge master && - ( yes "" | git mergetool subdir ) && + yes "" | git mergetool subdir && test "$(cat subdir/file3)" = "master new sub" ' @@ -410,10 +410,10 @@ test_expect_success 'deleted vs modified submodule' ' git checkout -b test$test_count.a test$test_count && test_must_fail git merge master && test -n "$(git ls-files -u)" && - ( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 ) && - ( yes "" | git mergetool both ) && - ( yes "d" | git mergetool file11 file12 ) && - ( yes "r" | git mergetool submod ) && + yes "" | git mergetool file1 file2 spaced\ name subdir/file3 && + yes "" | git mergetool both && + yes "d" | git mergetool file11 file12 && + yes "r" | git mergetool submod && rmdir submod && mv submod-movedaside submod && test "$(cat submod/bar)" = "branch1 submodule" && git submodule update -N && @@ -427,10 +427,10 @@ test_expect_success 'deleted vs modified submodule' ' git submodule update -N && test_must_fail git merge master && test -n "$(git ls-files -u)" && - ( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 ) && - ( yes "" | git mergetool both ) && - ( yes "d" | git mergetool file11 file12 ) && - ( yes "l" | git mergetool submod ) && + yes "" | git mergetool file1 file2 spaced\ name subdir/file3 && + yes "" | git mergetool both && + yes "d" | git mergetool file11 file12 && + yes "l" | git mergetool submod && test ! -e submod && output="$(git mergetool --no-prompt)" && test "$output" = "No files need merging" && @@ -441,10 +441,10 @@ test_expect_success 'deleted vs modified submodule' ' git submodule update -N && test_must_fail git merge test$test_count && test -n "$(git ls-files -u)" && - ( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 ) && - ( yes "" | git mergetool both ) && - ( yes "d" | git mergetool file11 file12 ) && - ( yes "r" | git mergetool submod ) && + yes "" | git mergetool file1 file2 spaced\ name subdir/file3 && + yes "" | git mergetool both && + yes "d" | git mergetool file11 file12 && + yes "r" | git mergetool submod && test ! -e submod && test -d submod.orig && git submodule update -N && @@ -457,10 +457,10 @@ test_expect_success 'deleted vs modified submodule' ' git submodule update -N && test_must_fail git merge test$test_count && test -n "$(git ls-files -u)" && - ( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 ) && - ( yes "" | git mergetool both ) && - ( yes "d" | git mergetool file11 file12 ) && - ( yes "l" | git mergetool submod ) && + yes "" | git mergetool file1 file2 spaced\ name subdir/file3 && + yes "" | git mergetool both && + yes "d" | git mergetool file11 file12 && + yes "l" | git mergetool submod && test "$(cat submod/bar)" = "master submodule" && git submodule update -N && test "$(cat submod/bar)" = "master submodule" && @@ -481,10 +481,10 @@ test_expect_success 'file vs modified submodule' ' git checkout -b test$test_count.a branch1 && test_must_fail git merge master && test -n "$(git ls-files -u)" && - ( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 ) && - ( yes "" | git mergetool both ) && - ( yes "d" | git mergetool file11 file12 ) && - ( yes "r" | git mergetool submod ) && + yes "" | git mergetool file1 file2 spaced\ name subdir/file3 && + yes "" | git mergetool both && + yes "d" | git mergetool file11 file12 && + yes "r" | git mergetool submod && rmdir submod && mv submod-movedaside submod && test "$(cat submod/bar)" = "branch1 submodule" && git submodule update -N && @@ -497,10 +497,10 @@ test_expect_success 'file vs modified submodule' ' git checkout -b test$test_count.b test$test_count && test_must_fail git merge master && test -n "$(git ls-files -u)" && - ( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 ) && - ( yes "" | git mergetool both ) && - ( yes "d" | git mergetool file11 file12 ) && - ( yes "l" | git mergetool submod ) && + yes "" | git mergetool file1 file2 spaced\ name subdir/file3 && + yes "" | git mergetool both && + yes "d" | git mergetool file11 file12 && + yes "l" | git mergetool submod && git submodule update -N && test "$(cat submod)" = "not a submodule" && output="$(git mergetool --no-prompt)" && @@ -513,10 +513,10 @@ test_expect_success 'file vs modified submodule' ' git submodule update -N && test_must_fail git merge test$test_count && test -n "$(git ls-files -u)" && - ( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 ) && - ( yes "" | git mergetool both ) && - ( yes "d" | git mergetool file11 file12 ) && - ( yes "r" | git mergetool submod ) && + yes "" | git mergetool file1 file2 spaced\ name subdir/file3 && + yes "" | git mergetool both && + yes "d" | git mergetool file11 file12 && + yes "r" | git mergetool submod && test -d submod.orig && git submodule update -N && test "$(cat submod)" = "not a submodule" && @@ -529,10 +529,10 @@ test_expect_success 'file vs modified submodule' ' git submodule update -N && test_must_fail git merge test$test_count && test -n "$(git ls-files -u)" && - ( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 ) && - ( yes "" | git mergetool both ) && - ( yes "d" | git mergetool file11 file12 ) && - ( yes "l" | git mergetool submod ) && + yes "" | git mergetool file1 file2 spaced\ name subdir/file3 && + yes "" | git mergetool both && + yes "d" | git mergetool file11 file12 && + yes "l" | git mergetool submod && test "$(cat submod/bar)" = "master submodule" && git submodule update -N && test "$(cat submod/bar)" = "master submodule" && @@ -587,7 +587,7 @@ test_expect_success 'submodule in subdirectory' ' test_must_fail git merge test$test_count.a && ( cd subdir && - ( yes "l" | git mergetool subdir_module ) + yes "l" | git mergetool subdir_module ) && test "$(cat subdir/subdir_module/file15)" = "test$test_count.b" && git submodule update -N && @@ -596,7 +596,7 @@ test_expect_success 'submodule in subdirectory' ' git submodule update -N && test_must_fail git merge test$test_count.a && - ( yes "r" | git mergetool subdir/subdir_module ) && + yes "r" | git mergetool subdir/subdir_module && test "$(cat subdir/subdir_module/file15)" = "test$test_count.b" && git submodule update -N && test "$(cat subdir/subdir_module/file15)" = "test$test_count.a" && @@ -615,7 +615,7 @@ test_expect_success 'directory vs modified submodule' ' test_must_fail git merge master && test -n "$(git ls-files -u)" && - ( yes "l" | git mergetool submod ) && + yes "l" | git mergetool submod && test "$(cat submod/file16)" = "not a submodule" && rm -rf submod.orig && @@ -623,7 +623,7 @@ test_expect_success 'directory vs modified submodule' ' test_must_fail git merge master && test -n "$(git ls-files -u)" && test ! -e submod.orig && - ( yes "r" | git mergetool submod ) && + yes "r" | git mergetool submod && test -d submod.orig && test "$(cat submod.orig/file16)" = "not a submodule" && rm -r submod.orig && @@ -638,7 +638,7 @@ test_expect_success 'directory vs modified submodule' ' git submodule update -N && test_must_fail git merge test$test_count && test -n "$(git ls-files -u)" && - ( yes "l" | git mergetool submod ) && + yes "l" | git mergetool submod && git submodule update -N && test "$(cat submod/bar)" = "master submodule" && @@ -647,7 +647,7 @@ test_expect_success 'directory vs modified submodule' ' test_must_fail git merge test$test_count && test -n "$(git ls-files -u)" && test ! -e submod.orig && - ( yes "r" | git mergetool submod ) && + yes "r" | git mergetool submod && test "$(cat submod/file16)" = "not a submodule" && git reset --hard master && -- 2.21.0.285.gc38d92e052 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 1/3] t7610-mergetool: do not place pipelines headed by `yes` in subshells 2019-06-10 8:58 ` [PATCH 1/3] t7610-mergetool: do not place pipelines headed by `yes` in subshells Johannes Sixt @ 2019-06-10 9:59 ` SZEDER Gábor 2019-06-10 17:23 ` Junio C Hamano 2019-06-10 18:29 ` Johannes Schindelin 0 siblings, 2 replies; 18+ messages in thread From: SZEDER Gábor @ 2019-06-10 9:59 UTC (permalink / raw) To: Johannes Sixt; +Cc: git On Mon, Jun 10, 2019 at 10:58:58AM +0200, Johannes Sixt wrote: > Subshells for pipelines are not required. This can save a number of > processes (if the shell does not optimize it away anyway). > > The patch was generated with the command > > sed -i 's/( *\(yes.*[^ ]\) *) *\&\&/\1 \&\&/' t7610-mergetool.sh > > with a manual fixup of the case having no && at the end. I think it would be great to include the corresponding numbers from the cover letter in each of the commit messages. > Signed-off-by: Johannes Sixt <j6t@kdbg.org> > --- > t/t7610-mergetool.sh | 170 +++++++++++++++++++++---------------------- > 1 file changed, 85 insertions(+), 85 deletions(-) > > diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh > index 5b61c10a9c..b67440882b 100755 > --- a/t/t7610-mergetool.sh > +++ b/t/t7610-mergetool.sh > @@ -131,13 +131,13 @@ test_expect_success 'custom mergetool' ' > git checkout -b test$test_count branch1 && > git submodule update -N && > test_must_fail git merge master && > - ( yes "" | git mergetool both ) && > - ( yes "" | git mergetool file1 file1 ) && > - ( yes "" | git mergetool file2 "spaced name" ) && > - ( yes "" | git mergetool subdir/file3 ) && > - ( yes "d" | git mergetool file11 ) && > - ( yes "d" | git mergetool file12 ) && > - ( yes "l" | git mergetool submod ) && > + yes "" | git mergetool both && > + yes "" | git mergetool file1 file1 && > + yes "" | git mergetool file2 "spaced name" && > + yes "" | git mergetool subdir/file3 && > + yes "d" | git mergetool file11 && > + yes "d" | git mergetool file12 && > + yes "l" | git mergetool submod && > test "$(cat file1)" = "master updated" && > test "$(cat file2)" = "master new" && > test "$(cat subdir/file3)" = "master new sub" && Another possibility for eliminating a few more subshells might be to turn these test "$(cat file1)" = "that"' checks into echo that >expect && test_cmp expect file1 because 'test_cmp' on Windows first compares the two files in shell and runs 'diff' only when there is a difference to report. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/3] t7610-mergetool: do not place pipelines headed by `yes` in subshells 2019-06-10 9:59 ` SZEDER Gábor @ 2019-06-10 17:23 ` Junio C Hamano 2019-06-10 17:56 ` SZEDER Gábor 2019-06-10 18:29 ` Johannes Schindelin 1 sibling, 1 reply; 18+ messages in thread From: Junio C Hamano @ 2019-06-10 17:23 UTC (permalink / raw) To: SZEDER Gábor; +Cc: Johannes Sixt, git SZEDER Gábor <szeder.dev@gmail.com> writes: > turn these > > test "$(cat file1)" = "that"' > > checks into > > echo that >expect && > test_cmp expect file1 > > because 'test_cmp' on Windows first compares the two files in shell > and runs 'diff' only when there is a difference to report. Needs measuring. Is two extra file I/Os that inexpensive? ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/3] t7610-mergetool: do not place pipelines headed by `yes` in subshells 2019-06-10 17:23 ` Junio C Hamano @ 2019-06-10 17:56 ` SZEDER Gábor 0 siblings, 0 replies; 18+ messages in thread From: SZEDER Gábor @ 2019-06-10 17:56 UTC (permalink / raw) To: Junio C Hamano; +Cc: Johannes Sixt, git On Mon, Jun 10, 2019 at 10:23:57AM -0700, Junio C Hamano wrote: > SZEDER Gábor <szeder.dev@gmail.com> writes: > > > turn these > > > > test "$(cat file1)" = "that"' > > > > checks into > > > > echo that >expect && > > test_cmp expect file1 > > > > because 'test_cmp' on Windows first compares the two files in shell > > and runs 'diff' only when there is a difference to report. > > Needs measuring. Is two extra file I/Os that inexpensive? Compared to subshells and external processes, yes. I run this on Linux: --- >8 --- #!/bin/sh test_description='test' . ./lib-bash.sh test_expect_success 'test' ' echo "master updated" >file1 && echo "original:" && time for i in $(seq 1000) do test "$(cat file1)" = "master updated" done && echo "using test_cmp:" && time for i in $(seq 1000) do echo "master updated" >expect && test_cmp expect file1 done && echo "using mingw_test_cmp:" && time for i in $(seq 1000) do echo "master updated" >expect && mingw_test_cmp expect file1 done ' test_done --- >8 --- And it produced: original: real 0m1.888s user 0m1.491s sys 0m0.532s using test_cmp: real 0m1.233s user 0m0.877s sys 0m0.432s using mingw_test_cmp: real 0m0.344s user 0m0.298s sys 0m0.026s ok 1 - test It's faster even on Linux, so it should be much faster on Windows, where both external commands and subshells have much higher overhead. However, there are only about 50 "$(cat ...)" in t7610 that can be eliminated this way, which is not all that much compared to the several thousand processes shown in the cover letter... Anyway, there is still the better developer experience in case of one of these checks were to fail without '-x' tracing... :) ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/3] t7610-mergetool: do not place pipelines headed by `yes` in subshells 2019-06-10 9:59 ` SZEDER Gábor 2019-06-10 17:23 ` Junio C Hamano @ 2019-06-10 18:29 ` Johannes Schindelin 2019-06-10 18:57 ` SZEDER Gábor 1 sibling, 1 reply; 18+ messages in thread From: Johannes Schindelin @ 2019-06-10 18:29 UTC (permalink / raw) To: SZEDER Gábor; +Cc: Johannes Sixt, git [-- Attachment #1: Type: text/plain, Size: 2147 bytes --] Hi, On Mon, 10 Jun 2019, SZEDER Gábor wrote: > > diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh > > index 5b61c10a9c..b67440882b 100755 > > --- a/t/t7610-mergetool.sh > > +++ b/t/t7610-mergetool.sh > > @@ -131,13 +131,13 @@ test_expect_success 'custom mergetool' ' > > git checkout -b test$test_count branch1 && > > git submodule update -N && > > test_must_fail git merge master && > > - ( yes "" | git mergetool both ) && > > - ( yes "" | git mergetool file1 file1 ) && > > - ( yes "" | git mergetool file2 "spaced name" ) && > > - ( yes "" | git mergetool subdir/file3 ) && > > - ( yes "d" | git mergetool file11 ) && > > - ( yes "d" | git mergetool file12 ) && > > - ( yes "l" | git mergetool submod ) && > > + yes "" | git mergetool both && > > + yes "" | git mergetool file1 file1 && > > + yes "" | git mergetool file2 "spaced name" && > > + yes "" | git mergetool subdir/file3 && > > + yes "d" | git mergetool file11 && > > + yes "d" | git mergetool file12 && > > + yes "l" | git mergetool submod && > > test "$(cat file1)" = "master updated" && > > test "$(cat file2)" = "master new" && > > test "$(cat subdir/file3)" = "master new sub" && > > Another possibility for eliminating a few more subshells might be to > turn these > > test "$(cat file1)" = "that"' > > checks into > > echo that >expect && > test_cmp expect file1 > > because 'test_cmp' on Windows first compares the two files in shell > and runs 'diff' only when there is a difference to report. When you remember that spawning processes is much more expensive on Windows, still, than I/O, you will realize that this adds even more expense. Instead of a spawn & read, you are suggesting essentially a write, spawn, read & read, and that is only the best case. In the worst case, it would be a write, spawn, read & read, spawn, read & read. (Even if the first spawn is an MSYS2 spawn on Windows, which is more expensive than the MINGW spawn for the `git diff`, if that is a `git diff` rather than `diff`, didn't check...) So I am rather negative about this suggestion ;-) Ciao, Dscho ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/3] t7610-mergetool: do not place pipelines headed by `yes` in subshells 2019-06-10 18:29 ` Johannes Schindelin @ 2019-06-10 18:57 ` SZEDER Gábor 0 siblings, 0 replies; 18+ messages in thread From: SZEDER Gábor @ 2019-06-10 18:57 UTC (permalink / raw) To: Johannes Schindelin; +Cc: Johannes Sixt, git On Mon, Jun 10, 2019 at 08:29:56PM +0200, Johannes Schindelin wrote: > Hi, > > On Mon, 10 Jun 2019, SZEDER Gábor wrote: > > > > diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh > > > index 5b61c10a9c..b67440882b 100755 > > > --- a/t/t7610-mergetool.sh > > > +++ b/t/t7610-mergetool.sh > > > @@ -131,13 +131,13 @@ test_expect_success 'custom mergetool' ' > > > git checkout -b test$test_count branch1 && > > > git submodule update -N && > > > test_must_fail git merge master && > > > - ( yes "" | git mergetool both ) && > > > - ( yes "" | git mergetool file1 file1 ) && > > > - ( yes "" | git mergetool file2 "spaced name" ) && > > > - ( yes "" | git mergetool subdir/file3 ) && > > > - ( yes "d" | git mergetool file11 ) && > > > - ( yes "d" | git mergetool file12 ) && > > > - ( yes "l" | git mergetool submod ) && > > > + yes "" | git mergetool both && > > > + yes "" | git mergetool file1 file1 && > > > + yes "" | git mergetool file2 "spaced name" && > > > + yes "" | git mergetool subdir/file3 && > > > + yes "d" | git mergetool file11 && > > > + yes "d" | git mergetool file12 && > > > + yes "l" | git mergetool submod && > > > test "$(cat file1)" = "master updated" && > > > test "$(cat file2)" = "master new" && > > > test "$(cat subdir/file3)" = "master new sub" && > > > > Another possibility for eliminating a few more subshells might be to > > turn these > > > > test "$(cat file1)" = "that"' > > > > checks into > > > > echo that >expect && > > test_cmp expect file1 > > > > because 'test_cmp' on Windows first compares the two files in shell > > and runs 'diff' only when there is a difference to report. > > When you remember that spawning processes is much more expensive on > Windows, still, than I/O, you will realize that this adds even more > expense. Instead of a spawn & read, you are suggesting essentially a > write, spawn, read & read, and that is only the best case. No, instead of a spawn (the subshell of the command substitution), spawn ('cat'), read, I suggest a write, read, read (no subshell or external process in 'mingw_test_cmp's main code path). > In the worst case, it would be a write, spawn, read & read, spawn, read & > read. It would be a write, read, read, only one spawn (diff), read read, but I assume that the tests succeed, so I ignore this worst case. > (Even if the first spawn is an MSYS2 spawn on Windows, which is more > expensive than the MINGW spawn for the `git diff`, if that is a `git diff` > rather than `diff`, didn't check...) > > So I am rather negative about this suggestion ;-) > > Ciao, > Dscho ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 2/3] mergetool: dissect strings with shell variable magic instead of `expr` 2019-06-10 8:58 [PATCH 0/3] Reduce number of processes spawned by git-mergetool Johannes Sixt 2019-06-10 8:58 ` [PATCH 1/3] t7610-mergetool: do not place pipelines headed by `yes` in subshells Johannes Sixt @ 2019-06-10 8:58 ` Johannes Sixt 2019-06-10 17:17 ` Junio C Hamano 2019-06-10 8:59 ` [PATCH 3/3] mergetool: use shell variable magic instead of `awk` Johannes Sixt 2019-06-12 16:33 ` [PATCH v2 0/4] Reduce number of processes spawned by git-mergetool Johannes Sixt 3 siblings, 1 reply; 18+ messages in thread From: Johannes Sixt @ 2019-06-10 8:58 UTC (permalink / raw) To: git; +Cc: Johannes Sixt git-mergetool spawns an enormous amount of processes. For this reason, the test script, t7610, is exceptionally slow, in particular, on Windows. Most of the processes are invocations of git, but there are also some that can be replaced with shell builtins. Do so with `expr`. Signed-off-by: Johannes Sixt <j6t@kdbg.org> --- git-mergetool.sh | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/git-mergetool.sh b/git-mergetool.sh index 88fa6a914a..8a937f680f 100755 --- a/git-mergetool.sh +++ b/git-mergetool.sh @@ -228,9 +228,8 @@ stage_submodule () { } checkout_staged_file () { - tmpfile=$(expr \ - "$(git checkout-index --temp --stage="$1" "$2" 2>/dev/null)" \ - : '\([^ ]*\) ') + tmpfile="$(git checkout-index --temp --stage="$1" "$2" 2>/dev/null)" && + tmpfile=${tmpfile%%' '*} if test $? -eq 0 && test -n "$tmpfile" then @@ -255,13 +254,16 @@ merge_file () { return 1 fi - if BASE=$(expr "$MERGED" : '\(.*\)\.[^/]*$') - then - ext=$(expr "$MERGED" : '.*\(\.[^/]*\)$') - else + # extract file extension from the last path component + case "${MERGED##*/}" in + *.*) + ext=.${MERGED##*.} + BASE=${MERGED%"$ext"} + ;; + *) BASE=$MERGED ext= - fi + esac mergetool_tmpdir_init @@ -406,7 +408,7 @@ main () { -t|--tool*) case "$#,$1" in *,*=*) - merge_tool=$(expr "z$1" : 'z-[^=]*=\(.*\)') + merge_tool=${1#*=} ;; 1,*) usage ;; -- 2.21.0.285.gc38d92e052 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 2/3] mergetool: dissect strings with shell variable magic instead of `expr` 2019-06-10 8:58 ` [PATCH 2/3] mergetool: dissect strings with shell variable magic instead of `expr` Johannes Sixt @ 2019-06-10 17:17 ` Junio C Hamano 2019-06-10 21:34 ` Johannes Sixt 0 siblings, 1 reply; 18+ messages in thread From: Junio C Hamano @ 2019-06-10 17:17 UTC (permalink / raw) To: Johannes Sixt; +Cc: git Johannes Sixt <j6t@kdbg.org> writes: > git-mergetool spawns an enormous amount of processes. For this reason, > the test script, t7610, is exceptionally slow, in particular, on > Windows. Most of the processes are invocations of git, but there are > also some that can be replaced with shell builtins. Do so with `expr`. I see these as improvements independent of whatever test may or may not be slow ;-) s/^.*/but there are/There are/. Thanks for working on it. > checkout_staged_file () { > - tmpfile=$(expr \ > - "$(git checkout-index --temp --stage="$1" "$2" 2>/dev/null)" \ > - : '\([^ ]*\) ') So this wants to grab leading non-HT substring that comes before an HT; we are trying to grab the name of the temorary picked by the checkout-index command, the output is ".merge_file_XXXXXX" followed by HT followed by the original filename "$2". > + tmpfile="$(git checkout-index --temp --stage="$1" "$2" 2>/dev/null)" && > + tmpfile=${tmpfile%%' '*} And this obviously is an equivalent, at least in the successful case. The ".merge_file_XXXXXX" temporary filename never has HT in it, and we are stripping everything after the first HT. And this rewrite makes the error behaviour much better. In the original, the exit code checked in the next "if test $? -eq 0" is that of "expr" (i.e. does the pattern match?); with this version, we are looking at the exit status of the checkout-index command. Good. > @@ -255,13 +254,16 @@ merge_file () { > return 1 > fi > > - if BASE=$(expr "$MERGED" : '\(.*\)\.[^/]*$') > - then > - ext=$(expr "$MERGED" : '.*\(\.[^/]*\)$') > - else > + # extract file extension from the last path component > + case "${MERGED##*/}" in > + *.*) > + ext=.${MERGED##*.} > + BASE=${MERGED%"$ext"} This rewrite can potentially change the behaviour, when $ext has glob metacharacters. Wouldn't BASE=${MERGED%.*} be more faithful conversion? > + ;; > + *) > BASE=$MERGED > ext= > - fi > + esac > @@ -406,7 +408,7 @@ main () { > -t|--tool*) > case "$#,$1" in > *,*=*) > - merge_tool=$(expr "z$1" : 'z-[^=]*=\(.*\)') > + merge_tool=${1#*=} OK, we strip leading substring before the first '=' out of "$1" and the case/esac ensures that there is such an equal '=' sign in "$1", so the rewrite is correct. Looks good. Thanks. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/3] mergetool: dissect strings with shell variable magic instead of `expr` 2019-06-10 17:17 ` Junio C Hamano @ 2019-06-10 21:34 ` Johannes Sixt 0 siblings, 0 replies; 18+ messages in thread From: Johannes Sixt @ 2019-06-10 21:34 UTC (permalink / raw) To: Junio C Hamano; +Cc: git Am 10.06.19 um 19:17 schrieb Junio C Hamano: > Johannes Sixt <j6t@kdbg.org> writes: >> git-mergetool spawns an enormous amount of processes. For this reason, >> the test script, t7610, is exceptionally slow, in particular, on >> Windows. Most of the processes are invocations of git, but there are >> also some that can be replaced with shell builtins. Do so with `expr`. > > I see these as improvements independent of whatever test may or may > not be slow ;-) s/^.*/but there are/There are/. Thanks for working > on it. Noted. >> @@ -255,13 +254,16 @@ merge_file () { >> return 1 >> fi >> >> - if BASE=$(expr "$MERGED" : '\(.*\)\.[^/]*$') >> - then >> - ext=$(expr "$MERGED" : '.*\(\.[^/]*\)$') >> - else >> + # extract file extension from the last path component >> + case "${MERGED##*/}" in >> + *.*) >> + ext=.${MERGED##*.} >> + BASE=${MERGED%"$ext"} > > This rewrite can potentially change the behaviour, when $ext has > glob metacharacters. Wouldn't BASE=${MERGED%.*} be more faithful > conversion? Since "$ext" is quoted inside the braces of the parameter expansion, the pattern counts as quoted, so all glob characters in $ext lose their special meaning. At least that's how I read the spec. I do see the symmetry in your proposed version. Nevertheless, I have a slight preference for my version because it specifies exactly what is to be removed from the end of value. -- Hannes ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 3/3] mergetool: use shell variable magic instead of `awk` 2019-06-10 8:58 [PATCH 0/3] Reduce number of processes spawned by git-mergetool Johannes Sixt 2019-06-10 8:58 ` [PATCH 1/3] t7610-mergetool: do not place pipelines headed by `yes` in subshells Johannes Sixt 2019-06-10 8:58 ` [PATCH 2/3] mergetool: dissect strings with shell variable magic instead of `expr` Johannes Sixt @ 2019-06-10 8:59 ` Johannes Sixt 2019-06-10 17:21 ` Junio C Hamano 2019-06-12 16:33 ` [PATCH v2 0/4] Reduce number of processes spawned by git-mergetool Johannes Sixt 3 siblings, 1 reply; 18+ messages in thread From: Johannes Sixt @ 2019-06-10 8:59 UTC (permalink / raw) To: git; +Cc: Johannes Sixt git-mergetool spawns an enormous amount of processes. For this reason, the test script, t7610, is exceptionally slow, in particular, on Windows. Most of the processes are invocations of git, but there are also some that can be replaced with shell builtins. Avoid repeated calls of `git ls-files` and `awk`. Signed-off-by: Johannes Sixt <j6t@kdbg.org> --- git-mergetool.sh | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/git-mergetool.sh b/git-mergetool.sh index 8a937f680f..e3f6d543fb 100755 --- a/git-mergetool.sh +++ b/git-mergetool.sh @@ -279,15 +279,30 @@ merge_file () { REMOTE="$MERGETOOL_TMPDIR/${BASE}_REMOTE_$$$ext" BASE="$MERGETOOL_TMPDIR/${BASE}_BASE_$$$ext" - base_mode=$(git ls-files -u -- "$MERGED" | awk '{if ($3==1) print $1;}') - local_mode=$(git ls-files -u -- "$MERGED" | awk '{if ($3==2) print $1;}') - remote_mode=$(git ls-files -u -- "$MERGED" | awk '{if ($3==3) print $1;}') + base_mode= local_mode= remote_mode= + + # here, $IFS is just a LF + for line in $f + do + mode=${line%% *} # 1st word + sha1=${line#"$mode "} + sha1=${sha1%% *} # 2nd word + case "${line#$mode $sha1 }" in # remainder + '1 '*) + base_mode=$mode + ;; + '2 '*) + local_mode=$mode local_sha1=$sha1 + ;; + '3 '*) + remote_mode=$mode remote_sha1=$sha1 + ;; + esac + done if is_submodule "$local_mode" || is_submodule "$remote_mode" then echo "Submodule merge conflict for '$MERGED':" - local_sha1=$(git ls-files -u -- "$MERGED" | awk '{if ($3==2) print $2;}') - remote_sha1=$(git ls-files -u -- "$MERGED" | awk '{if ($3==3) print $2;}') describe_file "$local_mode" "local" "$local_sha1" describe_file "$remote_mode" "remote" "$remote_sha1" resolve_submodule_merge -- 2.21.0.285.gc38d92e052 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 3/3] mergetool: use shell variable magic instead of `awk` 2019-06-10 8:59 ` [PATCH 3/3] mergetool: use shell variable magic instead of `awk` Johannes Sixt @ 2019-06-10 17:21 ` Junio C Hamano 2019-06-10 22:01 ` Johannes Sixt 0 siblings, 1 reply; 18+ messages in thread From: Junio C Hamano @ 2019-06-10 17:21 UTC (permalink / raw) To: Johannes Sixt; +Cc: git Johannes Sixt <j6t@kdbg.org> writes: > + # here, $IFS is just a LF > + for line in $f > + do > + mode=${line%% *} # 1st word > + sha1=${line#"$mode "} > + sha1=${sha1%% *} # 2nd word > + case "${line#$mode $sha1 }" in # remainder > + '1 '*) > + base_mode=$mode > + ;; > + '2 '*) > + local_mode=$mode local_sha1=$sha1 > + ;; > + '3 '*) > + remote_mode=$mode remote_sha1=$sha1 > + ;; > + esac > + done OK. $mode won't have any glob metacharacter, and there is only one invocation of "ls-files -u", which is now two fewer processes ;-) > > if is_submodule "$local_mode" || is_submodule "$remote_mode" > then > echo "Submodule merge conflict for '$MERGED':" > - local_sha1=$(git ls-files -u -- "$MERGED" | awk '{if ($3==2) print $2;}') > - remote_sha1=$(git ls-files -u -- "$MERGED" | awk '{if ($3==3) print $2;}') > describe_file "$local_mode" "local" "$local_sha1" > describe_file "$remote_mode" "remote" "$remote_sha1" > resolve_submodule_merge ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 3/3] mergetool: use shell variable magic instead of `awk` 2019-06-10 17:21 ` Junio C Hamano @ 2019-06-10 22:01 ` Johannes Sixt 0 siblings, 0 replies; 18+ messages in thread From: Johannes Sixt @ 2019-06-10 22:01 UTC (permalink / raw) To: Junio C Hamano; +Cc: git Am 10.06.19 um 19:21 schrieb Junio C Hamano: > Johannes Sixt <j6t@kdbg.org> writes: > >> + # here, $IFS is just a LF >> + for line in $f >> + do >> + mode=${line%% *} # 1st word >> + sha1=${line#"$mode "} >> + sha1=${sha1%% *} # 2nd word >> + case "${line#$mode $sha1 }" in # remainder >> + '1 '*) >> + base_mode=$mode >> + ;; >> + '2 '*) >> + local_mode=$mode local_sha1=$sha1 >> + ;; >> + '3 '*) >> + remote_mode=$mode remote_sha1=$sha1 >> + ;; >> + esac >> + done > > OK. $mode won't have any glob metacharacter, and there is only one > invocation of "ls-files -u", which is now two fewer processes ;-) I think it's better than that: there're about 9 fewer from the context that you didn't quote: - base_mode=$(git ls-files -u -- "$MERGED" | awk '{if ($3==1) print $1;}') - local_mode=$(git ls-files -u -- "$MERGED" | awk '{if ($3==2) print $1;}') - remote_mode=$(git ls-files -u -- "$MERGED" | awk '{if ($3==3) print $1;}') and sometimes the 6 from below on top of that. But I won't insist in having counted correctly ;-) >> >> if is_submodule "$local_mode" || is_submodule "$remote_mode" >> then >> echo "Submodule merge conflict for '$MERGED':" >> - local_sha1=$(git ls-files -u -- "$MERGED" | awk '{if ($3==2) print $2;}') >> - remote_sha1=$(git ls-files -u -- "$MERGED" | awk '{if ($3==3) print $2;}') >> describe_file "$local_mode" "local" "$local_sha1" >> describe_file "$remote_mode" "remote" "$remote_sha1" >> resolve_submodule_merge ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v2 0/4] Reduce number of processes spawned by git-mergetool 2019-06-10 8:58 [PATCH 0/3] Reduce number of processes spawned by git-mergetool Johannes Sixt ` (2 preceding siblings ...) 2019-06-10 8:59 ` [PATCH 3/3] mergetool: use shell variable magic instead of `awk` Johannes Sixt @ 2019-06-12 16:33 ` Johannes Sixt 2019-06-12 16:33 ` [PATCH v2 1/4] t7610-mergetool: do not place pipelines headed by `yes` in subshells Johannes Sixt ` (3 more replies) 3 siblings, 4 replies; 18+ messages in thread From: Johannes Sixt @ 2019-06-12 16:33 UTC (permalink / raw) To: git; +Cc: szeder.dev, gitster, Johannes Sixt git-mergetool spawns an enormous amount of processes. For this reason, the test script, t7610, is exceptionally slow, in particular, on Windows. Most of the processes are invocations of git. There are also some that can be replaced with shell builtins. I've measured the number of processes and the number of non-git commands invoked with strace -e execve,clone -f -o /tmp/git.strace ./t7610-mergetool.sh # processes: grep 'stack=NULL' /tmp/git.strace | wc -l # non-git commands: grep 'execve(' /tmp/git.strace | grep -v 'execve(".home' | wc -l non-git processes commands 13775 3082 baseline 13690 3082 t7610-mergetool: do not place pipelines headed by `yes` in subshells 13645 3082 t7610-mergetool: use test_cmp instead of test $(cat file) = $txt 13084 2521 mergetool: dissect strings with shell variable magic instead of `expr` 11542 2007 mergetool: use shell variable magic instead of `awk` Notes on v2: - Followed Gábor's suggestion to transform test ($cat ..) = txt (new patch 2). It does not help a lot, but neither does patch 1, the pipe-in-subshell conversion. Both are foremost clean-ups. - Commit message tweaks; see below. - Did not include measurement results in the commit messages, because I would have to repeat the measurement method every time. Johannes Sixt (4): t7610-mergetool: do not place pipelines headed by `yes` in subshells t7610-mergetool: use test_cmp instead of test $(cat file) = $txt mergetool: dissect strings with shell variable magic instead of `expr` mergetool: use shell variable magic instead of `awk` git-mergetool.sh | 45 +++++-- t/t7610-mergetool.sh | 309 +++++++++++++++++++++++++------------------ 2 files changed, 208 insertions(+), 146 deletions(-) Range-diff against v1: 1: 75c812bd48 = 1: 75c812bd48 t7610-mergetool: do not place pipelines headed by `yes` in subshells -: ---------- > 2: 82bafc90de t7610-mergetool: use test_cmp instead of test $(cat file) = $txt 2: 2a33ca20af ! 3: ad34a9a7ab mergetool: dissect strings with shell variable magic instead of `expr` @@ -4,7 +4,7 @@ git-mergetool spawns an enormous amount of processes. For this reason, the test script, t7610, is exceptionally slow, in particular, on - Windows. Most of the processes are invocations of git, but there are + Windows. Most of the processes are invocations of git. There are also some that can be replaced with shell builtins. Do so with `expr`. Signed-off-by: Johannes Sixt <j6t@kdbg.org> 3: d6675b0291 ! 4: 99a6a89339 mergetool: use shell variable magic instead of `awk` @@ -4,7 +4,7 @@ git-mergetool spawns an enormous amount of processes. For this reason, the test script, t7610, is exceptionally slow, in particular, on - Windows. Most of the processes are invocations of git, but there are + Windows. Most of the processes are invocations of git. There are also some that can be replaced with shell builtins. Avoid repeated calls of `git ls-files` and `awk`. -- 2.21.0.285.gc38d92e052 ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v2 1/4] t7610-mergetool: do not place pipelines headed by `yes` in subshells 2019-06-12 16:33 ` [PATCH v2 0/4] Reduce number of processes spawned by git-mergetool Johannes Sixt @ 2019-06-12 16:33 ` Johannes Sixt 2019-06-12 16:33 ` [PATCH v2 2/4] t7610-mergetool: use test_cmp instead of test $(cat file) = $txt Johannes Sixt ` (2 subsequent siblings) 3 siblings, 0 replies; 18+ messages in thread From: Johannes Sixt @ 2019-06-12 16:33 UTC (permalink / raw) To: git; +Cc: szeder.dev, gitster, Johannes Sixt Subshells for pipelines are not required. This can save a number of processes (if the shell does not optimize it away anyway). The patch was generated with the command sed -i 's/( *\(yes.*[^ ]\) *) *\&\&/\1 \&\&/' t7610-mergetool.sh with a manual fixup of the case having no && at the end. Signed-off-by: Johannes Sixt <j6t@kdbg.org> --- t/t7610-mergetool.sh | 170 +++++++++++++++++++++---------------------- 1 file changed, 85 insertions(+), 85 deletions(-) diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh index 5b61c10a9c..b67440882b 100755 --- a/t/t7610-mergetool.sh +++ b/t/t7610-mergetool.sh @@ -131,13 +131,13 @@ test_expect_success 'custom mergetool' ' git checkout -b test$test_count branch1 && git submodule update -N && test_must_fail git merge master && - ( yes "" | git mergetool both ) && - ( yes "" | git mergetool file1 file1 ) && - ( yes "" | git mergetool file2 "spaced name" ) && - ( yes "" | git mergetool subdir/file3 ) && - ( yes "d" | git mergetool file11 ) && - ( yes "d" | git mergetool file12 ) && - ( yes "l" | git mergetool submod ) && + yes "" | git mergetool both && + yes "" | git mergetool file1 file1 && + yes "" | git mergetool file2 "spaced name" && + yes "" | git mergetool subdir/file3 && + yes "d" | git mergetool file11 && + yes "d" | git mergetool file12 && + yes "l" | git mergetool submod && test "$(cat file1)" = "master updated" && test "$(cat file2)" = "master new" && test "$(cat subdir/file3)" = "master new sub" && @@ -153,13 +153,13 @@ test_expect_success 'gui mergetool' ' git checkout -b test$test_count branch1 && git submodule update -N && test_must_fail git merge master && - ( yes "" | git mergetool --gui both ) && - ( yes "" | git mergetool -g file1 file1 ) && - ( yes "" | git mergetool --gui file2 "spaced name" ) && - ( yes "" | git mergetool --gui subdir/file3 ) && - ( yes "d" | git mergetool --gui file11 ) && - ( yes "d" | git mergetool --gui file12 ) && - ( yes "l" | git mergetool --gui submod ) && + yes "" | git mergetool --gui both && + yes "" | git mergetool -g file1 file1 && + yes "" | git mergetool --gui file2 "spaced name" && + yes "" | git mergetool --gui subdir/file3 && + yes "d" | git mergetool --gui file11 && + yes "d" | git mergetool --gui file12 && + yes "l" | git mergetool --gui submod && test "$(cat file1)" = "gui master updated" && test "$(cat file2)" = "gui master new" && test "$(cat subdir/file3)" = "gui master new sub" && @@ -172,13 +172,13 @@ test_expect_success 'gui mergetool without merge.guitool set falls back to merge git checkout -b test$test_count branch1 && git submodule update -N && test_must_fail git merge master && - ( yes "" | git mergetool --gui both ) && - ( yes "" | git mergetool -g file1 file1 ) && - ( yes "" | git mergetool --gui file2 "spaced name" ) && - ( yes "" | git mergetool --gui subdir/file3 ) && - ( yes "d" | git mergetool --gui file11 ) && - ( yes "d" | git mergetool --gui file12 ) && - ( yes "l" | git mergetool --gui submod ) && + yes "" | git mergetool --gui both && + yes "" | git mergetool -g file1 file1 && + yes "" | git mergetool --gui file2 "spaced name" && + yes "" | git mergetool --gui subdir/file3 && + yes "d" | git mergetool --gui file11 && + yes "d" | git mergetool --gui file12 && + yes "l" | git mergetool --gui submod && test "$(cat file1)" = "master updated" && test "$(cat file2)" = "master new" && test "$(cat subdir/file3)" = "master new sub" && @@ -195,14 +195,14 @@ test_expect_success 'mergetool crlf' ' test_config core.autocrlf true && git checkout -b test$test_count branch1 && test_must_fail git merge master && - ( yes "" | git mergetool file1 ) && - ( yes "" | git mergetool file2 ) && - ( yes "" | git mergetool "spaced name" ) && - ( yes "" | git mergetool both ) && - ( yes "" | git mergetool subdir/file3 ) && - ( yes "d" | git mergetool file11 ) && - ( yes "d" | git mergetool file12 ) && - ( yes "r" | git mergetool submod ) && + yes "" | git mergetool file1 && + yes "" | git mergetool file2 && + yes "" | git mergetool "spaced name" && + yes "" | git mergetool both && + yes "" | git mergetool subdir/file3 && + yes "d" | git mergetool file11 && + yes "d" | git mergetool file12 && + yes "r" | git mergetool submod && test "$(printf x | cat file1 -)" = "$(printf "master updated\r\nx")" && test "$(printf x | cat file2 -)" = "$(printf "master new\r\nx")" && test "$(printf x | cat subdir/file3 -)" = "$(printf "master new sub\r\nx")" && @@ -218,7 +218,7 @@ test_expect_success 'mergetool in subdir' ' ( cd subdir && test_must_fail git merge master && - ( yes "" | git mergetool file3 ) && + yes "" | git mergetool file3 && test "$(cat file3)" = "master new sub" ) ' @@ -230,13 +230,13 @@ test_expect_success 'mergetool on file in parent dir' ' ( cd subdir && test_must_fail git merge master && - ( yes "" | git mergetool file3 ) && - ( yes "" | git mergetool ../file1 ) && - ( yes "" | git mergetool ../file2 ../spaced\ name ) && - ( yes "" | git mergetool ../both ) && - ( yes "d" | git mergetool ../file11 ) && - ( yes "d" | git mergetool ../file12 ) && - ( yes "l" | git mergetool ../submod ) && + yes "" | git mergetool file3 && + yes "" | git mergetool ../file1 && + yes "" | git mergetool ../file2 ../spaced\ name && + yes "" | git mergetool ../both && + yes "d" | git mergetool ../file11 && + yes "d" | git mergetool ../file12 && + yes "l" | git mergetool ../submod && test "$(cat ../file1)" = "master updated" && test "$(cat ../file2)" = "master new" && test "$(cat ../submod/bar)" = "branch1 submodule" && @@ -250,9 +250,9 @@ test_expect_success 'mergetool skips autoresolved' ' git submodule update -N && test_must_fail git merge master && test -n "$(git ls-files -u)" && - ( yes "d" | git mergetool file11 ) && - ( yes "d" | git mergetool file12 ) && - ( yes "l" | git mergetool submod ) && + yes "d" | git mergetool file11 && + yes "d" | git mergetool file12 && + yes "l" | git mergetool submod && output="$(git mergetool --no-prompt)" && test "$output" = "No files need merging" ' @@ -264,8 +264,8 @@ test_expect_success 'mergetool merges all from subdir (rerere disabled)' ' ( cd subdir && test_must_fail git merge master && - ( yes "r" | git mergetool ../submod ) && - ( yes "d" "d" | git mergetool --no-prompt ) && + yes "r" | git mergetool ../submod && + yes "d" "d" | git mergetool --no-prompt && test "$(cat ../file1)" = "master updated" && test "$(cat ../file2)" = "master new" && test "$(cat file3)" = "master new sub" && @@ -283,8 +283,8 @@ test_expect_success 'mergetool merges all from subdir (rerere enabled)' ' ( cd subdir && test_must_fail git merge master && - ( yes "r" | git mergetool ../submod ) && - ( yes "d" "d" | git mergetool --no-prompt ) && + yes "r" | git mergetool ../submod && + yes "d" "d" | git mergetool --no-prompt && test "$(cat ../file1)" = "master updated" && test "$(cat ../file2)" = "master new" && test "$(cat file3)" = "master new sub" && @@ -301,8 +301,8 @@ test_expect_success 'mergetool skips resolved paths when rerere is active' ' git checkout -b test$test_count branch1 && git submodule update -N && test_must_fail git merge master && - ( yes "l" | git mergetool --no-prompt submod ) && - ( yes "d" "d" | git mergetool --no-prompt ) && + yes "l" | git mergetool --no-prompt submod && + yes "d" "d" | git mergetool --no-prompt && git submodule update -N && output="$(yes "n" | git mergetool --no-prompt)" && test "$output" = "No files need merging" @@ -343,7 +343,7 @@ test_expect_success 'mergetool takes partial path' ' git submodule update -N && test_must_fail git merge master && - ( yes "" | git mergetool subdir ) && + yes "" | git mergetool subdir && test "$(cat subdir/file3)" = "master new sub" ' @@ -410,10 +410,10 @@ test_expect_success 'deleted vs modified submodule' ' git checkout -b test$test_count.a test$test_count && test_must_fail git merge master && test -n "$(git ls-files -u)" && - ( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 ) && - ( yes "" | git mergetool both ) && - ( yes "d" | git mergetool file11 file12 ) && - ( yes "r" | git mergetool submod ) && + yes "" | git mergetool file1 file2 spaced\ name subdir/file3 && + yes "" | git mergetool both && + yes "d" | git mergetool file11 file12 && + yes "r" | git mergetool submod && rmdir submod && mv submod-movedaside submod && test "$(cat submod/bar)" = "branch1 submodule" && git submodule update -N && @@ -427,10 +427,10 @@ test_expect_success 'deleted vs modified submodule' ' git submodule update -N && test_must_fail git merge master && test -n "$(git ls-files -u)" && - ( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 ) && - ( yes "" | git mergetool both ) && - ( yes "d" | git mergetool file11 file12 ) && - ( yes "l" | git mergetool submod ) && + yes "" | git mergetool file1 file2 spaced\ name subdir/file3 && + yes "" | git mergetool both && + yes "d" | git mergetool file11 file12 && + yes "l" | git mergetool submod && test ! -e submod && output="$(git mergetool --no-prompt)" && test "$output" = "No files need merging" && @@ -441,10 +441,10 @@ test_expect_success 'deleted vs modified submodule' ' git submodule update -N && test_must_fail git merge test$test_count && test -n "$(git ls-files -u)" && - ( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 ) && - ( yes "" | git mergetool both ) && - ( yes "d" | git mergetool file11 file12 ) && - ( yes "r" | git mergetool submod ) && + yes "" | git mergetool file1 file2 spaced\ name subdir/file3 && + yes "" | git mergetool both && + yes "d" | git mergetool file11 file12 && + yes "r" | git mergetool submod && test ! -e submod && test -d submod.orig && git submodule update -N && @@ -457,10 +457,10 @@ test_expect_success 'deleted vs modified submodule' ' git submodule update -N && test_must_fail git merge test$test_count && test -n "$(git ls-files -u)" && - ( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 ) && - ( yes "" | git mergetool both ) && - ( yes "d" | git mergetool file11 file12 ) && - ( yes "l" | git mergetool submod ) && + yes "" | git mergetool file1 file2 spaced\ name subdir/file3 && + yes "" | git mergetool both && + yes "d" | git mergetool file11 file12 && + yes "l" | git mergetool submod && test "$(cat submod/bar)" = "master submodule" && git submodule update -N && test "$(cat submod/bar)" = "master submodule" && @@ -481,10 +481,10 @@ test_expect_success 'file vs modified submodule' ' git checkout -b test$test_count.a branch1 && test_must_fail git merge master && test -n "$(git ls-files -u)" && - ( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 ) && - ( yes "" | git mergetool both ) && - ( yes "d" | git mergetool file11 file12 ) && - ( yes "r" | git mergetool submod ) && + yes "" | git mergetool file1 file2 spaced\ name subdir/file3 && + yes "" | git mergetool both && + yes "d" | git mergetool file11 file12 && + yes "r" | git mergetool submod && rmdir submod && mv submod-movedaside submod && test "$(cat submod/bar)" = "branch1 submodule" && git submodule update -N && @@ -497,10 +497,10 @@ test_expect_success 'file vs modified submodule' ' git checkout -b test$test_count.b test$test_count && test_must_fail git merge master && test -n "$(git ls-files -u)" && - ( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 ) && - ( yes "" | git mergetool both ) && - ( yes "d" | git mergetool file11 file12 ) && - ( yes "l" | git mergetool submod ) && + yes "" | git mergetool file1 file2 spaced\ name subdir/file3 && + yes "" | git mergetool both && + yes "d" | git mergetool file11 file12 && + yes "l" | git mergetool submod && git submodule update -N && test "$(cat submod)" = "not a submodule" && output="$(git mergetool --no-prompt)" && @@ -513,10 +513,10 @@ test_expect_success 'file vs modified submodule' ' git submodule update -N && test_must_fail git merge test$test_count && test -n "$(git ls-files -u)" && - ( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 ) && - ( yes "" | git mergetool both ) && - ( yes "d" | git mergetool file11 file12 ) && - ( yes "r" | git mergetool submod ) && + yes "" | git mergetool file1 file2 spaced\ name subdir/file3 && + yes "" | git mergetool both && + yes "d" | git mergetool file11 file12 && + yes "r" | git mergetool submod && test -d submod.orig && git submodule update -N && test "$(cat submod)" = "not a submodule" && @@ -529,10 +529,10 @@ test_expect_success 'file vs modified submodule' ' git submodule update -N && test_must_fail git merge test$test_count && test -n "$(git ls-files -u)" && - ( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 ) && - ( yes "" | git mergetool both ) && - ( yes "d" | git mergetool file11 file12 ) && - ( yes "l" | git mergetool submod ) && + yes "" | git mergetool file1 file2 spaced\ name subdir/file3 && + yes "" | git mergetool both && + yes "d" | git mergetool file11 file12 && + yes "l" | git mergetool submod && test "$(cat submod/bar)" = "master submodule" && git submodule update -N && test "$(cat submod/bar)" = "master submodule" && @@ -587,7 +587,7 @@ test_expect_success 'submodule in subdirectory' ' test_must_fail git merge test$test_count.a && ( cd subdir && - ( yes "l" | git mergetool subdir_module ) + yes "l" | git mergetool subdir_module ) && test "$(cat subdir/subdir_module/file15)" = "test$test_count.b" && git submodule update -N && @@ -596,7 +596,7 @@ test_expect_success 'submodule in subdirectory' ' git submodule update -N && test_must_fail git merge test$test_count.a && - ( yes "r" | git mergetool subdir/subdir_module ) && + yes "r" | git mergetool subdir/subdir_module && test "$(cat subdir/subdir_module/file15)" = "test$test_count.b" && git submodule update -N && test "$(cat subdir/subdir_module/file15)" = "test$test_count.a" && @@ -615,7 +615,7 @@ test_expect_success 'directory vs modified submodule' ' test_must_fail git merge master && test -n "$(git ls-files -u)" && - ( yes "l" | git mergetool submod ) && + yes "l" | git mergetool submod && test "$(cat submod/file16)" = "not a submodule" && rm -rf submod.orig && @@ -623,7 +623,7 @@ test_expect_success 'directory vs modified submodule' ' test_must_fail git merge master && test -n "$(git ls-files -u)" && test ! -e submod.orig && - ( yes "r" | git mergetool submod ) && + yes "r" | git mergetool submod && test -d submod.orig && test "$(cat submod.orig/file16)" = "not a submodule" && rm -r submod.orig && @@ -638,7 +638,7 @@ test_expect_success 'directory vs modified submodule' ' git submodule update -N && test_must_fail git merge test$test_count && test -n "$(git ls-files -u)" && - ( yes "l" | git mergetool submod ) && + yes "l" | git mergetool submod && git submodule update -N && test "$(cat submod/bar)" = "master submodule" && @@ -647,7 +647,7 @@ test_expect_success 'directory vs modified submodule' ' test_must_fail git merge test$test_count && test -n "$(git ls-files -u)" && test ! -e submod.orig && - ( yes "r" | git mergetool submod ) && + yes "r" | git mergetool submod && test "$(cat submod/file16)" = "not a submodule" && git reset --hard master && -- 2.21.0.285.gc38d92e052 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v2 2/4] t7610-mergetool: use test_cmp instead of test $(cat file) = $txt 2019-06-12 16:33 ` [PATCH v2 0/4] Reduce number of processes spawned by git-mergetool Johannes Sixt 2019-06-12 16:33 ` [PATCH v2 1/4] t7610-mergetool: do not place pipelines headed by `yes` in subshells Johannes Sixt @ 2019-06-12 16:33 ` Johannes Sixt 2019-06-12 16:33 ` [PATCH v2 3/4] mergetool: dissect strings with shell variable magic instead of `expr` Johannes Sixt 2019-06-12 16:33 ` [PATCH v2 4/4] mergetool: use shell variable magic instead of `awk` Johannes Sixt 3 siblings, 0 replies; 18+ messages in thread From: Johannes Sixt @ 2019-06-12 16:33 UTC (permalink / raw) To: git; +Cc: szeder.dev, gitster, Johannes Sixt Fix that anti-pattern by a sequence of echo and test_cmp. The patch was generated with this command: sed -i -e '/test.*(cat/s/^\(\t*\)test "..cat \(.*\))" = \(".*"\)\(.*\)/\1echo \3 >expect \&\&\n\1test_cmp expect \2\4/' t7610-mergetool.sh This helps on Windows, where test_cmp avoids spawning a process when there is no difference. Signed-off-by: Johannes Sixt <j6t@kdbg.org> --- t/t7610-mergetool.sh | 135 ++++++++++++++++++++++++++++--------------- 1 file changed, 90 insertions(+), 45 deletions(-) diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh index b67440882b..ad288ddc69 100755 --- a/t/t7610-mergetool.sh +++ b/t/t7610-mergetool.sh @@ -138,10 +138,14 @@ test_expect_success 'custom mergetool' ' yes "d" | git mergetool file11 && yes "d" | git mergetool file12 && yes "l" | git mergetool submod && - test "$(cat file1)" = "master updated" && - test "$(cat file2)" = "master new" && - test "$(cat subdir/file3)" = "master new sub" && - test "$(cat submod/bar)" = "branch1 submodule" && + echo "master updated" >expect && + test_cmp expect file1 && + echo "master new" >expect && + test_cmp expect file2 && + echo "master new sub" >expect && + test_cmp expect subdir/file3 && + echo "branch1 submodule" >expect && + test_cmp expect submod/bar && git commit -m "branch1 resolved with mergetool" ' @@ -160,10 +164,14 @@ test_expect_success 'gui mergetool' ' yes "d" | git mergetool --gui file11 && yes "d" | git mergetool --gui file12 && yes "l" | git mergetool --gui submod && - test "$(cat file1)" = "gui master updated" && - test "$(cat file2)" = "gui master new" && - test "$(cat subdir/file3)" = "gui master new sub" && - test "$(cat submod/bar)" = "branch1 submodule" && + echo "gui master updated" >expect && + test_cmp expect file1 && + echo "gui master new" >expect && + test_cmp expect file2 && + echo "gui master new sub" >expect && + test_cmp expect subdir/file3 && + echo "branch1 submodule" >expect && + test_cmp expect submod/bar && git commit -m "branch1 resolved with mergetool" ' @@ -179,10 +187,14 @@ test_expect_success 'gui mergetool without merge.guitool set falls back to merge yes "d" | git mergetool --gui file11 && yes "d" | git mergetool --gui file12 && yes "l" | git mergetool --gui submod && - test "$(cat file1)" = "master updated" && - test "$(cat file2)" = "master new" && - test "$(cat subdir/file3)" = "master new sub" && - test "$(cat submod/bar)" = "branch1 submodule" && + echo "master updated" >expect && + test_cmp expect file1 && + echo "master new" >expect && + test_cmp expect file2 && + echo "master new sub" >expect && + test_cmp expect subdir/file3 && + echo "branch1 submodule" >expect && + test_cmp expect submod/bar && git commit -m "branch1 resolved with mergetool" ' @@ -207,7 +219,8 @@ test_expect_success 'mergetool crlf' ' test "$(printf x | cat file2 -)" = "$(printf "master new\r\nx")" && test "$(printf x | cat subdir/file3 -)" = "$(printf "master new sub\r\nx")" && git submodule update -N && - test "$(cat submod/bar)" = "master submodule" && + echo "master submodule" >expect && + test_cmp expect submod/bar && git commit -m "branch1 resolved with mergetool - autocrlf" ' @@ -219,7 +232,8 @@ test_expect_success 'mergetool in subdir' ' cd subdir && test_must_fail git merge master && yes "" | git mergetool file3 && - test "$(cat file3)" = "master new sub" + echo "master new sub" >expect && + test_cmp expect file3 ) ' @@ -237,9 +251,12 @@ test_expect_success 'mergetool on file in parent dir' ' yes "d" | git mergetool ../file11 && yes "d" | git mergetool ../file12 && yes "l" | git mergetool ../submod && - test "$(cat ../file1)" = "master updated" && - test "$(cat ../file2)" = "master new" && - test "$(cat ../submod/bar)" = "branch1 submodule" && + echo "master updated" >expect && + test_cmp expect ../file1 && + echo "master new" >expect && + test_cmp expect ../file2 && + echo "branch1 submodule" >expect && + test_cmp expect ../submod/bar && git commit -m "branch1 resolved with mergetool - subdir" ) ' @@ -266,11 +283,15 @@ test_expect_success 'mergetool merges all from subdir (rerere disabled)' ' test_must_fail git merge master && yes "r" | git mergetool ../submod && yes "d" "d" | git mergetool --no-prompt && - test "$(cat ../file1)" = "master updated" && - test "$(cat ../file2)" = "master new" && - test "$(cat file3)" = "master new sub" && + echo "master updated" >expect && + test_cmp expect ../file1 && + echo "master new" >expect && + test_cmp expect ../file2 && + echo "master new sub" >expect && + test_cmp expect file3 && ( cd .. && git submodule update -N ) && - test "$(cat ../submod/bar)" = "master submodule" && + echo "master submodule" >expect && + test_cmp expect ../submod/bar && git commit -m "branch2 resolved by mergetool from subdir" ) ' @@ -285,11 +306,15 @@ test_expect_success 'mergetool merges all from subdir (rerere enabled)' ' test_must_fail git merge master && yes "r" | git mergetool ../submod && yes "d" "d" | git mergetool --no-prompt && - test "$(cat ../file1)" = "master updated" && - test "$(cat ../file2)" = "master new" && - test "$(cat file3)" = "master new sub" && + echo "master updated" >expect && + test_cmp expect ../file1 && + echo "master new" >expect && + test_cmp expect ../file2 && + echo "master new sub" >expect && + test_cmp expect file3 && ( cd .. && git submodule update -N ) && - test "$(cat ../submod/bar)" = "master submodule" && + echo "master submodule" >expect && + test_cmp expect ../submod/bar && git commit -m "branch2 resolved by mergetool from subdir" ) ' @@ -345,7 +370,8 @@ test_expect_success 'mergetool takes partial path' ' yes "" | git mergetool subdir && - test "$(cat subdir/file3)" = "master new sub" + echo "master new sub" >expect && + test_cmp expect subdir/file3 ' test_expect_success 'mergetool delete/delete conflict' ' @@ -415,9 +441,11 @@ test_expect_success 'deleted vs modified submodule' ' yes "d" | git mergetool file11 file12 && yes "r" | git mergetool submod && rmdir submod && mv submod-movedaside submod && - test "$(cat submod/bar)" = "branch1 submodule" && + echo "branch1 submodule" >expect && + test_cmp expect submod/bar && git submodule update -N && - test "$(cat submod/bar)" = "master submodule" && + echo "master submodule" >expect && + test_cmp expect submod/bar && output="$(git mergetool --no-prompt)" && test "$output" = "No files need merging" && git commit -m "Merge resolved by keeping module" && @@ -461,9 +489,11 @@ test_expect_success 'deleted vs modified submodule' ' yes "" | git mergetool both && yes "d" | git mergetool file11 file12 && yes "l" | git mergetool submod && - test "$(cat submod/bar)" = "master submodule" && + echo "master submodule" >expect && + test_cmp expect submod/bar && git submodule update -N && - test "$(cat submod/bar)" = "master submodule" && + echo "master submodule" >expect && + test_cmp expect submod/bar && output="$(git mergetool --no-prompt)" && test "$output" = "No files need merging" && git commit -m "Merge resolved by keeping module" @@ -486,9 +516,11 @@ test_expect_success 'file vs modified submodule' ' yes "d" | git mergetool file11 file12 && yes "r" | git mergetool submod && rmdir submod && mv submod-movedaside submod && - test "$(cat submod/bar)" = "branch1 submodule" && + echo "branch1 submodule" >expect && + test_cmp expect submod/bar && git submodule update -N && - test "$(cat submod/bar)" = "master submodule" && + echo "master submodule" >expect && + test_cmp expect submod/bar && output="$(git mergetool --no-prompt)" && test "$output" = "No files need merging" && git commit -m "Merge resolved by keeping module" && @@ -502,7 +534,8 @@ test_expect_success 'file vs modified submodule' ' yes "d" | git mergetool file11 file12 && yes "l" | git mergetool submod && git submodule update -N && - test "$(cat submod)" = "not a submodule" && + echo "not a submodule" >expect && + test_cmp expect submod && output="$(git mergetool --no-prompt)" && test "$output" = "No files need merging" && git commit -m "Merge resolved by keeping file" && @@ -519,7 +552,8 @@ test_expect_success 'file vs modified submodule' ' yes "r" | git mergetool submod && test -d submod.orig && git submodule update -N && - test "$(cat submod)" = "not a submodule" && + echo "not a submodule" >expect && + test_cmp expect submod && output="$(git mergetool --no-prompt)" && test "$output" = "No files need merging" && git commit -m "Merge resolved by keeping file" && @@ -533,9 +567,11 @@ test_expect_success 'file vs modified submodule' ' yes "" | git mergetool both && yes "d" | git mergetool file11 file12 && yes "l" | git mergetool submod && - test "$(cat submod/bar)" = "master submodule" && + echo "master submodule" >expect && + test_cmp expect submod/bar && git submodule update -N && - test "$(cat submod/bar)" = "master submodule" && + echo "master submodule" >expect && + test_cmp expect submod/bar && output="$(git mergetool --no-prompt)" && test "$output" = "No files need merging" && git commit -m "Merge resolved by keeping module" @@ -589,17 +625,21 @@ test_expect_success 'submodule in subdirectory' ' cd subdir && yes "l" | git mergetool subdir_module ) && - test "$(cat subdir/subdir_module/file15)" = "test$test_count.b" && + echo "test$test_count.b" >expect && + test_cmp expect subdir/subdir_module/file15 && git submodule update -N && - test "$(cat subdir/subdir_module/file15)" = "test$test_count.b" && + echo "test$test_count.b" >expect && + test_cmp expect subdir/subdir_module/file15 && git reset --hard && git submodule update -N && test_must_fail git merge test$test_count.a && yes "r" | git mergetool subdir/subdir_module && - test "$(cat subdir/subdir_module/file15)" = "test$test_count.b" && + echo "test$test_count.b" >expect && + test_cmp expect subdir/subdir_module/file15 && git submodule update -N && - test "$(cat subdir/subdir_module/file15)" = "test$test_count.a" && + echo "test$test_count.a" >expect && + test_cmp expect subdir/subdir_module/file15 && git commit -m "branch1 resolved with mergetool" ' @@ -616,7 +656,8 @@ test_expect_success 'directory vs modified submodule' ' test_must_fail git merge master && test -n "$(git ls-files -u)" && yes "l" | git mergetool submod && - test "$(cat submod/file16)" = "not a submodule" && + echo "not a submodule" >expect && + test_cmp expect submod/file16 && rm -rf submod.orig && git reset --hard && @@ -625,12 +666,14 @@ test_expect_success 'directory vs modified submodule' ' test ! -e submod.orig && yes "r" | git mergetool submod && test -d submod.orig && - test "$(cat submod.orig/file16)" = "not a submodule" && + echo "not a submodule" >expect && + test_cmp expect submod.orig/file16 && rm -r submod.orig && mv submod-movedaside/.git submod && ( cd submod && git clean -f && git reset --hard ) && git submodule update -N && - test "$(cat submod/bar)" = "master submodule" && + echo "master submodule" >expect && + test_cmp expect submod/bar && git reset --hard && rm -rf submod-movedaside && @@ -640,7 +683,8 @@ test_expect_success 'directory vs modified submodule' ' test -n "$(git ls-files -u)" && yes "l" | git mergetool submod && git submodule update -N && - test "$(cat submod/bar)" = "master submodule" && + echo "master submodule" >expect && + test_cmp expect submod/bar && git reset --hard && git submodule update -N && @@ -648,7 +692,8 @@ test_expect_success 'directory vs modified submodule' ' test -n "$(git ls-files -u)" && test ! -e submod.orig && yes "r" | git mergetool submod && - test "$(cat submod/file16)" = "not a submodule" && + echo "not a submodule" >expect && + test_cmp expect submod/file16 && git reset --hard master && ( cd submod && git clean -f && git reset --hard ) && -- 2.21.0.285.gc38d92e052 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v2 3/4] mergetool: dissect strings with shell variable magic instead of `expr` 2019-06-12 16:33 ` [PATCH v2 0/4] Reduce number of processes spawned by git-mergetool Johannes Sixt 2019-06-12 16:33 ` [PATCH v2 1/4] t7610-mergetool: do not place pipelines headed by `yes` in subshells Johannes Sixt 2019-06-12 16:33 ` [PATCH v2 2/4] t7610-mergetool: use test_cmp instead of test $(cat file) = $txt Johannes Sixt @ 2019-06-12 16:33 ` Johannes Sixt 2019-06-12 16:33 ` [PATCH v2 4/4] mergetool: use shell variable magic instead of `awk` Johannes Sixt 3 siblings, 0 replies; 18+ messages in thread From: Johannes Sixt @ 2019-06-12 16:33 UTC (permalink / raw) To: git; +Cc: szeder.dev, gitster, Johannes Sixt git-mergetool spawns an enormous amount of processes. For this reason, the test script, t7610, is exceptionally slow, in particular, on Windows. Most of the processes are invocations of git. There are also some that can be replaced with shell builtins. Do so with `expr`. Signed-off-by: Johannes Sixt <j6t@kdbg.org> --- git-mergetool.sh | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/git-mergetool.sh b/git-mergetool.sh index 88fa6a914a..8a937f680f 100755 --- a/git-mergetool.sh +++ b/git-mergetool.sh @@ -228,9 +228,8 @@ stage_submodule () { } checkout_staged_file () { - tmpfile=$(expr \ - "$(git checkout-index --temp --stage="$1" "$2" 2>/dev/null)" \ - : '\([^ ]*\) ') + tmpfile="$(git checkout-index --temp --stage="$1" "$2" 2>/dev/null)" && + tmpfile=${tmpfile%%' '*} if test $? -eq 0 && test -n "$tmpfile" then @@ -255,13 +254,16 @@ merge_file () { return 1 fi - if BASE=$(expr "$MERGED" : '\(.*\)\.[^/]*$') - then - ext=$(expr "$MERGED" : '.*\(\.[^/]*\)$') - else + # extract file extension from the last path component + case "${MERGED##*/}" in + *.*) + ext=.${MERGED##*.} + BASE=${MERGED%"$ext"} + ;; + *) BASE=$MERGED ext= - fi + esac mergetool_tmpdir_init @@ -406,7 +408,7 @@ main () { -t|--tool*) case "$#,$1" in *,*=*) - merge_tool=$(expr "z$1" : 'z-[^=]*=\(.*\)') + merge_tool=${1#*=} ;; 1,*) usage ;; -- 2.21.0.285.gc38d92e052 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v2 4/4] mergetool: use shell variable magic instead of `awk` 2019-06-12 16:33 ` [PATCH v2 0/4] Reduce number of processes spawned by git-mergetool Johannes Sixt ` (2 preceding siblings ...) 2019-06-12 16:33 ` [PATCH v2 3/4] mergetool: dissect strings with shell variable magic instead of `expr` Johannes Sixt @ 2019-06-12 16:33 ` Johannes Sixt 3 siblings, 0 replies; 18+ messages in thread From: Johannes Sixt @ 2019-06-12 16:33 UTC (permalink / raw) To: git; +Cc: szeder.dev, gitster, Johannes Sixt git-mergetool spawns an enormous amount of processes. For this reason, the test script, t7610, is exceptionally slow, in particular, on Windows. Most of the processes are invocations of git. There are also some that can be replaced with shell builtins. Avoid repeated calls of `git ls-files` and `awk`. Signed-off-by: Johannes Sixt <j6t@kdbg.org> --- git-mergetool.sh | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/git-mergetool.sh b/git-mergetool.sh index 8a937f680f..e3f6d543fb 100755 --- a/git-mergetool.sh +++ b/git-mergetool.sh @@ -279,15 +279,30 @@ merge_file () { REMOTE="$MERGETOOL_TMPDIR/${BASE}_REMOTE_$$$ext" BASE="$MERGETOOL_TMPDIR/${BASE}_BASE_$$$ext" - base_mode=$(git ls-files -u -- "$MERGED" | awk '{if ($3==1) print $1;}') - local_mode=$(git ls-files -u -- "$MERGED" | awk '{if ($3==2) print $1;}') - remote_mode=$(git ls-files -u -- "$MERGED" | awk '{if ($3==3) print $1;}') + base_mode= local_mode= remote_mode= + + # here, $IFS is just a LF + for line in $f + do + mode=${line%% *} # 1st word + sha1=${line#"$mode "} + sha1=${sha1%% *} # 2nd word + case "${line#$mode $sha1 }" in # remainder + '1 '*) + base_mode=$mode + ;; + '2 '*) + local_mode=$mode local_sha1=$sha1 + ;; + '3 '*) + remote_mode=$mode remote_sha1=$sha1 + ;; + esac + done if is_submodule "$local_mode" || is_submodule "$remote_mode" then echo "Submodule merge conflict for '$MERGED':" - local_sha1=$(git ls-files -u -- "$MERGED" | awk '{if ($3==2) print $2;}') - remote_sha1=$(git ls-files -u -- "$MERGED" | awk '{if ($3==3) print $2;}') describe_file "$local_mode" "local" "$local_sha1" describe_file "$remote_mode" "remote" "$remote_sha1" resolve_submodule_merge -- 2.21.0.285.gc38d92e052 ^ permalink raw reply related [flat|nested] 18+ messages in thread
end of thread, other threads:[~2019-06-12 16:34 UTC | newest] Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-06-10 8:58 [PATCH 0/3] Reduce number of processes spawned by git-mergetool Johannes Sixt 2019-06-10 8:58 ` [PATCH 1/3] t7610-mergetool: do not place pipelines headed by `yes` in subshells Johannes Sixt 2019-06-10 9:59 ` SZEDER Gábor 2019-06-10 17:23 ` Junio C Hamano 2019-06-10 17:56 ` SZEDER Gábor 2019-06-10 18:29 ` Johannes Schindelin 2019-06-10 18:57 ` SZEDER Gábor 2019-06-10 8:58 ` [PATCH 2/3] mergetool: dissect strings with shell variable magic instead of `expr` Johannes Sixt 2019-06-10 17:17 ` Junio C Hamano 2019-06-10 21:34 ` Johannes Sixt 2019-06-10 8:59 ` [PATCH 3/3] mergetool: use shell variable magic instead of `awk` Johannes Sixt 2019-06-10 17:21 ` Junio C Hamano 2019-06-10 22:01 ` Johannes Sixt 2019-06-12 16:33 ` [PATCH v2 0/4] Reduce number of processes spawned by git-mergetool Johannes Sixt 2019-06-12 16:33 ` [PATCH v2 1/4] t7610-mergetool: do not place pipelines headed by `yes` in subshells Johannes Sixt 2019-06-12 16:33 ` [PATCH v2 2/4] t7610-mergetool: use test_cmp instead of test $(cat file) = $txt Johannes Sixt 2019-06-12 16:33 ` [PATCH v2 3/4] mergetool: dissect strings with shell variable magic instead of `expr` Johannes Sixt 2019-06-12 16:33 ` [PATCH v2 4/4] mergetool: use shell variable magic instead of `awk` Johannes Sixt
Code repositories for project(s) associated with this public inbox https://80x24.org/mirrors/git.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).