Re: [PATCH] drop unnecessary copying in credential_ask_one

[Junio C Hamano <gitster@pobox.com>]

Jeff King <peff@peff.net> writes:

> 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.

I wonder if "expect" has already solved that for us.

> 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.

Agreed.