Re: [PATCH 3/4] am: drop tty requirement for --interactive
- Date: Thu, 23 May 2019 08:44:31 +0200 (CEST)
- From: Johannes Schindelin <Johannes.Schindelin@xxxxxx>
- Subject: Re: [PATCH 3/4] am: drop tty requirement for --interactive
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
> ' </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.
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
> 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.