* [PATCH v2 1/3] git-svn: Support retrieving passwords with GIT_ASKPASS [not found] <3.SQo> @ 2010-02-26 0:07 ` Frank Li 2010-02-26 6:33 ` Junio C Hamano ` (2 more replies) 0 siblings, 3 replies; 5+ messages in thread From: Frank Li @ 2010-02-26 0:07 UTC (permalink / raw) To: git; +Cc: davvid, Frank Li git-svn reads passwords from an interactive terminal. This behavior cause GUIs to hang waiting for git-svn to complete Fix this problem by allowing a password-retrieving command to be specified in GIT_ASKPASS. SSH_ASKPASS is supported as a fallback when GIT_ASKPASS is not provided. Signed-off-by: Frank Li <lznuaa@gmail.com> --- git-svn.perl | 37 +++++++++++++++++++++++++++---------- 1 files changed, 27 insertions(+), 10 deletions(-) diff --git a/git-svn.perl b/git-svn.perl index 265852f..cd39792 100755 --- a/git-svn.perl +++ b/git-svn.perl @@ -31,6 +31,16 @@ if (! exists $ENV{SVN_SSH}) { } } +if (! exists $ENV{GIT_ASKPASS}) { + if (exists $ENV{SSH_ASKPASS}) { + $ENV{GIT_ASKPASS} = $ENV{SSH_ASKPASS}; + if ($^O eq 'msys') { + $ENV{GIT_ASKPASS} =~ s/\\/\\\\/g; + $ENV{GIT_ASKPASS} =~ s/(.*)/"$1"/; + } + } +} + $Git::SVN::Log::TZ = $ENV{TZ}; $ENV{TZ} = 'UTC'; $| = 1; # unbuffer STDOUT @@ -3966,18 +3976,25 @@ sub username { sub _read_password { my ($prompt, $realm) = @_; - print STDERR $prompt; - STDERR->flush; - require Term::ReadKey; - Term::ReadKey::ReadMode('noecho'); my $password = ''; - while (defined(my $key = Term::ReadKey::ReadKey(0))) { - last if $key =~ /[\012\015]/; # \n\r - $password .= $key; + if (exists $ENV{GIT_ASKPASS}) { + open(PH, "$ENV{GIT_ASKPASS} \"$prompt\" |"); + $password = <PH>; + $password =~ s/[\012\015]//; # \n\r + close(PH); + } else { + print STDERR $prompt; + STDERR->flush; + require Term::ReadKey; + Term::ReadKey::ReadMode('noecho'); + while (defined(my $key = Term::ReadKey::ReadKey(0))) { + last if $key =~ /[\012\015]/; # \n\r + $password .= $key; + } + Term::ReadKey::ReadMode('restore'); + print STDERR "\n"; + STDERR->flush; } - Term::ReadKey::ReadMode('restore'); - print STDERR "\n"; - STDERR->flush; $password; } -- 1.7.0.85.g37fda.dirty ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2 1/3] git-svn: Support retrieving passwords with GIT_ASKPASS 2010-02-26 0:07 ` [PATCH v2 1/3] git-svn: Support retrieving passwords with GIT_ASKPASS Frank Li @ 2010-02-26 6:33 ` Junio C Hamano 2010-02-26 8:55 ` Frank Li 2010-02-26 10:05 ` Eric Wong 2010-02-26 17:41 ` Johannes Sixt 2 siblings, 1 reply; 5+ messages in thread From: Junio C Hamano @ 2010-02-26 6:33 UTC (permalink / raw) To: Frank Li; +Cc: git, davvid, Johannes Sixt Frank Li <lznuaa@gmail.com> writes: > +if (! exists $ENV{GIT_ASKPASS}) { > + if (exists $ENV{SSH_ASKPASS}) { > + $ENV{GIT_ASKPASS} = $ENV{SSH_ASKPASS}; > + if ($^O eq 'msys') { > + $ENV{GIT_ASKPASS} =~ s/\\/\\\\/g; > + $ENV{GIT_ASKPASS} =~ s/(.*)/"$1"/; > + } > + } > +} I've seen this code before, and you may not be the best person to answer this question, but this worries me and puzzles me a bit. On msys (and nowhere else), SSH_ASKPASS can be used as given by the user to launch the prompter, but GIT_ASKPASS must be quoted in some funny way. Why is that? Does this mean they must be given differently by the end user? In other words, if the end user wants to set GIT_ASKPASS himself, s/he needs to do this funny quoting, that is different from SSH_ASKPASS. I also notice that git-gui has support for SSH_ASKPASS (and its own implementation). Does it have the same quoting issues on msys? The reason I am asking is because: (1) if SSH_ASKPASS and GIT_ASKPASS cannot be specified exactly the same way, then [PATCH 3/3] would probably need a similar quoting magic? (2) With [PATCH 3/3], with quoting magic if necessary, we wouldn't need the above hunk, as it has already be done by the "git" potty. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 1/3] git-svn: Support retrieving passwords with GIT_ASKPASS 2010-02-26 6:33 ` Junio C Hamano @ 2010-02-26 8:55 ` Frank Li 0 siblings, 0 replies; 5+ messages in thread From: Frank Li @ 2010-02-26 8:55 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, davvid, Johannes Sixt 2010/2/26 Junio C Hamano <gitster@pobox.com>: > Frank Li <lznuaa@gmail.com> writes: > >> +if (! exists $ENV{GIT_ASKPASS}) { >> + if (exists $ENV{SSH_ASKPASS}) { >> + $ENV{GIT_ASKPASS} = $ENV{SSH_ASKPASS}; >> + if ($^O eq 'msys') { >> + $ENV{GIT_ASKPASS} =~ s/\\/\\\\/g; >> + $ENV{GIT_ASKPASS} =~ s/(.*)/"$1"/; >> + } >> + } >> +} > > I've seen this code before, and you may not be the best person to answer > this question, but this worries me and puzzles me a bit. Yes, I copy it from fall back SVN_SSH from GIT_SSH at git-svn.perl. I guess it seems related with windows path using space, such as c:\program files\bin\xxx. perl Open ('$ENV{GIT_ASKPASS} |") will be changed to open("c:\program files\bin\xxx |"). Perl will think c:\program as application, files\bin\xxx as first parameter. So add ". it equal to open ( "\"c:\program files\bin\xxx\" |"). perl can run correct application. > > On msys (and nowhere else), SSH_ASKPASS can be used as given by the user > to launch the prompter, but GIT_ASKPASS must be quoted in some funny way. > > Why is that? Does this mean they must be given differently by the end > user? In other words, if the end user wants to set GIT_ASKPASS himself, > s/he needs to do this funny quoting, that is different from SSH_ASKPASS. I should add code to check if there are a space at GIT_ASKPASS, if there are space in prompter path, add quote. So end user set GIT_ASKPASS and SSH_ASKPASS at the same ways, NO quoting. > > I also notice that git-gui has support for SSH_ASKPASS (and its own > implementation). Does it have the same quoting issues on msys? I think no because msys add prompter to PATH environment and needn't set full path. > > The reason I am asking is because: > > (1) if SSH_ASKPASS and GIT_ASKPASS cannot be specified exactly the same > way, then [PATCH 3/3] would probably need a similar quoting magic? SSH_ASKPASS and GIT_ASKPASS is the same. C code needn't quoting because start_command think $GIT_ASKPASS is full path and don't split $GIT_ASKPASS to application and parameter by space. > > (2) With [PATCH 3/3], with quoting magic if necessary, we wouldn't need > the above hunk, as it has already be done by the "git" potty. > quoting magic is not necessary at PATCH 3/3. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 1/3] git-svn: Support retrieving passwords with GIT_ASKPASS 2010-02-26 0:07 ` [PATCH v2 1/3] git-svn: Support retrieving passwords with GIT_ASKPASS Frank Li 2010-02-26 6:33 ` Junio C Hamano @ 2010-02-26 10:05 ` Eric Wong 2010-02-26 17:41 ` Johannes Sixt 2 siblings, 0 replies; 5+ messages in thread From: Eric Wong @ 2010-02-26 10:05 UTC (permalink / raw) To: Frank Li; +Cc: git, davvid Frank Li <lznuaa@gmail.com> wrote: > git-svn reads passwords from an interactive terminal. > This behavior cause GUIs to hang waiting for git-svn to > complete > > Fix this problem by allowing a password-retrieving command > to be specified in GIT_ASKPASS. SSH_ASKPASS is supported > as a fallback when GIT_ASKPASS is not provided. > > Signed-off-by: Frank Li <lznuaa@gmail.com> > --- > git-svn.perl | 37 +++++++++++++++++++++++++++---------- > 1 files changed, 27 insertions(+), 10 deletions(-) > > diff --git a/git-svn.perl b/git-svn.perl > index 265852f..cd39792 100755 > --- a/git-svn.perl > +++ b/git-svn.perl > @@ -31,6 +31,16 @@ if (! exists $ENV{SVN_SSH}) { > } > } > > +if (! exists $ENV{GIT_ASKPASS}) { > + if (exists $ENV{SSH_ASKPASS}) { > + $ENV{GIT_ASKPASS} = $ENV{SSH_ASKPASS}; > + if ($^O eq 'msys') { > + $ENV{GIT_ASKPASS} =~ s/\\/\\\\/g; > + $ENV{GIT_ASKPASS} =~ s/(.*)/"$1"/; > + } > + } > +} > + Hi Frank, Since this logic isn't SVN-specific, can we get this in Git.pm and/or git-var so other tools can use it? Thanks -- Eric Wong ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 1/3] git-svn: Support retrieving passwords with GIT_ASKPASS 2010-02-26 0:07 ` [PATCH v2 1/3] git-svn: Support retrieving passwords with GIT_ASKPASS Frank Li 2010-02-26 6:33 ` Junio C Hamano 2010-02-26 10:05 ` Eric Wong @ 2010-02-26 17:41 ` Johannes Sixt 2 siblings, 0 replies; 5+ messages in thread From: Johannes Sixt @ 2010-02-26 17:41 UTC (permalink / raw) To: Frank Li; +Cc: git, davvid Frank Li schrieb: > +if (! exists $ENV{GIT_ASKPASS}) { > + if (exists $ENV{SSH_ASKPASS}) { > + $ENV{GIT_ASKPASS} = $ENV{SSH_ASKPASS}; > + if ($^O eq 'msys') { > + $ENV{GIT_ASKPASS} =~ s/\\/\\\\/g; > + $ENV{GIT_ASKPASS} =~ s/(.*)/"$1"/; Don't quote GIT_ASKPASS here. > + if (exists $ENV{GIT_ASKPASS}) { > + open(PH, "$ENV{GIT_ASKPASS} \"$prompt\" |"); open(PH, "-|", $ENV{GIT_ASKPASS}, $prompt); and you don't have to do any quoting at all, no? -- Hannes ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2010-02-26 17:41 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <3.SQo>
2010-02-26 0:07 ` [PATCH v2 1/3] git-svn: Support retrieving passwords with GIT_ASKPASS Frank Li
2010-02-26 6:33 ` Junio C Hamano
2010-02-26 8:55 ` Frank Li
2010-02-26 10:05 ` Eric Wong
2010-02-26 17:41 ` Johannes Sixt
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.