Re: [PATCH] drop unnecessary copying in credential_ask_one

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Jeff King <peff@xxxxxxxx> writes:

> On Thu, Jan 02, 2014 at 11:08:51AM -0800, Junio C Hamano wrote:
>
>> Jeff King <peff@xxxxxxxx> 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.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]