patch, 5 partsSome fixes and an improvement for using CTest on Windows

> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  contrib/buildsystems/CMakeLists.txt | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt
> index 1b23f2440d8..4aee1e24342 100644
> --- a/contrib/buildsystems/CMakeLists.txt
> +++ b/contrib/buildsystems/CMakeLists.txt
> @@ -1088,7 +1088,7 @@ file(GLOB test_scipts "${CMAKE_SOURCE_DIR}/t/t[0-9]*.sh")
>  #test
>  foreach(tsh ${test_scipts})
>  	add_test(NAME ${tsh}
> -		COMMAND ${SH_EXE} ${tsh}
> +		COMMAND ${SH_EXE} ${tsh} --no-bin-wrappers --no-chain-lint -vx
>  		WORKING_DIRECTORY ${CMAKE_SOURCE_DIR}/t)
>  endforeach()

> "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
> writes:
>
> > While at it, enable the command trace via `-x` and verbose output via
> > `-v`, otherwise it would be near impossible to diagnose any problems.
>
> This sounds like a completely unrelated change.

> Hi Junio,
>
> On Wed, 10 Aug 2022, Junio C Hamano wrote:
>
>> "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
>> writes:
>>
>> > While at it, enable the command trace via `-x` and verbose output via
>> > `-v`, otherwise it would be near impossible to diagnose any problems.
>>
>> This sounds like a completely unrelated change.
>
> It may sound like it, but it is not. In order to make sense of the broken
> tests, I needed access to more verbose output than our test scripts
> provide by default.
>
> When running the test suite on the command-line, it is easy to tell the
> user "oh, if you need more information, just call the test script with
> these here options: ...".
>
> This is not an option when running the tests within Visual Studio.

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > On Wed, 10 Aug 2022, Junio C Hamano wrote:
> >
> >> "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
> >> writes:
> >>
> >> > While at it, enable the command trace via `-x` and verbose output via
> >> > `-v`, otherwise it would be near impossible to diagnose any problems.
> >>
> >> This sounds like a completely unrelated change.
> >
> > It may sound like it, but it is not. In order to make sense of the broken
> > tests, I needed access to more verbose output than our test scripts
> > provide by default.
> >
> > When running the test suite on the command-line, it is easy to tell the
> > user "oh, if you need more information, just call the test script with
> > these here options: ...".
> >
> > This is not an option when running the tests within Visual Studio.
>
> I gave an example of "CI environment it is cumbersome to go back and
> run only a single one" in the review you are responding to that may
> explain why such a change is needed, and you gave us exactly the
> context that was lacking here.  The environment does not let users
> run the tests as anything but a single monolithic ball of wax.

> > Does this clarify the intention and validity of the proposed patch? If so,
> > I will gladly try my best to improve the commit message to explain that
> > intention better.
>
> It explains why -x -v is needed and needs to be a part of VS+CMake
> topic, and it will help readers to have it in the explanation.  It
> still does not justify why it has to be a part of a step to omit
> bin-warppers and skip chain-lint that has nothing to do it, though.

> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> In Git's CI runs, the Windows tests are run with `--no-bin-wrappers` and
> `--no-chain-lint`, mainly to win back some time caused by the serious
> performance penalty paid for the tests relying so heavily on POSIX shell
> scripting, which only works by using a POSIX emulation layer.
>
> Let's do the same when running the tests, say, in Visual Studio.
>
> While at it, enable the command trace via `-x` and verbose output via
> `-v`, otherwise it would be near impossible to diagnose any problems.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  contrib/buildsystems/CMakeLists.txt | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt
> index 1b23f2440d8..4aee1e24342 100644
> --- a/contrib/buildsystems/CMakeLists.txt
> +++ b/contrib/buildsystems/CMakeLists.txt
> @@ -1088,7 +1088,7 @@ file(GLOB test_scipts "${CMAKE_SOURCE_DIR}/t/t[0-9]*.sh")
>  #test
>  foreach(tsh ${test_scipts})
>  	add_test(NAME ${tsh}
> -		COMMAND ${SH_EXE} ${tsh}
> +		COMMAND ${SH_EXE} ${tsh} --no-bin-wrappers --no-chain-lint -vx
>  		WORKING_DIRECTORY ${CMAKE_SOURCE_DIR}/t)
>  endforeach()

> From: Johannes Schindelin <johannes.schindelin@gmx.de>
> [...]
> However, this quirk is only in effect as long as `chmod` is run inside
> the pseudo Unix root directory structure or within the home directory.
> When run outside, such invocations fail like this:
>
> 	chmod: changing permissions of '<file>': Invalid argument

> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
> index 6da7273f1d5..7c63b22acab 100644
> --- a/t/test-lib-functions.sh
> +++ b/t/test-lib-functions.sh
> @@ -492,7 +492,10 @@ test_commit_bulk () {
>  # of a file in the working directory and add it to the index.
>  
>  test_chmod () {
> -	chmod "$@" &&
> +	if test_have_prereq !MINGW
> +	then
> +		chmod "$@"
> +	fi &&
>  	git update-index --add "--chmod=$@"
>  }
>  
> @@ -548,7 +551,10 @@ write_script () {
>  		echo "#!${2-"$SHELL_PATH"}" &&
>  		cat
>  	} >"$1" &&
> -	chmod +x "$1"
> +	if test_have_prereq !MINGW
> +	then
> +		chmod +x "$1"
> +	fi

> On Wed, Aug 10 2022, Johannes Schindelin via GitGitGadget wrote:
>
> > From: Johannes Schindelin <johannes.schindelin@gmx.de>
> > [...]
> > However, this quirk is only in effect as long as `chmod` is run inside
> > the pseudo Unix root directory structure or within the home directory.
> > When run outside, such invocations fail like this:
> >
> > 	chmod: changing permissions of '<file>': Invalid argument
>
> ..ok, but...
>
> > diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
> > index 6da7273f1d5..7c63b22acab 100644
> > --- a/t/test-lib-functions.sh
> > +++ b/t/test-lib-functions.sh
> > @@ -492,7 +492,10 @@ test_commit_bulk () {
> >  # of a file in the working directory and add it to the index.
> >
> >  test_chmod () {
> > -	chmod "$@" &&
> > +	if test_have_prereq !MINGW
> > +	then
> > +		chmod "$@"
> > +	fi &&
> >  	git update-index --add "--chmod=$@"
> >  }
> >
> > @@ -548,7 +551,10 @@ write_script () {
> >  		echo "#!${2-"$SHELL_PATH"}" &&
> >  		cat
> >  	} >"$1" &&
> > -	chmod +x "$1"
> > +	if test_have_prereq !MINGW
> > +	then
> > +		chmod +x "$1"
> > +	fi
>
> ... you get +x semantics by default, so we didn't need that "chmod +x"
> in the first place?

> If so isn't something in this direction (untested) a more targeted &
> obvious fix?:
>
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index 10258def7be..1c3b6692388 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -1690,6 +1690,16 @@ case $uname_s in
>  	find () {
>  		/usr/bin/find "$@"
>  	}
> +	chmod () {
> +		case "$1" in
> +		+x|-x)
> +			return;
> +			;;
> +		*)
> +			;;
> +		esac &&
> +		/usr/bin/chmod "$@"
> +	}
>  	# git sees Windows-style pwd
>  	pwd () {
>  		builtin pwd -W
>
>

> Instead of wasting the run time (even on non-Windows platforms!) to
> determine whether the `MINGW` prereq is set in order to skip the `chmod`
> call or not, we can make `chmod` a no-op explicitly in that `case
> $uname_s` block.
>
> I will make it so.

> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> In the interactive `add` operation, users can choose to jump to specific
> hunks, and Git will present the hunk list in that case. To avoid showing
> too many lines at once, only a maximum of 21 hunks are shown, skipping
> the "mode change" pseudo hunk.
>
> The comparison performed to skip the "mode change" pseudo hunk (if any)
> compares a signed integer `i` to the unsigned value `mode_change` (which
> can be 0 or 1 because it is a 1-bit type).
>
> According to section 6.3.1.8 of the C99 standard (see e.g.
> https://www.open-std.org/jtc1/sc22/WG14/www/docs/n1256.pdf), what should
> happen is an automatic conversion of the "lesser" type to the "greater"
> type, but since the types differ in signedness, it is ill-defined what
> is the correct "usual arithmetic conversion".
>
> Which means that Visual C's behavior can (and does) differ from GCC's:
> When compiling Git using the latter, `add -p`'s `goto` command shows no
> hunks by default because it casts a negative start offset to a pretty
> large unsigned value, breaking the "goto hunk" test case in
> `t3701-add-interactive.sh`.
>
> Let's avoid that by converting the unsigned bit explicitly to a signed
> integer.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---

>  add-patch.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/add-patch.c b/add-patch.c
> index 509ca04456b..3524555e2b0 100644
> --- a/add-patch.c
> +++ b/add-patch.c
> @@ -1547,7 +1547,7 @@ soft_increment:
>  			strbuf_remove(&s->answer, 0, 1);
>  			strbuf_trim(&s->answer);
>  			i = hunk_index - DISPLAY_HUNKS_LINES / 2;
> -			if (i < file_diff->mode_change)
> +			if (i < (int)file_diff->mode_change)
>  				i = file_diff->mode_change;
>  			while (s->answer.len == 0) {
>  				i = display_hunks(s, file_diff, i);

> "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
> writes:
>
> > From: Johannes Schindelin <johannes.schindelin@gmx.de>
> >
> > In the interactive `add` operation, users can choose to jump to specific
> > hunks, and Git will present the hunk list in that case. To avoid showing
> > too many lines at once, only a maximum of 21 hunks are shown, skipping
> > the "mode change" pseudo hunk.
> >
> > The comparison performed to skip the "mode change" pseudo hunk (if any)
> > compares a signed integer `i` to the unsigned value `mode_change` (which
> > can be 0 or 1 because it is a 1-bit type).
> >
> > According to section 6.3.1.8 of the C99 standard (see e.g.
> > https://www.open-std.org/jtc1/sc22/WG14/www/docs/n1256.pdf), what should
> > happen is an automatic conversion of the "lesser" type to the "greater"
> > type, but since the types differ in signedness, it is ill-defined what
> > is the correct "usual arithmetic conversion".
> >
> > Which means that Visual C's behavior can (and does) differ from GCC's:
> > When compiling Git using the latter, `add -p`'s `goto` command shows no
> > hunks by default because it casts a negative start offset to a pretty
> > large unsigned value, breaking the "goto hunk" test case in
> > `t3701-add-interactive.sh`.
> >
> > Let's avoid that by converting the unsigned bit explicitly to a signed
> > integer.
> >
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > ---
>
> This looks more like a fix to a general problem, not limited to
> windows or cmake, that we had since 9254bdfb (built-in add -p:
> implement the 'g' ("goto") command, 2019-12-13).
>
> Please pull this out of the series and let's have it reviewed
> separately.

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > The scope of this patch series is to fix running the tests in Visual
> > Studio when building using CMake.
>
> That's only the perspective of who cares about VS+CMake.  From my
> point of view who wants a healthy Git overall, the priority is
> different.  "add -p" fix is wider than the context of VS, and I do
> not discount the need that we need to fix it before we can call
> VS+CMake issue fixed (and I do not mean to say it is unnecessary to
> fix VS+CMake).  It's just this one can proceed with help by those
> who do not care about or have no environment to help with VS+CMake
> because it is more generic, and I do not think you mind to make the
> rest of the narrower VS+CMake topic _depend_ on it.

> > Pulling out this patch would break that patch series because it would
> > leave that breakage in place.
>
> We deal with topic that depends on another topic all the time, and I
> do not think there is anything that makes VS+CMake topic so special
> that it cannot be dependent on another topic.  It's just the matter
> of splitting this out and make it [1/1], and make the remainder to
> [1-4/4] and mark them rely on add-p fix when you send the latter
> out, isn't it?  Puzzled...

> From: Johannes Schindelin <johannes.schindelin@gmx.de>
> 
> In the interactive `add` operation, users can choose to jump to specific
> hunks, and Git will present the hunk list in that case. To avoid showing
> too many lines at once, only a maximum of 21 hunks are shown, skipping
> the "mode change" pseudo hunk.
> 
> The comparison performed to skip the "mode change" pseudo hunk (if any)
> compares a signed integer `i` to the unsigned value `mode_change` (which
> can be 0 or 1 because it is a 1-bit type).
> 
> According to section 6.3.1.8 of the C99 standard (see e.g.
> https://www.open-std.org/jtc1/sc22/WG14/www/docs/n1256.pdf), what should
> happen is an automatic conversion of the "lesser" type to the "greater"
> type, but since the types differ in signedness, it is ill-defined what
> is the correct "usual arithmetic conversion".
> 
> Which means that Visual C's behavior can (and does) differ from GCC's:
> When compiling Git using the latter, `add -p`'s `goto` command shows no
> hunks by default because it casts a negative start offset to a pretty
> large unsigned value, breaking the "goto hunk" test case in
> `t3701-add-interactive.sh`.
> 
> Let's avoid that by converting the unsigned bit explicitly to a signed
> integer.

> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>   add-patch.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/add-patch.c b/add-patch.c
> index 509ca04456b..3524555e2b0 100644
> --- a/add-patch.c
> +++ b/add-patch.c
> @@ -1547,7 +1547,7 @@ soft_increment:
>   			strbuf_remove(&s->answer, 0, 1);
>   			strbuf_trim(&s->answer);

> On 10/08/2022 16:02, Johannes Schindelin via GitGitGadget wrote:
>
> > diff --git a/add-patch.c b/add-patch.c
> > index 509ca04456b..3524555e2b0 100644
> > --- a/add-patch.c
> > +++ b/add-patch.c
> > @@ -1547,7 +1547,7 @@ soft_increment:
> >      strbuf_remove(&s->answer, 0, 1);
> >      strbuf_trim(&s->answer);
>
> Unrelated to this change but why don't we just call strbuf_reset() here?

> Hi Phillip,
> 
> On Thu, 11 Aug 2022, Phillip Wood wrote:
> 
>> On 10/08/2022 16:02, Johannes Schindelin via GitGitGadget wrote:
>>
>>> diff --git a/add-patch.c b/add-patch.c
>>> index 509ca04456b..3524555e2b0 100644
>>> --- a/add-patch.c
>>> +++ b/add-patch.c
>>> @@ -1547,7 +1547,7 @@ soft_increment:
>>>       strbuf_remove(&s->answer, 0, 1);
>>>       strbuf_trim(&s->answer);
>>
>> Unrelated to this change but why don't we just call strbuf_reset() here?
> 
> This part of the code is used when the `g` command (to "go to hunk") was
> issued by the user. And that `g` command allows for a number to be
> specified, e.g. `g1` to go to the first hunk.
> 
> The `strbuf_remove(&s->answer, 0, 1)` removes the `g` from the command.
> 
> The `strbuf_trim(&s->answer)` allows for whitespace between the `g` and
> the number, e.g. `g 1` should also go to the first hunk.
> 
> If we called `strbuf_reset()` here, we would remove the number completely.

> On 16/08/2022 11:00, Johannes Schindelin wrote:
>
> > On Thu, 11 Aug 2022, Phillip Wood wrote:
> >
> > > On 10/08/2022 16:02, Johannes Schindelin via GitGitGadget wrote:
> > >
> > > > diff --git a/add-patch.c b/add-patch.c
> > > > index 509ca04456b..3524555e2b0 100644
> > > > --- a/add-patch.c
> > > > +++ b/add-patch.c
> > > > @@ -1547,7 +1547,7 @@ soft_increment:
> > > >       strbuf_remove(&s->answer, 0, 1);
> > > >       strbuf_trim(&s->answer);
> > >
> > > Unrelated to this change but why don't we just call strbuf_reset() here?
> >
> > This part of the code is used when the `g` command (to "go to hunk") was
> > issued by the user. And that `g` command allows for a number to be
> > specified, e.g. `g1` to go to the first hunk.
> >
> > The `strbuf_remove(&s->answer, 0, 1)` removes the `g` from the command.
> >
> > The `strbuf_trim(&s->answer)` allows for whitespace between the `g` and
> > the number, e.g. `g 1` should also go to the first hunk.
> >
> > If we called `strbuf_reset()` here, we would remove the number completely.
>
> Oh so if the user is not using interactive.singleKey then they can skip the
> prompt which displays the hunks and asks which one they want to jump to by
> guessing the number of the hunk they want and typing "g <n>".

> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> In 7f5397a07c6c (cmake: support for testing git when building out of the
> source tree, 2020-06-26), we implemented support for running Git's test
> scripts even after building Git in a different directory than the source
> directory.
>
> The way we did this was to edit the file `t/test-lib.sh` to override
> `GIT_BUILD_DIR` to point somewhere else than the parent of the `t/`
> directory.
>
> This is unideal because it always leaves a tracked file marked as
> modified, and it is all too easy to commit that change by mistake.
>
> Let's change the strategy by teaching `t/test-lib.sh` to detect the
> presence of a file called `GIT-BUILD-DIR` in the source directory. If it
> exists, the contents are interpreted as the location to the _actual_
> build directory. We then write this file as part of the CTest
> definition.
>
> To support building Git via a regular `make` invocation after building
> it using CMake, we ensure that the `GIT-BUILD-DIR` file is deleted (for
> convenience, this is done as part of the Makefile rule that is already
> run with every `make` invocation to ensure that `GIT-BUILD-OPTIONS` is
> up to date).
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  .gitignore                          |  1 +
>  Makefile                            |  1 +
>  contrib/buildsystems/CMakeLists.txt |  7 +------
>  t/test-lib.sh                       | 11 ++++++++++-
>  4 files changed, 13 insertions(+), 7 deletions(-)
>
> diff --git a/.gitignore b/.gitignore
> index a4522157641..b72ddf09346 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -2,6 +2,7 @@
>  /fuzz_corpora
>  /fuzz-pack-headers
>  /fuzz-pack-idx
> +/GIT-BUILD-DIR
>  /GIT-BUILD-OPTIONS
>  /GIT-CFLAGS
>  /GIT-LDFLAGS
> diff --git a/Makefile b/Makefile
> index 04d0fd1fe60..9347ed90da7 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -3028,6 +3028,7 @@ else
>  	@echo RUNTIME_PREFIX=\'false\' >>$@+
>  endif
>  	@if cmp $@+ $@ >/dev/null 2>&1; then $(RM) $@+; else mv $@+ $@; fi
> +	@if test -f GIT-BUILD-DIR; then rm GIT-BUILD-DIR; fi
>  
>  ### Detect Python interpreter path changes
>  ifndef NO_PYTHON
> diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt
> index fe606c179f7..29d7e236ae1 100644
> --- a/contrib/buildsystems/CMakeLists.txt
> +++ b/contrib/buildsystems/CMakeLists.txt
> @@ -1067,14 +1067,9 @@ endif()
>  #Make the tests work when building out of the source tree
>  get_filename_component(CACHE_PATH ${CMAKE_CURRENT_LIST_DIR}/../../CMakeCache.txt ABSOLUTE)
>  if(NOT ${CMAKE_BINARY_DIR}/CMakeCache.txt STREQUAL ${CACHE_PATH})
> -	file(RELATIVE_PATH BUILD_DIR_RELATIVE ${CMAKE_SOURCE_DIR} ${CMAKE_BINARY_DIR}/CMakeCache.txt)
> -	string(REPLACE "/CMakeCache.txt" "" BUILD_DIR_RELATIVE ${BUILD_DIR_RELATIVE})
>  	#Setting the build directory in test-lib.sh before running tests
>  	file(WRITE ${CMAKE_BINARY_DIR}/CTestCustom.cmake
> -		"file(STRINGS ${CMAKE_SOURCE_DIR}/t/test-lib.sh GIT_BUILD_DIR_REPL REGEX \"GIT_BUILD_DIR=(.*)\")\n"
> -		"file(STRINGS ${CMAKE_SOURCE_DIR}/t/test-lib.sh content NEWLINE_CONSUME)\n"
> -		"string(REPLACE \"\${GIT_BUILD_DIR_REPL}\" \"GIT_BUILD_DIR=\\\"$TEST_DIRECTORY/../${BUILD_DIR_RELATIVE}\\\"\" content \"\${content}\")\n"
> -		"file(WRITE ${CMAKE_SOURCE_DIR}/t/test-lib.sh \${content})")
> +		"file(WRITE ${CMAKE_SOURCE_DIR}/GIT-BUILD-DIR \"${CMAKE_BINARY_DIR}\")")
>  	#misc copies
>  	file(COPY ${CMAKE_SOURCE_DIR}/t/chainlint.sed DESTINATION ${CMAKE_BINARY_DIR}/t/)
>  	file(COPY ${CMAKE_SOURCE_DIR}/po/is.po DESTINATION ${CMAKE_BINARY_DIR}/po/)
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index 55857af601b..4468ac51f25 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -42,7 +42,16 @@ then
>  	TEST_OUTPUT_DIRECTORY=$TEST_DIRECTORY
>  fi
>  GIT_BUILD_DIR="${TEST_DIRECTORY%/t}"
> -if test "$TEST_DIRECTORY" = "$GIT_BUILD_DIR"
> +if test -f "$GIT_BUILD_DIR/GIT-BUILD-DIR"
> +then
> +	GIT_BUILD_DIR="$(cat "$GIT_BUILD_DIR/GIT-BUILD-DIR")" || exit 1
> +	# On Windows, we must convert Windows paths lest they contain a colon
> +	case "$(uname -s)" in
> +	*MINGW*)
> +		GIT_BUILD_DIR="$(cygpath -au "$GIT_BUILD_DIR")"
> +		;;
> +	esac
> +elif test "$TEST_DIRECTORY" = "$GIT_BUILD_DIR"
>  then
>  	echo "PANIC: Running in a $TEST_DIRECTORY that doesn't end in '/t'?" >&2
>  	exit 1

> On Wed, Aug 10 2022, Johannes Schindelin via GitGitGadget wrote:
>
> > From: Johannes Schindelin <johannes.schindelin@gmx.de>
> >
> > In 7f5397a07c6c (cmake: support for testing git when building out of the
> > source tree, 2020-06-26), we implemented support for running Git's test
> > scripts even after building Git in a different directory than the source
> > directory.
> >
> > The way we did this was to edit the file `t/test-lib.sh` to override
> > `GIT_BUILD_DIR` to point somewhere else than the parent of the `t/`
> > directory.
> >
> > This is unideal because it always leaves a tracked file marked as
> > modified, and it is all too easy to commit that change by mistake.
> >
> > Let's change the strategy by teaching `t/test-lib.sh` to detect the
> > presence of a file called `GIT-BUILD-DIR` in the source directory. If it
> > exists, the contents are interpreted as the location to the _actual_
> > build directory. We then write this file as part of the CTest
> > definition.
> >
> > To support building Git via a regular `make` invocation after building
> > it using CMake, we ensure that the `GIT-BUILD-DIR` file is deleted (for
> > convenience, this is done as part of the Makefile rule that is already
> > run with every `make` invocation to ensure that `GIT-BUILD-OPTIONS` is
> > up to date).
> >
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > ---
> >  .gitignore                          |  1 +
> >  Makefile                            |  1 +
> >  contrib/buildsystems/CMakeLists.txt |  7 +------
> >  t/test-lib.sh                       | 11 ++++++++++-
> >  4 files changed, 13 insertions(+), 7 deletions(-)
> >
> > diff --git a/.gitignore b/.gitignore
> > index a4522157641..b72ddf09346 100644
> > --- a/.gitignore
> > +++ b/.gitignore
> > @@ -2,6 +2,7 @@
> >  /fuzz_corpora
> >  /fuzz-pack-headers
> >  /fuzz-pack-idx
> > +/GIT-BUILD-DIR
> >  /GIT-BUILD-OPTIONS
> >  /GIT-CFLAGS
> >  /GIT-LDFLAGS
> > diff --git a/Makefile b/Makefile
> > index 04d0fd1fe60..9347ed90da7 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -3028,6 +3028,7 @@ else
> >  	@echo RUNTIME_PREFIX=\'false\' >>$@+
> >  endif
> >  	@if cmp $@+ $@ >/dev/null 2>&1; then $(RM) $@+; else mv $@+ $@; fi
> > +	@if test -f GIT-BUILD-DIR; then rm GIT-BUILD-DIR; fi
> >
> >  ### Detect Python interpreter path changes
> >  ifndef NO_PYTHON
> > diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt
> > index fe606c179f7..29d7e236ae1 100644
> > --- a/contrib/buildsystems/CMakeLists.txt
> > +++ b/contrib/buildsystems/CMakeLists.txt
> > @@ -1067,14 +1067,9 @@ endif()
> >  #Make the tests work when building out of the source tree
> >  get_filename_component(CACHE_PATH ${CMAKE_CURRENT_LIST_DIR}/../../CMakeCache.txt ABSOLUTE)
> >  if(NOT ${CMAKE_BINARY_DIR}/CMakeCache.txt STREQUAL ${CACHE_PATH})
> > -	file(RELATIVE_PATH BUILD_DIR_RELATIVE ${CMAKE_SOURCE_DIR} ${CMAKE_BINARY_DIR}/CMakeCache.txt)
> > -	string(REPLACE "/CMakeCache.txt" "" BUILD_DIR_RELATIVE ${BUILD_DIR_RELATIVE})
> >  	#Setting the build directory in test-lib.sh before running tests
> >  	file(WRITE ${CMAKE_BINARY_DIR}/CTestCustom.cmake
> > -		"file(STRINGS ${CMAKE_SOURCE_DIR}/t/test-lib.sh GIT_BUILD_DIR_REPL REGEX \"GIT_BUILD_DIR=(.*)\")\n"
> > -		"file(STRINGS ${CMAKE_SOURCE_DIR}/t/test-lib.sh content NEWLINE_CONSUME)\n"
> > -		"string(REPLACE \"\${GIT_BUILD_DIR_REPL}\" \"GIT_BUILD_DIR=\\\"$TEST_DIRECTORY/../${BUILD_DIR_RELATIVE}\\\"\" content \"\${content}\")\n"
> > -		"file(WRITE ${CMAKE_SOURCE_DIR}/t/test-lib.sh \${content})")
> > +		"file(WRITE ${CMAKE_SOURCE_DIR}/GIT-BUILD-DIR \"${CMAKE_BINARY_DIR}\")")
> >  	#misc copies
> >  	file(COPY ${CMAKE_SOURCE_DIR}/t/chainlint.sed DESTINATION ${CMAKE_BINARY_DIR}/t/)
> >  	file(COPY ${CMAKE_SOURCE_DIR}/po/is.po DESTINATION ${CMAKE_BINARY_DIR}/po/)
> > diff --git a/t/test-lib.sh b/t/test-lib.sh
> > index 55857af601b..4468ac51f25 100644
> > --- a/t/test-lib.sh
> > +++ b/t/test-lib.sh
> > @@ -42,7 +42,16 @@ then
> >  	TEST_OUTPUT_DIRECTORY=$TEST_DIRECTORY
> >  fi
> >  GIT_BUILD_DIR="${TEST_DIRECTORY%/t}"
> > -if test "$TEST_DIRECTORY" = "$GIT_BUILD_DIR"
> > +if test -f "$GIT_BUILD_DIR/GIT-BUILD-DIR"
> > +then
> > +	GIT_BUILD_DIR="$(cat "$GIT_BUILD_DIR/GIT-BUILD-DIR")" || exit 1
> > +	# On Windows, we must convert Windows paths lest they contain a colon
> > +	case "$(uname -s)" in
> > +	*MINGW*)
> > +		GIT_BUILD_DIR="$(cygpath -au "$GIT_BUILD_DIR")"
> > +		;;
> > +	esac
> > +elif test "$TEST_DIRECTORY" = "$GIT_BUILD_DIR"
> >  then
> >  	echo "PANIC: Running in a $TEST_DIRECTORY that doesn't end in '/t'?" >&2
> >  	exit 1
>
> So we'll now skip the assertion added in 9dbf20e7f62 (test-lib: correct
> and assert TEST_DIRECTORY overriding, 2022-02-27), shouldn't these tests
> run inside a "t" directory generated in contrib/buildsystems/ ?
>
> This really seems like a hack-upon-hack, and will break some workflows
> when you switch back & forth, e.g.:
>
> 	make
> 	# run cmake
> 	make -C t
>
> Will now pick up this new file, and result in broken tests, as we'll
> rely on the now-stale file.
>
> IOW you're making the assumption that by piggy-backing on the
> GIT-BUILD-OPTIONS rule that anything that needs it will also *re-run
> it*, but that's not the case.
>
> I can think of ways to get around it, but it would be nasty as we'd need
> to complete the dependency graph between the two, and figure out the
> various interactions.
>
> But why do we need this at all? Right now I tried:
>
>  * (Manually) removing the existing hack
>  * Copying t/*lib*.sh over to contrib/buildsystems/t
>  * cd contrib/buildsystems/t
>  * ../../../t/t0001-init.sh

> Which (aside from a small fixable nit about oid-info) Just Works,
> because the cmake build is already creating a GIT-BUILD-OPTIONS.
>
> Presumably that "copying" step should be simlynking, or we'd be smarter
> about doing includes from test-lib.sh.
>
> But isn't that a much better approach? Rather than working around the
> directory discovery just teach it to run from the generated t/ directory
> directly?

> From: Johannes Schindelin <johannes.schindelin@gmx.de>
> 
> In 7f5397a07c6c (cmake: support for testing git when building out of the
> source tree, 2020-06-26), we implemented support for running Git's test
> scripts even after building Git in a different directory than the source
> directory.
> 
> The way we did this was to edit the file `t/test-lib.sh` to override
> `GIT_BUILD_DIR` to point somewhere else than the parent of the `t/`
> directory.
> 
> This is unideal because it always leaves a tracked file marked as
> modified, and it is all too easy to commit that change by mistake.
> 
> Let's change the strategy by teaching `t/test-lib.sh` to detect the
> presence of a file called `GIT-BUILD-DIR` in the source directory. If it
> exists, the contents are interpreted as the location to the _actual_
> build directory. We then write this file as part of the CTest
> definition.

> To support building Git via a regular `make` invocation after building
> it using CMake, we ensure that the `GIT-BUILD-DIR` file is deleted (for
> convenience, this is done as part of the Makefile rule that is already
> run with every `make` invocation to ensure that `GIT-BUILD-OPTIONS` is
> up to date).
> 
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>   .gitignore                          |  1 +
>   Makefile                            |  1 +
>   contrib/buildsystems/CMakeLists.txt |  7 +------
>   t/test-lib.sh                       | 11 ++++++++++-
>   4 files changed, 13 insertions(+), 7 deletions(-)
> 
> diff --git a/.gitignore b/.gitignore
> index a4522157641..b72ddf09346 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -2,6 +2,7 @@
>   /fuzz_corpora
>   /fuzz-pack-headers
>   /fuzz-pack-idx
> +/GIT-BUILD-DIR
>   /GIT-BUILD-OPTIONS
>   /GIT-CFLAGS
>   /GIT-LDFLAGS
> diff --git a/Makefile b/Makefile
> index 04d0fd1fe60..9347ed90da7 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -3028,6 +3028,7 @@ else
>   	@echo RUNTIME_PREFIX=\'false\' >>$@+
>   endif
>   	@if cmp $@+ $@ >/dev/null 2>&1; then $(RM) $@+; else mv $@+ $@; fi
> +	@if test -f GIT-BUILD-DIR; then rm GIT-BUILD-DIR; fi
>   
>   ### Detect Python interpreter path changes
>   ifndef NO_PYTHON
> diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt
> index fe606c179f7..29d7e236ae1 100644
> --- a/contrib/buildsystems/CMakeLists.txt
> +++ b/contrib/buildsystems/CMakeLists.txt
> @@ -1067,14 +1067,9 @@ endif()
>   #Make the tests work when building out of the source tree
>   get_filename_component(CACHE_PATH ${CMAKE_CURRENT_LIST_DIR}/../../CMakeCache.txt ABSOLUTE)
>   if(NOT ${CMAKE_BINARY_DIR}/CMakeCache.txt STREQUAL ${CACHE_PATH})
> -	file(RELATIVE_PATH BUILD_DIR_RELATIVE ${CMAKE_SOURCE_DIR} ${CMAKE_BINARY_DIR}/CMakeCache.txt)
> -	string(REPLACE "/CMakeCache.txt" "" BUILD_DIR_RELATIVE ${BUILD_DIR_RELATIVE})
>   	#Setting the build directory in test-lib.sh before running tests
>   	file(WRITE ${CMAKE_BINARY_DIR}/CTestCustom.cmake
> -		"file(STRINGS ${CMAKE_SOURCE_DIR}/t/test-lib.sh GIT_BUILD_DIR_REPL REGEX \"GIT_BUILD_DIR=(.*)\")\n"
> -		"file(STRINGS ${CMAKE_SOURCE_DIR}/t/test-lib.sh content NEWLINE_CONSUME)\n"
> -		"string(REPLACE \"\${GIT_BUILD_DIR_REPL}\" \"GIT_BUILD_DIR=\\\"$TEST_DIRECTORY/../${BUILD_DIR_RELATIVE}\\\"\" content \"\${content}\")\n"
> -		"file(WRITE ${CMAKE_SOURCE_DIR}/t/test-lib.sh \${content})")
> +		"file(WRITE ${CMAKE_SOURCE_DIR}/GIT-BUILD-DIR \"${CMAKE_BINARY_DIR}\")")
>   	#misc copies
>   	file(COPY ${CMAKE_SOURCE_DIR}/t/chainlint.sed DESTINATION ${CMAKE_BINARY_DIR}/t/)
>   	file(COPY ${CMAKE_SOURCE_DIR}/po/is.po DESTINATION ${CMAKE_BINARY_DIR}/po/)
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index 55857af601b..4468ac51f25 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -42,7 +42,16 @@ then
>   	TEST_OUTPUT_DIRECTORY=$TEST_DIRECTORY
>   fi
>   GIT_BUILD_DIR="${TEST_DIRECTORY%/t}"
> -if test "$TEST_DIRECTORY" = "$GIT_BUILD_DIR"
> +if test -f "$GIT_BUILD_DIR/GIT-BUILD-DIR"
> +then
> +	GIT_BUILD_DIR="$(cat "$GIT_BUILD_DIR/GIT-BUILD-DIR")" || exit 1
> +	# On Windows, we must convert Windows paths lest they contain a colon
> +	case "$(uname -s)" in
> +	*MINGW*)
> +		GIT_BUILD_DIR="$(cygpath -au "$GIT_BUILD_DIR")"
> +		;;
> +	esac
> +elif test "$TEST_DIRECTORY" = "$GIT_BUILD_DIR"
>   then
>   	echo "PANIC: Running in a $TEST_DIRECTORY that doesn't end in '/t'?" >&2
>   	exit 1

> On 10/08/2022 16:02, Johannes Schindelin via GitGitGadget wrote:
> > From: Johannes Schindelin <johannes.schindelin@gmx.de>
> >
> > In 7f5397a07c6c (cmake: support for testing git when building out of the
> > source tree, 2020-06-26), we implemented support for running Git's test
> > scripts even after building Git in a different directory than the source
> > directory.
> >
> > The way we did this was to edit the file `t/test-lib.sh` to override
> > `GIT_BUILD_DIR` to point somewhere else than the parent of the `t/`
> > directory.
> >
> > This is unideal because it always leaves a tracked file marked as
> > modified, and it is all too easy to commit that change by mistake.
> >
> > Let's change the strategy by teaching `t/test-lib.sh` to detect the
> > presence of a file called `GIT-BUILD-DIR` in the source directory. If it
> > exists, the contents are interpreted as the location to the _actual_
> > build directory. We then write this file as part of the CTest
> > definition.
>
> I think it is really good to get away from editing the test files, but one of
> the nice things about CMake's out of tree builds is that you can have several
> build directories with different build configurations and this change does not
> support that. Could we pass the build directory to the test scripts as a
> commandline option or environment variable instead? e.g.
>
>   foreach(tsh ${test_scipts})
>    	add_test(NAME ${tsh}
> -		COMMAND ${SH_EXE} ${tsh} --no-bin-wrappers --no-chain-lint -vx
> +		COMMAND ${SH_EXE} ${tsh} --no-bin-wrappers --no-chain-lint
> --build-dir=${CMAKE_BINARY_DIR} -vx
>
>   		WORKING_DIRECTORY ${CMAKE_SOURCE_DIR}/t)
>   endforeach()
>
> Doing that would avoid changing the main Makefile to remove a file which
> almost certainly does not exist every time make is invoked as well.

> Hi Phillip,
> 
> On Thu, 11 Aug 2022, Phillip Wood wrote:
> 
>> On 10/08/2022 16:02, Johannes Schindelin via GitGitGadget wrote:
>>> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>>>
>>> In 7f5397a07c6c (cmake: support for testing git when building out of the
>>> source tree, 2020-06-26), we implemented support for running Git's test
>>> scripts even after building Git in a different directory than the source
>>> directory.
>>>
>>> The way we did this was to edit the file `t/test-lib.sh` to override
>>> `GIT_BUILD_DIR` to point somewhere else than the parent of the `t/`
>>> directory.
>>>
>>> This is unideal because it always leaves a tracked file marked as
>>> modified, and it is all too easy to commit that change by mistake.
>>>
>>> Let's change the strategy by teaching `t/test-lib.sh` to detect the
>>> presence of a file called `GIT-BUILD-DIR` in the source directory. If it
>>> exists, the contents are interpreted as the location to the _actual_
>>> build directory. We then write this file as part of the CTest
>>> definition.
>>
>> I think it is really good to get away from editing the test files, but one of
>> the nice things about CMake's out of tree builds is that you can have several
>> build directories with different build configurations and this change does not
>> support that. Could we pass the build directory to the test scripts as a
>> commandline option or environment variable instead? e.g.
>>
>>    foreach(tsh ${test_scipts})
>>     	add_test(NAME ${tsh}
>> -		COMMAND ${SH_EXE} ${tsh} --no-bin-wrappers --no-chain-lint -vx
>> +		COMMAND ${SH_EXE} ${tsh} --no-bin-wrappers --no-chain-lint
>> --build-dir=${CMAKE_BINARY_DIR} -vx
>>
>>    		WORKING_DIRECTORY ${CMAKE_SOURCE_DIR}/t)
>>    endforeach()
>>
>> Doing that would avoid changing the main Makefile to remove a file which
>> almost certainly does not exist every time make is invoked as well.
> 
> This is indeed tempting, but one of the things I recommend to Visual
> Studio users for ages already is to run the tests in a Git Bash, i.e.
> outside of Visual Studio, to allow for passing `--run=...` options and the
> likes. This recommendation predates support for CTest, naturally, but it
> is still valid.
> 
> And that recommended way of running tests would be broken by the suggested
> change because the tests would no longer run except when using CTest.

> Besides, while the semantics look tempting, the implementation details do
> not. The reason is that we use `GIT_BUILD_DIR` to source the
> `GIT-BUILD-OPTIONS` and to validate that Git was built, long before we
> parse the command-line in `t/test-lib.sh`. I want to refrain from the
> prerequisite extensive refactoring required to support this, at least in
> the context of this here patch series.

> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> In 7f5397a07c6c (cmake: support for testing git when building out of the
> source tree, 2020-06-26), we implemented support for running Git's test
> scripts even after building Git in a different directory than the source
> directory.
>
> The way we did this was to edit the file `t/test-lib.sh` to override
> `GIT_BUILD_DIR` to point somewhere else than the parent of the `t/`
> directory.
>
> This is unideal because it always leaves a tracked file marked as
> modified, and it is all too easy to commit that change by mistake.
>
> Let's change the strategy by teaching `t/test-lib.sh` to detect the
> presence of a file called `GIT-BUILD-DIR` in the source directory. If it
> exists, the contents are interpreted as the location to the _actual_
> build directory. We then write this file as part of the CTest
> definition.
>
> To support building Git via a regular `make` invocation after building
> it using CMake, we ensure that the `GIT-BUILD-DIR` file is deleted (for
> convenience, this is done as part of the Makefile rule that is already
> run with every `make` invocation to ensure that `GIT-BUILD-OPTIONS` is
> up to date).
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  .gitignore                          |  1 +
>  Makefile                            |  1 +
>  contrib/buildsystems/CMakeLists.txt |  7 +------
>  t/test-lib.sh                       | 11 ++++++++++-
>  4 files changed, 13 insertions(+), 7 deletions(-)
>
> diff --git a/.gitignore b/.gitignore
> index a4522157641..b72ddf09346 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -2,6 +2,7 @@
>  /fuzz_corpora
>  /fuzz-pack-headers
>  /fuzz-pack-idx
> +/GIT-BUILD-DIR
>  /GIT-BUILD-OPTIONS
>  /GIT-CFLAGS
>  /GIT-LDFLAGS
> diff --git a/Makefile b/Makefile
> index 04d0fd1fe60..9347ed90da7 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -3028,6 +3028,7 @@ else
>  	@echo RUNTIME_PREFIX=\'false\' >>$@+
>  endif
>  	@if cmp $@+ $@ >/dev/null 2>&1; then $(RM) $@+; else mv $@+ $@; fi
> +	@if test -f GIT-BUILD-DIR; then rm GIT-BUILD-DIR; fi
>
>  ### Detect Python interpreter path changes
>  ifndef NO_PYTHON
> diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt
> index fe606c179f7..29d7e236ae1 100644
> --- a/contrib/buildsystems/CMakeLists.txt
> +++ b/contrib/buildsystems/CMakeLists.txt
> @@ -1067,14 +1067,9 @@ endif()
>  #Make the tests work when building out of the source tree
>  get_filename_component(CACHE_PATH ${CMAKE_CURRENT_LIST_DIR}/../../CMakeCache.txt ABSOLUTE)
>  if(NOT ${CMAKE_BINARY_DIR}/CMakeCache.txt STREQUAL ${CACHE_PATH})
> -	file(RELATIVE_PATH BUILD_DIR_RELATIVE ${CMAKE_SOURCE_DIR} ${CMAKE_BINARY_DIR}/CMakeCache.txt)
> -	string(REPLACE "/CMakeCache.txt" "" BUILD_DIR_RELATIVE ${BUILD_DIR_RELATIVE})
>  	#Setting the build directory in test-lib.sh before running tests
>  	file(WRITE ${CMAKE_BINARY_DIR}/CTestCustom.cmake
> -		"file(STRINGS ${CMAKE_SOURCE_DIR}/t/test-lib.sh GIT_BUILD_DIR_REPL REGEX \"GIT_BUILD_DIR=(.*)\")\n"
> -		"file(STRINGS ${CMAKE_SOURCE_DIR}/t/test-lib.sh content NEWLINE_CONSUME)\n"
> -		"string(REPLACE \"\${GIT_BUILD_DIR_REPL}\" \"GIT_BUILD_DIR=\\\"$TEST_DIRECTORY/../${BUILD_DIR_RELATIVE}\\\"\" content \"\${content}\")\n"
> -		"file(WRITE ${CMAKE_SOURCE_DIR}/t/test-lib.sh \${content})")
> +		"file(WRITE ${CMAKE_SOURCE_DIR}/GIT-BUILD-DIR \"${CMAKE_BINARY_DIR}\")")
>  	#misc copies
>  	file(COPY ${CMAKE_SOURCE_DIR}/t/chainlint.sed DESTINATION ${CMAKE_BINARY_DIR}/t/)
>  	file(COPY ${CMAKE_SOURCE_DIR}/po/is.po DESTINATION ${CMAKE_BINARY_DIR}/po/)
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index 55857af601b..4468ac51f25 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -42,7 +42,16 @@ then
>  	TEST_OUTPUT_DIRECTORY=$TEST_DIRECTORY
>  fi
>  GIT_BUILD_DIR="${TEST_DIRECTORY%/t}"
> -if test "$TEST_DIRECTORY" = "$GIT_BUILD_DIR"
> +if test -f "$GIT_BUILD_DIR/GIT-BUILD-DIR"
> +then
> +	GIT_BUILD_DIR="$(cat "$GIT_BUILD_DIR/GIT-BUILD-DIR")" || exit 1
> +	# On Windows, we must convert Windows paths lest they contain a colon
> +	case "$(uname -s)" in
> +	*MINGW*)
> +		GIT_BUILD_DIR="$(cygpath -au "$GIT_BUILD_DIR")"
> +		;;
> +	esac
> +elif test "$TEST_DIRECTORY" = "$GIT_BUILD_DIR"
>  then
>  	echo "PANIC: Running in a $TEST_DIRECTORY that doesn't end in '/t'?" >&2
>  	exit 1

> On Tue, Aug 23 2022, Johannes Schindelin via GitGitGadget wrote:
>
> > From: Johannes Schindelin <johannes.schindelin@gmx.de>
> >
> > In 7f5397a07c6c (cmake: support for testing git when building out of the
> > source tree, 2020-06-26), we implemented support for running Git's test
> > scripts even after building Git in a different directory than the source
> > directory.
> >
> > The way we did this was to edit the file `t/test-lib.sh` to override
> > `GIT_BUILD_DIR` to point somewhere else than the parent of the `t/`
> > directory.
> >
> > This is unideal because it always leaves a tracked file marked as
> > modified, and it is all too easy to commit that change by mistake.
> >
> > Let's change the strategy by teaching `t/test-lib.sh` to detect the
> > presence of a file called `GIT-BUILD-DIR` in the source directory. If it
> > exists, the contents are interpreted as the location to the _actual_
> > build directory. We then write this file as part of the CTest
> > definition.
> >
> > To support building Git via a regular `make` invocation after building
> > it using CMake, we ensure that the `GIT-BUILD-DIR` file is deleted (for
> > convenience, this is done as part of the Makefile rule that is already
> > run with every `make` invocation to ensure that `GIT-BUILD-OPTIONS` is
> > up to date).
> >
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > ---
> >  .gitignore                          |  1 +
> >  Makefile                            |  1 +
> >  contrib/buildsystems/CMakeLists.txt |  7 +------
> >  t/test-lib.sh                       | 11 ++++++++++-
> >  4 files changed, 13 insertions(+), 7 deletions(-)
> >
> > diff --git a/.gitignore b/.gitignore
> > index a4522157641..b72ddf09346 100644
> > --- a/.gitignore
> > +++ b/.gitignore
> > @@ -2,6 +2,7 @@
> >  /fuzz_corpora
> >  /fuzz-pack-headers
> >  /fuzz-pack-idx
> > +/GIT-BUILD-DIR
> >  /GIT-BUILD-OPTIONS
> >  /GIT-CFLAGS
> >  /GIT-LDFLAGS
> > diff --git a/Makefile b/Makefile
> > index 04d0fd1fe60..9347ed90da7 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -3028,6 +3028,7 @@ else
> >  	@echo RUNTIME_PREFIX=\'false\' >>$@+
> >  endif
> >  	@if cmp $@+ $@ >/dev/null 2>&1; then $(RM) $@+; else mv $@+ $@; fi
> > +	@if test -f GIT-BUILD-DIR; then rm GIT-BUILD-DIR; fi
> >
> >  ### Detect Python interpreter path changes
> >  ifndef NO_PYTHON
> > diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt
> > index fe606c179f7..29d7e236ae1 100644
> > --- a/contrib/buildsystems/CMakeLists.txt
> > +++ b/contrib/buildsystems/CMakeLists.txt
> > @@ -1067,14 +1067,9 @@ endif()
> >  #Make the tests work when building out of the source tree
> >  get_filename_component(CACHE_PATH ${CMAKE_CURRENT_LIST_DIR}/../../CMakeCache.txt ABSOLUTE)
> >  if(NOT ${CMAKE_BINARY_DIR}/CMakeCache.txt STREQUAL ${CACHE_PATH})
> > -	file(RELATIVE_PATH BUILD_DIR_RELATIVE ${CMAKE_SOURCE_DIR} ${CMAKE_BINARY_DIR}/CMakeCache.txt)
> > -	string(REPLACE "/CMakeCache.txt" "" BUILD_DIR_RELATIVE ${BUILD_DIR_RELATIVE})
> >  	#Setting the build directory in test-lib.sh before running tests
> >  	file(WRITE ${CMAKE_BINARY_DIR}/CTestCustom.cmake
> > -		"file(STRINGS ${CMAKE_SOURCE_DIR}/t/test-lib.sh GIT_BUILD_DIR_REPL REGEX \"GIT_BUILD_DIR=(.*)\")\n"
> > -		"file(STRINGS ${CMAKE_SOURCE_DIR}/t/test-lib.sh content NEWLINE_CONSUME)\n"
> > -		"string(REPLACE \"\${GIT_BUILD_DIR_REPL}\" \"GIT_BUILD_DIR=\\\"$TEST_DIRECTORY/../${BUILD_DIR_RELATIVE}\\\"\" content \"\${content}\")\n"
> > -		"file(WRITE ${CMAKE_SOURCE_DIR}/t/test-lib.sh \${content})")
> > +		"file(WRITE ${CMAKE_SOURCE_DIR}/GIT-BUILD-DIR \"${CMAKE_BINARY_DIR}\")")
> >  	#misc copies
> >  	file(COPY ${CMAKE_SOURCE_DIR}/t/chainlint.sed DESTINATION ${CMAKE_BINARY_DIR}/t/)
> >  	file(COPY ${CMAKE_SOURCE_DIR}/po/is.po DESTINATION ${CMAKE_BINARY_DIR}/po/)
> > diff --git a/t/test-lib.sh b/t/test-lib.sh
> > index 55857af601b..4468ac51f25 100644
> > --- a/t/test-lib.sh
> > +++ b/t/test-lib.sh
> > @@ -42,7 +42,16 @@ then
> >  	TEST_OUTPUT_DIRECTORY=$TEST_DIRECTORY
> >  fi
> >  GIT_BUILD_DIR="${TEST_DIRECTORY%/t}"
> > -if test "$TEST_DIRECTORY" = "$GIT_BUILD_DIR"
> > +if test -f "$GIT_BUILD_DIR/GIT-BUILD-DIR"
> > +then
> > +	GIT_BUILD_DIR="$(cat "$GIT_BUILD_DIR/GIT-BUILD-DIR")" || exit 1
> > +	# On Windows, we must convert Windows paths lest they contain a colon
> > +	case "$(uname -s)" in
> > +	*MINGW*)
> > +		GIT_BUILD_DIR="$(cygpath -au "$GIT_BUILD_DIR")"
> > +		;;
> > +	esac
> > +elif test "$TEST_DIRECTORY" = "$GIT_BUILD_DIR"
> >  then
> >  	echo "PANIC: Running in a $TEST_DIRECTORY that doesn't end in '/t'?" >&2
> >  	exit 1
>
> As pointed out in the v1 this breaks the cmake<->make interaction in
> some scenarios, but from some brief testing there seemed to be an easy
> workaround which didn't suffer from that problem:
> https://lore.kernel.org/git/220811.86sfm3ov5z.gmgdl@evledraar.gmail.com/

> Hi Ævar,
>
> On Thu, 8 Sep 2022, Ævar Arnfjörð Bjarmason wrote:
>
>> On Tue, Aug 23 2022, Johannes Schindelin via GitGitGadget wrote:
>>
>> > From: Johannes Schindelin <johannes.schindelin@gmx.de>
>> >
>> > In 7f5397a07c6c (cmake: support for testing git when building out of the
>> > source tree, 2020-06-26), we implemented support for running Git's test
>> > scripts even after building Git in a different directory than the source
>> > directory.
>> >
>> > The way we did this was to edit the file `t/test-lib.sh` to override
>> > `GIT_BUILD_DIR` to point somewhere else than the parent of the `t/`
>> > directory.
>> >
>> > This is unideal because it always leaves a tracked file marked as
>> > modified, and it is all too easy to commit that change by mistake.
>> >
>> > Let's change the strategy by teaching `t/test-lib.sh` to detect the
>> > presence of a file called `GIT-BUILD-DIR` in the source directory. If it
>> > exists, the contents are interpreted as the location to the _actual_
>> > build directory. We then write this file as part of the CTest
>> > definition.
>> >
>> > To support building Git via a regular `make` invocation after building
>> > it using CMake, we ensure that the `GIT-BUILD-DIR` file is deleted (for
>> > convenience, this is done as part of the Makefile rule that is already
>> > run with every `make` invocation to ensure that `GIT-BUILD-OPTIONS` is
>> > up to date).
>> >
>> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
>> > ---
>> >  .gitignore                          |  1 +
>> >  Makefile                            |  1 +
>> >  contrib/buildsystems/CMakeLists.txt |  7 +------
>> >  t/test-lib.sh                       | 11 ++++++++++-
>> >  4 files changed, 13 insertions(+), 7 deletions(-)
>> >
>> > diff --git a/.gitignore b/.gitignore
>> > index a4522157641..b72ddf09346 100644
>> > --- a/.gitignore
>> > +++ b/.gitignore
>> > @@ -2,6 +2,7 @@
>> >  /fuzz_corpora
>> >  /fuzz-pack-headers
>> >  /fuzz-pack-idx
>> > +/GIT-BUILD-DIR
>> >  /GIT-BUILD-OPTIONS
>> >  /GIT-CFLAGS
>> >  /GIT-LDFLAGS
>> > diff --git a/Makefile b/Makefile
>> > index 04d0fd1fe60..9347ed90da7 100644
>> > --- a/Makefile
>> > +++ b/Makefile
>> > @@ -3028,6 +3028,7 @@ else
>> >  	@echo RUNTIME_PREFIX=\'false\' >>$@+
>> >  endif
>> >  	@if cmp $@+ $@ >/dev/null 2>&1; then $(RM) $@+; else mv $@+ $@; fi
>> > +	@if test -f GIT-BUILD-DIR; then rm GIT-BUILD-DIR; fi
>> >
>> >  ### Detect Python interpreter path changes
>> >  ifndef NO_PYTHON
>> > diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt
>> > index fe606c179f7..29d7e236ae1 100644
>> > --- a/contrib/buildsystems/CMakeLists.txt
>> > +++ b/contrib/buildsystems/CMakeLists.txt
>> > @@ -1067,14 +1067,9 @@ endif()
>> >  #Make the tests work when building out of the source tree
>> >  get_filename_component(CACHE_PATH ${CMAKE_CURRENT_LIST_DIR}/../../CMakeCache.txt ABSOLUTE)
>> >  if(NOT ${CMAKE_BINARY_DIR}/CMakeCache.txt STREQUAL ${CACHE_PATH})
>> > -	file(RELATIVE_PATH BUILD_DIR_RELATIVE ${CMAKE_SOURCE_DIR} ${CMAKE_BINARY_DIR}/CMakeCache.txt)
>> > -	string(REPLACE "/CMakeCache.txt" "" BUILD_DIR_RELATIVE ${BUILD_DIR_RELATIVE})
>> >  	#Setting the build directory in test-lib.sh before running tests
>> >  	file(WRITE ${CMAKE_BINARY_DIR}/CTestCustom.cmake
>> > -		"file(STRINGS ${CMAKE_SOURCE_DIR}/t/test-lib.sh GIT_BUILD_DIR_REPL REGEX \"GIT_BUILD_DIR=(.*)\")\n"
>> > -		"file(STRINGS ${CMAKE_SOURCE_DIR}/t/test-lib.sh content NEWLINE_CONSUME)\n"
>> > -		"string(REPLACE \"\${GIT_BUILD_DIR_REPL}\" \"GIT_BUILD_DIR=\\\"$TEST_DIRECTORY/../${BUILD_DIR_RELATIVE}\\\"\" content \"\${content}\")\n"
>> > -		"file(WRITE ${CMAKE_SOURCE_DIR}/t/test-lib.sh \${content})")
>> > +		"file(WRITE ${CMAKE_SOURCE_DIR}/GIT-BUILD-DIR \"${CMAKE_BINARY_DIR}\")")
>> >  	#misc copies
>> >  	file(COPY ${CMAKE_SOURCE_DIR}/t/chainlint.sed DESTINATION ${CMAKE_BINARY_DIR}/t/)
>> >  	file(COPY ${CMAKE_SOURCE_DIR}/po/is.po DESTINATION ${CMAKE_BINARY_DIR}/po/)
>> > diff --git a/t/test-lib.sh b/t/test-lib.sh
>> > index 55857af601b..4468ac51f25 100644
>> > --- a/t/test-lib.sh
>> > +++ b/t/test-lib.sh
>> > @@ -42,7 +42,16 @@ then
>> >  	TEST_OUTPUT_DIRECTORY=$TEST_DIRECTORY
>> >  fi
>> >  GIT_BUILD_DIR="${TEST_DIRECTORY%/t}"
>> > -if test "$TEST_DIRECTORY" = "$GIT_BUILD_DIR"
>> > +if test -f "$GIT_BUILD_DIR/GIT-BUILD-DIR"
>> > +then
>> > +	GIT_BUILD_DIR="$(cat "$GIT_BUILD_DIR/GIT-BUILD-DIR")" || exit 1
>> > +	# On Windows, we must convert Windows paths lest they contain a colon
>> > +	case "$(uname -s)" in
>> > +	*MINGW*)
>> > +		GIT_BUILD_DIR="$(cygpath -au "$GIT_BUILD_DIR")"
>> > +		;;
>> > +	esac
>> > +elif test "$TEST_DIRECTORY" = "$GIT_BUILD_DIR"
>> >  then
>> >  	echo "PANIC: Running in a $TEST_DIRECTORY that doesn't end in '/t'?" >&2
>> >  	exit 1
>>
>> As pointed out in the v1 this breaks the cmake<->make interaction in
>> some scenarios, but from some brief testing there seemed to be an easy
>> workaround which didn't suffer from that problem:
>> https://lore.kernel.org/git/220811.86sfm3ov5z.gmgdl@evledraar.gmail.com/
>
> I do not think that the CMake <-> make interaction will come up in any
> other scenario than your and my tests in the context of this mailing list
> thread.
>
> Therefore, I am certain that we need not cater to that scenario at all.

> From: Johannes Schindelin <johannes.schindelin@gmx.de>
> 
> In 7f5397a07c6c (cmake: support for testing git when building out of the
> source tree, 2020-06-26), we implemented support for running Git's test
> scripts even after building Git in a different directory than the source
> directory.
> 
> The way we did this was to edit the file `t/test-lib.sh` to override
> `GIT_BUILD_DIR` to point somewhere else than the parent of the `t/`
> directory.
> 
> This is unideal because it always leaves a tracked file marked as
> modified, and it is all too easy to commit that change by mistake.
> 
> Let's change the strategy by teaching `t/test-lib.sh` to detect the
> presence of a file called `GIT-BUILD-DIR` in the source directory. If it
> exists, the contents are interpreted as the location to the _actual_
> build directory. We then write this file as part of the CTest
> definition.
> 
> To support building Git via a regular `make` invocation after building
> it using CMake, we ensure that the `GIT-BUILD-DIR` file is deleted (for
> convenience, this is done as part of the Makefile rule that is already
> run with every `make` invocation to ensure that `GIT-BUILD-OPTIONS` is
> up to date).

> 
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  .gitignore                          |  1 +
>  Makefile                            |  1 +
>  contrib/buildsystems/CMakeLists.txt |  7 +------
>  t/test-lib.sh                       | 11 ++++++++++-
>  4 files changed, 13 insertions(+), 7 deletions(-)
> 
> diff --git a/.gitignore b/.gitignore
> index a4522157641..b72ddf09346 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -2,6 +2,7 @@
>  /fuzz_corpora
>  /fuzz-pack-headers
>  /fuzz-pack-idx
> +/GIT-BUILD-DIR
>  /GIT-BUILD-OPTIONS
>  /GIT-CFLAGS
>  /GIT-LDFLAGS
> diff --git a/Makefile b/Makefile
> index 04d0fd1fe60..9347ed90da7 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -3028,6 +3028,7 @@ else
>  	@echo RUNTIME_PREFIX=\'false\' >>$@+
>  endif
>  	@if cmp $@+ $@ >/dev/null 2>&1; then $(RM) $@+; else mv $@+ $@; fi
> +	@if test -f GIT-BUILD-DIR; then rm GIT-BUILD-DIR; fi
>  
>  ### Detect Python interpreter path changes
>  ifndef NO_PYTHON
> diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt
> index fe606c179f7..29d7e236ae1 100644
> --- a/contrib/buildsystems/CMakeLists.txt
> +++ b/contrib/buildsystems/CMakeLists.txt
> @@ -1067,14 +1067,9 @@ endif()
>  #Make the tests work when building out of the source tree
>  get_filename_component(CACHE_PATH ${CMAKE_CURRENT_LIST_DIR}/../../CMakeCache.txt ABSOLUTE)
>  if(NOT ${CMAKE_BINARY_DIR}/CMakeCache.txt STREQUAL ${CACHE_PATH})
> -	file(RELATIVE_PATH BUILD_DIR_RELATIVE ${CMAKE_SOURCE_DIR} ${CMAKE_BINARY_DIR}/CMakeCache.txt)
> -	string(REPLACE "/CMakeCache.txt" "" BUILD_DIR_RELATIVE ${BUILD_DIR_RELATIVE})
>  	#Setting the build directory in test-lib.sh before running tests
>  	file(WRITE ${CMAKE_BINARY_DIR}/CTestCustom.cmake
> -		"file(STRINGS ${CMAKE_SOURCE_DIR}/t/test-lib.sh GIT_BUILD_DIR_REPL REGEX \"GIT_BUILD_DIR=(.*)\")\n"
> -		"file(STRINGS ${CMAKE_SOURCE_DIR}/t/test-lib.sh content NEWLINE_CONSUME)\n"
> -		"string(REPLACE \"\${GIT_BUILD_DIR_REPL}\" \"GIT_BUILD_DIR=\\\"$TEST_DIRECTORY/../${BUILD_DIR_RELATIVE}\\\"\" content \"\${content}\")\n"
> -		"file(WRITE ${CMAKE_SOURCE_DIR}/t/test-lib.sh \${content})")
> +		"file(WRITE ${CMAKE_SOURCE_DIR}/GIT-BUILD-DIR \"${CMAKE_BINARY_DIR}\")")
>  	#misc copies
>  	file(COPY ${CMAKE_SOURCE_DIR}/t/chainlint.sed DESTINATION ${CMAKE_BINARY_DIR}/t/)
>  	file(COPY ${CMAKE_SOURCE_DIR}/po/is.po DESTINATION ${CMAKE_BINARY_DIR}/po/)
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index 55857af601b..4468ac51f25 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -42,7 +42,16 @@ then
>  	TEST_OUTPUT_DIRECTORY=$TEST_DIRECTORY
>  fi
>  GIT_BUILD_DIR="${TEST_DIRECTORY%/t}"
> -if test "$TEST_DIRECTORY" = "$GIT_BUILD_DIR"
> +if test -f "$GIT_BUILD_DIR/GIT-BUILD-DIR"
> +then
> +	GIT_BUILD_DIR="$(cat "$GIT_BUILD_DIR/GIT-BUILD-DIR")" || exit 1
> +	# On Windows, we must convert Windows paths lest they contain a colon
> +	case "$(uname -s)" in
> +	*MINGW*)
> +		GIT_BUILD_DIR="$(cygpath -au "$GIT_BUILD_DIR")"
> +		;;
> +	esac
> +elif test "$TEST_DIRECTORY" = "$GIT_BUILD_DIR"
>  then
>  	echo "PANIC: Running in a $TEST_DIRECTORY that doesn't end in '/t'?" >&2
>  	exit 1

> Johannes Schindelin via GitGitGadget wrote:
>> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>>
>> In 7f5397a07c6c (cmake: support for testing git when building out of the
>> source tree, 2020-06-26), we implemented support for running Git's test
>> scripts even after building Git in a different directory than the source
>> directory.
>>
>> The way we did this was to edit the file `t/test-lib.sh` to override
>> `GIT_BUILD_DIR` to point somewhere else than the parent of the `t/`
>> directory.
>>
>> This is unideal because it always leaves a tracked file marked as
>> modified, and it is all too easy to commit that change by mistake.
>>
>> Let's change the strategy by teaching `t/test-lib.sh` to detect the
>> presence of a file called `GIT-BUILD-DIR` in the source directory. If it
>> exists, the contents are interpreted as the location to the _actual_
>> build directory. We then write this file as part of the CTest
>> definition.
>>
>> To support building Git via a regular `make` invocation after building
>> it using CMake, we ensure that the `GIT-BUILD-DIR` file is deleted (for
>> convenience, this is done as part of the Makefile rule that is already
>> run with every `make` invocation to ensure that `GIT-BUILD-OPTIONS` is
>> up to date).
> 
> While I like that this removes a user error case, it sacrifices some of the
> separation between contrib/ and the main Git tree by adding logic to
> 'test-lib.sh' that only really benefits the CMake build.
> 
> To your point in [1]:

> 
>> Can we maybe agree that the proposed patch is a net improvement over the
>> status quo, and think about a better solution independently (without
>> blocking this here patch)?
> 
> I don't think it does more harm than good, but I wouldn't go so far as to
> call it a definitive "net improvement." I'd personally very much prefer a
> solution that didn't involve adding 'GIT-BUILD-DIR' just for the sake of
> CMake. Unfortunately, after spending a pretty substantial amount of time
> looking for an alternative, I couldn't think of anything that didn't either
> 1) change how users ran tests or 2) change where CMake builds wrote Git's
> binary files. 
> 
> So, I could go either way on this patch - if others feel strongly that it
> should be dropped, I'll defer to that. Otherwise, I'm fine keeping it unless
> someone can think of a better alternative.
> 

>> diff --git a/t/test-lib.sh b/t/test-lib.sh
>> index 55857af601b..4468ac51f25 100644
>> --- a/t/test-lib.sh
>> +++ b/t/test-lib.sh
>> @@ -42,7 +42,16 @@ then
>>  	TEST_OUTPUT_DIRECTORY=$TEST_DIRECTORY
>>  fi
>>  GIT_BUILD_DIR="${TEST_DIRECTORY%/t}"
>> -if test "$TEST_DIRECTORY" = "$GIT_BUILD_DIR"
>> +if test -f "$GIT_BUILD_DIR/GIT-BUILD-DIR"
>> +then
>> +	GIT_BUILD_DIR="$(cat "$GIT_BUILD_DIR/GIT-BUILD-DIR")" || exit 1
>> +	# On Windows, we must convert Windows paths lest they contain a colon
>> +	case "$(uname -s)" in
>> +	*MINGW*)
>> +		GIT_BUILD_DIR="$(cygpath -au "$GIT_BUILD_DIR")"
>> +		;;
>> +	esac
>> +elif test "$TEST_DIRECTORY" = "$GIT_BUILD_DIR"
>>  then
>>  	echo "PANIC: Running in a $TEST_DIRECTORY that doesn't end in '/t'?" >&2
>>  	exit 1
> Referring to Ævar's review in [1] - while I'm not overly concerned about the

> "switching between make & CMake" file staleness (if I'm not mistaken, the
> same thing can happen now with the modified 'test-lib.sh', so this patch
> doesn't really make anything worse), I do think the changes to 'test-lib.sh'
> should be rearranged to preserve the "PANIC" check:
> 
> ----------------->8----------------->8----------------->8-----------------
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index 4468ac51f2..7b57f55c37 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -42,6 +42,11 @@ then
>  	TEST_OUTPUT_DIRECTORY=$TEST_DIRECTORY
>  fi
>  GIT_BUILD_DIR="${TEST_DIRECTORY%/t}"
> +if test "$TEST_DIRECTORY" = "$GIT_BUILD_DIR"
> +then
> +	echo "PANIC: Running in a $TEST_DIRECTORY that doesn't end in '/t'?" >&2
> +	exit 1
> +fi
>  if test -f "$GIT_BUILD_DIR/GIT-BUILD-DIR"
>  then
>  	GIT_BUILD_DIR="$(cat "$GIT_BUILD_DIR/GIT-BUILD-DIR")" || exit 1
> @@ -51,10 +56,6 @@ then
>  		GIT_BUILD_DIR="$(cygpath -au "$GIT_BUILD_DIR")"
>  		;;
>  	esac
> -elif test "$TEST_DIRECTORY" = "$GIT_BUILD_DIR"
> -then
> -	echo "PANIC: Running in a $TEST_DIRECTORY that doesn't end in '/t'?" >&2
> -	exit 1
>  fi
>  
>  # Prepend a string to a VAR using an arbitrary ":" delimiter, not
> -----------------8<-----------------8<-----------------8<-----------------
> 
> Otherwise, a user could run the tests from outside a 't/' directory if they
> built Git with CMake, which doesn't appear to be part of the intended
> behavior of this patch.

> I don't think it does more harm than good, but I wouldn't go so far as to
> call it a definitive "net improvement." I'd personally very much prefer a
> solution that didn't involve adding 'GIT-BUILD-DIR' just for the sake of
> CMake.
> ...
> ... "switching between make & CMake" file staleness (if I'm not mistaken, the
> same thing can happen now with the modified 'test-lib.sh', so this patch
> doesn't really make anything worse), I do think the changes to 'test-lib.sh'
> should be rearranged to preserve the "PANIC" check:

> Johannes Schindelin via GitGitGadget wrote:
>
> > [...]
> > diff --git a/t/test-lib.sh b/t/test-lib.sh
> > index 55857af601b..4468ac51f25 100644
> > --- a/t/test-lib.sh
> > +++ b/t/test-lib.sh
> > @@ -42,7 +42,16 @@ then
> >  	TEST_OUTPUT_DIRECTORY=$TEST_DIRECTORY
> >  fi
> >  GIT_BUILD_DIR="${TEST_DIRECTORY%/t}"
> > -if test "$TEST_DIRECTORY" = "$GIT_BUILD_DIR"
> > +if test -f "$GIT_BUILD_DIR/GIT-BUILD-DIR"
> > +then
> > +	GIT_BUILD_DIR="$(cat "$GIT_BUILD_DIR/GIT-BUILD-DIR")" || exit 1
> > +	# On Windows, we must convert Windows paths lest they contain a colon
> > +	case "$(uname -s)" in
> > +	*MINGW*)
> > +		GIT_BUILD_DIR="$(cygpath -au "$GIT_BUILD_DIR")"
> > +		;;
> > +	esac
> > +elif test "$TEST_DIRECTORY" = "$GIT_BUILD_DIR"
> >  then
> >  	echo "PANIC: Running in a $TEST_DIRECTORY that doesn't end in '/t'?" >&2
> >  	exit 1
> Referring to Ævar's review in [1] - while I'm not overly concerned about the
> "switching between make & CMake" file staleness (if I'm not mistaken, the
> same thing can happen now with the modified 'test-lib.sh', so this patch
> doesn't really make anything worse), I do think the changes to 'test-lib.sh'
> should be rearranged to preserve the "PANIC" check:
>
> ----------------->8----------------->8----------------->8-----------------
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index 4468ac51f2..7b57f55c37 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -42,6 +42,11 @@ then
>  	TEST_OUTPUT_DIRECTORY=$TEST_DIRECTORY
>  fi
>  GIT_BUILD_DIR="${TEST_DIRECTORY%/t}"
> +if test "$TEST_DIRECTORY" = "$GIT_BUILD_DIR"
> +then
> +	echo "PANIC: Running in a $TEST_DIRECTORY that doesn't end in '/t'?" >&2
> +	exit 1
> +fi
>  if test -f "$GIT_BUILD_DIR/GIT-BUILD-DIR"
>  then
>  	GIT_BUILD_DIR="$(cat "$GIT_BUILD_DIR/GIT-BUILD-DIR")" || exit 1
> @@ -51,10 +56,6 @@ then
>  		GIT_BUILD_DIR="$(cygpath -au "$GIT_BUILD_DIR")"
>  		;;
>  	esac
> -elif test "$TEST_DIRECTORY" = "$GIT_BUILD_DIR"
> -then
> -	echo "PANIC: Running in a $TEST_DIRECTORY that doesn't end in '/t'?" >&2
> -	exit 1
>  fi
>
>  # Prepend a string to a VAR using an arbitrary ":" delimiter, not
> -----------------8<-----------------8<-----------------8<-----------------
>
> Otherwise, a user could run the tests from outside a 't/' directory if they
> built Git with CMake, which doesn't appear to be part of the intended
> behavior of this patch.

> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> As suggested in
> https://github.com/git-for-windows/git/issues/3966#issuecomment-1221264238,
> t7112 can run for well over one hour, which seems to be the default
> maximum run time at least when running CTest-based tests in Visual
> Studio.
>
> Let's increase the time-out as a stop gap to unblock developers wishing
> to run Git's test suite in Visual Studio.
>
> Note: The actual run time is highly dependent on the circumstances. For
> example, in Git's CI runs, the Windows-based tests typically take a bit
> over 5 minutes to run. CI runs have the added benefit that Windows
> Defender (the common anti-malware scanner on Windows) is turned off,
> something many developers are not at liberty to do on their work
> stations. When Defender is turned on, even on this developer's high-end
> Ryzen system, t7112 takes over 15 minutes to run.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  contrib/buildsystems/CMakeLists.txt | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt
> index 29d7e236ae1..b1306f95256 100644
> --- a/contrib/buildsystems/CMakeLists.txt
> +++ b/contrib/buildsystems/CMakeLists.txt
> @@ -1088,4 +1088,8 @@ foreach(tsh ${test_scipts})
>  		WORKING_DIRECTORY ${CMAKE_SOURCE_DIR}/t)
>  endforeach()
>  
> +# This test script takes an extremely long time and is known to time out even
> +# on fast machines because it requires in excess of one hour to run
> +set_tests_properties("${CMAKE_SOURCE_DIR}/t/t7112-reset-submodule.sh" PROPERTIES TIMEOUT 4000)
> +
>  endif()#BUILD_TESTING

> 
> On Tue, Aug 23 2022, Johannes Schindelin via GitGitGadget wrote:
> 
>> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>>
>> As suggested in
>> https://github.com/git-for-windows/git/issues/3966#issuecomment-1221264238,
>> t7112 can run for well over one hour, which seems to be the default
>> maximum run time at least when running CTest-based tests in Visual
>> Studio.
>>
>> Let's increase the time-out as a stop gap to unblock developers wishing
>> to run Git's test suite in Visual Studio.
>>
>> Note: The actual run time is highly dependent on the circumstances. For
>> example, in Git's CI runs, the Windows-based tests typically take a bit
>> over 5 minutes to run. CI runs have the added benefit that Windows
>> Defender (the common anti-malware scanner on Windows) is turned off,
>> something many developers are not at liberty to do on their work
>> stations. When Defender is turned on, even on this developer's high-end
>> Ryzen system, t7112 takes over 15 minutes to run.
>>
>> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
>> ---
>>  contrib/buildsystems/CMakeLists.txt | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt
>> index 29d7e236ae1..b1306f95256 100644
>> --- a/contrib/buildsystems/CMakeLists.txt
>> +++ b/contrib/buildsystems/CMakeLists.txt
>> @@ -1088,4 +1088,8 @@ foreach(tsh ${test_scipts})
>>  		WORKING_DIRECTORY ${CMAKE_SOURCE_DIR}/t)
>>  endforeach()
>>  
>> +# This test script takes an extremely long time and is known to time out even
>> +# on fast machines because it requires in excess of one hour to run
>> +set_tests_properties("${CMAKE_SOURCE_DIR}/t/t7112-reset-submodule.sh" PROPERTIES TIMEOUT 4000)
>> +
>>  endif()#BUILD_TESTING
> 
> I don't see per [1] that it would have any negative effect to just bump
> the timeout a lot more, and for all the tests.

> 
> If we're running into 3600 seconds, then setting this to 4000 seconds
> seems to be a smal stopgap at best. That's just over a 10% increase, so
> if one person ran into it it 3600 someone with a slightly slower system
> should be running into the same still, or if we just add a few more
> tests to 7112 (or some other slow test).

> 
> So why not set this ta 3600*10 or whatever for *all* of the test scripts
> instead of playing whack-a-mole?
> 
> 1. https://cmake.org/cmake/help/latest/prop_test/TIMEOUT.html

> From: Johannes Schindelin <johannes.schindelin@gmx.de>
> 
> When a test script fails in Git's test suite, the usual course of action
> is to re-run it using options to increase the verbosity of the output,
> e.g. `-v` and `-x`.
> 
> Like in Git's CI runs, when running the tests in Visual Studio via the
> CTest route, it is cumbersome or at least requires a very unintuitive
> approach to pass options to the test scripts.

> 
> While at it, also imitate what the Windows jobs in Git's CI runs do to
> accelerate running the test scripts: pass the `--no-bin-wrappers` and
> `--no-chain-lint` options. This makes the test runs noticeably faster
> because the `bin-wrappers/` scripts as well as the `chain-lint` code
> make heavy use of POSIX shell scripting, which is really, really slow on
> Windows due to the need to emulate POSIX behavior via the MSYS2 runtime.

> 
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  contrib/buildsystems/CMakeLists.txt | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt
> index 1b23f2440d8..4aee1e24342 100644
> --- a/contrib/buildsystems/CMakeLists.txt
> +++ b/contrib/buildsystems/CMakeLists.txt
> @@ -1088,7 +1088,7 @@ file(GLOB test_scipts "${CMAKE_SOURCE_DIR}/t/t[0-9]*.sh")
>  #test
>  foreach(tsh ${test_scipts})
>  	add_test(NAME ${tsh}
> -		COMMAND ${SH_EXE} ${tsh}
> +		COMMAND ${SH_EXE} ${tsh} --no-bin-wrappers --no-chain-lint -vx

> Johannes Schindelin via GitGitGadget wrote:
> > From: Johannes Schindelin <johannes.schindelin@gmx.de>
> >
> > When a test script fails in Git's test suite, the usual course of action
> > is to re-run it using options to increase the verbosity of the output,
> > e.g. `-v` and `-x`.
> >
> > Like in Git's CI runs, when running the tests in Visual Studio via the
> > CTest route, it is cumbersome or at least requires a very unintuitive
> > approach to pass options to the test scripts.
>
> At first I wondered whether there's a way to make arg specification more
> intuitive, rather than silently changing defaults. Unfortunately, it looks
> like even in the latest versions of CTest don't really support passing
> arguments through to tests [1] (and the workarounds are unpleasant at best).
>
> But then, you mentioned that there *is* a cumbersome/unintuitive approach to
> passing the options; what approach were you thinking?

> [1] https://gitlab.kitware.com/cmake/cmake/-/issues/20470
>
> >
> > So let's just pass those options by default: This will not clutter any
> > output window but the log that is written to a log file will have
> > information necessary to figure out test failures.
>
> Makes sense, I don't see any harm in providing more verbose output by
> default here.
>
> >
> > While at it, also imitate what the Windows jobs in Git's CI runs do to
> > accelerate running the test scripts: pass the `--no-bin-wrappers` and
> > `--no-chain-lint` options. This makes the test runs noticeably faster
> > because the `bin-wrappers/` scripts as well as the `chain-lint` code
> > make heavy use of POSIX shell scripting, which is really, really slow on
> > Windows due to the need to emulate POSIX behavior via the MSYS2 runtime.
>
> I'm a bit more hesitant on including these. I see how the performance
> benefit (on Windows in particular) would make typical user experience nicer.
> But, if someone develops locally with '--no-chain-lint' specified, they'll
> be much more likely to miss a broken && chain (personally, I get caught by
> chain lint errors *all the time* when I'm adding/updating tests) and cause
> unnecessary churn in their patch submissions. So, my recommendation would be
> to drop '--no-chain-lint' here (unless it's less important to the average
> developer than I think it is).
>
> It's also possible that '--no-bin-wrappers' does something weird with their
> local installation of Git but I think it's safe enough to make the default
> if the performance gain is substantial.
>
> >
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > ---
> >  contrib/buildsystems/CMakeLists.txt | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt
> > index 1b23f2440d8..4aee1e24342 100644
> > --- a/contrib/buildsystems/CMakeLists.txt
> > +++ b/contrib/buildsystems/CMakeLists.txt
> > @@ -1088,7 +1088,7 @@ file(GLOB test_scipts "${CMAKE_SOURCE_DIR}/t/t[0-9]*.sh")
> >  #test
> >  foreach(tsh ${test_scipts})
> >  	add_test(NAME ${tsh}
> > -		COMMAND ${SH_EXE} ${tsh}
> > +		COMMAND ${SH_EXE} ${tsh} --no-bin-wrappers --no-chain-lint -vx
>
> As you have it here, I don't think there's a way for a user to override
> these defaults (unless there's something about the manual workaround you
> mentioned earlier that allows it). Since a user could feasibly want to set
> their own options, could you add a build variable to CMakeLists.txt like
> 'GIT_TEST_OPTS'? You could use it to set the default options you have here,
> but a user could still specify their own value at build time to override.

>
> >  		WORKING_DIRECTORY ${CMAKE_SOURCE_DIR}/t)
> >  endforeach()
> >
>
>

> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> When a test script fails in Git's test suite, the usual course of action
> is to re-run it using options to increase the verbosity of the output,
> e.g. `-v` and `-x`.
>
> Like in Git's CI runs, when running the tests in Visual Studio via the
> CTest route, it is cumbersome or at least requires a very unintuitive
> approach to pass options to the test scripts.
>
> So let's just pass those options by default: This will not clutter any
> output window but the log that is written to a log file will have
> information necessary to figure out test failures.
>
> While at it, also imitate what the Windows jobs in Git's CI runs do to
> accelerate running the test scripts: pass the `--no-bin-wrappers` and
> `--no-chain-lint` options. This makes the test runs noticeably faster
> because the `bin-wrappers/` scripts as well as the `chain-lint` code
> make heavy use of POSIX shell scripting, which is really, really slow on
> Windows due to the need to emulate POSIX behavior via the MSYS2 runtime.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  contrib/buildsystems/CMakeLists.txt | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt
> index 1b23f2440d8..4aee1e24342 100644
> --- a/contrib/buildsystems/CMakeLists.txt
> +++ b/contrib/buildsystems/CMakeLists.txt
> @@ -1088,7 +1088,7 @@ file(GLOB test_scipts "${CMAKE_SOURCE_DIR}/t/t[0-9]*.sh")
>  #test
>  foreach(tsh ${test_scipts})
>  	add_test(NAME ${tsh}
> -		COMMAND ${SH_EXE} ${tsh}
> +		COMMAND ${SH_EXE} ${tsh} --no-bin-wrappers --no-chain-lint -vx
>  		WORKING_DIRECTORY ${CMAKE_SOURCE_DIR}/t)
>  endforeach()

> On Tue, Aug 23 2022, Johannes Schindelin via GitGitGadget wrote:
> > When a test script fails in Git's test suite, the usual course of action
> > is to re-run it using options to increase the verbosity of the output,
> > e.g. `-v` and `-x`.
> > [...]
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > ---
> > diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt
> > +             COMMAND ${SH_EXE} ${tsh} --no-bin-wrappers --no-chain-lint -vx
>
> I think the "--no-bin-wrappers --no-chain-lint" and the "while at it"
> here should be stripped out, and put into its own commit.
>
> So, perhaps we should have a performance hack due to Windows's slowness,
> but:
>
>  C. Since I sent [1] we've had submitted chainlint.pl in-flight
>     series. It's partly trying to take special considerations to be fast
>     on Windows. I don't think it's the case with that series that this
>     needs to be skipped on Windows anymore (and if it is, Eric would
>     like to know).

> Visual Studio users enjoy support for running the test suite via CTest,
> thanks to Git's CMake definition.
> 
> In https://github.com/git-for-windows/git/issues/3966, it has been reported
> that this does not work out of the box, though, but causes a couple of test
> failures instead. These problems are not caught by Git's CI runs because the
> vs-tests jobs actually use prove to run the test suite, not CTest.
> 
> In addition to fixing these problems, this patch series also addresses a
> long-standing gripe I have with the way Git's CMake definition supports
> CTest: It edits t/test-lib.sh, which leaves this file eternally modified
> (but these modification should never be committed, they refer to a
> local-only, configuration-dependent directory).
> 
> Note: The signed/unsigned comparison bug in git add -p that is fixed in this
> here patch series is a relatively big one, and it merits further
> investigation whether there are similar bugs lurking in Git's code base.
> However, this is a much bigger project than can be addressed as part of this
> patch series, in particular because the analysis would require tools other
> than GCC's -Wsign-compare option (which totally misses the instance that is
> fixed in this here patch series).
> 
> Changes since v1:
> 
>  * Clarified why it is a good idea to pass --no-bin-wrappers and
>    --no-chain-lint when running on Windows.
>  * Clarified why the add -p bug has not been caught earlier.
>  * Clarified the scope of this patch series to fix running Git's tests
>    within Visual Studio.
>  * Increased the time-out for the very slow t7112 test script.
>  * The test_chmod was determined to be not only faulty, but unneeded, and
>    was dropped.
> 
> Johannes Schindelin (5):
>   cmake: make it easier to diagnose regressions in CTest runs
>   cmake: copy the merge tools for testing
>   add -p: avoid ambiguous signed/unsigned comparison
>   cmake: avoid editing t/test-lib.sh
>   cmake: increase time-out for a long-running test

> 
>  .gitignore                          |  1 +
>  Makefile                            |  1 +
>  add-patch.c                         |  2 +-
>  contrib/buildsystems/CMakeLists.txt | 16 ++++++++--------
>  t/test-lib.sh                       | 11 ++++++++++-
>  5 files changed, 21 insertions(+), 10 deletions(-)
> 
> 
> base-commit: bbea4dcf42b28eb7ce64a6306cdde875ae5d09ca
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1320%2Fdscho%2Fctest-on-windows-v2
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1320/dscho/ctest-on-windows-v2
> Pull-Request: https://github.com/gitgitgadget/git/pull/1320
> 
> Range-diff vs v1:
> 
>  1:  9cf14984c0a ! 1:  e00cb37b98a cmake: align CTest definition with Git's CI runs
>      @@ Metadata
>       Author: Johannes Schindelin <Johannes.Schindelin@gmx.de>
>       
>        ## Commit message ##
>      -    cmake: align CTest definition with Git's CI runs
>      +    cmake: make it easier to diagnose regressions in CTest runs
>       
>      -    In Git's CI runs, the Windows tests are run with `--no-bin-wrappers` and
>      -    `--no-chain-lint`, mainly to win back some time caused by the serious
>      -    performance penalty paid for the tests relying so heavily on POSIX shell
>      -    scripting, which only works by using a POSIX emulation layer.
>      +    When a test script fails in Git's test suite, the usual course of action
>      +    is to re-run it using options to increase the verbosity of the output,
>      +    e.g. `-v` and `-x`.
>       
>      -    Let's do the same when running the tests, say, in Visual Studio.
>      +    Like in Git's CI runs, when running the tests in Visual Studio via the
>      +    CTest route, it is cumbersome or at least requires a very unintuitive
>      +    approach to pass options to the test scripts.
>       
>      -    While at it, enable the command trace via `-x` and verbose output via
>      -    `-v`, otherwise it would be near impossible to diagnose any problems.
>      +    So let's just pass those options by default: This will not clutter any
>      +    output window but the log that is written to a log file will have
>      +    information necessary to figure out test failures.
>      +
>      +    While at it, also imitate what the Windows jobs in Git's CI runs do to
>      +    accelerate running the test scripts: pass the `--no-bin-wrappers` and
>      +    `--no-chain-lint` options. This makes the test runs noticeably faster
>      +    because the `bin-wrappers/` scripts as well as the `chain-lint` code
>      +    make heavy use of POSIX shell scripting, which is really, really slow on
>      +    Windows due to the need to emulate POSIX behavior via the MSYS2 runtime.
>       
>           Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
>       
>  2:  86ab58b6508 = 2:  de7b47a9aa7 cmake: copy the merge tools for testing
>  3:  79abfa82c32 < -:  ----------- tests: explicitly skip `chmod` calls on Windows
>  4:  4d24a4345ba ! 3:  f96d5ab484c add -p: avoid ambiguous signed/unsigned comparison
>      @@ Commit message
>           Let's avoid that by converting the unsigned bit explicitly to a signed
>           integer.
>       
>      +    Note: This is a long-standing bug in the Visual C build of Git, but it
>      +    has never been caught because t3701 is skipped when `NO_PERL` is set,
>      +    which is the case in the `vs-test` jobs of Git's CI runs.
>      +
>           Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
>       
>        ## add-patch.c ##
>  5:  c7fc5a4ee4c = 4:  22473d6b8f3 cmake: avoid editing t/test-lib.sh
>  -:  ----------- > 5:  6aaa675301c cmake: increase time-out for a long-running test
> 

> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> When a test script fails in Git's test suite, the usual course of action
> is to re-run it using options to increase the verbosity of the output,
> e.g. `-v` and `-x`.
>
> Like in Git's CI runs, when running the tests in Visual Studio via the
> CTest route, it is cumbersome or at least requires a very unintuitive
> approach to pass options to the test scripts: the CMakeLists.txt file
> would have to be modified, passing the desired options to _all_ test
> scripts, and then the CMake Cache would have to be reconfigured before
> running the test in question individually. Unintuitive at best, and
> opposite to the niceties IDE users expect.
>
> So let's just pass those options by default: This will not clutter any
> output window but the log that is written to a log file will have
> information necessary to figure out test failures.
>
> While at it, also imitate what the Windows jobs in Git's CI runs do to
> accelerate running the test scripts: pass the `--no-bin-wrappers` and
> `--no-chain-lint` options.
>
> This makes the test runs noticeably faster because the `bin-wrappers/`
> scripts as well as the `chain-lint` code make heavy use of POSIX shell
> scripting, which is really, really slow on Windows due to the need to
> emulate POSIX behavior via the MSYS2 runtime. In a test by Eric
> Sunshine, it added two minutes (!) just to perform the chain-lint task.
>
> The idea of adding a CMake config option (á la `GIT_TEST_OPTS`) was
> considered during the development of this patch, but then dropped: such
> a setting is global, across _all_ tests, where e.g. `--run=...` would
> not make sense. Users wishing to override these new defaults are better
> advised running the test script manually, in a Git Bash, with full
> control over the command line.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  contrib/buildsystems/CMakeLists.txt | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt
> index 2237109b57f..6ac20bc5054 100644
> --- a/contrib/buildsystems/CMakeLists.txt
> +++ b/contrib/buildsystems/CMakeLists.txt
> @@ -1088,7 +1088,7 @@ file(GLOB test_scipts "${CMAKE_SOURCE_DIR}/t/t[0-9]*.sh")
>  #test
>  foreach(tsh ${test_scipts})
>  	add_test(NAME ${tsh}
> -		COMMAND ${SH_EXE} ${tsh}
> +		COMMAND ${SH_EXE} ${tsh} --no-bin-wrappers --no-chain-lint -vx
>  		WORKING_DIRECTORY ${CMAKE_SOURCE_DIR}/t)
>  endforeach()

> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> Even when running the tests via CTest, t7609 and t7610 rely on more than
> only a few mergetools to be copied to the build directory. Let's make it
> so.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  contrib/buildsystems/CMakeLists.txt | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt
> index 6ac20bc5054..0c741e7d878 100644
> --- a/contrib/buildsystems/CMakeLists.txt
> +++ b/contrib/buildsystems/CMakeLists.txt
> @@ -1078,7 +1078,8 @@ if(NOT ${CMAKE_BINARY_DIR}/CMakeCache.txt STREQUAL ${CACHE_PATH})
>  	#misc copies
>  	file(COPY ${CMAKE_SOURCE_DIR}/t/chainlint.sed DESTINATION ${CMAKE_BINARY_DIR}/t/)
>  	file(COPY ${CMAKE_SOURCE_DIR}/po/is.po DESTINATION ${CMAKE_BINARY_DIR}/po/)
> -	file(COPY ${CMAKE_SOURCE_DIR}/mergetools/tkdiff DESTINATION ${CMAKE_BINARY_DIR}/mergetools/)
> +	file(GLOB mergetools "${CMAKE_SOURCE_DIR}/mergetools/*")
> +	file(COPY ${mergetools} DESTINATION ${CMAKE_BINARY_DIR}/mergetools/)
>  	file(COPY ${CMAKE_SOURCE_DIR}/contrib/completion/git-prompt.sh DESTINATION ${CMAKE_BINARY_DIR}/contrib/completion/)
>  	file(COPY ${CMAKE_SOURCE_DIR}/contrib/completion/git-completion.bash DESTINATION ${CMAKE_BINARY_DIR}/contrib/completion/)
>  endif()

> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> In the interactive `add` operation, users can choose to jump to specific
> hunks, and Git will present the hunk list in that case. To avoid showing
> too many lines at once, only a maximum of 21 hunks are shown, skipping
> the "mode change" pseudo hunk.
>
> The comparison performed to skip the "mode change" pseudo hunk (if any)
> compares a signed integer `i` to the unsigned value `mode_change` (which
> can be 0 or 1 because it is a 1-bit type).
>
> According to section 6.3.1.8 of the C99 standard (see e.g.
> https://www.open-std.org/jtc1/sc22/WG14/www/docs/n1256.pdf), what should
> happen is an automatic conversion of the "lesser" type to the "greater"
> type, but since the types differ in signedness, it is ill-defined what
> is the correct "usual arithmetic conversion".
>
> Which means that Visual C's behavior can (and does) differ from GCC's:
> When compiling Git using the latter, `add -p`'s `goto` command shows no
> hunks by default because it casts a negative start offset to a pretty
> large unsigned value, breaking the "goto hunk" test case in
> `t3701-add-interactive.sh`.
>
> Let's avoid that by converting the unsigned bit explicitly to a signed
> integer.
>
> Note: This is a long-standing bug in the Visual C build of Git, but it
> has never been caught because t3701 is skipped when `NO_PERL` is set,
> which is the case in the `vs-test` jobs of Git's CI runs.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  add-patch.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/add-patch.c b/add-patch.c
> index 509ca04456b..3524555e2b0 100644
> --- a/add-patch.c
> +++ b/add-patch.c
> @@ -1547,7 +1547,7 @@ soft_increment:
>  			strbuf_remove(&s->answer, 0, 1);
>  			strbuf_trim(&s->answer);
>  			i = hunk_index - DISPLAY_HUNKS_LINES / 2;
> -			if (i < file_diff->mode_change)
> +			if (i < (int)file_diff->mode_change)
>  				i = file_diff->mode_change;
>  			while (s->answer.len == 0) {
>  				i = display_hunks(s, file_diff, i);

> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> In 7f5397a07c6c (cmake: support for testing git when building out of the
> source tree, 2020-06-26), we implemented support for running Git's test
> scripts even after building Git in a different directory than the source
> directory.
>
> The way we did this was to edit the file `t/test-lib.sh` to override
> `GIT_BUILD_DIR` to point somewhere else than the parent of the `t/`
> directory.
>
> This is unideal because it always leaves a tracked file marked as
> modified, and it is all too easy to commit that change by mistake.
>
> Let's change the strategy by teaching `t/test-lib.sh` to detect the
> presence of a file called `GIT-BUILD-DIR` in the source directory. If it
> exists, the contents are interpreted as the location to the _actual_
> build directory. We then write this file as part of the CTest
> definition.
>
> To support building Git via a regular `make` invocation after building
> it using CMake, we ensure that the `GIT-BUILD-DIR` file is deleted (for
> convenience, this is done as part of the Makefile rule that is already
> run with every `make` invocation to ensure that `GIT-BUILD-OPTIONS` is
> up to date).
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

> diff --git a/Makefile b/Makefile
> index 88178c5b466..886614340c7 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -3029,6 +3029,7 @@ else
>  	@echo RUNTIME_PREFIX=\'false\' >>$@+
>  endif
>  	@if cmp $@+ $@ >/dev/null 2>&1; then $(RM) $@+; else mv $@+ $@; fi
> +	@if test -f GIT-BUILD-DIR; then rm GIT-BUILD-DIR; fi

>  ### Detect Python interpreter path changes
>  ifndef NO_PYTHON
> diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt
> index 0c741e7d878..1d8cebb4cfe 100644
> --- a/contrib/buildsystems/CMakeLists.txt
> +++ b/contrib/buildsystems/CMakeLists.txt
> @@ -1067,14 +1067,9 @@ endif()
>  #Make the tests work when building out of the source tree
>  get_filename_component(CACHE_PATH ${CMAKE_CURRENT_LIST_DIR}/../../CMakeCache.txt ABSOLUTE)
>  if(NOT ${CMAKE_BINARY_DIR}/CMakeCache.txt STREQUAL ${CACHE_PATH})
> -	file(RELATIVE_PATH BUILD_DIR_RELATIVE ${CMAKE_SOURCE_DIR} ${CMAKE_BINARY_DIR}/CMakeCache.txt)
> -	string(REPLACE "/CMakeCache.txt" "" BUILD_DIR_RELATIVE ${BUILD_DIR_RELATIVE})
>  	#Setting the build directory in test-lib.sh before running tests
>  	file(WRITE ${CMAKE_BINARY_DIR}/CTestCustom.cmake
> -		"file(STRINGS ${CMAKE_SOURCE_DIR}/t/test-lib.sh GIT_BUILD_DIR_REPL REGEX \"GIT_BUILD_DIR=(.*)\")\n"
> -		"file(STRINGS ${CMAKE_SOURCE_DIR}/t/test-lib.sh content NEWLINE_CONSUME)\n"
> -		"string(REPLACE \"\${GIT_BUILD_DIR_REPL}\" \"GIT_BUILD_DIR=\\\"$TEST_DIRECTORY/../${BUILD_DIR_RELATIVE}\\\"\" content \"\${content}\")\n"
> -		"file(WRITE ${CMAKE_SOURCE_DIR}/t/test-lib.sh \${content})")
> +		"file(WRITE ${CMAKE_SOURCE_DIR}/GIT-BUILD-DIR \"${CMAKE_BINARY_DIR}\")")
>  	#misc copies
>  	file(COPY ${CMAKE_SOURCE_DIR}/t/chainlint.sed DESTINATION ${CMAKE_BINARY_DIR}/t/)
>  	file(COPY ${CMAKE_SOURCE_DIR}/po/is.po DESTINATION ${CMAKE_BINARY_DIR}/po/)

> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index 120f11812c3..dfc0144ed3b 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -47,6 +47,16 @@ then
>  	echo "PANIC: Running in a $TEST_DIRECTORY that doesn't end in '/t'?" >&2
>  	exit 1
>  fi
> +if test -f "$GIT_BUILD_DIR/GIT-BUILD-DIR"
> +then
> +	GIT_BUILD_DIR="$(cat "$GIT_BUILD_DIR/GIT-BUILD-DIR")" || exit 1
> +	# On Windows, we must convert Windows paths lest they contain a colon
> +	case "$(uname -s)" in
> +	*MINGW*)
> +		GIT_BUILD_DIR="$(cygpath -au "$GIT_BUILD_DIR")"
> +		;;
> +	esac
> +fi

>
> On Tue, Oct 18 2022, Johannes Schindelin via GitGitGadget wrote:
>
> > From: Johannes Schindelin <johannes.schindelin@gmx.de>
> >
> > In 7f5397a07c6c (cmake: support for testing git when building out of the
> > source tree, 2020-06-26), we implemented support for running Git's test
> > scripts even after building Git in a different directory than the source
> > directory.
> >
> > The way we did this was to edit the file `t/test-lib.sh` to override
> > `GIT_BUILD_DIR` to point somewhere else than the parent of the `t/`
> > directory.
> >
> > This is unideal because it always leaves a tracked file marked as
> > modified, and it is all too easy to commit that change by mistake.
> >
> > Let's change the strategy by teaching `t/test-lib.sh` to detect the
> > presence of a file called `GIT-BUILD-DIR` in the source directory. If it
> > exists, the contents are interpreted as the location to the _actual_
> > build directory. We then write this file as part of the CTest
> > definition.
> >
> > To support building Git via a regular `make` invocation after building
> > it using CMake, we ensure that the `GIT-BUILD-DIR` file is deleted (for
> > convenience, this is done as part of the Makefile rule that is already
> > run with every `make` invocation to ensure that `GIT-BUILD-OPTIONS` is
> > up to date).
> >
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> Re my earlier feedback, I came up with this as an alternative, which
> nicely allows us to have "cmake" and "make" play together, you can even
> run them concurrently!:
>
> 	https://github.com/avar/git/commit/30f2265fd07aee97ea66f6e84a824d85d241e245