Subject: Re: [PATCH] honour GIT_ASKPASS for querying username in git-svn

From: Jakub Narebski <[email-masked]>
Date: Tue, 27 Dec 2011 17:00:57 +0100

On Tue, 27 Dec 2011, Sven Strickroth wrote:
> Am 27.12.2011 15:33 schrieb Jakub Narebski:

>>> +sub prompt {
>>> +	my ($self, $prompt) = _maybe_self(@_);
>>> +	if (exists $ENV{'GIT_ASKPASS'}) {
>>> +		return _prompt($ENV{'GIT_ASKPASS'}, $prompt);
>>> +	} elsif (exists $ENV{'SSH_ASKPASS'}) {
>>> +		return _prompt($ENV{'SSH_ASKPASS'}, $prompt);
>>> +	} else {
>>> +		return undef;
>>> +	}
>>> +}
>> 
>> ...and provide some kind of fallback even if neither of GIT_ASKPASS
>> nor SSH_ASKPASS are set (perhaps assuming that some Perl packages from
>> CPAN are installed).
> 
> If neither of GIT_ASKPASS nor SSH_ASKPASS are set the caller has to
> handle the request. This has to be done this way, because of lots of
> different needs (username, password (no echo) and so on).

I think that Git.pm and therefore git commands written in Perl should
behave the same as git command written in C; and I think builtins do
use common gitprompt fallback.
 
>>> +sub _prompt {
>>> +	my ($self, $askpass, $prompt) = _maybe_self(@_);
>>> +	my $ret;
>>> +	open(PH, "-|", $askpass, $prompt);
>>> +	$ret = <PH>;
>>> +	$ret =~ s/[\012\015]//g; # strip \n\r
>>> +	close(PH);
>>> +	return $ret;
>>> +}
>> 
>> Please, use modern Perl, in particula use lexical filehandles instead
>> of typeglobs (which are global variables), i.e.
> 
> I used the same style as I found in Git.pm (see lines I removed in patch 2).

Yes, that should be fixed (together with host of other issues), but
one should use modern and _better_ way (no possibility of action at
distance).

-- 
Jakub Narebski
Poland