From: Johannes Schindelin <johannes.schindelin@gmx.de>
To: git@vger.kernel.org
Cc: Junio C Hamano <gitster@pobox.com>,
Pranit Bauva <pranit.bauva@gmail.com>,
Johannes Sixt <j6t@kdbg.org>, Beat Bolli <dev+git@drbeat.li>
Subject: [PATCH v3 0/3] Really fix the isatty() problem on Windows
Date: Fri, 23 Dec 2016 00:16:03 +0100 (CET) [thread overview]
Message-ID: <cover.1482448531.git.johannes.schindelin@gmx.de> (raw)
In-Reply-To: <cover.1482426497.git.johannes.schindelin@gmx.de>
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
next prev parent reply other threads:[~2016-12-22 23:16 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-12-21 17:53 [PATCH 0/2] Really fix the isatty() problem on Windows Johannes Schindelin
2016-12-21 17:53 ` [PATCH 1/2] mingw: adjust is_console() to work with stdin Johannes Schindelin
2016-12-21 17:53 ` [PATCH 2/2] mingw: replace isatty() hack Johannes Schindelin
2016-12-21 18:45 ` Junio C Hamano
2016-12-21 21:15 ` [PATCH 0/2] Really fix the isatty() problem on Windows Johannes Sixt
2016-12-21 21:21 ` Junio C Hamano
2016-12-22 18:48 ` Johannes Sixt
2016-12-22 21:33 ` Johannes Schindelin
2016-12-22 17:08 ` [PATCH v2 0/3] " Johannes Schindelin
2016-12-22 17:08 ` [PATCH v2 1/3] mingw: adjust is_console() to work with stdin Johannes Schindelin
2016-12-22 23:04 ` Beat Bolli
2016-12-22 23:18 ` Junio C Hamano
2016-12-23 9:30 ` Johannes Schindelin
2016-12-23 12:51 ` Beat Bolli
2016-12-22 17:09 ` [PATCH v2 2/3] mingw: fix colourization on Cygwin pseudo terminals Johannes Schindelin
2016-12-22 17:09 ` [PATCH v2 3/3] mingw: replace isatty() hack Johannes Schindelin
2016-12-22 20:26 ` Johannes Sixt
2016-12-22 21:37 ` Johannes Schindelin
2016-12-22 22:28 ` Johannes Sixt
2016-12-22 23:18 ` Johannes Schindelin
2016-12-22 17:49 ` [PATCH v2 0/3] Really fix the isatty() problem on Windows Junio C Hamano
2016-12-22 17:59 ` Johannes Schindelin
2016-12-22 18:32 ` Junio C Hamano
2016-12-22 18:19 ` Junio C Hamano
2016-12-22 23:16 ` Johannes Schindelin [this message]
2016-12-22 23:16 ` [PATCH v3 1/3] mingw: adjust is_console() to work with stdin Johannes Schindelin
2016-12-22 23:16 ` [PATCH v3 2/3] mingw: fix colourization on Cygwin pseudo terminals Johannes Schindelin
2016-12-22 23:16 ` [PATCH v3 3/3] mingw: replace isatty() hack Johannes Schindelin
2017-01-18 12:13 ` Johannes Schindelin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=cover.1482448531.git.johannes.schindelin@gmx.de \
--to=johannes.schindelin@gmx.de \
--cc=dev+git@drbeat.li \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=j6t@kdbg.org \
--cc=pranit.bauva@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
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).