git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] drop unnecessary copying in credential_ask_one

From
Junio C Hamano <gitster@pobox.com>
Date
Jan 7, 2014, 19:44 UTC
Message-ID
<xmqqlhyrd1bz.fsf@gitster.dls.corp.google.com>
In-Reply-To
<20140107175009.GA19691@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 25 quoted lines
> 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.
Show 9 quoted lines
> 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.
Previous: Jeff KingNext: Jeff King
Message 6 of 7 in “drop unnecessary copying in credential_ask_one”
  1. drop unnecessary copying in credential_ask_oneTay Ray Chuan, Jan 2, 2014
  2. Jeff KingJan 2, 2014
  3. Jeff KingJan 2, 2014
  4. Junio C HamanoJan 2, 2014
  5. Jeff KingJan 7, 2014
  6. Junio C HamanoJan 7, 2014
  7. Jeff KingJan 7, 2014

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.