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 >
