Proactively fix a test issue with v3.x of the MSYS2 runtime
cover Proactively fix a test issue with v3.x of the MSYS2 runtime
I saw this in one of my builds that followed the bleeding edge of Git for Windows' SDK: git.exe has a different idea of the test script's PID than the test script itself. Yet another of the quirks Git for Windows has to deal with...
Johannes Schindelin (1):
t6500(mingw): use the Windows PID of the shell
t/t6500-gc.sh | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)
base-commit: 8104ec994ea3849a968b4667d072fedd1e688642
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-185%2Fdscho%2Ft6500-and-msys2-runtime-v3.x-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-185/dscho/t6500-and-msys2-runtime-v3.x-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/185
Signature · 1 line
gitgitgadget
PATCH 1/1 t6500(mingw): use the Windows PID of the shell
From: Johannes Schindelin <[email protected]>
In Git for Windows, we use the MSYS2 Bash which inherits a non-standard PID model from Cygwin's POSIX emulation layer: every MSYS2 process has a regular Windows PID, and in addition it has an MSYS2 PID (which corresponds to a shadow process that emulates Unix-style signal handling).
With the upgrade to the MSYS2 runtime v3.x, this shadow process cannot be accessed via OpenProcess() any longer, and therefore t6500 thought incorrectly that the process referenced in gc.pid (which is not actually a real gc process in this context, but the current shell) no longer exists.
Let's fix this by making sure that the Windows PID is written into gc.pid in this test script soo that git.exe is able to understand that that process does indeed still exist.
--- t/t6500-gc.sh | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)
t/t6500-gc.sh +9 -1
@@ -162,7 +162,15 @@ test_expect_success 'background auto gc respects lock for all operations' ' # now fake a concurrent gc that holds the lock; we can use our # shell pid so that it looks valid. hostname=$(hostname || echo unknown) && - printf "$$ %s" "$hostname" >.git/gc.pid && + shell_pid=$$ && + if test_have_prereq MINGW && test -f /proc/$shell_pid/winpid + then + # In Git for Windows, Bash (actually, the MSYS2 runtime) has a + # different idea of PIDs than git.exe (actually Windows). Use + # the Windows PID in this case. + shell_pid=$(cat /proc/$shell_pid/winpid) + fi && + printf "%d %s" "$shell_pid" "$hostname" >.git/gc.pid && # our gc should exit zero without doing anything run_and_wait_for_auto_gc &&
Signature · 1 line
gitgitgadget
Re: [PATCH 1/1] t6500(mingw): use the Windows PID of the shell
Quoted 8 lines · Johannes Schindelin via GitGitGadget
[...]
Let's fix this by making sure that the Windows PID is written intogc.pidin this test script soo thatgit.exeis able to understand that that process does indeed still exist.Signed-off-by Johannes Schindelin <[email protected]>---
t/t6500-gc.sh test_expect_success 'background auto gc respects lock for all operations' ' · line 170
Quoted 9 lines
t/t6500-gc.sh +9
-0@@ -162,7 +162,15 @@ test_expect_success 'background auto gc respects lock for all operations' ' + shell_pid=$$ && + if test_have_prereq MINGW && test -f /proc/$shell_pid/winpid + then + # In Git for Windows, Bash (actually, the MSYS2 runtime) has a + # different idea of PIDs than git.exe (actually Windows). Use + # the Windows PID in this case. + shell_pid=$(cat /proc/$shell_pid/winpid) + fi && + printf "%d %s" "$shell_pid" "$hostname" >.git/gc.pid &&
I wonder if it would make sense to abstract this away behind a shell function named shell_pid() which can be specialized for MINGW, much like the shell function pwd() is specialized on Windows.
Re: [PATCH 1/1] t6500(mingw): use the Windows PID of the shell
Quoted 7 lines · Eric Sunshine
Quoted 5 lines · Johannes Schindelin via GitGitGadget
[...]
Let's fix this by making sure that the Windows PID is written intogc.pidin this test script soo thatgit.exeis able to understand that that process does indeed still exist.
Quoted 16 lines
t/t6500-gc.sh test_expect_success 'background auto gc respects lock for all operations' ' · line 170
Quoted 12 lines
Signed-off-by Johannes Schindelin <[email protected]>---
t/t6500-gc.sh +9
-0@@ -162,7 +162,15 @@ test_expect_success 'background auto gc respects lock for all operations' ' + shell_pid=$$ && + if test_have_prereq MINGW && test -f /proc/$shell_pid/winpid + then + # In Git for Windows, Bash (actually, the MSYS2 runtime) has a + # different idea of PIDs than git.exe (actually Windows). Use + # the Windows PID in this case. + shell_pid=$(cat /proc/$shell_pid/winpid) + fi && + printf "%d %s" "$shell_pid" "$hostname" >.git/gc.pid &&I wonder if it would make sense to abstract this away behind a shell function named
shell_pid()which can be specialized for MINGW, much like the shell functionpwd()is specialized on Windows.
It would be, especially when we need the next such invocation. I'd find it easier to follow if it were
if test_have_prereq ... then shell_pid=$(cat /proc/$$/winpid) else shell_pid=$$ fi && ...
simply because in each of the cases the mental burden gets smaller (those trying to see what happens on MINGW do not have to recall shell_pid was originally taken from $$ after seeing 'then'; others do not have to wonder if lack of 'else' to cover the platforms they are reading for is deliberate and shell_pid is the only thing expected out of the if/then/fi construct)
And I would suggest doing such a rewrite when it becomes a helper shell function, but what is written here is good enough until then, I would think.
Re: [PATCH 1/1] t6500(mingw): use the Windows PID of the shell
Quoted 7 lines · Junio C Hamano
Quoted 5 lines · Eric Sunshine
Quoted 3 lines · Johannes Schindelin via GitGitGadget
[...]
Let's fix this by making sure that the Windows PID is written intogc.pidin this test script soo thatgit.exeis able to understand
I amended s/soo/so/ while queuing; I did not make any other changes, as I described why in the message I am responding to.
Thanks.