On Mon, Nov 28, 2011 at 12:31 PM, Jeff King <peff@xxxxxxxx> wrote:
> On Mon, Nov 28, 2011 at 10:36:21AM +0100, Erik Faye-Lund wrote:
>
>> > 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.
>
>> > 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.
>
>> 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.
>
Yes, that's certainly cleaner implementation-wise. But didn't you
change it to only do the storage-part in the last round, or did I
misunderstand the updated series?
> 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.
Windows doesn't really have that strict norms when it comes to console
applications, but it'd be nice if it didn't do anything obviously
wrong when the GUI isn't available (non-interactive sessions,
PowerShell remote commands, CopSSH, etc). So I guess this is yet
another argument to stay with the credential-helper instead, if
possible...
--
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