public inbox for cygwin-developers@cygwin.com
 help / color / mirror / Atom feed
* winpty injection
@ 2018-04-04 17:07 Thomas Wolff
  2018-04-04 22:04 ` spawn - where can I get it Mark A. Engel
                   ` (2 more replies)
  0 siblings, 3 replies; 18+ messages in thread
From: Thomas Wolff @ 2018-04-04 17:07 UTC (permalink / raw)
  To: cygwin-developers

[-- Attachment #1: Type: text/plain, Size: 1733 bytes --]

The interworking problems of pty-based terminals with native Windows 
programs have long been discussed and raise recurring user reports and 
complaints. Mintty issues https://github.com/mintty/mintty/issues/56 
(Improve support for native console programs) and 
https://github.com/mintty/mintty/issues/376 (Use different encodings for 
native Windows commands output and remote output) have dozens of duplicates.
There is the fairly well-known wrapper winpty which solves most of the 
interworking problems. However, it needs to be involved explicitly and 
selectively for Windows command-line applications.

So I had the idea that cygwin could apply winpty wrapping automatically 
when it detects that a non-cygwin command-line program is being called. 
The attached patch demonstrates how this would work. It is a 
proof-of-concept, not ready for integration but provided for discussion 
of the approach (thus not sent to cygwin-patches).
If it is appreciated, there are a number of open issues:
* The patch offers two methods. The activated one (#ifdef 
inject_winpty_recursively) works but leaves a memory leak in the argv1 
array needed to expand the argv with the injected wrapper. The other 
one, which tries to inject winpty into argv and then continue the 
function linearly, would solve the memory leak but does not work (yet)...
* The winpty tool is not even a cygwin package yet. If the injection 
approach is approved, it should be packaged as a base package, or even 
its code could be migrated into cygwin.
* As some people may object such a magic (and it might in fact raise 
unexpected problems), it could also be made dependent on a setting in 
the CYGWIN environment variable.

Looking forward to feedback
Thomas

[-- Attachment #2: 0000-winpty-injection-proof-of-concept.patch --]
[-- Type: text/plain, Size: 3338 bytes --]

From 2233529613af5a3b569a078d4e33334bbf8bd7e2 Mon Sep 17 00:00:00 2001
From: Thomas Wolff <towo@towo.net>
Date: Fri, 23 Mar 2018 22:34:11 +0100
Subject: [PATCH] winpty injection (proof of concept)

If a non-cygwin program is run from a pty-based environment,
the winpty wrapper is injected to adapt:
* pty handling, especially raw input
* character set conversion
* limited signal handling interworking
---
 winsup/cygwin/spawn.cc | 50 +++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 49 insertions(+), 1 deletion(-)

diff --git a/winsup/cygwin/spawn.cc b/winsup/cygwin/spawn.cc
index 37db526..3cd442e 100644
--- a/winsup/cygwin/spawn.cc
+++ b/winsup/cygwin/spawn.cc
@@ -292,6 +292,7 @@ child_info_spawn::worker (const char *prog_arg, const char *const *argv,
   av newargv;
   linebuf cmd;
   PWCHAR envblock = NULL;
+  char * * argv1 = 0;
   path_conv real_path;
   bool reset_sendsig = false;
 
@@ -332,10 +333,53 @@ child_info_spawn::worker (const char *prog_arg, const char *const *argv,
 	}
 
       res = newargv.setup (prog_arg, real_path, ext, ac, argv, p_type_exec);
-
       if (res)
 	__leave;
 
+      /* winpty injection */
+
+      // provide a check for proper installation of winpty;
+      // this slows us down a bit (thus called last below),
+      // maybe it could check winpty.exe only or nothing...
+#define winpty_available()	\
+		   !access ("/bin/winpty.exe", R_OK | X_OK) \
+		&& !access ("/bin/winpty.dll", R_OK | X_OK) \
+		&& !access ("/bin/winpty-agent.exe", R_OK | X_OK)
+
+      bool ispty = !strncmp (ttyname (0), "/dev/pty", 8);
+      bool isgui = false;
+      // we could check for a GUI program, as determined by `file` ("(GUI)"),
+      // and possibly others (not "(console)"), which do not need the wrapper
+      debug_printf("winpty injection check %s cyg %d pty %d\n", prog_arg, real_path.iscygexec (), ispty);
+      if (!real_path.iscygexec () && ispty && !isgui && winpty_available ())
+	{
+	  debug_printf("argv1 allocating\n");
+	  argv1 = (char**) malloc (sizeof(char*) * (ac + 2));
+	  for (ac = 0; argv[ac]; ac++)
+	    argv1[ac + 1] = (char*) argv[ac];
+	  argv1[ac + 1] = 0;
+	  argv1[0] = (char*) prog_arg;
+
+#define inject_winpty_recursively
+
+#ifdef inject_winpty_recursively
+	  res = worker ("/bin/winpty", argv1, envp, mode, in__stdin, in__stdout);
+	  debug_printf("worker -> %d\n", res);
+	  // if we return here, probably winpty was not found, so we could 
+	  // try to continue and skip __leave, but that does not work
+	  __leave;
+#else
+	  /* repeat newargv setup above */
+	  // this method does not work, somehow the parameters are mangled
+	  // but it would resolve the argv1 memory leak
+	  res = newargv.setup ("/bin/winpty", real_path, ext, ac, argv1, p_type_exec);
+	  free (argv1); argv1 = 0; // with this method, drop free() below
+	  if (res)
+	    __leave;
+	  debug_printf("continuing after argv1 adjustment\n");
+#endif
+	}
+
       if (!real_path.iscygexec () && ::cygheap->cwd.get_error ())
 	{
 	  small_printf ("Error: Current working directory %s.\n"
@@ -846,6 +890,10 @@ child_info_spawn::worker (const char *prog_arg, const char *const *argv,
   this->cleanup ();
   if (envblock)
     free (envblock);
+  if (argv1) {
+    debug_printf("argv1 freeing\n", argv1);
+    free (argv1);
+  }
   return (int) res;
 }
 
-- 
2.16.2


^ permalink raw reply	[flat|nested] 18+ messages in thread

* spawn - where can I get it
  2018-04-04 17:07 winpty injection Thomas Wolff
@ 2018-04-04 22:04 ` Mark A. Engel
  2018-04-04 22:55   ` Victor Corral
  2018-04-05  6:49 ` winpty injection David Macek
  2018-04-05  8:47 ` Corinna Vinschen
  2 siblings, 1 reply; 18+ messages in thread
From: Mark A. Engel @ 2018-04-04 22:04 UTC (permalink / raw)
  To: cygwin-developers

cygwin team,
I am trying to install cross compiler to cygwin environment.  The build 
stops when it can't find the "spawn" command.  I tried to download it 
with the cygwin64_setup tool.  I can't find it there and nowhere in the 
internet.  Do you have any idea where I might get/download the spawn 
command for cygwing64?

-- 
Mark Engel
Software Engineer
630 960-5300
mark.engel@comcast.net

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: spawn - where can I get it
  2018-04-04 22:04 ` spawn - where can I get it Mark A. Engel
@ 2018-04-04 22:55   ` Victor Corral
  2018-04-04 23:57     ` Mark A. Engel
  0 siblings, 1 reply; 18+ messages in thread
From: Victor Corral @ 2018-04-04 22:55 UTC (permalink / raw)
  To: cygwin-developers

you have to connet to ftp://ges.redhat.com

On Wednesday, April 4, 2018, Mark A. Engel <mark.engel@comcast.net> wrote:

> cygwin team,
> I am trying to install cross compiler to cygwin environment.  The build
> stops when it can't find the "spawn" command.  I tried to download it with
> the cygwin64_setup tool.  I can't find it there and nowhere in the
> internet.  Do you have any idea where I might get/download the spawn
> command for cygwing64?u
>
> --
> Mark Engel
> Software Engineer
> 630 960-5300
> mark.engel@comcast.net
>
>

-- 
Victor Corral

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: spawn - where can I get it
  2018-04-04 22:55   ` Victor Corral
@ 2018-04-04 23:57     ` Mark A. Engel
  2018-04-05  5:39       ` Mark Geisert
  0 siblings, 1 reply; 18+ messages in thread
From: Mark A. Engel @ 2018-04-04 23:57 UTC (permalink / raw)
  To: cygwin-developers

Victor,
Thanks but I get the "Address not found" screen.  Also it looks like 
that is rehat site.  I am running cygwin64 on my Windows 10 laptop, so 
will this run in cygwin or do I need some other site. Either case  - 
address not found.

Victor Corral wrote:
> you have to connet to ftp://ges.redhat.com
>
> On Wednesday, April 4, 2018, Mark A. Engel <mark.engel@comcast.net> wrote:
>
>> cygwin team,
>> I am trying to install cross compiler to cygwin environment.  The build
>> stops when it can't find the "spawn" command.  I tried to download it with
>> the cygwin64_setup tool.  I can't find it there and nowhere in the
>> internet.  Do you have any idea where I might get/download the spawn
>> command for cygwing64?u
>>
>> --
>> Mark Engel
>> Software Engineer
>> 630 960-5300
>> mark.engel@comcast.net
>>
>>

-- 
Mark Engel
Software Engineer
630 960-5300
mark.engel@comcast.net

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: spawn - where can I get it
  2018-04-04 23:57     ` Mark A. Engel
@ 2018-04-05  5:39       ` Mark Geisert
  0 siblings, 0 replies; 18+ messages in thread
From: Mark Geisert @ 2018-04-05  5:39 UTC (permalink / raw)
  To: cygwin; +Cc: cygwin-developers

On Wed, 4 Apr 2018, Mark A. Engel wrote:
> Victor,
> Thanks but I get the "Address not found" screen.  Also it looks like that is 
> rehat site.  I am running cygwin64 on my Windows 10 laptop, so will this run 
> in cygwin or do I need some other site. Either case  - address not found.
>
> Victor Corral wrote:
>> you have to connet to ftp://ges.redhat.com
>> 
>> On Wednesday, April 4, 2018, Mark A. Engel <mark.engel@comcast.net> wrote:
>> 
>>> cygwin team,
>>> I am trying to install cross compiler to cygwin environment.  The build
>>> stops when it can't find the "spawn" command.  I tried to download it with
>>> the cygwin64_setup tool.  I can't find it there and nowhere in the
>>> internet.  Do you have any idea where I might get/download the spawn
>>> command for cygwing64?u

(1) Please don't start new email threads by replying to an unrelated 
email.  Start with a new email to the correct list.

(2) This is not the correct list for your question.  It should have been 
sent to cygwin@cygwin.com.  I'm CCing that list so this query can be 
continued there.

(3) It's not clear from your email if you need a 'spawn' command as part 
of the execution of the build itself, or if one of the source files being 
built requires a spawn() function which has not been found in an include 
file.

Could you please reply quoting a few lines around the error in your build 
log?  (But do that on the correct mailing list please.)

..mark

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: winpty injection
  2018-04-04 17:07 winpty injection Thomas Wolff
  2018-04-04 22:04 ` spawn - where can I get it Mark A. Engel
@ 2018-04-05  6:49 ` David Macek
  2018-04-05  8:47 ` Corinna Vinschen
  2 siblings, 0 replies; 18+ messages in thread
From: David Macek @ 2018-04-05  6:49 UTC (permalink / raw)
  To: cygwin-developers

[-- Attachment #1: Type: text/plain, Size: 227 bytes --]

IMO:

It makes sense to have this functionality folded into Cygwin, same way as it currently supports running inside Conhost.  The process tree doesn't seem right though (but maybe that's a non-issue).

-- 
David Macek


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4002 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: winpty injection
  2018-04-04 17:07 winpty injection Thomas Wolff
  2018-04-04 22:04 ` spawn - where can I get it Mark A. Engel
  2018-04-05  6:49 ` winpty injection David Macek
@ 2018-04-05  8:47 ` Corinna Vinschen
  2018-04-05 13:25   ` Johannes Schindelin
  2 siblings, 1 reply; 18+ messages in thread
From: Corinna Vinschen @ 2018-04-05  8:47 UTC (permalink / raw)
  To: cygwin-developers

[-- Attachment #1: Type: text/plain, Size: 5119 bytes --]

On Apr  4 19:07, Thomas Wolff wrote:
> The interworking problems of pty-based terminals with native Windows
> programs have long been discussed and raise recurring user reports and
> complaints. Mintty issues https://github.com/mintty/mintty/issues/56
> (Improve support for native console programs) and
> https://github.com/mintty/mintty/issues/376 (Use different encodings for
> native Windows commands output and remote output) have dozens of duplicates.
> There is the fairly well-known wrapper winpty which solves most of the
> interworking problems. However, it needs to be involved explicitly and
> selectively for Windows command-line applications.
> 
> So I had the idea that cygwin could apply winpty wrapping automatically when
> it detects that a non-cygwin command-line program is being called. The
> attached patch demonstrates how this would work. It is a proof-of-concept,
> not ready for integration but provided for discussion of the approach (thus
> not sent to cygwin-patches).

The idea is not bad, in general.  A few points, though.

> If it is appreciated, there are a number of open issues:
> * The patch offers two methods. The activated one (#ifdef
> inject_winpty_recursively) works but leaves a memory leak in the argv1 array
> needed to expand the argv with the injected wrapper. The other one, which
> tries to inject winpty into argv and then continue the function linearly,
> would solve the memory leak but does not work (yet)...

The problem is that you're trying to do the stuff in
child_info_spawn::worker, while the interesting action for this case
occurs in av::setup.  You should be able to do your stuff there without
too much problems.

Additionally this function opens the executable anyway to check if it's
a Cygwin executable.  Check out the __try/__except block starting at
spawn.cc line 1090.  The hook_or_detect_cygwin function conveniently
returns a value in subsys.  It's not used at the moment, but this is
the PE/COFF header info returning the subsystem.  I.e.
IMAGE_SUBSYSTEM_WINDOWS_GUI = 2, IMAGE_SUBSYSTEM_WINDOWS_CUI = 3, ...

> * The winpty tool is not even a cygwin package yet. If the injection
> approach is approved, it should be packaged as a base package, or even its
> code could be migrated into cygwin.

Sure for the Cygwin package.  Not so sure if the agent part could be
included into Cygwin.  This may be better discussed with the original
author of the code.

> * As some people may object such a magic (and it might in fact raise
> unexpected problems), it could also be made dependent on a setting in the
> CYGWIN environment variable.

Nah.  If it improves the situation, nobody will complain.  However, it
may slow down calling native Windows apps considerably when used a lot
from scripts.

> Looking forward to feedback
> Thomas

> From 2233529613af5a3b569a078d4e33334bbf8bd7e2 Mon Sep 17 00:00:00 2001
> From: Thomas Wolff <towo@towo.net>
> Date: Fri, 23 Mar 2018 22:34:11 +0100
> Subject: [PATCH] winpty injection (proof of concept)
> 
> If a non-cygwin program is run from a pty-based environment,
> the winpty wrapper is injected to adapt:
> * pty handling, especially raw input
> * character set conversion
> * limited signal handling interworking
> ---
>  winsup/cygwin/spawn.cc | 50 +++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 49 insertions(+), 1 deletion(-)
> 
> diff --git a/winsup/cygwin/spawn.cc b/winsup/cygwin/spawn.cc
> index 37db526..3cd442e 100644
> --- a/winsup/cygwin/spawn.cc
> +++ b/winsup/cygwin/spawn.cc
> @@ -292,6 +292,7 @@ child_info_spawn::worker (const char *prog_arg, const char *const *argv,
>    av newargv;
>    linebuf cmd;
>    PWCHAR envblock = NULL;
> +  char * * argv1 = 0;

Style.  Please make sure you stick to the style of the surrounding
code.  Here:   char **argv1 = NULL;

>    path_conv real_path;
>    bool reset_sendsig = false;
>  
> @@ -332,10 +333,53 @@ child_info_spawn::worker (const char *prog_arg, const char *const *argv,
>  	}
>  
>        res = newargv.setup (prog_arg, real_path, ext, ac, argv, p_type_exec);
> -
>        if (res)
>  	__leave;
>  
> +      /* winpty injection */
> +
> +      // provide a check for proper installation of winpty;
> +      // this slows us down a bit (thus called last below),
> +      // maybe it could check winpty.exe only or nothing...

Please use /* ... */ except for very short oneliner comments at the end
of an expression (and preferredly not even then).

> +#define winpty_available()	\
> +		   !access ("/bin/winpty.exe", R_OK | X_OK) \
> +		&& !access ("/bin/winpty.dll", R_OK | X_OK) \
> +		&& !access ("/bin/winpty-agent.exe", R_OK | X_OK)

Ideally this check should only actually check for the /bin/winpty.exe
executable and ideally this information is stored in shared storage
so it's only called once or, at least, very seldom.


Thanks,
Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Maintainer                 cygwin AT cygwin DOT com
Red Hat

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: winpty injection
  2018-04-05  8:47 ` Corinna Vinschen
@ 2018-04-05 13:25   ` Johannes Schindelin
  2018-04-05 18:33     ` Thomas Wolff
  0 siblings, 1 reply; 18+ messages in thread
From: Johannes Schindelin @ 2018-04-05 13:25 UTC (permalink / raw)
  To: cygwin-developers

Hi Corinna,

On Thu, 5 Apr 2018, Corinna Vinschen wrote:

> On Apr  4 19:07, Thomas Wolff wrote:
> 
> > * As some people may object such a magic (and it might in fact raise
> > unexpected problems), it could also be made dependent on a setting in the
> > CYGWIN environment variable.
> 
> Nah.  If it improves the situation, nobody will complain.  However, it
> may slow down calling native Windows apps considerably when used a lot
> from scripts.

We suggest `winpty` quite a bit in Git for Windows' bug tracker. And it is
not without problems... for example, when calling Maven in MinTTY via
`winpty mvn`, the `TERM` variable is still set as appropriate for MinTTY,
and consequently, the color codes of Maven's output show up as ugly
special characters...

Ciao,
Dscho

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: winpty injection
  2018-04-05 13:25   ` Johannes Schindelin
@ 2018-04-05 18:33     ` Thomas Wolff
  2018-04-05 19:41       ` Johannes Schindelin
  0 siblings, 1 reply; 18+ messages in thread
From: Thomas Wolff @ 2018-04-05 18:33 UTC (permalink / raw)
  To: cygwin-developers

Am 05.04.2018 um 15:25 schrieb Johannes Schindelin:
> Hi Corinna,
>
> On Thu, 5 Apr 2018, Corinna Vinschen wrote:
>
>> On Apr  4 19:07, Thomas Wolff wrote:
>>
>>> * As some people may object such a magic (and it might in fact raise
>>> unexpected problems), it could also be made dependent on a setting in the
>>> CYGWIN environment variable.
>> Nah.  If it improves the situation, nobody will complain.  However, it
>> may slow down calling native Windows apps considerably when used a lot
>> from scripts.
> We suggest `winpty` quite a bit in Git for Windows' bug tracker. And it is
> not without problems... for example, when calling Maven in MinTTY via
> `winpty mvn`, the `TERM` variable is still set as appropriate for MinTTY,
> and consequently, the color codes of Maven's output show up as ugly
> special characters...
This sounds weird, as ANSI colour controls do not differ among 
terminals. What if you set TERM=cygwin or unset TERM before `winpty mvn`?
What's even weirder is that I cannot even reproduce the test case, as 
`winpty mvn` tells me "Could not start 'mvn.exe': The system could not 
find the file. (error 0x2)", although `mvn` works.
Thomas

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: winpty injection
  2018-04-05 18:33     ` Thomas Wolff
@ 2018-04-05 19:41       ` Johannes Schindelin
  2018-04-05 22:07         ` Thomas Wolff
  0 siblings, 1 reply; 18+ messages in thread
From: Johannes Schindelin @ 2018-04-05 19:41 UTC (permalink / raw)
  To: Thomas Wolff; +Cc: cygwin-developers

Hi Thomas,

On Thu, 5 Apr 2018, Thomas Wolff wrote:

> Am 05.04.2018 um 15:25 schrieb Johannes Schindelin:
> >
> > On Thu, 5 Apr 2018, Corinna Vinschen wrote:
> >
> > > On Apr  4 19:07, Thomas Wolff wrote:
> > >
> > > > * As some people may object such a magic (and it might in fact raise
> > > > unexpected problems), it could also be made dependent on a setting in
> > > > the
> > > > CYGWIN environment variable.
> > >
> > > Nah.  If it improves the situation, nobody will complain.  However, it
> > > may slow down calling native Windows apps considerably when used a lot
> > > from scripts.
> >
> > We suggest `winpty` quite a bit in Git for Windows' bug tracker. And it is
> > not without problems... for example, when calling Maven in MinTTY via
> > `winpty mvn`, the `TERM` variable is still set as appropriate for MinTTY,
> > and consequently, the color codes of Maven's output show up as ugly
> > special characters...
>
> This sounds weird, as ANSI colour controls do not differ among
> terminals. What if you set TERM=cygwin or unset TERM before `winpty
> mvn`?  What's even weirder is that I cannot even reproduce the test
> case, as `winpty mvn` tells me "Could not start 'mvn.exe': The system
> could not find the file.  (error 0x2)", although `mvn` works.

In Git for Windows, we set `TERM=xterm`, not `TERM=cygwin` before
launching MinTTY. That might explain the difference?

Also, the reporter claimed the same about `winpty mvn`, as winpty does not
use the shell to execute shell scripts on the `PATH`, and the solution was
to run `winpty sh $(which mvn)`.

See
https://github.com/git-for-windows/git/issues/1470#issuecomment-365618938
for details.

Ciao,
Johannes

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: winpty injection
  2018-04-05 19:41       ` Johannes Schindelin
@ 2018-04-05 22:07         ` Thomas Wolff
  2018-04-06 10:31           ` Johannes Schindelin
  0 siblings, 1 reply; 18+ messages in thread
From: Thomas Wolff @ 2018-04-05 22:07 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: cygwin-developers

Am 05.04.2018 um 21:41 schrieb Johannes Schindelin:
> Hi Thomas,
>
> On Thu, 5 Apr 2018, Thomas Wolff wrote:
>
>> Am 05.04.2018 um 15:25 schrieb Johannes Schindelin:
>>> On Thu, 5 Apr 2018, Corinna Vinschen wrote:
>>>
>>>> On Apr  4 19:07, Thomas Wolff wrote:
>>>>
>>>>> * As some people may object such a magic (and it might in fact raise
>>>>> unexpected problems), it could also be made dependent on a setting in
>>>>> the
>>>>> CYGWIN environment variable.
>>>> Nah.  If it improves the situation, nobody will complain.  However, it
>>>> may slow down calling native Windows apps considerably when used a lot
>>>> from scripts.
>>> We suggest `winpty` quite a bit in Git for Windows' bug tracker. And it is
>>> not without problems... for example, when calling Maven in MinTTY via
>>> `winpty mvn`, the `TERM` variable is still set as appropriate for MinTTY,
>>> and consequently, the color codes of Maven's output show up as ugly
>>> special characters...
>> This sounds weird, as ANSI colour controls do not differ among
>> terminals. What if you set TERM=cygwin or unset TERM before `winpty
>> mvn`?  What's even weirder is that I cannot even reproduce the test
>> case, as `winpty mvn` tells me "Could not start 'mvn.exe': The system
>> could not find the file.  (error 0x2)", although `mvn` works.
> In Git for Windows, we set `TERM=xterm`, not `TERM=cygwin` before
> launching MinTTY. That might explain the difference?
>
> Also, the reporter claimed the same about `winpty mvn`, as winpty does not
> use the shell to execute shell scripts on the `PATH`, and the solution was
> to run `winpty sh $(which mvn)`.
OK, winpty cannot start mvn because that's a shell script, not an exe. 
winpty can start mvn.cmd directly, and in this case (after 
back-converting JAVA_HOME to Windows syntax) I can reproduce the issue. 
However, I do not see where the text output comes from; it's not from 
the mvn wrapper and anything else is Java libraries. So the question is, 
what does the Java runtime do with an escape character output by the 
Java code? Especially as winpty seems to unset TERM (test case: winpty 
cmd, then echo %TERM%). winpty itself lets escape characters pass 
through, at least (test case: winpty echoesc.cmd, which contains echo 
"...." with a colouring escape sequence with verbatim escape characters).
These symptoms suggest to me: winpty is not the culprit, but its 
presence in the invocation chain seems to trigger the effect in a yet 
unclear way.
Thomas

> See
> https://github.com/git-for-windows/git/issues/1470#issuecomment-365618938
> for details.
>
> Ciao,
> Johannes

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: winpty injection
  2018-04-05 22:07         ` Thomas Wolff
@ 2018-04-06 10:31           ` Johannes Schindelin
  2018-04-06 18:44             ` Thomas Wolff
  0 siblings, 1 reply; 18+ messages in thread
From: Johannes Schindelin @ 2018-04-06 10:31 UTC (permalink / raw)
  To: Thomas Wolff; +Cc: cygwin-developers

Hi Thomas,

On Fri, 6 Apr 2018, Thomas Wolff wrote:

> These symptoms suggest to me: winpty is not the culprit, but its
> presence in the invocation chain seems to trigger the effect in a yet
> unclear way.

Sure, `winpty` is not the culprit. But running certain software (such as
Maven) through `winpty` is *not* as unproblematic as we would want.

Therefore, it would make sense *not* to make it the default. Unless you
have a really good strategy in mind how to communicate this change to
users in a way that lets them know about this issue and how to work around
it.

Ciao,
Johannes

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: winpty injection
  2018-04-06 10:31           ` Johannes Schindelin
@ 2018-04-06 18:44             ` Thomas Wolff
  2018-04-06 19:22               ` Johannes Schindelin
  0 siblings, 1 reply; 18+ messages in thread
From: Thomas Wolff @ 2018-04-06 18:44 UTC (permalink / raw)
  To: Johannes Schindelin, cygwin-developers

Hi,

Am 06.04.2018 um 12:31 schrieb Johannes Schindelin:
> Hi Thomas,
>
> On Fri, 6 Apr 2018, Thomas Wolff wrote:
>
>> These symptoms suggest to me: winpty is not the culprit, but its
>> presence in the invocation chain seems to trigger the effect in a yet
>> unclear way.
> Sure, `winpty` is not the culprit.
Actually, as it turns out, winpty *is* the culprit. I've raised winpty 
issue https://github.com/rprichard/winpty/issues/140 about it.
> But running certain software (such as Maven) through `winpty` is *not* as unproblematic as we would want.
>
> Therefore, it would make sense *not* to make it the default. Unless you
> have a really good strategy in mind how to communicate this change to
> users in a way that lets them know about this issue and how to work around it.
Assuming, we're getting winpty tweaked as to the above issue, any 
further concerns?
Cheers
Thomas

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: winpty injection
  2018-04-06 18:44             ` Thomas Wolff
@ 2018-04-06 19:22               ` Johannes Schindelin
  2018-04-09  9:57                 ` Corinna Vinschen
  0 siblings, 1 reply; 18+ messages in thread
From: Johannes Schindelin @ 2018-04-06 19:22 UTC (permalink / raw)
  To: Thomas Wolff; +Cc: cygwin-developers

Hi Thomas,

On Fri, 6 Apr 2018, Thomas Wolff wrote:

> Am 06.04.2018 um 12:31 schrieb Johannes Schindelin:
> >
> > On Fri, 6 Apr 2018, Thomas Wolff wrote:
> >
> > > These symptoms suggest to me: winpty is not the culprit, but its
> > > presence in the invocation chain seems to trigger the effect in a
> > > yet unclear way.
> >
> > Sure, `winpty` is not the culprit.
> >
> Actually, as it turns out, winpty *is* the culprit. I've raised winpty
> issue https://github.com/rprichard/winpty/issues/140 about it.

I am not sure you understand the issue here. The `winpty` helper opens a
Win32 console for the native Windows application to use. Then it polls
this (hidden) console for changes and tries to reflect them in the Cygwin
pty.

If that Windows application writes something to that console containing
Escape sequences, then those Escape sequences occupy certain cells in that
matrix of rows and columns making up that console.

However, if the Windows application uses random-access functions to put
individual characters into cells specified by given absolute positions,
winpty cannot tell the difference. So what winpty would be asked to
consider an ANSI sequence may never have been an ANSI sequence.

Sure, this is a construed example, but it shows that you should not be so
sure that winpty *can* detect ANSI sequences and handle them in a way that
*you* want.

Apart from that, winpty's polling also adds quite a bit of a performance
burden, using more CPU, and lagging in screen updates. Which some users do
not take kindly to.

> > But running certain software (such as Maven) through `winpty` is *not*
> > as unproblematic as we would want.
> >
> > Therefore, it would make sense *not* to make it the default. Unless
> > you have a really good strategy in mind how to communicate this change
> > to users in a way that lets them know about this issue and how to work
> > around it.
>
> Assuming, we're getting winpty tweaked as to the above issue, any further
> concerns?

Just like you seem to have a preference for removing all the structure of
my mails when quoting them by removing all the empty lines, making things
substantially harder to read for me (not to mention uglier to my eyes),
this issue boils down to a matter of personal preferences.

For an individual person, therefore, it would be totally appropriate to
change the default on a whim, according to one person's personal bias.

But not for a widely-used software such as Cygwin.

In the least, therefore, it should be configurable. And I would even argue
that the default behavior should remain the same as current in Cygwin: do
not use winpty by default.

Of course, this is just my opinion, and I am but a user and infrequent
contributor to Cygwin. But I would hope that the Cygwin maintainers use a
lot of care and reluctant deliberation when considering a potentially
disruptive change such as this, a change that may very well occupy a lot
of time in dealing with the unwanted fallout.

Ciao,
Johannes

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: winpty injection
  2018-04-06 19:22               ` Johannes Schindelin
@ 2018-04-09  9:57                 ` Corinna Vinschen
  2018-08-16  7:41                   ` Thomas Wolff
  0 siblings, 1 reply; 18+ messages in thread
From: Corinna Vinschen @ 2018-04-09  9:57 UTC (permalink / raw)
  To: cygwin-developers

[-- Attachment #1: Type: text/plain, Size: 2524 bytes --]

On Apr  6 21:22, Johannes Schindelin wrote:
> Hi Thomas,
> 
> On Fri, 6 Apr 2018, Thomas Wolff wrote:
> 
> > Am 06.04.2018 um 12:31 schrieb Johannes Schindelin:
> > >
> > > On Fri, 6 Apr 2018, Thomas Wolff wrote:
> > >
> > > > These symptoms suggest to me: winpty is not the culprit, but its
> > > > presence in the invocation chain seems to trigger the effect in a
> > > > yet unclear way.
> > >
> > > Sure, `winpty` is not the culprit.
> > >
> > Actually, as it turns out, winpty *is* the culprit. I've raised winpty
> > issue https://github.com/rprichard/winpty/issues/140 about it.
> 
> I am not sure you understand the issue here. The `winpty` helper opens a
> Win32 console for the native Windows application to use. Then it polls
> this (hidden) console for changes and tries to reflect them in the Cygwin
> pty.
> 
> If that Windows application writes something to that console containing
> Escape sequences, then those Escape sequences occupy certain cells in that
> matrix of rows and columns making up that console.
> 
> However, if the Windows application uses random-access functions to put
> individual characters into cells specified by given absolute positions,
> winpty cannot tell the difference. So what winpty would be asked to
> consider an ANSI sequence may never have been an ANSI sequence.
> 
> Sure, this is a construed example, but it shows that you should not be so
> sure that winpty *can* detect ANSI sequences and handle them in a way that
> *you* want.
> [...]
> In the least, therefore, it should be configurable. And I would even argue
> that the default behavior should remain the same as current in Cygwin: do
> not use winpty by default.
> 
> Of course, this is just my opinion, and I am but a user and infrequent
> contributor to Cygwin. But I would hope that the Cygwin maintainers use a
> lot of care and reluctant deliberation when considering a potentially
> disruptive change such as this, a change that may very well occupy a lot
> of time in dealing with the unwanted fallout.

Point taken.  Nicely explained.

However, that makes calling winpty from Cygwin a somewhat questionable
endeavor.  Adding optional code paths which are in all likelyhood not
used very often, thus not tested very often, thus bound to rot, are not
really something I'm looking forward to.


Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Maintainer                 cygwin AT cygwin DOT com
Red Hat

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: winpty injection
  2018-04-09  9:57                 ` Corinna Vinschen
@ 2018-08-16  7:41                   ` Thomas Wolff
  2018-08-16  8:54                     ` Johannes Schindelin
  0 siblings, 1 reply; 18+ messages in thread
From: Thomas Wolff @ 2018-08-16  7:41 UTC (permalink / raw)
  To: cygwin-developers

Am 09.04.2018 um 11:57 schrieb Corinna Vinschen:
> On Apr  6 21:22, Johannes Schindelin wrote:
>> Hi Thomas,
>>
>> On Fri, 6 Apr 2018, Thomas Wolff wrote:
>>
>>> Am 06.04.2018 um 12:31 schrieb Johannes Schindelin:
>>>> On Fri, 6 Apr 2018, Thomas Wolff wrote:
>>>>
>>>>> These symptoms suggest to me: winpty is not the culprit, but its
>>>>> presence in the invocation chain seems to trigger the effect in a
>>>>> yet unclear way.
>>>> Sure, `winpty` is not the culprit.
>>>>
>>> Actually, as it turns out, winpty *is* the culprit. I've raised winpty
>>> issue https://github.com/rprichard/winpty/issues/140 about it.
>> I am not sure you understand the issue here. The `winpty` helper opens a
>> Win32 console for the native Windows application to use. Then it polls
>> this (hidden) console for changes and tries to reflect them in the Cygwin
>> pty.
>>
>> If that Windows application writes something to that console containing
>> Escape sequences, then those Escape sequences occupy certain cells in that
>> matrix of rows and columns making up that console.
>>
>> However, if the Windows application uses random-access functions to put
>> individual characters into cells specified by given absolute positions,
>> winpty cannot tell the difference. So what winpty would be asked to
>> consider an ANSI sequence may never have been an ANSI sequence.
>>
>> Sure, this is a construed example, but it shows that you should not be so
>> sure that winpty *can* detect ANSI sequences and handle them in a way that
>> *you* want.
>> [...]
>> In the least, therefore, it should be configurable. And I would even argue
>> that the default behavior should remain the same as current in Cygwin: do
>> not use winpty by default.
>>
>> Of course, this is just my opinion, and I am but a user and infrequent
>> contributor to Cygwin. But I would hope that the Cygwin maintainers use a
>> lot of care and reluctant deliberation when considering a potentially
>> disruptive change such as this, a change that may very well occupy a lot
>> of time in dealing with the unwanted fallout.
> Point taken.  Nicely explained.
>
> However, that makes calling winpty from Cygwin a somewhat questionable
> endeavor.  Adding optional code paths which are in all likelyhood not
> used very often, thus not tested very often, thus bound to rot, are not
> really something I'm looking forward to.
It seems that the new Windows ConPTY API will be a more reliable 
solution to these issues:
https://blogs.msdn.microsoft.com/commandline/2018/08/02/windows-command-line-introducing-the-windows-pseudo-console-conpty/

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: winpty injection
  2018-08-16  7:41                   ` Thomas Wolff
@ 2018-08-16  8:54                     ` Johannes Schindelin
  2018-08-16 10:36                       ` Thomas Wolff
  0 siblings, 1 reply; 18+ messages in thread
From: Johannes Schindelin @ 2018-08-16  8:54 UTC (permalink / raw)
  To: Thomas Wolff; +Cc: cygwin-developers

Hi Thomas,

On Thu, 16 Aug 2018, Thomas Wolff wrote:

> Am 09.04.2018 um 11:57 schrieb Corinna Vinschen:
> > On Apr  6 21:22, Johannes Schindelin wrote:
> > >
> > > On Fri, 6 Apr 2018, Thomas Wolff wrote:
> > >
> > > > Am 06.04.2018 um 12:31 schrieb Johannes Schindelin:
> > > > > On Fri, 6 Apr 2018, Thomas Wolff wrote:
> > > > >
> > > > > > These symptoms suggest to me: winpty is not the culprit, but its
> > > > > > presence in the invocation chain seems to trigger the effect in a
> > > > > > yet unclear way.
> > > > > Sure, `winpty` is not the culprit.
> > > > >
> > > > Actually, as it turns out, winpty *is* the culprit. I've raised winpty
> > > > issue https://github.com/rprichard/winpty/issues/140 about it.
> > > I am not sure you understand the issue here. The `winpty` helper opens a
> > > Win32 console for the native Windows application to use. Then it polls
> > > this (hidden) console for changes and tries to reflect them in the Cygwin
> > > pty.
> > >
> > > If that Windows application writes something to that console containing
> > > Escape sequences, then those Escape sequences occupy certain cells in that
> > > matrix of rows and columns making up that console.
> > >
> > > However, if the Windows application uses random-access functions to put
> > > individual characters into cells specified by given absolute positions,
> > > winpty cannot tell the difference. So what winpty would be asked to
> > > consider an ANSI sequence may never have been an ANSI sequence.
> > >
> > > Sure, this is a construed example, but it shows that you should not be so
> > > sure that winpty *can* detect ANSI sequences and handle them in a way that
> > > *you* want.
> > > [...]
> > > In the least, therefore, it should be configurable. And I would even argue
> > > that the default behavior should remain the same as current in Cygwin: do
> > > not use winpty by default.
> > >
> > > Of course, this is just my opinion, and I am but a user and infrequent
> > > contributor to Cygwin. But I would hope that the Cygwin maintainers use a
> > > lot of care and reluctant deliberation when considering a potentially
> > > disruptive change such as this, a change that may very well occupy a lot
> > > of time in dealing with the unwanted fallout.
> > Point taken.  Nicely explained.
> >
> > However, that makes calling winpty from Cygwin a somewhat questionable
> > endeavor.  Adding optional code paths which are in all likelyhood not
> > used very often, thus not tested very often, thus bound to rot, are not
> > really something I'm looking forward to.
> It seems that the new Windows ConPTY API will be a more reliable solution to
> these issues:
> https://blogs.msdn.microsoft.com/commandline/2018/08/02/windows-command-line-introducing-the-windows-pseudo-console-conpty/

And of course this will not apply to any Windows 7 or Windows 8 users. Or
to users who are for some reason stuck with a Windows 10 that is not
updated to the latest and greatest update.

Don't get me wrong, I am delighted what my colleagues do there, they are
doing great work, and Windows 10 will offer very nice features that Cygwin
can use to make things more reliable and/or faster.

Cygwin wants to support more Windows versions, though, so winpty will
still be needed, methinks.

Ciao,
Johannes

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: winpty injection
  2018-08-16  8:54                     ` Johannes Schindelin
@ 2018-08-16 10:36                       ` Thomas Wolff
  0 siblings, 0 replies; 18+ messages in thread
From: Thomas Wolff @ 2018-08-16 10:36 UTC (permalink / raw)
  To: Johannes Schindelin, cygwin-developers


Am 16.08.2018 um 10:54 schrieb Johannes Schindelin:
> Hi Thomas,
>
> On Thu, 16 Aug 2018, Thomas Wolff wrote:
>
>> Am 09.04.2018 um 11:57 schrieb Corinna Vinschen:
>>> On Apr  6 21:22, Johannes Schindelin wrote:
>>>> On Fri, 6 Apr 2018, Thomas Wolff wrote:
>>>>
>>>>> Am 06.04.2018 um 12:31 schrieb Johannes Schindelin:
>>>>>> On Fri, 6 Apr 2018, Thomas Wolff wrote:
>>>>>>
>>>>>>> These symptoms suggest to me: winpty is not the culprit, but its
>>>>>>> presence in the invocation chain seems to trigger the effect in a
>>>>>>> yet unclear way.
>>>>>> Sure, `winpty` is not the culprit.
>>>>>>
>>>>> Actually, as it turns out, winpty *is* the culprit. I've raised winpty
>>>>> issue https://github.com/rprichard/winpty/issues/140 about it.
>>>> I am not sure you understand the issue here. The `winpty` helper opens a
>>>> Win32 console for the native Windows application to use. Then it polls
>>>> this (hidden) console for changes and tries to reflect them in the Cygwin
>>>> pty.
>>>>
>>>> If that Windows application writes something to that console containing
>>>> Escape sequences, then those Escape sequences occupy certain cells in that
>>>> matrix of rows and columns making up that console.
>>>>
>>>> However, if the Windows application uses random-access functions to put
>>>> individual characters into cells specified by given absolute positions,
>>>> winpty cannot tell the difference. So what winpty would be asked to
>>>> consider an ANSI sequence may never have been an ANSI sequence.
>>>>
>>>> Sure, this is a construed example, but it shows that you should not be so
>>>> sure that winpty *can* detect ANSI sequences and handle them in a way that
>>>> *you* want.
>>>> [...]
>>>> In the least, therefore, it should be configurable. And I would even argue
>>>> that the default behavior should remain the same as current in Cygwin: do
>>>> not use winpty by default.
>>>>
>>>> Of course, this is just my opinion, and I am but a user and infrequent
>>>> contributor to Cygwin. But I would hope that the Cygwin maintainers use a
>>>> lot of care and reluctant deliberation when considering a potentially
>>>> disruptive change such as this, a change that may very well occupy a lot
>>>> of time in dealing with the unwanted fallout.
>>> Point taken.  Nicely explained.
>>>
>>> However, that makes calling winpty from Cygwin a somewhat questionable
>>> endeavor.  Adding optional code paths which are in all likelyhood not
>>> used very often, thus not tested very often, thus bound to rot, are not
>>> really something I'm looking forward to.
>> It seems that the new Windows ConPTY API will be a more reliable solution to
>> these issues:
>> https://blogs.msdn.microsoft.com/commandline/2018/08/02/windows-command-line-introducing-the-windows-pseudo-console-conpty/
> And of course this will not apply to any Windows 7 or Windows 8 users. Or
> to users who are for some reason stuck with a Windows 10 that is not
> updated to the latest and greatest update.
>
> Don't get me wrong, I am delighted what my colleagues do there, they are
> doing great work, and Windows 10 will offer very nice features that Cygwin
> can use to make things more reliable and/or faster.
>
> Cygwin wants to support more Windows versions, though, so winpty will
> still be needed, methinks.
Sure, I wasn't suggesting winpty might become obsolete. I only meant to 
refer to the "implicit winpty injection" solution I had suggested to be 
embedded into cygwin. With ConPTY, cygwin might provide better 
integration of native Windows apps at least on newer Windows systems.

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2018-08-16 10:36 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-04 17:07 winpty injection Thomas Wolff
2018-04-04 22:04 ` spawn - where can I get it Mark A. Engel
2018-04-04 22:55   ` Victor Corral
2018-04-04 23:57     ` Mark A. Engel
2018-04-05  5:39       ` Mark Geisert
2018-04-05  6:49 ` winpty injection David Macek
2018-04-05  8:47 ` Corinna Vinschen
2018-04-05 13:25   ` Johannes Schindelin
2018-04-05 18:33     ` Thomas Wolff
2018-04-05 19:41       ` Johannes Schindelin
2018-04-05 22:07         ` Thomas Wolff
2018-04-06 10:31           ` Johannes Schindelin
2018-04-06 18:44             ` Thomas Wolff
2018-04-06 19:22               ` Johannes Schindelin
2018-04-09  9:57                 ` Corinna Vinschen
2018-08-16  7:41                   ` Thomas Wolff
2018-08-16  8:54                     ` Johannes Schindelin
2018-08-16 10:36                       ` Thomas Wolff

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).