Re: [PATCH 0/6] echo usernames as they are typed

From: Jeff King <peff@peff.net>
Date: Nov 28, 2011, 11:31 UTC
Message-ID: <20111128113127.GA23408@sigill.intra.peff.net>
In-Reply-To: <CABPQNSbLvWh-ivaqBk-Du+kwZvV3t+ajEJhHATRzyGZbHYyM=Q@mail.gmail.com>

On Mon, Nov 28, 2011 at 10:36:21AM +0100, Erik Faye-Lund wrote:

Show 7 quoted lines

> > You really want to open "/dev/tty" on most Unix systems (which is what
> > getpass() does).
> 
> Yes, you're right. Opening "/dev/tty" is much better. But what happens
> for processes started by GUI applications (with no easily observable
> tty, if any)? Does open simply fail? If so, is it desirable for us to
> fail in that case?

Yes, the open will fail (on Linux, I get ENXIO).

And yes, we should fail in that case. getpass() will generally return NULL in that instance, and the current implementation of git_getpass() will die(), explaining that we could not get the password.

Show 6 quoted lines

> > I have no idea what would be appropriate on Windows.
> 
> It's pretty similar, but not exactly: CreateFile("CONIN$", ...) or
> CreateFile("CONOUT$", ...), depending on if you want the read-handle
> or the write-handle... I can probably cook up something a bit more
> concrete, though.

OK, that maps to the /dev/tty concept quite well. Though I suspect the code for turning off character echo-ing is going to also be different.

Show 13 quoted lines

> But _getch() that we already use always reads from the console
> (according to MSDN, I haven't actually tested this myself:
> http://msdn.microsoft.com/en-us/library/078sfkak%28v=VS.80%29.aspx).
> But I don't think this allows us to fail when no console is attached.
> Question is, should we fail in such cases? Windows does have an API to
> prompt for passwords in a GUI window... Perhaps fallbacking to those
> are the way to go? Something like:
> 
> if (GetConsoleWindow()) {
> 	/*  normal console-stuff */
> } else {
> 	/* call CredUIPromptForWindowsCredentials(...) instead */
> }

Certainly on non-Windows something like that would not be welcome. The user can already have specified GIT_ASKPASS if they don't have a terminal. And once the credential-helper code is in, they can use a platform-specific helper that provides a nice dialog if they want it.

So I would say trying to do something graphical would be surprising and unwelcome. But then, I am a very Unix-y kind of guy. Maybe on Windows something like that would be more favorable. I'll leave that decision to people who know more.

-Peff