winpty injection

[Thomas Wolff]

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
-------------- next part --------------
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