Fix hanging t3701-add-interactive in Git's test suite #51
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…rtup." When Git for Windows upgraded its MSYS2 runtime from v3.3.* to v3.4.* at long last, the t3701-add-interactive test of Git's test suite started to hang consistently, timing out. The culprit is a `git add -p` call that wants to read from a diff filter. This diff filter is `cat.exe`, i.e. nothing special, but that diff filter gets input that is larger than the pipe buffer, and therefore must not block. Yet that is exactly what it does. This was a regression that introduced by upgrading from MSYS2 runtime v3.3.*. After a tedious, multi-day bisection session, the commit introducing that regression was identified as 9078882 (Cygwin: pipe: Restore blocking mode for cygwin process at startup., 2021-11-17), which we hereby revert. So what bug does this reversion reintroduce? The commit seems to have been in response to https://inbox.sourceware.org/cygwin/CAEv6GOB8PXHgHoz7hdJy6Bia2GWEmUDnTd252gTGinz2vuv=hA@mail.gmail.com/ which reported a bug when a C# program writes 0-byte content to a Cygwin pipe. (Irony of ironies, this report originated from Git's realm, over at git-ecosystem/git-credential-manager#509.) The analysis revealed, back in December '21, that a `CYGWIN=pipe_byte` would work around the bug, too, but that did not fix the regression in Git's test suite. That leaves us with the somewhat unsatisfying conclusion that we _might_ reintroduce a regression when Git Credential Manager tries to talk to an _MSYS_ `git.exe`. But since we do not need that in Git for Windows, and since we need to have CI builds that do not time out after 6h causing failures, it is better to revert that commit, and in the hopefully unlikely event that this causes _other_ problems in Git for Windows' ecosystem, we will simply have to revisit this issue in more depth. This fixes git-for-windows/git#4422. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
dscho
added a commit
to dscho/msys2-runtime
that referenced
this pull request
Jun 16, 2023
Fix hanging t3701-add-interactive in Git's test suite (git-for-windows#51) Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
dscho
added a commit
to dscho/msys2-runtime
that referenced
this pull request
Jun 19, 2023
Fix hanging t3701-add-interactive in Git's test suite (git-for-windows#51) Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
dscho
added a commit
to dscho/msys2-runtime
that referenced
this pull request
Sep 6, 2023
Fix hanging t3701-add-interactive in Git's test suite (git-for-windows#51) Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
dscho
added a commit
to dscho/msys2-runtime
that referenced
this pull request
Nov 29, 2023
Fix hanging t3701-add-interactive in Git's test suite (git-for-windows#51) Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
dscho
added a commit
to dscho/msys2-runtime
that referenced
this pull request
Aug 30, 2024
Fix hanging t3701-add-interactive in Git's test suite (git-for-windows#51) Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
dscho
added a commit
to dscho/msys2-runtime
that referenced
this pull request
Sep 5, 2024
Fix hanging t3701-add-interactive in Git's test suite (git-for-windows#51) Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
dscho
added a commit
to dscho/msys2-runtime
that referenced
this pull request
Sep 18, 2024
Fix hanging t3701-add-interactive in Git's test suite (git-for-windows#51) Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When Git for Windows upgraded its MSYS2 runtime from v3.3.* to v3.4.* at long last, the t3701-add-interactive test of Git's test suite started to hang consistently, timing out.
The hang happens when a
git add -pcall that wants to read from a diff filter that iscat.exe, i.e. nothing special, receiving input that is larger than the pipe buffer.The commit introducing that regression was identified as 9078882 (Cygwin: pipe: Restore blocking mode for cygwin process at startup., 2021-11-17), which we hereby revert.
So what bug does this reversion reintroduce?
The commit seems to have been in response to
https://inbox.sourceware.org/cygwin/CAEv6GOB8PXHgHoz7hdJy6Bia2GWEmUDnTd252gTGinz2vuv=hA@mail.gmail.com/ which reported a bug when a C# program writes 0-byte content to a Cygwin pipe. (Irony of ironies, this report originated from Git's realm, over at git-ecosystem/git-credential-manager#509.)
The analysis revealed, back in December '21, that a
CYGWIN=pipe_bytewould work around the bug, too, but that did not fix the regression in Git's test suite.That leaves us with the somewhat unsatisfying conclusion that we might reintroduce a regression when Git Credential Manager tries to talk to an MSYS
git.exe. But since we do not need that in Git for Windows, and since we need to have CI builds that do not time out after 6h causing failures, it is better to revert that commit, and in the hopefully unlikely event that this causes other problems in Git for Windows' ecosystem, we will simply have to revisit this issue in more depth.This fixes git-for-windows/git#4422.