Hi Takashi, On Tue, 15 Jul 2025, Takashi Yano wrote:
> On Mon, 7 Jul 2025 12:50:24 +0200 (CEST) > Johannes Schindelin wrote: > > > > On Thu, 3 Jul 2025, Takashi Yano wrote: > > > > > On Thu, 3 Jul 2025 11:15:44 +0200 (CEST) > > > Johannes Schindelin wrote: > > > > > > > > On Thu, 3 Jul 2025, Takashi Yano wrote: > > > > > > > > > I noticed this patch needs additional fix. Please apply also > > > > > https://cygwin.com/pipermail/cygwin-patches/2025q3/014053.html > > > > > > > > Thank you for the update! > > > > > > > > I am curious, though: Under what circumstances does this patch make a > > > > difference? I tried to deduce this from the diff and the commit > > > > message but was unable to figure it out. > > > > > > In my environment, the command cat | /cygdrive/c/windows/system32/ping > > > -t localhost in Command Prompt cannt stop with single Ctrl-C. ping is > > > stopped, but cat remains without the sencond patch, IIRC. > > > > I have added this as an (AutoHotKey-based) integration test to > > https://github.com/git-for-windows/msys2-runtime/pull/105 and was able to > > verify that your fix is necessary to let that test pass. > > > > Speaking of tests: Have you had any time to consider how to accompany your > > fix by a regression test in `winsup/testsuite/`? > > > > For several days, I tried to find a way to reproduce a way to reproduce > > the SSH hang using combinations of Cygwin programs and MINGW > > programs/Node.JS scripts and did not find any. FWIW I don't think that > > MINGW programs or Node.JS scripts would be allowed in the test suite, > > anyway, but I wanted to see whether I could replicate the conditions > > necessary for the hang without resorting to SSH and `git.exe` _at all_. > > > > I deem it crucial to start including tests with your fixes that can be run > > automatically, and that catch regressions in the CI builds. > > To be honest, I already have local test suites that check the behavior > of special keys for both pty and console. However, I currently have no > clear idea how to integrate them into winsup/testsuite... If Cygwin were merely a personal project of yours, I would understand and probably agree. However, Cygwin is used (via the MSYS2 runtime) in Git for Windows, and by extension millions of users rely on it. Therefore, it would be good to at least publish those local tests. Ideally, a good deal of thought should be spent on figuring out a way to integrate the tests into the CI builds. You mentioned winsup/testsuite, and I do agree that it sounds more than just tricky to integrate the tests there. Essentially, you would probably end up reimplementing AutoHotKey's fundamental functionality: sending keystrokes and inspecting the results. Now, to be sure, running AutoHotKey-based tests is a lot more finicky than running winsup/testsuite. In the absence of any better idea, though, I would take the confidence from having tests over not having tests, any day. After all, you and I are both fully aware of the unfortunate pattern in the code under discussion where on multiple occasions, bug fixes introduced new bugs whose fixes introduced yet other bugs, etc ad nauseam. If AutoHotKey-based tests can help break that pattern, let's integrate them. Ciao, Johannes
