Re: [PATCH] drop unnecessary copying in credential_ask_one

[Jeff King <peff@peff.net>]

On Thu, Jan 02, 2014 at 11:08:51AM -0800, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > ... But the test suite, of course, always uses askpass because it
> > cannot rely on accessing a terminal (we'd have to do some magic with
> > lib-terminal, I think).
> >
> > So it doesn't detect the problem in your patch, but I wonder if it is
> > worth applying the patch below anyway, as it makes the test suite
> > slightly more robust.
> 
> Sounds like a good first step in the right direction.  Thanks.

I took a brief look at adding "real" terminal tests for the credential
code using our test-terminal/lib-terminal.sh setup. Unfortunately, it
falls short of what we need.

test-terminal only handles stdout and stderr streams as fake terminals.
We could pretty easily add stdin for input, as it uses fork() to work
asynchronously.  But the credential code does not actually read from
stdin. It opens and reads from /dev/tty explicitly. So I think we'd have
to actually fake setting up a controlling terminal. And that means magic
with setsid() and ioctl(TIOCSCTTY), which in turn sounds like a
portability headache.

So it's definitely possible under Linux, and probably under most Unixes.
But I'm not sure it's worth the effort, given that review already caught
the potential bug here.

Another option would be to instrument git_terminal_prompt with a
mock-terminal interface (say, reading from a file specified in an
environment variable). But I really hate polluting the code with test
cruft, and it would not actually be testing an interesting segment of
the code, anyway.

-Peff