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

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Jun 10, 2019, 18:29 UTC
Message-ID: <nycvar.QRO.7.76.6.1906102026030.789@QRFXGBC-DHN364S.ybpnyqbznva>
In-Reply-To: <20190610095942.GA19035@szeder.dev>

Hi,

On Mon, 10 Jun 2019, SZEDER Gábor wrote:

Show 38 quoted lines

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