Thread (2 messages) flat view 2 messages, 2 authors, 2016-12-22

Re: [PATCH v2 0/3] Really fix the isatty() problem on Windows

From: Junio C Hamano <hidden>
Date: 2016-12-22 17:49:45

Johannes Schindelin [off-list ref] writes:
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 v1:

- rebased onto master

- unsquashed 2/3 which was improperly snuck in before,
As Windows specific changes, I didn't notice these two were independent.
- noted that Beat Bolli tested this (see
  https://github.com/git-for-windows/git/issues/997#issuecomment-268764693)

- fixed the confusing commit message by using Junio's suggested
  replacement
Sorry, but I didn't mean to "suggest replacement".  I was just
testing my understanding by attempt to rephrase the gist of it.

There was one thing I still wasn't clear in my "summary of my
understanding".  Is the "replacement originally done for compiling
with VC++" a solution that still peeks into MSVC runtime internals
but is usable with both old and more recent one?  Or is it a more
kosher approach that does not play with the internals to make it
unlikely that it would have to change again in the future?

Your "use this opportunity to actually clean up" above suggests that
the answer is the latter, but if you took my "summary of my
understanding", it is likely that that fact is not captured in the
resulting log message.

The interdiff obviously looks good.  Let's move this series forward.
I'll see if it can be merged down to 'maint', too, but it probably
would not matter that much.

Thanks.

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help