Re: [PATCH] Avoid crippled getpass function on Solaris

From: Jeff King <peff@peff.net>
Date: Aug 6, 2012, 19:39 UTC
Message-ID: <20120806193958.GA10039@sigill.intra.peff.net>
In-Reply-To: <1344220427-sup-3468@pinkfloyd.chass.utoronto.ca>

On Sun, Aug 05, 2012 at 10:35:06PM -0400, Ben Walton wrote:

Show 11 quoted lines

> Excerpts from Junio C Hamano's message of Sun Aug 05 21:59:48 -0400 2012:
> > Wouldn't
> > 
> >     #if solaris
> >         #define getpass getpassphrase
> >         #endif
> > 
> > without anything else be more than sufficient?
> 
> Yes, it would, but I was hoping to make it more explicit that the
> function getpass may be substituted with something else.

I don't think that's important. Either the thing is a drop-in replica of getpass, or it is not. In the former case, it's OK for it to be transparent that it has been replaced. In the latter case, it should not be a #define replacement at all, but should be its own alternative in compat/terminal.c (just like HAVE_DEV_TTY is). From my reading of getpassphrase, it does seem to be a drop-in replacement.

So I'm OK conceptually with the patch if we can't do any better. But getpass still sucks. It doesn't handle echoing, and it may or may not fall back to reading from stdin if the tty isn't available (which is disastrous for remote-curl, whose stdin is speaking the remote-helper protocol to git). So I'd really prefer to make HAVE_DEV_TTY work with Solaris if we can.

I'm happy to spend a few cycles on it. I don't have access to any real Solaris boxes these days, but I imagine I can get OpenSolaris running under VirtualBox without too much trouble...

-Peff

PS If we do go the getpassphrase route, does it make sense to introduce
   HAVE_GETPASSPHRASE? We usually try to provide one layer of
   indirection by naming our #defines after features, and then
   connecting systems to the feature defines via the Makefile. But maybe
   Solaris is the only system that has getpassphrase.