On Thu, Jul 26, 2012 at 10:59:51AM -0700, Junio C Hamano wrote:

> > The credential code uses git_terminal_prompt, which actually opens
> > /dev/tty directly. So it is probably sane to use for your new prompt,
> > but it does not (and should not) rely on isatty.
> I think using git_terminal_prompt() after doing a looser "does the
> user sit at a terminal and is capable of answering interactive
> prompt" check with isatty(2) is OK, as long as we know that all
> implementations of git_terminal_prompt() never read from whatever
> happens to be connected to the standard input.

I don't think isatty(2) is correct, though. It would yield false
negatives when the user has redirected stderr but /dev/tty is still
available. I don't know if it possible for getpass to fallback to stdin
when stderr is a tty (it would mean that opening /dev/tty failed, which
would mean that we have no controlling terminal _but_ our stderr is
still connected to some terminal. That might be bizarre enough not to
care about).

I think the right answer would be a real is_prompt_available() that
checked /dev/tty when HAVE_DEV_TTY was set, and otherwise checked
isatty(2) (or whatever was appropriate for the platform).

> The function falls back to getpass() on platforms without DEV_TTY,
> and if getpass() on some platforms reads from the standard input,
> that would be a disaster.  I wasn't sure about that part.

Yeah, although it is already a disaster in those cases, as the main
caller of git_terminal_prompt is remote-curl, whose stdin is connected
to git via the remote-helper protocol. Which isn't to say this wouldn't
make things worse. It would, but the real solution is to implement a
sane git_terminal_prompt for those platforms. Erik had a patch for
Windows to use their magical CONIN$, but I think it is temporarily
stalled. I don't know if there are any other platforms that do not have
/dev/tty (I know we do not set HAVE_DEV_TTY by default, but that is only
because I was being conservative and waiting for people on particular
platforms to confirm that it works before tweaking our Makefile

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

Reply via email to