Skip to content

mingw-isatty-v1

I finally got a chance to debug the problems with the ever-timing-out
test runs of `pu` on Windows. Turns out that pb/bisect uncovered a
really old, really bad bug: on Windows, isatty() does not do what Git
expects, at least not completely: it detects interactive terminals *and
character devices*.

Why is this such a big deal?

One such character device is NUL, Windows' equivalent of /dev/null. And
guess what happens when the new tests of the bisect--helper run, with
stdin redirected from /dev/null? Precisely. Git asks "the user" for
reassurance that it may really continue, waiting forever. Or for Ctrl+C.

As we know what Git's source code wants, we have to make extra certain
to test whether isatty() reports success for a Console. The common way
to do this is to run GetConsoleMode() for input file descriptors, and
GetConsoleScreenBufferInfo() for output file descriptors.

One additional note: the new winansi_isatty() function was put into this
particular spot not only because it vaguely makes sense to put
tty-related stuff into compat/winansi.c, but with required future
changes in mind:

The current way in which Git for Windows makes sure that isatty()
returns non-zero for Git Bash (which runs in a terminal emulator called
MinTTY that does *not* have any Windows Console associated with it, and
therefore Windows' _isatty() would actually return 0 if it was not for
our detect_msys_tty() function) is hacky and needs to be fixed properly.

It is hacky because it relies on internals of the MSVC runtime that do
not hold true for the new Universal runtimes, which are used when
compiling with Visual C.

We already have experimental code to future-proof this method, and we
use that already when compiling Git for Windows in Visual Studio.

The place in which winansi_isatty() now lives will hopefully make it
possible to unify the code paths again, so that both GCC and Visual C
use detect_msys_tty() through winansi_isatty().

This will also fix a bug where current Visual C-built Git may misdetect
a reopened stdin to be connected to an interactive terminal.

Johannes Schindelin (1):
  mingw: intercept isatty() to handle /dev/null as Git expects it

 compat/mingw.h   |  3 +++
 compat/winansi.c | 33 +++++++++++++++++++++++++++++++++
 2 files changed, 36 insertions(+)

base-commit: 8d7a455ed52e2a96debc080dfc011b6bb00db5d2
--
2.11.0.rc3.windows.1

From 42ddc270ea04e01e899cc479063e5d602e4a4448 Mon Sep 17 00:00:00 2001
Message-Id: <42ddc270ea04e01e899cc479063e5d602e4a4448.1481454992.git.johannes.schindelin@gmx.de>
In-Reply-To: <cover.1481454992.git.johannes.schindelin@gmx.de>
References: <cover.1481454992.git.johannes.schindelin@gmx.de>
From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Sun, 11 Dec 2016 11:39:55 +0100
Subject: [PATCH 1/1] mingw: intercept isatty() to handle /dev/null as Git
 expects it
Content-Type: text/plain; charset=UTF-8
Fcc: Sent
To: git@vger.kernel.org
Cc: Junio C Hamano <gitster@pobox.com>,
    Pranit Bauva <pranit.bauva@gmail.com>

When Git's source code calls isatty(), it really asks whether the
respective file descriptor is connected to an interactive terminal.

Windows' _isatty() function, however, determines whether the file
descriptor is associated with a character device. And NUL, Windows'
equivalent of /dev/null, is a character device.

Which means that for years, Git mistakenly detected an associated
interactive terminal when being run through the test suite, which
almost always redirects stdin, stdout and stderr to /dev/null.

This bug only became obvious, and painfully so, when the new
bisect--helper entered the `pu` branch and made the automatic build & test
time out because t6030 was waiting for an answer.

For details, see

	https://msdn.microsoft.com/en-us/library/f4s0ddew.aspx

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 compat/mingw.h   |  3 +++
 compat/winansi.c | 33 +++++++++++++++++++++++++++++++++
 2 files changed, 36 insertions(+)

diff --git a/compat/mingw.h b/compat/mingw.h
index 034fff9479..3350169555 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -384,6 +384,9 @@ int mingw_raise(int sig);
  * ANSI emulation wrappers
  */

+int winansi_isatty(int fd);
+#define isatty winansi_isatty
+
 void winansi_init(void);
 HANDLE winansi_get_osfhandle(int fd);

diff --git a/compat/winansi.c b/compat/winansi.c
index db4a5b0a37..cb725fb02f 100644
--- a/compat/winansi.c
+++ b/compat/winansi.c
@@ -7,6 +7,9 @@
 #include <wingdi.h>
 #include <winreg.h>

+/* In this file, we actually want to use Windows' own isatty(). */
+#undef isatty
+
 /*
  ANSI codes used by git: m, K

@@ -570,6 +573,36 @@ static void detect_msys_tty(int fd)

 #endif

+int winansi_isatty(int fd)
+{
+	int res = isatty(fd);
+
+	if (res) {
+		/*
+		 * Make sure that /dev/null is not fooling Git into believing
+		 * that we are connected to a terminal, as "_isatty() returns a
+		 * nonzero value if the descriptor is associated with a
+		 * character device."; for more information, see
+		 *
+		 * https://msdn.microsoft.com/en-us/library/f4s0ddew.aspx
+		 */
+		HANDLE handle = (HANDLE)_get_osfhandle(fd);
+		if (fd == STDIN_FILENO) {
+			DWORD dummy;
+
+			if (!GetConsoleMode(handle, &dummy))
+				res = 0;
+		} else if (fd == STDOUT_FILENO || fd == STDERR_FILENO) {
+			CONSOLE_SCREEN_BUFFER_INFO dummy;
+
+			if (!GetConsoleScreenBufferInfo(handle, &dummy))
+				res = 0;
+		}
+	}
+
+	return res;
+}
+
 void winansi_init(void)
 {
 	int con1, con2;
--
2.11.0.rc3.windows.1

Submitted-As: https://public-inbox.org/git/cover.1481454992.git.johannes.schindelin@gmx.de
Assets 2