diff mbox series

[1/3] t7610-mergetool: do not place pipelines headed by `yes` in subshells

Message ID 75c812bd4838f6f35b6f42b97ae396ebb28d8b95.1560152205.git.j6t@kdbg.org (mailing list archive)
State New, archived
Headers show
Series Reduce number of processes spawned by git-mergetool | expand

Commit Message

Johannes Sixt June 10, 2019, 8:58 a.m. UTC 
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(-)

Comments

SZEDER Gábor June 10, 2019, 9:59 a.m. UTC | #1 
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.
Junio C Hamano June 10, 2019, 5:23 p.m. UTC | #2 
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?
SZEDER Gábor June 10, 2019, 5:56 p.m. UTC | #3 
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... :)
Johannes Schindelin June 10, 2019, 6:29 p.m. UTC | #4 
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
SZEDER Gábor June 10, 2019, 6:57 p.m. UTC | #5 
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
diff mbox series

Patch

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 &&