Re: [PATCH 3/4] am: drop tty requirement for --interactive

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

Hi Peff,

On Mon, 20 May 2019, Jeff King wrote:

> On Mon, May 20, 2019 at 08:11:13AM -0400, Jeff King wrote:
>
> > We have required that the stdin of "am --interactive" be a tty since
> > a1451104ac (git-am: interactive should fail gracefully., 2005-10-12).
> > However, this isn't strictly necessary, and makes the tool harder to
> > test (and is unlike all of our other --interactive commands).
>
> I think this is worth doing for simplicity and consistency. But as you
> might guess, my ulterior motive was making it easier to add tests.
>
> In theory we _should_ be able to use test_terminal for this, but it
> seems to be racy, because it will quickly read all input and close the
> descriptor (to give the reader EOF). But after that close, isatty() will
> no longer report it correctly. E.g., if I run this:
>
>   perl test-terminal.perl sh -c '
> 	for i in 0 1 2; do
> 		echo $i is $(test -t $i || echo not) a tty
> 	done
>   ' </dev/null
>
> it _usually_ says "0 is a tty", but racily may say "not a tty". If you
> put a sleep into the beginning of the shell, then it will basically
> always lose the race and say "not".

This is just another nail in the coffin for `test-terminal.perl`, as far
as I am concerned.

In the built-in `add -i` patch series, I followed a strategy where I move
totally away from `test-terminal`, in favor of using some knobs to force
Git into thinking that we are in a terminal.

But at the same time, I *also* remove the limitation (for most cases) of
"read from /dev/tty", in favor of reading from stdin, and making things
testable, and more importantly: scriptable.

So I am *very* much in favor of this here patch.

Thanks,
Dscho

P.S.: There are even more reasons to get rid of `test-terminal`, of
course: it is an unnecessary dependency on Perl, works only when certain
Perl modules are installed (that are *not* installed on Ubuntu by default,
for example), and it requires pseudo terminals, so it will *never* work on
Windows.

> It might be possible to overcome this by making test-terminal more
> clever (i.e., is there a way for us to send an "EOF" over the pty
> without actually _closing_ it? That would behave like a real terminal,
> where you can hit ^D to generate an EOF but then type more).
>
> But barring that, this works by just avoiding it entirely. :)
>
> Curiously, my script above also reports consistently that stdout is not
> a tty, but that stderr is. I'm not sure why this is, but it no tests
> seem to care either way.
>
> -Peff
>