[PATCH v3 0/3] Really fix the isatty() problem on Windows

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



My previous fix may have fixed running the new
t/t6030-bisect-porcelain.sh script that tested the new bisect--helper,
which in turn used isatty() to determine whether it was running
interactively and was fooled by being redirected to /dev/null.

But it not only broke paging when running in a CMD window, due to
testing in the wrong worktree I also missed that it broke paging in Git
for Windows 2.x' Git Bash (i.e. a MinTTY terminal emulator).

Let's use this opportunity to actually clean up the entire isatty() mess
once and for all, as part of the problem was introduced by a clever hack
that messes with internals of the Microsoft C runtime, and which changed
recently, so it was not such a clever hack to begin with.

Happily, one of my colleagues had to address that latter problem
recently when he was tasked to make Git compile with Microsoft Visual C
(the rationale: debugging facilities of Visual Studio are really
outstanding, try them if you get a chance).

And incidentally, replacing the previous hack with the clean, new
solution, which specifies explicitly for the file descriptors 0, 1 and 2
whether we detected an MSYS2 pseudo-tty, whether we detected a real
Win32 Console, and whether we had to swap out a real Win32 Console for a
pipe to allow child processes to inherit it.

While at it (or, actually, more like: as I already made this part of v1
by mistake), upstream the patch carried in Git for Windows that supports
color when running Git for Windows in Cygwin terminals.

Changes since v2:

- reworded the comment about "recycling handles"

- moved the reassignment of the `console` variable before the dup2()
  call so that it is valid at all times

- removed the "handle = INVALID_HANDLE_VALUE" assignment, as the local
  variable `handle` is not used afterwards anyway

- generated the patches with --patience in the hope to improve
  readability


Alan Davies (1):
  mingw: fix colourization on Cygwin pseudo terminals

Jeff Hostetler (1):
  mingw: replace isatty() hack

Johannes Schindelin (1):
  mingw: adjust is_console() to work with stdin

 compat/winansi.c | 195 +++++++++++++++++++++++--------------------------------
 1 file changed, 81 insertions(+), 114 deletions(-)


base-commit: 1d1bdafd64266e5ee3bd46c6965228f32e4022ea
Published-As: https://github.com/dscho/git/releases/tag/mingw-isatty-fixup-v3
Fetch-It-Via: git fetch https://github.com/dscho/git mingw-isatty-fixup-v3

Interdiff vs v2:

 diff --git a/compat/winansi.c b/compat/winansi.c
 index 477209fce7..56658ec4f8 100644
 --- a/compat/winansi.c
 +++ b/compat/winansi.c
 @@ -494,19 +494,16 @@ static HANDLE swap_osfhnd(int fd, HANDLE new_handle)
  	 * It is because of this implicit close() that we created the
  	 * copy of the original.
  	 *
 -	 * Note that the OS can recycle HANDLE (numbers) just like it
 -	 * recycles fd (numbers), so we must update the cached value
 -	 * of "console".  You can use GetFileType() to see that
 -	 * handle and _get_osfhandle(fd) may have the same number
 -	 * value, but they refer to different actual files now.
 +	 * Note that we need to update the cached console handle to the
 +	 * duplicated one because the dup2() call will implicitly close
 +	 * the original one.
  	 *
  	 * Note that dup2() when given target := {0,1,2} will also
  	 * call SetStdHandle(), so we don't need to worry about that.
  	 */
 -	dup2(new_fd, fd);
  	if (console == handle)
  		console = duplicate;
 -	handle = INVALID_HANDLE_VALUE;
 +	dup2(new_fd, fd);
  
  	/* Close the temp fd.  This explicitly closes "new_handle"
  	 * (because it has been associated with it).

-- 
2.11.0.rc3.windows.1




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]