Re: [PATCH] Cygwin: pty: detect pcon-backed pty for non-Cygwin-spawned children

Hi Takashi,

On Fri, 29 May 2026, Takashi Yano wrote:

> On Thu, 28 May 2026 15:48:24 +0200 (CEST)
> Johannes Schindelin wrote:
> > Hi Takashi,
> > 
> > On Tue, 5 May 2026, Takashi Yano wrote:
> > 
> > > On Thu, 30 Apr 2026 15:04:04 +0000
> > > "Johannes Schindelin via GitGitGadget" wrote:
> > > > From: Johannes Schindelin <[email protected]>
> > > > 
> > > > diff --git a/winsup/cygwin/dtable.cc b/winsup/cygwin/dtable.cc
> > > > index 7303f7eac..ce29f4608 100644
> > > > --- a/winsup/cygwin/dtable.cc
> > > > +++ b/winsup/cygwin/dtable.cc
> > > > @@ -327,7 +327,17 @@ dtable::init_std_file_from_handle (int fd, HANDLE handle)
> > > >  	dev.parse (myself->ctty);
> > > >        else
> > > >  	{
> > > > -	  dev.parse (FH_CONSOLE);
> > > > +	  /* Check whether the inherited console is actually a pseudo
> > > > +	     console bridging a pty.  This happens when our non-Cygwin
> > > > +	     parent was itself spawned by a Cygwin process from a pty
> > > > +	     (e.g. bash spawning git.exe which then spawns vim).  In
> > > > +	     that case, connect to the pty slave instead of treating
> > > > +	     the handle as a real console. */
> > > > +	  int pcon_minor = cygwin_shared->tty.find_pcon_pty ();
> > > > +	  if (pcon_minor >= 0)
> > > > +	    dev.parse (FHDEV (DEV_PTYS_MAJOR, pcon_minor));
> > > > +	  else
> > > > +	    dev.parse (FH_CONSOLE);
> > > >  	  CloseHandle (handle);
> > > >  	  handle = INVALID_HANDLE_VALUE;
> > > 
> > > The lines:
> > > CloseHandle (handle);
> > > handle = INVALID_HANDLE_VALUE;
> > > are dropped in master branch. Do you think that these two lines
> > > are necessary for this patch when applying this patch to cygwin
> > > master branch?
> > 
> > Those two lines are not necessary, all added code ignores the handle
> > entirely.
> 
> OK. I think this patch is not a bug fix but a behavioral change,
> so it should go only to the master branch. What do you think?

Seeing as this fixes a regression in Git for Windows, where `git commit`'s
spawning a `vim` that will clear the screen after exiting instead of
restoring the previous non-alternate screen, I would characterize it as a
bug fix.

But I can make that bug fix downstream-only, in Git for Windows' fork of
the MSYS2 runtime, if you want to keep the commit in Cygwin's `master`
only and not backport it to `cygwin-3_6-branch`. Either solution is fine
for me.

> If so, the patch does not apply to master branch due to these
> two lines. Could you please update the patch for master branch?
> 
> In addition, I would appreciate it if you could consider incorporating
> the two additional patches I posted earlier.

Sorry, I could not find the time to finish the analysis and validation via
those AutoHotKey-based tests earlier.

Ciao,
Johannes