[PATCH] Cygwin: pty: Introduce disable_pcon in environment CYGWIN.

classic Classic list List threaded Threaded
12 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] Cygwin: pty: Introduce disable_pcon in environment CYGWIN.

Takashi Yano
- For programs which does not work properly with pseudo console,
  disable_pcon in environment CYGWIN is introduced. If disable_pcon
  is set, pseudo console support is disabled.
---
 winsup/cygwin/environ.cc      | 1 +
 winsup/cygwin/fhandler_tty.cc | 2 ++
 winsup/cygwin/globals.cc      | 1 +
 3 files changed, 4 insertions(+)

diff --git a/winsup/cygwin/environ.cc b/winsup/cygwin/environ.cc
index 8c5ce64e1..7eb4780a8 100644
--- a/winsup/cygwin/environ.cc
+++ b/winsup/cygwin/environ.cc
@@ -120,6 +120,7 @@ static struct parse_thing
   {"reset_com", {&reset_com}, setbool, NULL, {{false}, {true}}},
   {"wincmdln", {&wincmdln}, setbool, NULL, {{false}, {true}}},
   {"winsymlinks", {func: set_winsymlinks}, isfunc, NULL, {{0}, {0}}},
+  {"disable_pcon", {&disable_pcon}, setbool, NULL, {{false}, {true}}},
   {NULL, {0}, setdword, 0, {{0}, {0}}}
 };
 
diff --git a/winsup/cygwin/fhandler_tty.cc b/winsup/cygwin/fhandler_tty.cc
index fff5bebe3..a5db0967b 100644
--- a/winsup/cygwin/fhandler_tty.cc
+++ b/winsup/cygwin/fhandler_tty.cc
@@ -3132,6 +3132,8 @@ is_running_as_service (void)
 bool
 fhandler_pty_master::setup_pseudoconsole ()
 {
+  if (disable_pcon)
+    return false;
   /* If the legacy console mode is enabled, pseudo console seems
      not to work as expected. To determine console mode, registry
      key ForceV2 in HKEY_CURRENT_USER\Console is checked. */
diff --git a/winsup/cygwin/globals.cc b/winsup/cygwin/globals.cc
index ebe8b569f..a9648fe6a 100644
--- a/winsup/cygwin/globals.cc
+++ b/winsup/cygwin/globals.cc
@@ -71,6 +71,7 @@ bool pipe_byte;
 bool reset_com;
 bool wincmdln;
 winsym_t allow_winsymlinks = WSYM_sysfile;
+bool disable_pcon;
 
 bool NO_COPY in_forkee;
 
--
2.21.0

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Cygwin: pty: Introduce disable_pcon in environment CYGWIN.

Corinna Vinschen-2
On Jan 20 11:50, Takashi Yano wrote:
> - For programs which does not work properly with pseudo console,
>   disable_pcon in environment CYGWIN is introduced. If disable_pcon
>   is set, pseudo console support is disabled.

Oh well, do we really need that?  Anyway, this patch also requires
an addition to the documentation in winsup/doc/cygwinenv.xml.


Thanks,
Corinna

--
Corinna Vinschen
Cygwin Maintainer

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Cygwin: pty: Introduce disable_pcon in environment CYGWIN.

Takashi Yano
Hi Corinna,

On Mon, 20 Jan 2020 11:06:46 +0100
Corinna Vinschen wrote:
> On Jan 20 11:50, Takashi Yano wrote:
> > - For programs which does not work properly with pseudo console,
> >   disable_pcon in environment CYGWIN is introduced. If disable_pcon
> >   is set, pseudo console support is disabled.
> Oh well, do we really need that?

This is, for example, needed to solve the issue reported in
https://www.cygwin.com/ml/cygwin/2020-01/msg00147.html.

I looked into this problem, and found that cgdb read output of
gdb from pty master and write it to ncurses. The output from
pty master includes a lot of escape sequences which are generated
by pseudo console, however, ncurses does not pass-through them
and shows garbages. This is the cause of that issue.

cgdb is the only program do such things so far, however, there
may be more programs which do not expect escape sequences read
from pty.

There is no way to control pseudo console not to generate
escape sequences, therefore, I proposed this patch.

--
Takashi Yano <[hidden email]>
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Cygwin: pty: Introduce disable_pcon in environment CYGWIN.

Jon TURNEY
On 20/01/2020 12:41, Takashi Yano wrote:

> Hi Corinna,
>
> On Mon, 20 Jan 2020 11:06:46 +0100
> Corinna Vinschen wrote:
>> On Jan 20 11:50, Takashi Yano wrote:
>>> - For programs which does not work properly with pseudo console,
>>>    disable_pcon in environment CYGWIN is introduced. If disable_pcon
>>>    is set, pseudo console support is disabled.
>> Oh well, do we really need that?
>
> This is, for example, needed to solve the issue reported in
> https://www.cygwin.com/ml/cygwin/2020-01/msg00147.html.
>
> I looked into this problem, and found that cgdb read output of
> gdb from pty master and write it to ncurses. The output from
> pty master includes a lot of escape sequences which are generated
> by pseudo console, however, ncurses does not pass-through them
> and shows garbages. This is the cause of that issue.
>
> cgdb is the only program do such things so far, however, there
> may be more programs which do not expect escape sequences read
> from pty.
>
> There is no way to control pseudo console not to generate
> escape sequences, therefore, I proposed this patch.
>

I think this may actually be an issue with cgdb being old.

The latest gdb enables "output styling" using ANSI escape sequences by
default, but our cgdb can't handle them?

See: https://github.com/cgdb/cgdb/issues/211
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Cygwin: pty: Introduce disable_pcon in environment CYGWIN.

Ken Brown-6
[Adding the cgdb maintainer to the CC.]

On 1/20/2020 9:18 AM, Jon Turney wrote:

> On 20/01/2020 12:41, Takashi Yano wrote:
>> Hi Corinna,
>>
>> On Mon, 20 Jan 2020 11:06:46 +0100
>> Corinna Vinschen wrote:
>>> On Jan 20 11:50, Takashi Yano wrote:
>>>> - For programs which does not work properly with pseudo console,
>>>>    disable_pcon in environment CYGWIN is introduced. If disable_pcon
>>>>    is set, pseudo console support is disabled.
>>> Oh well, do we really need that?
>>
>> This is, for example, needed to solve the issue reported in
>> https://www.cygwin.com/ml/cygwin/2020-01/msg00147.html.
>>
>> I looked into this problem, and found that cgdb read output of
>> gdb from pty master and write it to ncurses. The output from
>> pty master includes a lot of escape sequences which are generated
>> by pseudo console, however, ncurses does not pass-through them
>> and shows garbages. This is the cause of that issue.
>>
>> cgdb is the only program do such things so far, however, there
>> may be more programs which do not expect escape sequences read
>> from pty.
>>
>> There is no way to control pseudo console not to generate
>> escape sequences, therefore, I proposed this patch.
>>
>
> I think this may actually be an issue with cgdb being old.
>
> The latest gdb enables "output styling" using ANSI escape sequences by default,
> but our cgdb can't handle them?
>
> See: https://github.com/cgdb/cgdb/issues/211
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Cygwin: pty: Introduce disable_pcon in environment CYGWIN.

Takashi Yano
On Mon, 20 Jan 2020 14:58:52 +0000
Ken Brown wrote:

> [Adding the cgdb maintainer to the CC.]
>
> On 1/20/2020 9:18 AM, Jon Turney wrote:
> > On 20/01/2020 12:41, Takashi Yano wrote:
> >> Hi Corinna,
> >>
> >> On Mon, 20 Jan 2020 11:06:46 +0100
> >> Corinna Vinschen wrote:
> >>> On Jan 20 11:50, Takashi Yano wrote:
> >>>> - For programs which does not work properly with pseudo console,
> >>>>    disable_pcon in environment CYGWIN is introduced. If disable_pcon
> >>>>    is set, pseudo console support is disabled.
> >>> Oh well, do we really need that?
> >>
> >> This is, for example, needed to solve the issue reported in
> >> https://www.cygwin.com/ml/cygwin/2020-01/msg00147.html.
> >>
> >> I looked into this problem, and found that cgdb read output of
> >> gdb from pty master and write it to ncurses. The output from
> >> pty master includes a lot of escape sequences which are generated
> >> by pseudo console, however, ncurses does not pass-through them
> >> and shows garbages. This is the cause of that issue.
> >>
> >> cgdb is the only program do such things so far, however, there
> >> may be more programs which do not expect escape sequences read
> >> from pty.
> >>
> >> There is no way to control pseudo console not to generate
> >> escape sequences, therefore, I proposed this patch.
> >>
> >
> > I think this may actually be an issue with cgdb being old.
> >
> > The latest gdb enables "output styling" using ANSI escape sequences by default,
> > but our cgdb can't handle them?
> >
> > See: https://github.com/cgdb/cgdb/issues/211

I downloaded latest cgdb source from
https://github.com/cgdb/cgdb
and
https://github.com/atotic/cgdb
then built them in cygwin, however, both do not work under
cygwin pseudo console enabled.

Therefore I think this patch is needed for now.

--
Takashi Yano <[hidden email]>
Reply | Threaded
Open this post in threaded view
|

[PATCH v2] Cygwin: pty: Introduce disable_pcon in environment CYGWIN.

Takashi Yano
- For programs which does not work properly with pseudo console,
  disable_pcon in environment CYGWIN is introduced. If disable_pcon
  is set, pseudo console support is disabled.
---
 winsup/cygwin/environ.cc      | 1 +
 winsup/cygwin/fhandler_tty.cc | 2 ++
 winsup/cygwin/globals.cc      | 1 +
 winsup/doc/cygwinenv.xml      | 6 ++++++
 4 files changed, 10 insertions(+)

diff --git a/winsup/cygwin/environ.cc b/winsup/cygwin/environ.cc
index 8c5ce64e1..7eb4780a8 100644
--- a/winsup/cygwin/environ.cc
+++ b/winsup/cygwin/environ.cc
@@ -120,6 +120,7 @@ static struct parse_thing
   {"reset_com", {&reset_com}, setbool, NULL, {{false}, {true}}},
   {"wincmdln", {&wincmdln}, setbool, NULL, {{false}, {true}}},
   {"winsymlinks", {func: set_winsymlinks}, isfunc, NULL, {{0}, {0}}},
+  {"disable_pcon", {&disable_pcon}, setbool, NULL, {{false}, {true}}},
   {NULL, {0}, setdword, 0, {{0}, {0}}}
 };
 
diff --git a/winsup/cygwin/fhandler_tty.cc b/winsup/cygwin/fhandler_tty.cc
index fff5bebe3..a5db0967b 100644
--- a/winsup/cygwin/fhandler_tty.cc
+++ b/winsup/cygwin/fhandler_tty.cc
@@ -3132,6 +3132,8 @@ is_running_as_service (void)
 bool
 fhandler_pty_master::setup_pseudoconsole ()
 {
+  if (disable_pcon)
+    return false;
   /* If the legacy console mode is enabled, pseudo console seems
      not to work as expected. To determine console mode, registry
      key ForceV2 in HKEY_CURRENT_USER\Console is checked. */
diff --git a/winsup/cygwin/globals.cc b/winsup/cygwin/globals.cc
index ebe8b569f..a9648fe6a 100644
--- a/winsup/cygwin/globals.cc
+++ b/winsup/cygwin/globals.cc
@@ -71,6 +71,7 @@ bool pipe_byte;
 bool reset_com;
 bool wincmdln;
 winsym_t allow_winsymlinks = WSYM_sysfile;
+bool disable_pcon;
 
 bool NO_COPY in_forkee;
 
diff --git a/winsup/doc/cygwinenv.xml b/winsup/doc/cygwinenv.xml
index 6f67cb95d..1bc02f986 100644
--- a/winsup/doc/cygwinenv.xml
+++ b/winsup/doc/cygwinenv.xml
@@ -99,6 +99,12 @@ system call will immediately fail.</para>
 <xref linkend="pathnames-symlinks"></xref>.</para>
 </listitem>
 
+<listitem>
+<para><envar>disable_pcon</envar> - if set, pseudo console support in
+pty will be disabled. This is for programs which does not work properly
+under pty with pseudo console enabled. Defaults to not set.
+</listitem>
+
 </itemizedlist>
 
 </sect2>
--
2.21.0

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2] Cygwin: pty: Introduce disable_pcon in environment CYGWIN.

Corinna Vinschen-2
On Jan 21 22:25, Takashi Yano wrote:
> - For programs which does not work properly with pseudo console,
>   disable_pcon in environment CYGWIN is introduced. If disable_pcon
>   is set, pseudo console support is disabled.

Pushed.  I just fixed a missing </para> in the doc text.


Thanks,
Corinna

--
Corinna Vinschen
Cygwin Maintainer

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2] Cygwin: pty: Introduce disable_pcon in environment CYGWIN.

Thomas Wolff
On 22.01.2020 11:06, Corinna Vinschen wrote:
> On Jan 21 22:25, Takashi Yano wrote:
>> - For programs which does not work properly with pseudo console,
>>    disable_pcon in environment CYGWIN is introduced. If disable_pcon
>>    is set, pseudo console support is disabled.
> Pushed.  I just fixed a missing </para> in the doc text.
>
Sorry I didn't notice this before. I think rather than having to decide
and unconditionally switch on or off, a better approach would be to
automatically enable pseudo console when forking a non-cygwin program
only, or have that as a third option. (I think I had suggested this before.)
It's good we had pseudo console in unconditionally now for a while, as
that apparently helped identifying a bunch of issues, but targetting it
to where it's really needed would further help to avoid future trouble,
including any performance issues as recently reported.
I'm willing to prepare a patch if desired, as I had implemented that
condition already for my earlier "winpty injection" proposal.
Thomas
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2] Cygwin: pty: Introduce disable_pcon in environment CYGWIN.

Corinna Vinschen-2
On Feb  8 18:13, Thomas Wolff wrote:

> On 22.01.2020 11:06, Corinna Vinschen wrote:
> > On Jan 21 22:25, Takashi Yano wrote:
> > > - For programs which does not work properly with pseudo console,
> > >    disable_pcon in environment CYGWIN is introduced. If disable_pcon
> > >    is set, pseudo console support is disabled.
> > Pushed.  I just fixed a missing </para> in the doc text.
> >
> Sorry I didn't notice this before. I think rather than having to decide and
> unconditionally switch on or off, a better approach would be to
> automatically enable pseudo console when forking a non-cygwin program only,
> or have that as a third option. (I think I had suggested this before.)
> It's good we had pseudo console in unconditionally now for a while, as that
> apparently helped identifying a bunch of issues, but targetting it to where
> it's really needed would further help to avoid future trouble, including any
> performance issues as recently reported.
> I'm willing to prepare a patch if desired, as I had implemented that
> condition already for my earlier "winpty injection" proposal.
> Thomas
Interesting idea, but given that all the Pseudo Console code in
Cygwin is from Takashi, he should decide how to go forward.

Takashi?  What do you think?


Thanks,
Corinna

--
Corinna Vinschen
Cygwin Maintainer

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2] Cygwin: pty: Introduce disable_pcon in environment CYGWIN.

Takashi Yano
On Mon, 10 Feb 2020 11:07:10 +0100
Corinna Vinschen wrote:

> On Feb  8 18:13, Thomas Wolff wrote:
> > On 22.01.2020 11:06, Corinna Vinschen wrote:
> > > On Jan 21 22:25, Takashi Yano wrote:
> > > > - For programs which does not work properly with pseudo console,
> > > >    disable_pcon in environment CYGWIN is introduced. If disable_pcon
> > > >    is set, pseudo console support is disabled.
> > > Pushed.  I just fixed a missing </para> in the doc text.
> > >
> > Sorry I didn't notice this before. I think rather than having to decide and
> > unconditionally switch on or off, a better approach would be to
> > automatically enable pseudo console when forking a non-cygwin program only,
> > or have that as a third option. (I think I had suggested this before.)
> > It's good we had pseudo console in unconditionally now for a while, as that
> > apparently helped identifying a bunch of issues, but targetting it to where
> > it's really needed would further help to avoid future trouble, including any
> > performance issues as recently reported.
> > I'm willing to prepare a patch if desired, as I had implemented that
> > condition already for my earlier "winpty injection" proposal.
> > Thomas
>
> Interesting idea, but given that all the Pseudo Console code in
> Cygwin is from Takashi, he should decide how to go forward.
>
> Takashi?  What do you think?

I cannot imagine how to realize this right now. Let me consider.

--
Takashi Yano <[hidden email]>
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2] Cygwin: pty: Introduce disable_pcon in environment CYGWIN.

Thomas Wolff
Am 10.02.2020 um 13:24 schrieb Takashi Yano:

> On Mon, 10 Feb 2020 11:07:10 +0100
> Corinna Vinschen wrote:
>> On Feb  8 18:13, Thomas Wolff wrote:
>>> On 22.01.2020 11:06, Corinna Vinschen wrote:
>>>> On Jan 21 22:25, Takashi Yano wrote:
>>>>> - For programs which does not work properly with pseudo console,
>>>>>     disable_pcon in environment CYGWIN is introduced. If disable_pcon
>>>>>     is set, pseudo console support is disabled.
>>>> Pushed.  I just fixed a missing </para> in the doc text.
>>>>
>>> Sorry I didn't notice this before. I think rather than having to decide and
>>> unconditionally switch on or off, a better approach would be to
>>> automatically enable pseudo console when forking a non-cygwin program only,
>>> or have that as a third option. (I think I had suggested this before.)
>>> It's good we had pseudo console in unconditionally now for a while, as that
>>> apparently helped identifying a bunch of issues, but targetting it to where
>>> it's really needed would further help to avoid future trouble, including any
>>> performance issues as recently reported.
>>> I'm willing to prepare a patch if desired, as I had implemented that
>>> condition already for my earlier "winpty injection" proposal.
>>> Thomas
>> Interesting idea, but given that all the Pseudo Console code in
>> Cygwin is from Takashi, he should decide how to go forward.
>>
>> Takashi?  What do you think?
> I cannot imagine how to realize this right now. Let me consider.
>
See https://cygwin.com/ml/cygwin-developers/2018-04/msg00002.html for my
patch.