git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
* [PATCH 0/1] Proactively fix a test issue with v3.x of the MSYS2 runtime
@ 2019-05-07 21:51 Johannes Schindelin via GitGitGadget
  2019-05-07 21:51 ` [PATCH 1/1] t6500(mingw): use the Windows PID of the shell Johannes Schindelin via GitGitGadget
  0 siblings, 1 reply; 5+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2019-05-07 21:51 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/1] t6500(mingw): use the Windows PID of the shell
  2019-05-07 21:51 [PATCH 0/1] Proactively fix a test issue with v3.x of the MSYS2 runtime Johannes Schindelin via GitGitGadget
@ 2019-05-07 21:51 ` Johannes Schindelin via GitGitGadget
  2019-05-07 22:25   ` Eric Sunshine
  0 siblings, 1 reply; 5+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2019-05-07 21:51 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Johannes Schindelin

From: Johannes Schindelin <johannes.schindelin@gmx.de>

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.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 t/t6500-gc.sh | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/t/t6500-gc.sh b/t/t6500-gc.sh
index 4684d06552..53258d45a1 100755
--- a/t/t6500-gc.sh
+++ b/t/t6500-gc.sh
@@ -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 &&
-- 
gitgitgadget

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/1] t6500(mingw): use the Windows PID of the shell
  2019-05-07 21:51 ` [PATCH 1/1] t6500(mingw): use the Windows PID of the shell Johannes Schindelin via GitGitGadget
@ 2019-05-07 22:25   ` Eric Sunshine
  2019-05-08  3:53     ` Junio C Hamano
  0 siblings, 1 reply; 5+ messages in thread
From: Eric Sunshine @ 2019-05-07 22:25 UTC (permalink / raw)
  To: Johannes Schindelin via GitGitGadget
  Cc: Git List, Junio C Hamano, Johannes Schindelin

On Tue, May 7, 2019 at 5:51 PM Johannes Schindelin via GitGitGadget
<gitgitgadget@gmail.com> wrote:
> [...]
> 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.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
> @@ -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.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/1] t6500(mingw): use the Windows PID of the shell
  2019-05-07 22:25   ` Eric Sunshine
@ 2019-05-08  3:53     ` Junio C Hamano
  2019-05-08  6:18       ` Junio C Hamano
  0 siblings, 1 reply; 5+ messages in thread
From: Junio C Hamano @ 2019-05-08  3:53 UTC (permalink / raw)
  To: Eric Sunshine
  Cc: Johannes Schindelin via GitGitGadget, Git List, Johannes Schindelin

Eric Sunshine <sunshine@sunshineco.com> writes:

> On Tue, May 7, 2019 at 5:51 PM Johannes Schindelin via GitGitGadget
> <gitgitgadget@gmail.com> wrote:
>> [...]
>> 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.
>>
>> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
>> ---
>> @@ -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.

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.



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/1] t6500(mingw): use the Windows PID of the shell
  2019-05-08  3:53     ` Junio C Hamano
@ 2019-05-08  6:18       ` Junio C Hamano
  0 siblings, 0 replies; 5+ messages in thread
From: Junio C Hamano @ 2019-05-08  6:18 UTC (permalink / raw)
  To: Eric Sunshine
  Cc: Johannes Schindelin via GitGitGadget, Git List, Johannes Schindelin

Junio C Hamano <gitster@pobox.com> writes:

> Eric Sunshine <sunshine@sunshineco.com> writes:
>
>> On Tue, May 7, 2019 at 5:51 PM Johannes Schindelin via GitGitGadget
>> <gitgitgadget@gmail.com> wrote:
>>> [...]
>>> 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

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.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2019-05-08  6:19 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-07 21:51 [PATCH 0/1] Proactively fix a test issue with v3.x of the MSYS2 runtime Johannes Schindelin via GitGitGadget
2019-05-07 21:51 ` [PATCH 1/1] t6500(mingw): use the Windows PID of the shell Johannes Schindelin via GitGitGadget
2019-05-07 22:25   ` Eric Sunshine
2019-05-08  3:53     ` Junio C Hamano
2019-05-08  6:18       ` Junio C Hamano

Code repositories for project(s) associated with this inbox:

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).