{"thread":{"id":"51048","subject":"[PATCH 0/1] Proactively fix a test issue with v3.x of the MSYS2 runtime","startedAt":"2019-05-07T21:51:32Z","lastAt":"2019-05-08T06:19:03Z","messageCount":5,"participants":["Johannes Schindelin via GitGitGadget","Eric Sunshine","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":1},"messages":[{"id":"375059","messageId":"pull.185.git.gitgitgadget@gmail.com","threadId":"51048","inReplyTo":null,"subject":"[PATCH 0/1] Proactively fix a test issue with v3.x of the MSYS2 runtime","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-05-07T21:51:29Z","receivedAt":"2019-05-07T21:51:32Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"I saw this in one of my builds that followed the bleeding edge of Git for\nWindows' SDK: git.exe has a different idea of the test script's PID than the\ntest script itself. Yet another of the quirks Git for Windows has to deal\nwith...\n\nJohannes Schindelin (1):\n  t6500(mingw): use the Windows PID of the shell\n\n t/t6500-gc.sh | 10 +++++++++-\n 1 file changed, 9 insertions(+), 1 deletion(-)\n\n\nbase-commit: 8104ec994ea3849a968b4667d072fedd1e688642\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-185%2Fdscho%2Ft6500-and-msys2-runtime-v3.x-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-185/dscho/t6500-and-msys2-runtime-v3.x-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/185\n-- \ngitgitgadget\n"},{"id":"375060","messageId":"ba78a47b873c5f044cbfb147b30a801cc90fb0ac.1557265888.git.gitgitgadget@gmail.com","threadId":"51048","inReplyTo":"pull.185.git.gitgitgadget@gmail.com","subject":"[PATCH 1/1] t6500(mingw): use the Windows PID of the shell","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-05-07T21:51:30Z","receivedAt":"2019-05-07T21:51:35Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nIn Git for Windows, we use the MSYS2 Bash which inherits a non-standard\nPID model from Cygwin's POSIX emulation layer: every MSYS2 process has a\nregular Windows PID, and in addition it has an MSYS2 PID (which\ncorresponds to a shadow process that emulates Unix-style signal\nhandling).\n\nWith the upgrade to the MSYS2 runtime v3.x, this shadow process cannot\nbe accessed via `OpenProcess()` any longer, and therefore t6500 thought\nincorrectly that the process referenced in `gc.pid` (which is not\nactually a real `gc` process in this context, but the current shell) no\nlonger exists.\n\nLet's fix this by making sure that the Windows PID is written into\n`gc.pid` in this test script soo that `git.exe` is able to understand\nthat that process does indeed still exist.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n t/t6500-gc.sh | 10 +++++++++-\n 1 file changed, 9 insertions(+), 1 deletion(-)\n\ndiff --git a/t/t6500-gc.sh b/t/t6500-gc.sh\nindex 4684d06552..53258d45a1 100755\n--- a/t/t6500-gc.sh\n+++ b/t/t6500-gc.sh\n@@ -162,7 +162,15 @@ test_expect_success 'background auto gc respects lock for all operations' '\n \t# now fake a concurrent gc that holds the lock; we can use our\n \t# shell pid so that it looks valid.\n \thostname=$(hostname || echo unknown) &&\n-\tprintf \"$$ %s\" \"$hostname\" >.git/gc.pid &&\n+\tshell_pid=$$ &&\n+\tif test_have_prereq MINGW && test -f /proc/$shell_pid/winpid\n+\tthen\n+\t\t# In Git for Windows, Bash (actually, the MSYS2 runtime) has a\n+\t\t# different idea of PIDs than git.exe (actually Windows). Use\n+\t\t# the Windows PID in this case.\n+\t\tshell_pid=$(cat /proc/$shell_pid/winpid)\n+\tfi &&\n+\tprintf \"%d %s\" \"$shell_pid\" \"$hostname\" >.git/gc.pid &&\n \n \t# our gc should exit zero without doing anything\n \trun_and_wait_for_auto_gc &&\n-- \ngitgitgadget\n"},{"id":"375062","messageId":"CAPig+cQG4+A+G+i+8RqpDAY2PveULVJ5iNR4HYEUPAd_4Ub04w@mail.gmail.com","threadId":"51048","inReplyTo":"ba78a47b873c5f044cbfb147b30a801cc90fb0ac.1557265888.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/1] t6500(mingw): use the Windows PID of the shell","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2019-05-07T22:25:20Z","receivedAt":"2019-05-07T22:25:34Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, May 7, 2019 at 5:51 PM Johannes Schindelin via GitGitGadget\n<gitgitgadget@gmail.com> wrote:\n> [...]\n> Let's fix this by making sure that the Windows PID is written into\n> `gc.pid` in this test script soo that `git.exe` is able to understand\n> that that process does indeed still exist.\n>\n> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n> ---\n> @@ -162,7 +162,15 @@ test_expect_success 'background auto gc respects lock for all operations' '\n> +       shell_pid=$$ &&\n> +       if test_have_prereq MINGW && test -f /proc/$shell_pid/winpid\n> +       then\n> +               # In Git for Windows, Bash (actually, the MSYS2 runtime) has a\n> +               # different idea of PIDs than git.exe (actually Windows). Use\n> +               # the Windows PID in this case.\n> +               shell_pid=$(cat /proc/$shell_pid/winpid)\n> +       fi &&\n> +       printf \"%d %s\" \"$shell_pid\" \"$hostname\" >.git/gc.pid &&\n\nI wonder if it would make sense to abstract this away behind a shell\nfunction named shell_pid() which can be specialized for MINGW, much\nlike the shell function pwd() is specialized on Windows.\n"},{"id":"375090","messageId":"xmqqef59iou2.fsf@gitster-ct.c.googlers.com","threadId":"51048","inReplyTo":"CAPig+cQG4+A+G+i+8RqpDAY2PveULVJ5iNR4HYEUPAd_4Ub04w@mail.gmail.com","subject":"Re: [PATCH 1/1] t6500(mingw): use the Windows PID of the shell","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-05-08T03:53:41Z","receivedAt":"2019-05-08T03:53:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine <sunshine@sunshineco.com> writes:\n\n> On Tue, May 7, 2019 at 5:51 PM Johannes Schindelin via GitGitGadget\n> <gitgitgadget@gmail.com> wrote:\n>> [...]\n>> Let's fix this by making sure that the Windows PID is written into\n>> `gc.pid` in this test script soo that `git.exe` is able to understand\n>> that that process does indeed still exist.\n>>\n>> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n>> ---\n>> @@ -162,7 +162,15 @@ test_expect_success 'background auto gc respects lock for all operations' '\n>> +       shell_pid=$$ &&\n>> +       if test_have_prereq MINGW && test -f /proc/$shell_pid/winpid\n>> +       then\n>> +               # In Git for Windows, Bash (actually, the MSYS2 runtime) has a\n>> +               # different idea of PIDs than git.exe (actually Windows). Use\n>> +               # the Windows PID in this case.\n>> +               shell_pid=$(cat /proc/$shell_pid/winpid)\n>> +       fi &&\n>> +       printf \"%d %s\" \"$shell_pid\" \"$hostname\" >.git/gc.pid &&\n>\n> I wonder if it would make sense to abstract this away behind a shell\n> function named shell_pid() which can be specialized for MINGW, much\n> like the shell function pwd() is specialized on Windows.\n\nIt would be, especially when we need the next such invocation.  I'd\nfind it easier to follow if it were\n\n\tif test_have_prereq ...\n\tthen\n\t\tshell_pid=$(cat /proc/$$/winpid)\n\telse\n\t\tshell_pid=$$\n\tfi &&\n\t...\n\nsimply because in each of the cases the mental burden gets smaller\n(those trying to see what happens on MINGW do not have to recall\nshell_pid was originally taken from $$ after seeing 'then'; others\ndo not have to wonder if lack of 'else' to cover the platforms they\nare reading for is deliberate and shell_pid is the only thing\nexpected out of the if/then/fi construct)\n\nAnd I would suggest doing such a rewrite when it becomes a helper\nshell function, but what is written here is good enough until then,\nI would think.\n\n\n"},{"id":"375094","messageId":"xmqqr299h3ji.fsf@gitster-ct.c.googlers.com","threadId":"51048","inReplyTo":"xmqqef59iou2.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 1/1] t6500(mingw): use the Windows PID of the shell","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-05-08T06:18:57Z","receivedAt":"2019-05-08T06:19:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Eric Sunshine <sunshine@sunshineco.com> writes:\n>\n>> On Tue, May 7, 2019 at 5:51 PM Johannes Schindelin via GitGitGadget\n>> <gitgitgadget@gmail.com> wrote:\n>>> [...]\n>>> Let's fix this by making sure that the Windows PID is written into\n>>> `gc.pid` in this test script soo that `git.exe` is able to understand\n\nI amended s/soo/so/ while queuing; I did not make any other changes,\nas I described why in the message I am responding to.\n\nThanks.\n"}]}