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

Hi Johannes,

On Sat, 27 Jun 2026 10:38:50 +0200 (CEST)
Johannes Schindelin wrote:
> 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.

I still do not think this is a bug fix, but rather a significant behavioral
change. So I'd keep the patch series only in master branch.

-- 
Takashi Yano <[email protected]>