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 defaults). -Peff -- 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
