{"thread":{"id":"28961","subject":"[PATCH] honour GIT_ASKPASS for querying username in git-svn","startedAt":"2011-11-17T15:15:20Z","lastAt":"2012-12-18T00:57:25Z","messageCount":82,"participants":["Sven Strickroth","Erik Faye-Lund","Jeff King","Jakub Narebski","Junio C Hamano","Thomas Adam","Ævar Arnfjörð Bjarmason"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"179640","messageId":"4EC52508.9070907@tu-clausthal.de","threadId":"28961","inReplyTo":null,"subject":"[PATCH] honour GIT_ASKPASS for querying username in git-svn","fromName":"Sven Strickroth","fromEmail":"sven.strickroth@tu-clausthal.de","sentAt":"2011-11-17T15:15:20Z","receivedAt":"2011-11-17T15:15:20Z","isPatch":true,"sender":{"key":"sven.strickroth@tu-clausthal.de","avatar":null},"body":">From 8e576705ca949c32ff22d3216006073ee70652eb Mon Sep 17 00:00:00 2001\nFrom: Sven Strickroth \nDate: Thu, 17 Nov 2011 15:43:25 +0100\nSubject: [PATCH 1/2] honour GIT_ASKPASS for querying username\n\ngit-svn reads usernames from an interactive terminal.\nThis behavior cause GUIs to hang waiting for git-svn to\ncomplete (http://code.google.com/p/tortoisegit/issues/detail?id=967).\n\nAlso see commit 56a853b62c0ae7ebaad0a7a0a704f5ef561eb795.\n\nSigned-off-by: Sven Strickroth \n---\n git-svn.perl | 5 +++++\n 1 files changed, 5 insertions(+), 0 deletions(-)\n\ndiff --git a/git-svn.perl b/git-svn.perl\nindex e30df22..8ec3dfc 100755\n--- a/git-svn.perl\n+++ b/git-svn.perl\n@@ -4403,6 +4403,11 @@ sub username {\n \tmy $username;\n \tif (defined $_username) {\n \t\t$username = $_username;\n+\t} else if (exists $ENV{GIT_ASKPASS}) {\n+\t\topen(PH, \"-|\", $ENV{GIT_ASKPASS}, \"Username: \");\n+\t\t$username = ;\n+\t\t$username =~ s/[\\012\\015]//; # \\n\\r\n+\t\tclose(PH);\n \t} else {\n \t\tprint STDERR \"Username: \";\n \t\tSTDERR->flush;\n-- \n1.7.7.1.msysgit.0\n\n-- \nBest regards,\n Sven Strickroth\n"},{"id":"179694","messageId":"CABPQNSZ0iPAE+BnDU6Nz8_PkrAtPbjL4RoJuQS=Um2wxPt-2DQ@mail.gmail.com","threadId":"28961","inReplyTo":"4EC52508.9070907@tu-clausthal.de","subject":"Re: [PATCH] honour GIT_ASKPASS for querying username in git-svn","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2011-11-18T11:36:31Z","receivedAt":"2011-11-18T11:36:31Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Thu, Nov 17, 2011 at 4:15 PM, Sven Strickroth\n wrote:\n> From 8e576705ca949c32ff22d3216006073ee70652eb Mon Sep 17 00:00:00 2001\n> From: Sven Strickroth \n> Date: Thu, 17 Nov 2011 15:43:25 +0100\n> Subject: [PATCH 1/2] honour GIT_ASKPASS for querying username\n>\n> git-svn reads usernames from an interactive terminal.\n> This behavior cause GUIs to hang waiting for git-svn to\n> complete (http://code.google.com/p/tortoisegit/issues/detail?id=967).\n>\n> Also see commit 56a853b62c0ae7ebaad0a7a0a704f5ef561eb795.\n>\n> Signed-off-by: Sven Strickroth \n\nIIUC, GIT_ASKPASS is intended for passwords and not usernames. Won't\nthis cause console-users to not see their username prompted anymore?\n"},{"id":"179698","messageId":"4EC65DE4.90005@tu-clausthal.de","threadId":"28961","inReplyTo":"CABPQNSZ0iPAE+BnDU6Nz8_PkrAtPbjL4RoJuQS=Um2wxPt-2DQ@mail.gmail.com","subject":"Re: [PATCH] honour GIT_ASKPASS for querying username in git-svn","fromName":"Sven Strickroth","fromEmail":"sven.strickroth@tu-clausthal.de","sentAt":"2011-11-18T13:30:12Z","receivedAt":"2011-11-18T13:30:12Z","isPatch":true,"sender":{"key":"sven.strickroth@tu-clausthal.de","avatar":null},"body":"Am 18.11.2011 12:36 schrieb Erik Faye-Lund:\n> On Thu, Nov 17, 2011 at 4:15 PM, Sven Strickroth\n> wrote:\n>> From 8e576705ca949c32ff22d3216006073ee70652eb Mon Sep 17 00:00:00 2001\n>> From: Sven Strickroth \n>> Date: Thu, 17 Nov 2011 15:43:25 +0100\n>> Subject: [PATCH 1/2] honour GIT_ASKPASS for querying username\n>>\n>> git-svn reads usernames from an interactive terminal.\n>> This behavior cause GUIs to hang waiting for git-svn to\n>> complete (http://code.google.com/p/tortoisegit/issues/detail?id=967).\n>>\n>> Also see commit 56a853b62c0ae7ebaad0a7a0a704f5ef561eb795.\n>>\n>> Signed-off-by: Sven Strickroth \n> \n> IIUC, GIT_ASKPASS is intended for passwords and not usernames. Won't\n> this cause console-users to not see their username prompted anymore?\n\ngit also asks for username using the GIT_ASKPASS tool (if GIT_ASKPASS is\nset).\n\n-- \nBest regards,\n Sven Strickroth\n ClamAV, a GPL anti-virus toolkit http://www.clamav.net\n PGP key id F5A9D4C4 @ any key-server\n"},{"id":"179703","messageId":"CABPQNSbfM0JRVPk3fxfSEq7QaO-fynHM8FBGpPribdgeRqpZKA@mail.gmail.com","threadId":"28961","inReplyTo":"4EC65DE4.90005@tu-clausthal.de","subject":"Re: [PATCH] honour GIT_ASKPASS for querying username in git-svn","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2011-11-18T14:19:37Z","receivedAt":"2011-11-18T14:19:37Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Fri, Nov 18, 2011 at 2:30 PM, Sven Strickroth\n wrote:\n> Am 18.11.2011 12:36 schrieb Erik Faye-Lund:\n>> On Thu, Nov 17, 2011 at 4:15 PM, Sven Strickroth\n>> wrote:\n>>> From 8e576705ca949c32ff22d3216006073ee70652eb Mon Sep 17 00:00:00 2001\n>>> From: Sven Strickroth \n>>> Date: Thu, 17 Nov 2011 15:43:25 +0100\n>>> Subject: [PATCH 1/2] honour GIT_ASKPASS for querying username\n>>>\n>>> git-svn reads usernames from an interactive terminal.\n>>> This behavior cause GUIs to hang waiting for git-svn to\n>>> complete (http://code.google.com/p/tortoisegit/issues/detail?id=967).\n>>>\n>>> Also see commit 56a853b62c0ae7ebaad0a7a0a704f5ef561eb795.\n>>>\n>>> Signed-off-by: Sven Strickroth \n>>\n>> IIUC, GIT_ASKPASS is intended for passwords and not usernames. Won't\n>> this cause console-users to not see their username prompted anymore?\n>\n> git also asks for username using the GIT_ASKPASS tool (if GIT_ASKPASS is\n> set).\n>\n\nYou are right, it does. Documentation/config.txt documents it as being\nfor passwords without mentioning that it also affects usernames,\nthat's why I wondered. I've also verified what happens here on my\nconfig, and git-svn doesn't prompt my username here without the patch\neither. So consider my comment withdrawn ;)\n"},{"id":"179984","messageId":"4ED0CE8B.70205@tu-clausthal.de","threadId":"28961","inReplyTo":"CABPQNSbfM0JRVPk3fxfSEq7QaO-fynHM8FBGpPribdgeRqpZKA@mail.gmail.com","subject":"Re: [PATCH] honour GIT_ASKPASS for querying username in git-svn","fromName":"Sven Strickroth","fromEmail":"sven.strickroth@tu-clausthal.de","sentAt":"2011-11-26T11:33:31Z","receivedAt":"2011-11-26T11:33:31Z","isPatch":true,"sender":{"key":"sven.strickroth@tu-clausthal.de","avatar":null},"body":"Hi,\n\nthere's also another point where git-svn doesn't honour GIT_ASKPASS:\n\n>From 632c264d0de127c35fbe45866ed81e832f357d56 Mon Sep 17 00:00:00 2001\nFrom: Sven Strickroth \nDate: Sat, 26 Nov 2011 12:01:19 +0100\nSubject: [PATCH] honour GIT_ASKPASS for querying further actions on unknown\n certificates\n\ngit-svn reads user answers from an interactive terminal.\nThis behavior cause GUIs to hang waiting for git-svn to\ncomplete.\n\nSigned-off-by: Sven Strickroth \n---\n git-svn.perl | 9 +++++++++--\n 1 files changed, 8 insertions(+), 1 deletions(-)\n\ndiff --git a/git-svn.perl b/git-svn.perl\nindex e30df22..d7aeb11 100755\n--- a/git-svn.perl\n+++ b/git-svn.perl\n@@ -4361,7 +4361,14 @@ prompt:\n \t \"(R)eject, accept (t)emporarily or accept (p)ermanently? \" :\n \t \"(R)eject or accept (t)emporarily? \";\n \tSTDERR->flush;\n-\t$choice = lc(substr( || 'R', 0, 1));\n+\tif (exists $ENV{GIT_ASKPASS}) {\n+\t\tprint STDERR \"\\n\";\n+\t\topen(PH, \"-|\", $ENV{GIT_ASKPASS}, \"Certificate unknown\");\n+\t\t$choice = lc(substr( || 'R', 0, 1));\n+\t\tclose(PH);\n+\t} else {\n+\t\t$choice = lc(substr( || 'R', 0, 1));\n+\t}\n \tif ($choice =~ /^t$/i) {\n \t\t$cred->may_save(undef);\n \t} elsif ($choice =~ /^r$/i) {\n-- \n1.7.7.1.msysgit.0\n\n-- \nBest regards,\n Sven Strickroth\n ClamAV, a GPL anti-virus toolkit http://www.clamav.net\n PGP key id F5A9D4C4 @ any key-server\n"},{"id":"180137","messageId":"20111130064401.GC5317@sigill.intra.peff.net","threadId":"28961","inReplyTo":"4ED0CE8B.70205@tu-clausthal.de","subject":"Re: [PATCH] honour GIT_ASKPASS for querying username in git-svn","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-11-30T06:44:01Z","receivedAt":"2011-11-30T06:44:01Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Nov 26, 2011 at 12:33:31PM +0100, Sven Strickroth wrote:\n\n> diff --git a/git-svn.perl b/git-svn.perl\n> index e30df22..d7aeb11 100755\n> --- a/git-svn.perl\n> +++ b/git-svn.perl\n> @@ -4361,7 +4361,14 @@ prompt:\n> \t \"(R)eject, accept (t)emporarily or accept (p)ermanently? \" :\n> \t \"(R)eject or accept (t)emporarily? \";\n> \tSTDERR->flush;\n> -\t$choice = lc(substr( || 'R', 0, 1));\n> +\tif (exists $ENV{GIT_ASKPASS}) {\n> +\t\tprint STDERR \"\\n\";\n> +\t\topen(PH, \"-|\", $ENV{GIT_ASKPASS}, \"Certificate unknown\");\n> +\t\t$choice = lc(substr( || 'R', 0, 1));\n> +\t\tclose(PH);\n> +\t} else {\n> +\t\t$choice = lc(substr( || 'R', 0, 1));\n> +\t}\n\nWhy is the prompt simply \"Certificate unknown\"? Shouldn't it mention\nthat the right answers are \"(R)eject, accept (t)emporarily, ...\"?\n\nThat aside, I think this is an improvement over the current code. But\nhaving just been looking at regular git's askpass code, I notice there\nare some subtle differences:\n\n 1. Regular git will also respect SSH_ASKPASS\n\n 2. Regular git will ignore an askpass variable that is set but empty.\n\nPerhaps git-svn should be refactored to have a reusable \"prompt\"\nfunction that respects askpass and tries to behave like C git? It could\neven go into the Git perl module.\n\n-Peff\n"},{"id":"181706","messageId":"4EF907F1.1030801@tu-clausthal.de","threadId":"28961","inReplyTo":"20111130064401.GC5317@sigill.intra.peff.net","subject":"Re: [PATCH] honour GIT_ASKPASS for querying username in git-svn","fromName":"Sven Strickroth","fromEmail":"sven.strickroth@tu-clausthal.de","sentAt":"2011-12-26T23:49:05Z","receivedAt":"2011-12-26T23:49:05Z","isPatch":true,"sender":{"key":"sven.strickroth@tu-clausthal.de","avatar":null},"body":"Hi,\n\nAm 30.11.2011 07:44 schrieb Jeff King:\n> That aside, I think this is an improvement over the current code.\n> 1. Regular git will also respect SSH_ASKPASS\n> 2. Regular git will ignore an askpass variable that is set but empty.\n> Perhaps git-svn should be refactored to have a reusable \"prompt\"\n> function that respects askpass and tries to behave like C git? It could\n> even go into the Git perl module.\n\nI honoured all your ideas. Hopefully the patches can be applied now. The new patches\nfollow (you can also pull from git://github.com/csware/git.git askpass-prompt):\n\n>From b760546c59d1b9982296c19f8eaea6dc225b5a4f Mon Sep 17 00:00:00 2001\nFrom: Sven Strickroth \nDate: Tue, 27 Dec 2011 00:33:46 +0100\nSubject: [PATCH 1/4] add central method for prompting a user using\n GIT_ASKPASS or SSH_ASKPASS\n\nSigned-off-by: Sven Strickroth \n---\n perl/Git.pm | 31 ++++++++++++++++++++++++++++++-\n 1 files changed, 30 insertions(+), 1 deletions(-)\n\ndiff --git a/perl/Git.pm b/perl/Git.pm\nindex f7ce511..8176d47 100644\n--- a/perl/Git.pm\n+++ b/perl/Git.pm\n@@ -58,7 +58,7 @@ require Exporter;\n command_output_pipe command_input_pipe command_close_pipe\n command_bidi_pipe command_close_bidi_pipe\n version exec_path html_path hash_object git_cmd_try\n- remote_refs\n+ remote_refs prompt\n temp_acquire temp_release temp_reset temp_path);\n\n\n@@ -512,6 +512,35 @@ C). Useful mostly only internally.\n sub html_path { command_oneline('--html-path') }\n\n\n+=item prompt ( PROMPT)\n+\n+Checks if GIT_ASKPASS or SSH_ASKPASS is set, and if yes\n+use it and return answer from user.\n+\n+=cut\n+\n+sub prompt {\n+\tmy ($self, $prompt) = _maybe_self(@_);\n+\tif (exists $ENV{'GIT_ASKPASS'}) {\n+\t\treturn _prompt($ENV{'GIT_ASKPASS'}, $prompt);\n+\t} elsif (exists $ENV{'SSH_ASKPASS'}) {\n+\t\treturn _prompt($ENV{'SSH_ASKPASS'}, $prompt);\n+\t} else {\n+\t\treturn undef;\n+\t}\n+}\n+\n+sub _prompt {\n+\tmy ($self, $askpass, $prompt) = _maybe_self(@_);\n+\tmy $ret;\n+\topen(PH, \"-|\", $askpass, $prompt);\n+\t$ret = ;\n+\t$ret =~ s/[\\012\\015]//g; # strip \\n\\r\n+\tclose(PH);\n+\treturn $ret;\n+}\n+\n+\n =item repo_path ()\n\n Return path to the git repository. Must be called on a repository instance.\n-- \n1.7.7.1.msysgit.0\n\n>From ef4c6557d1b0e33440d13c64742d44b2a22143f3 Mon Sep 17 00:00:00 2001\nFrom: Sven Strickroth \nDate: Tue, 27 Dec 2011 00:34:09 +0100\nSubject: [PATCH 2/4] switch to central prompt method\n\nSigned-off-by: Sven Strickroth \n---\n git-svn.perl | 9 ++-------\n 1 files changed, 2 insertions(+), 7 deletions(-)\n\ndiff --git a/git-svn.perl b/git-svn.perl\nindex eeb83d3..4fd4eca 100755\n--- a/git-svn.perl\n+++ b/git-svn.perl\n@@ -4415,13 +4415,8 @@ sub username {\n\n sub _read_password {\n \tmy ($prompt, $realm) = @_;\n-\tmy $password = '';\n-\tif (exists $ENV{GIT_ASKPASS}) {\n-\t\topen(PH, \"-|\", $ENV{GIT_ASKPASS}, $prompt);\n-\t\t$password = ;\n-\t\t$password =~ s/[\\012\\015]//; # \\n\\r\n-\t\tclose(PH);\n-\t} else {\n+\tmy $password = Git->prompt($prompt);;\n+\tif (!defined $password) {\n \t\tprint STDERR $prompt;\n \t\tSTDERR->flush;\n \t\trequire Term::ReadKey;\n-- \n1.7.7.1.msysgit.0\n\n>From d58f41d7b9b8e690c9839f6f7539774da88aa3a4 Mon Sep 17 00:00:00 2001\nFrom: Sven Strickroth \nDate: Tue, 27 Dec 2011 00:37:43 +0100\nSubject: [PATCH 3/4] honour *_ASKPASS for querying username and for querying\n further actions on unknown certificates\n\ngit-svn reads usernames (and answers for certificate errors) from an interactive terminal.\nThis behavior cause GUIs to hang waiting for git-svn to complete (http://code.google.com/p/tortoisegit/issues/detail?id=967).\n\nAlso see commit 56a853b62c0ae7ebaad0a7a0a704f5ef561eb795.\n\nSigned-off-by: Sven Strickroth \n---\n git-svn.perl | 13 ++++++++++---\n 1 files changed, 10 insertions(+), 3 deletions(-)\n\ndiff --git a/git-svn.perl b/git-svn.perl\nindex 4fd4eca..b85a7de 100755\n--- a/git-svn.perl\n+++ b/git-svn.perl\n@@ -4357,11 +4357,15 @@ sub ssl_server_trust {\n \t issuer_dname fingerprint);\n \tmy $choice;\n prompt:\n-\tprint STDERR $may_save ?\n+\tmy $options = $may_save ?\n \t \"(R)eject, accept (t)emporarily or accept (p)ermanently? \" :\n \t \"(R)eject or accept (t)emporarily? \";\n-\tSTDERR->flush;\n-\t$choice = lc(substr( || 'R', 0, 1));\n+\t$choice = Git->prompt(\"Certificate unknown. \" . $options);\n+\tif (!defined $choice) {\n+\t\tprint STDERR $options;\n+\t\tSTDERR->flush;\n+\t\t$choice = lc(substr( || 'R', 0, 1));\n+\t}\n \tif ($choice =~ /^t$/i) {\n \t\t$cred->may_save(undef);\n \t} elsif ($choice =~ /^r$/i) {\n@@ -4404,6 +4408,9 @@ sub username {\n \tif (defined $_username) {\n \t\t$username = $_username;\n \t} else {\n+\t\t$username = Git->prompt(\"Username\");\n+\t}\n+\tif (!defined $username) {\n \t\tprint STDERR \"Username: \";\n \t\tSTDERR->flush;\n \t\tchomp($username = );\n-- \n1.7.7.1.msysgit.0\n\n>From 2c1dbdae8024f28d17abfbdc7e45865a1277151a Mon Sep 17 00:00:00 2001\nFrom: Sven Strickroth \nDate: Tue, 27 Dec 2011 00:42:07 +0100\nSubject: [PATCH 4/4] ignore empty *_ASKPASS variables\n\nSigned-off-by: Sven Strickroth \n---\n perl/Git.pm | 3 +++\n 1 files changed, 3 insertions(+), 0 deletions(-)\n\ndiff --git a/perl/Git.pm b/perl/Git.pm\nindex 8176d47..fade617 100644\n--- a/perl/Git.pm\n+++ b/perl/Git.pm\n@@ -532,6 +532,9 @@ sub prompt {\n\n sub _prompt {\n \tmy ($self, $askpass, $prompt) = _maybe_self(@_);\n+\tunless ($askpass) {\n+\t\treturn undef;\n+\t}\n \tmy $ret;\n \topen(PH, \"-|\", $askpass, $prompt);\n \t$ret = ;\n-- \n1.7.7.1.msysgit.0\n\n-- \nBest regards,\n Sven Strickroth\n ClamAV, a GPL anti-virus toolkit http://www.clamav.net\n PGP key id F5A9D4C4 @ any key-server\n"},{"id":"181714","messageId":"m3d3baf5kd.fsf@localhost.localdomain","threadId":"28961","inReplyTo":"4EF907F1.1030801@tu-clausthal.de","subject":"Re: [PATCH] honour GIT_ASKPASS for querying username in git-svn","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2011-12-27T14:33:51Z","receivedAt":"2011-12-27T14:33:51Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Sven Strickroth writes:\n\n> +=item prompt ( PROMPT)\n> +\n> +Checks if GIT_ASKPASS or SSH_ASKPASS is set, and if yes\n> +use it and return answer from user.\n> +\n> +=cut\n\nI think it would be good idea to describe what this function is for...\n\n> +sub prompt {\n> +\tmy ($self, $prompt) = _maybe_self(@_);\n> +\tif (exists $ENV{'GIT_ASKPASS'}) {\n> +\t\treturn _prompt($ENV{'GIT_ASKPASS'}, $prompt);\n> +\t} elsif (exists $ENV{'SSH_ASKPASS'}) {\n> +\t\treturn _prompt($ENV{'SSH_ASKPASS'}, $prompt);\n> +\t} else {\n> +\t\treturn undef;\n> +\t}\n> +}\n\n...and provide some kind of fallback even if neither of GIT_ASKPASS\nnor SSH_ASKPASS are set (perhaps assuming that some Perl packages from\nCPAN are installed).\n\n> +sub _prompt {\n> +\tmy ($self, $askpass, $prompt) = _maybe_self(@_);\n> +\tmy $ret;\n> +\topen(PH, \"-|\", $askpass, $prompt);\n> +\t$ret = ;\n> +\t$ret =~ s/[\\012\\015]//g; # strip \\n\\r\n> +\tclose(PH);\n> +\treturn $ret;\n> +}\n\nPlease, use modern Perl, in particula use lexical filehandles instead\nof typeglobs (which are global variables), i.e.\n\n +\topen my $fh, \"-|\", $askpass, $prompt\n +\t\tor die \"...\";\n +\t$ret = <$fh>;\n +\tchomp($ret);\n +\tclose($fh)\n +\t\tor die \"...\";\n\n\n> -- \n> 1.7.7.1.msysgit.0\n> \n> From ef4c6557d1b0e33440d13c64742d44b2a22143f3 Mon Sep 17 00:00:00 2001\n> From: Sven Strickroth \n> Date: Tue, 27 Dec 2011 00:34:09 +0100\n> Subject: [PATCH 2/4] switch to central prompt method\n> \n> Signed-off-by: Sven Strickroth \n\nPlease send those patches as a separate emails, not concatenated in a\nsingle email (perhaps even with cover letter).\n\nSee Documentation/SubmittingPatches\n\n[...]\n-- \nJakub Narebski\n"},{"id":"181715","messageId":"4EF9D8B9.9060106@tu-clausthal.de","threadId":"28961","inReplyTo":"m3d3baf5kd.fsf@localhost.localdomain","subject":"Re: [PATCH] honour GIT_ASKPASS for querying username in git-svn","fromName":"Sven Strickroth","fromEmail":"sven.strickroth@tu-clausthal.de","sentAt":"2011-12-27T14:39:53Z","receivedAt":"2011-12-27T14:39:53Z","isPatch":true,"sender":{"key":"sven.strickroth@tu-clausthal.de","avatar":null},"body":"Am 27.12.2011 15:33 schrieb Jakub Narebski:\n>> +sub prompt {\n>> +\tmy ($self, $prompt) = _maybe_self(@_);\n>> +\tif (exists $ENV{'GIT_ASKPASS'}) {\n>> +\t\treturn _prompt($ENV{'GIT_ASKPASS'}, $prompt);\n>> +\t} elsif (exists $ENV{'SSH_ASKPASS'}) {\n>> +\t\treturn _prompt($ENV{'SSH_ASKPASS'}, $prompt);\n>> +\t} else {\n>> +\t\treturn undef;\n>> +\t}\n>> +}\n> \n> ...and provide some kind of fallback even if neither of GIT_ASKPASS\n> nor SSH_ASKPASS are set (perhaps assuming that some Perl packages from\n> CPAN are installed).\n\nIf neither of GIT_ASKPASS nor SSH_ASKPASS are set the caller has to\nhandle the request. This has to be done this way, because of lots of\ndifferent needs (username, password (no echo) and so on).\n\n>> +sub _prompt {\n>> +\tmy ($self, $askpass, $prompt) = _maybe_self(@_);\n>> +\tmy $ret;\n>> +\topen(PH, \"-|\", $askpass, $prompt);\n>> +\t$ret = ;\n>> +\t$ret =~ s/[\\012\\015]//g; # strip \\n\\r\n>> +\tclose(PH);\n>> +\treturn $ret;\n>> +}\n> \n> Please, use modern Perl, in particula use lexical filehandles instead\n> of typeglobs (which are global variables), i.e.\n\nI used the same style as I found in Git.pm (see lines I removed in patch 2).\n\n-- \nBest regards,\n Sven Strickroth\n ClamAV, a GPL anti-virus toolkit http://www.clamav.net\n PGP key id F5A9D4C4 @ any key-server\n"},{"id":"181717","messageId":"201112271700.58078.jnareb@gmail.com","threadId":"28961","inReplyTo":"4EF9D8B9.9060106@tu-clausthal.de","subject":"Re: [PATCH] honour GIT_ASKPASS for querying username in git-svn","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2011-12-27T16:00:57Z","receivedAt":"2011-12-27T16:00:57Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Tue, 27 Dec 2011, Sven Strickroth wrote:\n> Am 27.12.2011 15:33 schrieb Jakub Narebski:\n\n>>> +sub prompt {\n>>> +\tmy ($self, $prompt) = _maybe_self(@_);\n>>> +\tif (exists $ENV{'GIT_ASKPASS'}) {\n>>> +\t\treturn _prompt($ENV{'GIT_ASKPASS'}, $prompt);\n>>> +\t} elsif (exists $ENV{'SSH_ASKPASS'}) {\n>>> +\t\treturn _prompt($ENV{'SSH_ASKPASS'}, $prompt);\n>>> +\t} else {\n>>> +\t\treturn undef;\n>>> +\t}\n>>> +}\n>> \n>> ...and provide some kind of fallback even if neither of GIT_ASKPASS\n>> nor SSH_ASKPASS are set (perhaps assuming that some Perl packages from\n>> CPAN are installed).\n> \n> If neither of GIT_ASKPASS nor SSH_ASKPASS are set the caller has to\n> handle the request. This has to be done this way, because of lots of\n> different needs (username, password (no echo) and so on).\n\nI think that Git.pm and therefore git commands written in Perl should\nbehave the same as git command written in C; and I think builtins do\nuse common gitprompt fallback.\n \n>>> +sub _prompt {\n>>> +\tmy ($self, $askpass, $prompt) = _maybe_self(@_);\n>>> +\tmy $ret;\n>>> +\topen(PH, \"-|\", $askpass, $prompt);\n>>> +\t$ret = ;\n>>> +\t$ret =~ s/[\\012\\015]//g; # strip \\n\\r\n>>> +\tclose(PH);\n>>> +\treturn $ret;\n>>> +}\n>> \n>> Please, use modern Perl, in particula use lexical filehandles instead\n>> of typeglobs (which are global variables), i.e.\n> \n> I used the same style as I found in Git.pm (see lines I removed in patch 2).\n\nYes, that should be fixed (together with host of other issues), but\none should use modern and _better_ way (no possibility of action at\ndistance).\n\n-- \nJakub Narebski\nPoland\n"},{"id":"181718","messageId":"4EF9EBF4.7070200@tu-clausthal.de","threadId":"28961","inReplyTo":"4EF9D8B9.9060106@tu-clausthal.de","subject":"[PATCH 0/5] honour *_ASKPASS for querying user in git-svn","fromName":"Sven Strickroth","fromEmail":"sven.strickroth@tu-clausthal.de","sentAt":"2011-12-27T16:01:56Z","receivedAt":"2011-12-27T16:01:56Z","isPatch":true,"sender":{"key":"sven.strickroth@tu-clausthal.de","avatar":null},"body":"Hi,\n\ngit-svn reads usernames (and other stuff) from an interactive terminal.\nThis behavior cause GUIs to hang waiting for git-svn to complete\n(http://code.google.com/p/tortoisegit/issues/detail?id=967).\n\nAgain I honoured all your ideas. Hopefully the patches can be applied\nnow. The new patches follow in the next mails (you can also pull from\ngit://github.com/csware/git.git askpass-prompt).\n\nThe first four patches implement the raw functionality. The last and\nfifth patch extends the prompt-method so that it can be used for all\npurposes in order to query users.\n\n-- \nBest regards,\n Sven Strickroth\n ClamAV, a GPL anti-virus toolkit http://www.clamav.net\n PGP key id F5A9D4C4 @ any key-server\n"},{"id":"181719","messageId":"4EF9ECC0.606@tu-clausthal.de","threadId":"28961","inReplyTo":"4EF9EBF4.7070200@tu-clausthal.de","subject":"[PATCH 1/5] add central method for prompting a user using GIT_ASKPASS or SSH_ASKPASS","fromName":"Sven Strickroth","fromEmail":"sven.strickroth@tu-clausthal.de","sentAt":"2011-12-27T16:05:20Z","receivedAt":"2011-12-27T16:05:20Z","isPatch":true,"sender":{"key":"sven.strickroth@tu-clausthal.de","avatar":null},"body":"\nSigned-off-by: Sven Strickroth \n---\n perl/Git.pm | 36 +++++++++++++++++++++++++++++++++++-\n 1 files changed, 35 insertions(+), 1 deletions(-)\n\ndiff --git a/perl/Git.pm b/perl/Git.pm\nindex f7ce511..7fdf805 100644\n--- a/perl/Git.pm\n+++ b/perl/Git.pm\n@@ -58,7 +58,7 @@ require Exporter;\n command_output_pipe command_input_pipe command_close_pipe\n command_bidi_pipe command_close_bidi_pipe\n version exec_path html_path hash_object git_cmd_try\n- remote_refs\n+ remote_refs askpass_prompt\n temp_acquire temp_release temp_reset temp_path);\n\n\n@@ -512,6 +512,40 @@ C). Useful mostly only internally.\n sub html_path { command_oneline('--html-path') }\n\n\n+=item askpass_prompt ( PROMPT)\n+\n+Asks user using *_ASKPASS programs and return answer from user.\n+\n+Checks if GIT_ASKPASS or SSH_ASKPASS is set, and use first matching for querying\n+user and returns answer.\n+\n+If no *_ASKPASS variable is set, the variable is empty or an error occours,\n+it returns undef and the caller has to ask the user (e.g. on terminal).\n+\n+=cut\n+\n+sub askpass_prompt {\n+\tmy ($self, $prompt) = _maybe_self(@_);\n+\tif (exists $ENV{'GIT_ASKPASS'}) {\n+\t\treturn _askpass_prompt($ENV{'GIT_ASKPASS'}, $prompt);\n+\t} elsif (exists $ENV{'SSH_ASKPASS'}) {\n+\t\treturn _askpass_prompt($ENV{'SSH_ASKPASS'}, $prompt);\n+\t} else {\n+\t\treturn undef;\n+\t}\n+}\n+\n+sub _askpass_prompt {\n+\tmy ($self, $askpass, $prompt) = _maybe_self(@_);\n+\tmy $ret;\n+\topen my $fh, \"-|\", $askpass, $prompt || return undef;\n+\t$ret = <$fh>;\n+\t$ret =~ s/[\\012\\015]//g; # strip \\n\\r, chomp does not work on all systems (i.e. windows) as expected\n+\tclose ($fh);\n+\treturn $ret;\n+}\n+\n+\n =item repo_path ()\n\n Return path to the git repository. Must be called on a repository instance.\n\n-- \nBest regards,\n Sven Strickroth\n ClamAV, a GPL anti-virus toolkit http://www.clamav.net\n PGP key id F5A9D4C4 @ any key-server\n"},{"id":"181720","messageId":"4EF9ECEF.6020403@tu-clausthal.de","threadId":"28961","inReplyTo":"4EF9EBF4.7070200@tu-clausthal.de","subject":"[PATCH 2/5] switch to central prompt method","fromName":"Sven Strickroth","fromEmail":"sven.strickroth@tu-clausthal.de","sentAt":"2011-12-27T16:06:07Z","receivedAt":"2011-12-27T16:06:07Z","isPatch":true,"sender":{"key":"sven.strickroth@tu-clausthal.de","avatar":null},"body":"\nSigned-off-by: Sven Strickroth \n---\n git-svn.perl | 9 ++-------\n 1 files changed, 2 insertions(+), 7 deletions(-)\n\ndiff --git a/git-svn.perl b/git-svn.perl\nindex eeb83d3..25d5da7 100755\n--- a/git-svn.perl\n+++ b/git-svn.perl\n@@ -4415,13 +4415,8 @@ sub username {\n\n sub _read_password {\n \tmy ($prompt, $realm) = @_;\n-\tmy $password = '';\n-\tif (exists $ENV{GIT_ASKPASS}) {\n-\t\topen(PH, \"-|\", $ENV{GIT_ASKPASS}, $prompt);\n-\t\t$password = ;\n-\t\t$password =~ s/[\\012\\015]//; # \\n\\r\n-\t\tclose(PH);\n-\t} else {\n+\tmy $password = Git->askpass_prompt($prompt);;\n+\tif (!defined $password) {\n \t\tprint STDERR $prompt;\n \t\tSTDERR->flush;\n \t\trequire Term::ReadKey;\n-- \nBest regards,\n Sven Strickroth\n ClamAV, a GPL anti-virus toolkit http://www.clamav.net\n PGP key id F5A9D4C4 @ any key-server\n"},{"id":"181721","messageId":"4EF9ED24.2040902@tu-clausthal.de","threadId":"28961","inReplyTo":"4EF9EBF4.7070200@tu-clausthal.de","subject":"[PATCH 3/5] honour *_ASKPASS for querying username and for querying further actions like unknown certificates","fromName":"Sven Strickroth","fromEmail":"sven.strickroth@tu-clausthal.de","sentAt":"2011-12-27T16:07:00Z","receivedAt":"2011-12-27T16:07:00Z","isPatch":true,"sender":{"key":"sven.strickroth@tu-clausthal.de","avatar":null},"body":"git-svn reads usernames (and other stuff) from an interactive terminal.\nThis behavior cause GUIs to hang waiting for git-svn to complete (http://code.google.com/p/tortoisegit/issues/detail?id=967).\n\nAlso see commit 56a853b62c0ae7ebaad0a7a0a704f5ef561eb795.\n\nSigned-off-by: Sven Strickroth \n---\n git-svn.perl | 22 ++++++++++++++++------\n 1 files changed, 16 insertions(+), 6 deletions(-)\n\ndiff --git a/git-svn.perl b/git-svn.perl\nindex 25d5da7..2c99aaa 100755\n--- a/git-svn.perl\n+++ b/git-svn.perl\n@@ -4357,11 +4357,15 @@ sub ssl_server_trust {\n \t issuer_dname fingerprint);\n \tmy $choice;\n prompt:\n-\tprint STDERR $may_save ?\n+\tmy $options = $may_save ?\n \t \"(R)eject, accept (t)emporarily or accept (p)ermanently? \" :\n \t \"(R)eject or accept (t)emporarily? \";\n-\tSTDERR->flush;\n-\t$choice = lc(substr( || 'R', 0, 1));\n+\t$choice = Git->askpass_prompt(\"Certificate unknown. \" . $options);\n+\tif (!defined $choice) {\n+\t\tprint STDERR $options;\n+\t\tSTDERR->flush;\n+\t\t$choice = lc(substr( || 'R', 0, 1));\n+\t}\n \tif ($choice =~ /^t$/i) {\n \t\t$cred->may_save(undef);\n \t} elsif ($choice =~ /^r$/i) {\n@@ -4378,9 +4382,12 @@ prompt:\n sub ssl_client_cert {\n \tmy ($cred, $realm, $may_save, $pool) = @_;\n \t$may_save = undef if $_no_auth_cache;\n-\tprint STDERR \"Client certificate filename: \";\n-\tSTDERR->flush;\n-\tchomp(my $filename = );\n+\tmy $filename = Git->askpass_prompt(\"Client certificate filename:\");\n+\tif (!defined $filename) {\n+\t\tprint STDERR \"Client certificate filename: \";\n+\t\tSTDERR->flush;\n+\t\tchomp($filename = );\n+\t}\n \t$cred->cert_file($filename);\n \t$cred->may_save($may_save);\n \t$SVN::_Core::SVN_NO_ERROR;\n@@ -4404,6 +4411,9 @@ sub username {\n \tif (defined $_username) {\n \t\t$username = $_username;\n \t} else {\n+\t\t$username = Git->askpass_prompt(\"Username\");\n+\t}\n+\tif (!defined $username) {\n \t\tprint STDERR \"Username: \";\n \t\tSTDERR->flush;\n \t\tchomp($username = );\n-- \nBest regards,\n Sven Strickroth\n ClamAV, a GPL anti-virus toolkit http://www.clamav.net\n PGP key id F5A9D4C4 @ any key-server\n"},{"id":"181722","messageId":"4EF9ED38.9010502@tu-clausthal.de","threadId":"28961","inReplyTo":"4EF9EBF4.7070200@tu-clausthal.de","subject":"[PATCH 4/5] ignore empty *_ASKPASS variables","fromName":"Sven Strickroth","fromEmail":"sven.strickroth@tu-clausthal.de","sentAt":"2011-12-27T16:07:20Z","receivedAt":"2011-12-27T16:07:20Z","isPatch":true,"sender":{"key":"sven.strickroth@tu-clausthal.de","avatar":null},"body":"\nSigned-off-by: Sven Strickroth \n---\n perl/Git.pm | 3 +++\n 1 files changed, 3 insertions(+), 0 deletions(-)\n\ndiff --git a/perl/Git.pm b/perl/Git.pm\nindex 7fdf805..c6b3e11 100644\n--- a/perl/Git.pm\n+++ b/perl/Git.pm\n@@ -537,6 +537,9 @@ sub askpass_prompt {\n\n sub _askpass_prompt {\n \tmy ($self, $askpass, $prompt) = _maybe_self(@_);\n+\tunless ($askpass) {\n+\t\treturn undef;\n+\t}\n \tmy $ret;\n \topen my $fh, \"-|\", $askpass, $prompt || return undef;\n \t$ret = <$fh>;\n-- \nBest regards,\n Sven Strickroth\n ClamAV, a GPL anti-virus toolkit http://www.clamav.net\n PGP key id F5A9D4C4 @ any key-server\n"},{"id":"181723","messageId":"4EF9ED58.8080205@tu-clausthal.de","threadId":"28961","inReplyTo":"4EF9EBF4.7070200@tu-clausthal.de","subject":"[PATCH 5/5] make askpass_prompt a global prompt method for asking users","fromName":"Sven Strickroth","fromEmail":"sven.strickroth@tu-clausthal.de","sentAt":"2011-12-27T16:07:52Z","receivedAt":"2011-12-27T16:07:52Z","isPatch":true,"sender":{"key":"sven.strickroth@tu-clausthal.de","avatar":null},"body":"\nSigned-off-by: Sven Strickroth \n---\n git-svn.perl | 37 ++++---------------------------------\n perl/Git.pm | 43 +++++++++++++++++++++++++++++++------------\n 2 files changed, 35 insertions(+), 45 deletions(-)\n\ndiff --git a/git-svn.perl b/git-svn.perl\nindex 2c99aaa..1f30dc2 100755\n--- a/git-svn.perl\n+++ b/git-svn.perl\n@@ -4360,12 +4360,7 @@ prompt:\n \tmy $options = $may_save ?\n \t \"(R)eject, accept (t)emporarily or accept (p)ermanently? \" :\n \t \"(R)eject or accept (t)emporarily? \";\n-\t$choice = Git->askpass_prompt(\"Certificate unknown. \" . $options);\n-\tif (!defined $choice) {\n-\t\tprint STDERR $options;\n-\t\tSTDERR->flush;\n-\t\t$choice = lc(substr( || 'R', 0, 1));\n-\t}\n+\t$choice = substr(Git->prompt(\"Certificate unknown. \" . $options) || 'R', 0, 1);\n \tif ($choice =~ /^t$/i) {\n \t\t$cred->may_save(undef);\n \t} elsif ($choice =~ /^r$/i) {\n@@ -4382,13 +4377,7 @@ prompt:\n sub ssl_client_cert {\n \tmy ($cred, $realm, $may_save, $pool) = @_;\n \t$may_save = undef if $_no_auth_cache;\n-\tmy $filename = Git->askpass_prompt(\"Client certificate filename:\");\n-\tif (!defined $filename) {\n-\t\tprint STDERR \"Client certificate filename: \";\n-\t\tSTDERR->flush;\n-\t\tchomp($filename = );\n-\t}\n-\t$cred->cert_file($filename);\n+\t$cred->cert_file(Git->prompt(\"Client certificate filename: \"));\n \t$cred->may_save($may_save);\n \t$SVN::_Core::SVN_NO_ERROR;\n }\n@@ -4411,12 +4400,7 @@ sub username {\n \tif (defined $_username) {\n \t\t$username = $_username;\n \t} else {\n-\t\t$username = Git->askpass_prompt(\"Username\");\n-\t}\n-\tif (!defined $username) {\n-\t\tprint STDERR \"Username: \";\n-\t\tSTDERR->flush;\n-\t\tchomp($username = );\n+\t\t$username = Git->prompt(\"Username: \");\n \t}\n \t$cred->username($username);\n \t$cred->may_save($may_save);\n@@ -4425,20 +4409,7 @@ sub username {\n\n sub _read_password {\n \tmy ($prompt, $realm) = @_;\n-\tmy $password = Git->askpass_prompt($prompt);;\n-\tif (!defined $password) {\n-\t\tprint STDERR $prompt;\n-\t\tSTDERR->flush;\n-\t\trequire Term::ReadKey;\n-\t\tTerm::ReadKey::ReadMode('noecho');\n-\t\twhile (defined(my $key = Term::ReadKey::ReadKey(0))) {\n-\t\t\tlast if $key =~ /[\\012\\015]/; # \\n\\r\n-\t\t\t$password .= $key;\n-\t\t}\n-\t\tTerm::ReadKey::ReadMode('restore');\n-\t\tprint STDERR \"\\n\";\n-\t\tSTDERR->flush;\n-\t}\n+\tmy $password = Git->prompt($prompt, 1);\n \t$password;\n }\n\ndiff --git a/perl/Git.pm b/perl/Git.pm\nindex c6b3e11..acc00b4 100644\n--- a/perl/Git.pm\n+++ b/perl/Git.pm\n@@ -58,7 +58,7 @@ require Exporter;\n command_output_pipe command_input_pipe command_close_pipe\n command_bidi_pipe command_close_bidi_pipe\n version exec_path html_path hash_object git_cmd_try\n- remote_refs askpass_prompt\n+ remote_refs prompt\n temp_acquire temp_release temp_reset temp_path);\n\n\n@@ -512,30 +512,49 @@ C). Useful mostly only internally.\n sub html_path { command_oneline('--html-path') }\n\n\n-=item askpass_prompt ( PROMPT)\n+=item prompt ( PROMPT , ISPASSWORD )\n\n-Asks user using *_ASKPASS programs and return answer from user.\n+Asks user using *_ASKPASS programs or terminal the prompt C and return answer from user.\n\n Checks if GIT_ASKPASS or SSH_ASKPASS is set, and use first matching for querying\n user and returns answer.\n\n If no *_ASKPASS variable is set, the variable is empty or an error occours,\n-it returns undef and the caller has to ask the user (e.g. on terminal).\n+the querying the user using terminal is tried. If C is set and true,\n+the terminal disables echo.\n\n =cut\n\n-sub askpass_prompt {\n-\tmy ($self, $prompt) = _maybe_self(@_);\n+sub prompt {\n+\tmy ($self, $prompt, $isPassword) = _maybe_self(@_);\n+\tmy $ret;\n \tif (exists $ENV{'GIT_ASKPASS'}) {\n-\t\treturn _askpass_prompt($ENV{'GIT_ASKPASS'}, $prompt);\n-\t} elsif (exists $ENV{'SSH_ASKPASS'}) {\n-\t\treturn _askpass_prompt($ENV{'SSH_ASKPASS'}, $prompt);\n-\t} else {\n-\t\treturn undef;\n+\t\t$ret = _prompt($ENV{'GIT_ASKPASS'}, $prompt);\n \t}\n+\tif (!defined $ret && exists $ENV{'SSH_ASKPASS'}) {\n+\t\t$ret = _prompt($ENV{'SSH_ASKPASS'}, $prompt);\n+\t}\n+\tif (!defined $ret) {\n+\t\tprint STDERR $prompt;\n+\t\tSTDERR->flush;\n+\t\tif (defined $isPassword && $isPassword) {\n+\t\t\trequire Term::ReadKey;\n+\t\t\tTerm::ReadKey::ReadMode('noecho');\n+\t\t\twhile (defined(my $key = Term::ReadKey::ReadKey(0))) {\n+\t\t\t\tlast if $key =~ /[\\012\\015]/; # \\n\\r\n+\t\t\t\t$ret .= $key;\n+\t\t\t}\n+\t\t\tTerm::ReadKey::ReadMode('restore');\n+\t\t\tprint STDERR \"\\n\";\n+\t\t\tSTDERR->flush;\n+\t\t} else {\n+\t\t\tchomp($ret = );\n+\t\t}\n+\t}\n+\treturn $ret;\n }\n\n-sub _askpass_prompt {\n+sub _prompt {\n \tmy ($self, $askpass, $prompt) = _maybe_self(@_);\n \tunless ($askpass) {\n \t\treturn undef;\n-- \nBest regards,\n Sven Strickroth\n ClamAV, a GPL anti-virus toolkit http://www.clamav.net\n PGP key id F5A9D4C4 @ any key-server\n"},{"id":"181728","messageId":"7vwr9h68t9.fsf@alter.siamese.dyndns.org","threadId":"28961","inReplyTo":"4EF9ECC0.606@tu-clausthal.de","subject":"Re: [PATCH 1/5] add central method for prompting a user using GIT_ASKPASS or SSH_ASKPASS","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-12-27T20:47:30Z","receivedAt":"2011-12-27T20:47:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sven Strickroth writes:\n\n> Signed-off-by: Sven Strickroth \n> ---\n\nThanks.\n\nPlease indicate what area you are touching on the Subject, e.g.\n\n\tSubject: [PATCH 1/5] perl/Git.pm: add askpass_prompt() method\n\nand explain what problem it tries to solve, how it tries to solve it, and\nwhy this particular way of solving that problem is a good thing to have in\nthe proposed commit log message (but see the comments to 2/5 before doing\nso).\n\n> perl/Git.pm | 36 +++++++++++++++++++++++++++++++++++-\n> 1 files changed, 35 insertions(+), 1 deletions(-)\n>\n> diff --git a/perl/Git.pm b/perl/Git.pm\n> index f7ce511..7fdf805 100644\n> --- a/perl/Git.pm\n> +++ b/perl/Git.pm\n> @@ -58,7 +58,7 @@ require Exporter;\n> command_output_pipe command_input_pipe command_close_pipe\n> command_bidi_pipe command_close_bidi_pipe\n> version exec_path html_path hash_object git_cmd_try\n> - remote_refs\n> + remote_refs askpass_prompt\n> temp_acquire temp_release temp_reset temp_path);\n>\n>\n> @@ -512,6 +512,40 @@ C). Useful mostly only internally.\n> sub html_path { command_oneline('--html-path') }\n>\n>\n> +=item askpass_prompt ( PROMPT)\n\nIs the unbalanced spacing around parentheses your finger slippage?\n\n> +\n> +Asks user using *_ASKPASS programs and return answer from user.\n\nOther =item entries in this file (I only checked a couple of earlier ones)\nbegin with \"Construct a new ...\", \"Execute the given git command ...\",\ni.e. in imperative mood.\n\nAlso I agree with Jakub that it would be more appropriate to start the\ndescription with the purpose and the effect, i.e. what the function is\nfor, than the internal implementation, i.e. what the function does.\n\n> +Checks if GIT_ASKPASS or SSH_ASKPASS is set, and use first matching for querying\n> +user and returns answer.\n> +\n> +If no *_ASKPASS variable is set, the variable is empty or an error occours,\n> +it returns undef and the caller has to ask the user (e.g. on terminal).\n> +\n> +=cut\n> +\n> +sub askpass_prompt {\n> +\tmy ($self, $prompt) = _maybe_self(@_);\n> +\tif (exists $ENV{'GIT_ASKPASS'}) {\n> +\t\treturn _askpass_prompt($ENV{'GIT_ASKPASS'}, $prompt);\n> +\t} elsif (exists $ENV{'SSH_ASKPASS'}) {\n> +\t\treturn _askpass_prompt($ENV{'SSH_ASKPASS'}, $prompt);\n> +\t} else {\n> +\t\treturn undef;\n\nTwo problems with this if/elsif/else cascade.\n\n - If _askpass_prompt() fails to open the pipe to ENV{'GIT_ASKPASS'}, it\n will return 'undef' to us. Don't we want to fall back to SSH_ASKPASS in\n such a case?\n\n - The last \"return undef\" makes all callers of this method to implement a\n fall-back way somehow. I find it very likely that they will want to use\n the fall-back code that uses Term::ReadKey found in _read_password, and\n they will hate the above askpass_prompt implementation for focing them\n to duplicate the code more than they will appreciate the flexibility\n that they could implement a different fall-back.\n\n> +}\n> +\n> +sub _askpass_prompt {\n> +\tmy ($self, $askpass, $prompt) = _maybe_self(@_);\n\nI am not sure why you would want _maybe_self() here for this internal\nhelper function _askpass_prompt that is not even called as a method of\nanything.\n\n> +\tmy $ret;\n> +\topen my $fh, \"-|\", $askpass, $prompt || return undef;\n> +\t$ret = <$fh>;\n> +\t$ret =~ s/[\\012\\015]//g; # strip \\n\\r, chomp does not work on all systems (i.e. windows) as expected\n> +\tclose ($fh);\n> +\treturn $ret;\n> +}\n> +\n> +\n> =item repo_path ()\n>\n> Return path to the git repository. Must be called on a repository instance.\n"},{"id":"181729","messageId":"7vpqf968sr.fsf@alter.siamese.dyndns.org","threadId":"28961","inReplyTo":"4EF9ECEF.6020403@tu-clausthal.de","subject":"Re: [PATCH 2/5] switch to central prompt method","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-12-27T20:47:48Z","receivedAt":"2011-12-27T20:47:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sven Strickroth writes:\n\n> Signed-off-by: Sven Strickroth \n> ---\n\nIt would be better to have this and the previous patch squashed into one\ncommit, for three reasons:\n\n - It is far easier to review the patch that way, because it will show the\n old way to get the password from the user as the removed code and new\n way to do so as the added helper function, enabling reviewers to\n compare the two in a single review session, to see if the change keeps\n the feature bug-to-bug compatible, introduces any regression, and/or\n offers improvements over existing code.\n\n - If there were a bug in the implementation of askpass_prompt method\n introduced in patch 1 without being used, and the calling codepath\n introduced in patch 2 is bug-free (i.e. it correctly follows the\n calling convention of the new method, only the implementation of the\n method is buggy), bisection will still point at patch 2 that dumped the\n old proven working way and started using the buggy new implementation.\n\n Of course, it is possible that patch 1 perfectly implements the new\n method and a bug exists in the way the caller introduced in patch 2\n calls the method and in such a case bisection will correctly point out\n that the caller is at fault, but the point of this refactoring is to\n make it harder for callers to make such mistakes.\n\n - As we can see above the three-dash line, even the author of the series\n could not come up with any justification why the proposed change is a\n good thing in the proposed commit log message for this patch alone (or\n the previous patch alone for that matter). Combining these patches\n together would make it clearer why it may be a good thing, which would\n make it easier to come up with a better log message.\n\n> git-svn.perl | 9 ++-------\n> 1 files changed, 2 insertions(+), 7 deletions(-)\n>\n> diff --git a/git-svn.perl b/git-svn.perl\n> index eeb83d3..25d5da7 100755\n> --- a/git-svn.perl\n> +++ b/git-svn.perl\n> @@ -4415,13 +4415,8 @@ sub username {\n>\n> sub _read_password {\n> \tmy ($prompt, $realm) = @_;\n> -\tmy $password = '';\n> -\tif (exists $ENV{GIT_ASKPASS}) {\n> -\t\topen(PH, \"-|\", $ENV{GIT_ASKPASS}, $prompt);\n> -\t\t$password = ;\n> -\t\t$password =~ s/[\\012\\015]//; # \\n\\r\n> -\t\tclose(PH);\n> -\t} else {\n> +\tmy $password = Git->askpass_prompt($prompt);;\n> +\tif (!defined $password) {\n> \t\tprint STDERR $prompt;\n> \t\tSTDERR->flush;\n> \t\trequire Term::ReadKey;\n"},{"id":"181730","messageId":"7vlipx68dk.fsf@alter.siamese.dyndns.org","threadId":"28961","inReplyTo":"4EF9ED24.2040902@tu-clausthal.de","subject":"Re: [PATCH 3/5] honour *_ASKPASS for querying username and for querying further actions like unknown certificates","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-12-27T20:56:55Z","receivedAt":"2011-12-27T20:56:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sven Strickroth writes:\n\n> git-svn reads usernames (and other stuff) from an interactive terminal.\n> This behavior cause GUIs to hang waiting for git-svn to complete (http://code.google.com/p/tortoisegit/issues/detail?id=967).\n\n(style) Wrap the line perhaps after \"to complete\".\n\nDoes the above mean \"GUIs hang, until the user goes back to the terminal\nand authenticates\"? Where is the \"interactive terminal\" connected when\nrunning the GUI?\n\nWith that bit information, I think the above is a decent problem\ndescription (i.e. \"what problem is this change trying to solve? is it\nworth solving?\").\n\nThe second paragraph (missing) should then discuss what approach is taken\nby the proposed patch to solve that problem. Something like\n\n Instead of using hand-rolled prompt-response code that only works with\n the interactive terminal, use the git_prompt() method introduced in\n the earlier commit.\n\nwould suffice (I didn't check what method name you used, though).\n\n> Also see commit 56a853b62c0ae7ebaad0a7a0a704f5ef561eb795.\n\nI checked that commit, and what you wanted to say is unclear. Are you\nsaying this patch attempts to fix the breakage by that commit? That commit\ntried to go in a right direction but did not go far enough and you are\ntrying to enhance it? Somerthing else?\n\nWhich means that you shouldn't have said \"Also see...\" at all and instead\ndirectly said what you wanted to say here.\n"},{"id":"181731","messageId":"7vhb0l6883.fsf@alter.siamese.dyndns.org","threadId":"28961","inReplyTo":"4EF9ED38.9010502@tu-clausthal.de","subject":"Re: [PATCH 4/5] ignore empty *_ASKPASS variables","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-12-27T21:00:12Z","receivedAt":"2011-12-27T21:00:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sven Strickroth writes:\n\n> Signed-off-by: Sven Strickroth \n> ---\n\nI *suspect* that this is a fix-up to a bug in patch 1/5 that lets callers\ncall _askpass_prompt helper without checking the value of the \"askpass\",\nand if that is the case, this patch should be squashed there.\n\nBut there is no justification in the proposed log message above, so I\ncannot tell.\n\n> perl/Git.pm | 3 +++\n> 1 files changed, 3 insertions(+), 0 deletions(-)\n>\n> diff --git a/perl/Git.pm b/perl/Git.pm\n> index 7fdf805..c6b3e11 100644\n> --- a/perl/Git.pm\n> +++ b/perl/Git.pm\n> @@ -537,6 +537,9 @@ sub askpass_prompt {\n>\n> sub _askpass_prompt {\n> \tmy ($self, $askpass, $prompt) = _maybe_self(@_);\n> +\tunless ($askpass) {\n> +\t\treturn undef;\n> +\t}\n> \tmy $ret;\n> \topen my $fh, \"-|\", $askpass, $prompt || return undef;\n> \t$ret = <$fh>;\n"},{"id":"181732","messageId":"7vd3b967ql.fsf@alter.siamese.dyndns.org","threadId":"28961","inReplyTo":"4EF9ED58.8080205@tu-clausthal.de","subject":"Re: [PATCH 5/5] make askpass_prompt a global prompt method for asking users","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-12-27T21:10:42Z","receivedAt":"2011-12-27T21:10:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sven Strickroth writes:\n\n> Signed-off-by: Sven Strickroth \n> ---\n\nI think the end result of having a prompt function that can be used with\nor without echoing like Peff's git_prompt() series does is a very good\nthing. But then we just should introduce \"prompt()\" from day one of the\nseries, instead of introducing a half-featured one \"askpass_prompt()\" and\nthen later renaming the callers like this patch does.\n\nIt may a good idea to take the stepwise approach like this series does,\nbut in that case, the proposed log message must explain what the new\n\"prompt()\" function is and does. It is derived from askpass_prompt() and\napparently it does more than its ancestor, but what are differences\nbetween the two?\n\nFor example, it is totally unclear why these two are equivalent without\nany explanation in the commit log message.\n\n> -\t$choice = Git->askpass_prompt(\"Certificate unknown. \" . $options);\n> -\tif (!defined $choice) {\n> -\t\tprint STDERR $options;\n> -\t\tSTDERR->flush;\n> -\t\t$choice = lc(substr( || 'R', 0, 1));\n> -\t}\n> +\t$choice = substr(Git->prompt(\"Certificate unknown. \" . $options) || 'R', 0, 1);\n\nor this:\n\n> -\tmy $filename = Git->askpass_prompt(\"Client certificate filename:\");\n> -\tif (!defined $filename) {\n> -\t\tprint STDERR \"Client certificate filename: \";\n> -\t\tSTDERR->flush;\n> -\t\tchomp($filename = );\n> -\t}\n> -\t$cred->cert_file($filename);\n> +\t$cred->cert_file(Git->prompt(\"Client certificate filename: \"));\n\nI *suspect* the difference is that you discarded that \"return false at the\nend to let the caller do whatever they want\" found in patch 1/5 and have\nthe fallback inside the prompt() funtion now. And if that is the primary\ndifference between the old \"askpass_prompt()\" and the new \"prompt()\", I\ntend to think that the series should be restructured to use the \"prompt()\"\nsemantics from the beginning. No reason to start with a known-to-be-wrong\nway to do a thing and then fix it in a series that is new to the codebase.\n\nThanks.\n"},{"id":"181734","messageId":"7vty4l4rr8.fsf@alter.siamese.dyndns.org","threadId":"28961","inReplyTo":"7vd3b967ql.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 5/5] make askpass_prompt a global prompt method for asking users","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-12-27T21:41:15Z","receivedAt":"2011-12-27T21:41:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> I *suspect* the difference is that you discarded that \"return false at the\n> end to let the caller do whatever they want\" found in patch 1/5 and have\n> the fallback inside the prompt() funtion now. And if that is the primary\n> difference between the old \"askpass_prompt()\" and the new \"prompt()\", I\n> tend to think that the series should be restructured to use the \"prompt()\"\n> semantics from the beginning. No reason to start with a known-to-be-wrong\n> way to do a thing and then fix it in a series that is new to the codebase.\n\nAfter reading the series again, I think the right structure of this patch\nseries should be more like this:\n\n (1/3) Add Git->prompt($prompt) and make _read_password in git-svn.perl to\n use it. The prompt method should implement the Term::ReadKey based\n fallback, so that _read_password do not have to roll its own. IOW,\n a squash of your 1/5, 2/5, and a part of 5/5, plus possibly 4/5.\n\n (2/3) Enhance Git->prompt($prompt, $is_password), and convert the various\n existing terminal interacters to use it. The fallback in the prompt\n method, when it is not reading in a noecho mode, should read a\n single line from the standard input in cooked mode like your 5/5\n does. IOW, a squash of your 3/5 and a part of 5/5.\n\n (3/3) Possibly tests and docs.\n\nThanks.\n"},{"id":"181737","messageId":"CA+39Oz5J82GVyLfzWbWz20VS=Gp=8q9WsHQY33GuOKT1PyFCbQ@mail.gmail.com","threadId":"28961","inReplyTo":"7vwr9h68t9.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/5] add central method for prompting a user using GIT_ASKPASS or SSH_ASKPASS","fromName":"Thomas Adam","fromEmail":"thomas@xteddy.org","sentAt":"2011-12-27T23:12:04Z","receivedAt":"2011-12-27T23:12:04Z","isPatch":true,"sender":{"key":"thomas@xteddy.org","avatar":"https://gravatar.com/avatar/e7256db4738e501e5d2e84f00bb0bd99503165729573848a03330301fc2adc4a?d=mp&s=160"},"body":"On 27 December 2011 20:47, Junio C Hamano wrote:\n> Sven Strickroth writes:\n>> +sub askpass_prompt {\n>> +     my ($self, $prompt) = _maybe_self(@_);\n>> +     if (exists $ENV{'GIT_ASKPASS'}) {\n>> +             return _askpass_prompt($ENV{'GIT_ASKPASS'}, $prompt);\n>> +     } elsif (exists $ENV{'SSH_ASKPASS'}) {\n>> +             return _askpass_prompt($ENV{'SSH_ASKPASS'}, $prompt);\n>> +     } else {\n>> +             return undef;\n>\n> Two problems with this if/elsif/else cascade.\n>\n>  - If _askpass_prompt() fails to open the pipe to ENV{'GIT_ASKPASS'}, it\n>   will return 'undef' to us. Don't we want to fall back to SSH_ASKPASS in\n>   such a case?\n>\n>  - The last \"return undef\" makes all callers of this method to implement a\n>   fall-back way somehow. I find it very likely that they will want to use\n\nNot only that, \"return undef\" will have nasty side-effects if this\nsubroutine is called in list-context -- it's usually discouraged to\nhave explicit returns of \"undef\", where in scalar context that might\nbe OK, but in list context, the caller will see:\n\n(undef)\n\nand not:\n\n()\n\ni.e., the empty list.\n\n-- Thomas Adam\n"},{"id":"181739","messageId":"7vaa6d4mhg.fsf@alter.siamese.dyndns.org","threadId":"28961","inReplyTo":"CA+39Oz5J82GVyLfzWbWz20VS=Gp=8q9WsHQY33GuOKT1PyFCbQ@mail.gmail.com","subject":"Re: [PATCH 1/5] add central method for prompting a user using GIT_ASKPASS or SSH_ASKPASS","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-12-27T23:35:07Z","receivedAt":"2011-12-27T23:35:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thomas Adam writes:\n\n> On 27 December 2011 20:47, Junio C Hamano wrote:\n>> Sven Strickroth writes:\n>>> +sub askpass_prompt {\n>>> +     my ($self, $prompt) = _maybe_self(@_);\n>>> +     if (exists $ENV{'GIT_ASKPASS'}) {\n>>> +             return _askpass_prompt($ENV{'GIT_ASKPASS'}, $prompt);\n>>> +     } elsif (exists $ENV{'SSH_ASKPASS'}) {\n>>> +             return _askpass_prompt($ENV{'SSH_ASKPASS'}, $prompt);\n>>> +     } else {\n>>> +             return undef;\n>>\n>> Two problems with this if/elsif/else cascade.\n>>\n>>  - If _askpass_prompt() fails to open the pipe to ENV{'GIT_ASKPASS'}, it\n>>   will return 'undef' to us. Don't we want to fall back to SSH_ASKPASS in\n>>   such a case?\n>>\n>>  - The last \"return undef\" makes all callers of this method to implement a\n>>   fall-back way somehow. I find it very likely that they will want to use\n>\n> Not only that, \"return undef\" will have nasty side-effects if this\n> subroutine is called in list-context -- it's usually discouraged to\n> have explicit returns of \"undef\", where in scalar context that might\n> be OK, but in list context, the caller will see:\n>\n> (undef)\n>\n> and not:\n>\n> ()\n>\n> i.e., the empty list.\n\nWell, for this particular function whose interface is \"I'll give you a\nprompt, use it to interact with the user and give me what the user gave us\nin response\", a scalar caller would do\n\n\tmy $response = askpass_prompt(\"What is your password?\");\n\nwhile a list context caller would instead do\n\n\tmy ($response) = askpass_prompt(\"What is your password?\");\n\nor\n\n\tmy @answer = askpass_prompt(\"What is your password?\");\n my $response = $answer[0];\n\nand all three callers would get \"undef\" in $response. I suspect returning\n(undef) is a better thing to do, than relying that\n\n my @answer = ();\n my $response = $answer[0];\n\nhappes to give undef to $response because the access goes beyond the end\nof the array, no?\n"},{"id":"181741","messageId":"4EFA5EB3.4000802@tu-clausthal.de","threadId":"28961","inReplyTo":"7vty4l4rr8.fsf@alter.siamese.dyndns.org","subject":"[PATCH 1/2] git-svn, perl/Git.pm: add central method for prompting passwords honoring GIT_ASKPASS and SSH_ASKPASS","fromName":"Sven Strickroth","fromEmail":"sven.strickroth@tu-clausthal.de","sentAt":"2011-12-28T00:11:31Z","receivedAt":"2011-12-28T00:11:31Z","isPatch":true,"sender":{"key":"sven.strickroth@tu-clausthal.de","avatar":null},"body":"git-svn reads passwords from an interactive terminal or by using\nGIT_ASKPASS helper tool. But if GIT_ASKPASS environment variable is not\nset, git-svn does not try to use SSH_ASKPASS as git-core does. This\ncause GUIs (w/o STDIN connected) to hang waiting forever for git-svn to\ncomplete (http://code.google.com/p/tortoisegit/issues/detail?id=967).\n\nCommit 56a853b62c0ae7ebaad0a7a0a704f5ef561eb795 tried to solve this\nissue, but was incomplete as described above.\n\nInstead of using hand-rolled prompt-response code that only works with\nthe interactive terminal, a reusable prompt() method is introduced in\nthis commit. This change also adds a fallback to SSH_ASKPASS.\n\nSigned-off-by: Sven Strickroth \n---\n git-svn.perl | 20 +-------------------\n perl/Git.pm | 51 ++++++++++++++++++++++++++++++++++++++++++++++++++-\n 2 files changed, 51 insertions(+), 20 deletions(-)\n\ndiff --git a/git-svn.perl b/git-svn.perl\nindex eeb83d3..bcee8aa 100755\n--- a/git-svn.perl\n+++ b/git-svn.perl\n@@ -4415,25 +4415,7 @@ sub username {\n\n sub _read_password {\n \tmy ($prompt, $realm) = @_;\n-\tmy $password = '';\n-\tif (exists $ENV{GIT_ASKPASS}) {\n-\t\topen(PH, \"-|\", $ENV{GIT_ASKPASS}, $prompt);\n-\t\t$password = ;\n-\t\t$password =~ s/[\\012\\015]//; # \\n\\r\n-\t\tclose(PH);\n-\t} else {\n-\t\tprint STDERR $prompt;\n-\t\tSTDERR->flush;\n-\t\trequire Term::ReadKey;\n-\t\tTerm::ReadKey::ReadMode('noecho');\n-\t\twhile (defined(my $key = Term::ReadKey::ReadKey(0))) {\n-\t\t\tlast if $key =~ /[\\012\\015]/; # \\n\\r\n-\t\t\t$password .= $key;\n-\t\t}\n-\t\tTerm::ReadKey::ReadMode('restore');\n-\t\tprint STDERR \"\\n\";\n-\t\tSTDERR->flush;\n-\t}\n+\tmy $password = Git->prompt($prompt);\n \t$password;\n }\n\ndiff --git a/perl/Git.pm b/perl/Git.pm\nindex f7ce511..b1c7c50 100644\n--- a/perl/Git.pm\n+++ b/perl/Git.pm\n@@ -58,7 +58,7 @@ require Exporter;\n command_output_pipe command_input_pipe command_close_pipe\n command_bidi_pipe command_close_bidi_pipe\n version exec_path html_path hash_object git_cmd_try\n- remote_refs\n+ remote_refs prompt\n temp_acquire temp_release temp_reset temp_path);\n\n\n@@ -512,6 +512,55 @@ C). Useful mostly only internally.\n sub html_path { command_oneline('--html-path') }\n\n\n+=item prompt ( PROMPT )\n+\n+Query user C and return answer from user.\n+\n+Check if GIT_ASKPASS or SSH_ASKPASS is set, use first matching for querying\n+user and return answer. If no *_ASKPASS variable is set, the variable is\n+empty or an error occoured, the terminal is tried as a fallback.\n+\n+=cut\n+\n+sub prompt {\n+\tmy ($self, $prompt) = _maybe_self(@_);\n+\tmy $ret;\n+\tif (exists $ENV{'GIT_ASKPASS'}) {\n+\t\t$ret = _prompt($ENV{'GIT_ASKPASS'}, $prompt);\n+\t}\n+\tif (!defined $ret && exists $ENV{'SSH_ASKPASS'}) {\n+\t\t$ret = _prompt($ENV{'SSH_ASKPASS'}, $prompt);\n+\t}\n+\tif (!defined $ret) {\n+\t\tprint STDERR $prompt;\n+\t\tSTDERR->flush;\n+\t\trequire Term::ReadKey;\n+\t\tTerm::ReadKey::ReadMode('noecho');\n+\t\twhile (defined(my $key = Term::ReadKey::ReadKey(0))) {\n+\t\t\tlast if $key =~ /[\\012\\015]/; # \\n\\r\n+\t\t\t$ret .= $key;\n+\t\t}\n+\t\tTerm::ReadKey::ReadMode('restore');\n+\t\tprint STDERR \"\\n\";\n+\t\tSTDERR->flush;\n+\t}\n+\treturn $ret;\n+}\n+\n+sub _prompt {\n+\tmy ($askpass, $prompt) = @_;\n+\tunless ($askpass) {\n+\t\treturn undef;\n+\t}\n+\tmy $ret;\n+\topen my $fh, \"-|\", $askpass, $prompt || return undef;\n+\t$ret = <$fh>;\n+\t$ret =~ s/[\\012\\015]//g; # strip \\n\\r, chomp does not work on all systems (i.e. windows) as expected\n+\tclose ($fh);\n+\treturn $ret;\n+}\n+\n+\n =item repo_path ()\n\n Return path to the git repository. Must be called on a repository instance.\n-- \nBest regards,\n Sven Strickroth\n ClamAV, a GPL anti-virus toolkit http://www.clamav.net\n PGP key id F5A9D4C4 @ any key-server\n"},{"id":"181742","messageId":"4EFA5F08.2060705@tu-clausthal.de","threadId":"28961","inReplyTo":"7vty4l4rr8.fsf@alter.siamese.dyndns.org","subject":"[PATCH 2/2] git-svn, perl/Git.pm: extend and use Git->prompt method for querying users","fromName":"Sven Strickroth","fromEmail":"sven.strickroth@tu-clausthal.de","sentAt":"2011-12-28T00:12:56Z","receivedAt":"2011-12-28T00:12:56Z","isPatch":true,"sender":{"key":"sven.strickroth@tu-clausthal.de","avatar":null},"body":"git-svn reads usernames and other user queries from an interactive\nterminal. This cause GUIs (w/o STDIN connected) to hang waiting forever\nfor git-svn to complete (http://code.google.com/p/tortoisegit/issues/detail?id=967).\n\nThis change extends the Git->prompt method, so that it can also be used\nfor non password queries, and makes use of it instead of using\nhand-rolled prompt-response code that only works with the interactive\nterminal.\n\nSigned-off-by: Sven Strickroth \n---\n git-svn.perl | 16 +++++-----------\n perl/Git.pm | 25 +++++++++++++++----------\n 2 files changed, 20 insertions(+), 21 deletions(-)\n\ndiff --git a/git-svn.perl b/git-svn.perl\nindex bcee8aa..1f30dc2 100755\n--- a/git-svn.perl\n+++ b/git-svn.perl\n@@ -4357,11 +4357,10 @@ sub ssl_server_trust {\n \t issuer_dname fingerprint);\n \tmy $choice;\n prompt:\n-\tprint STDERR $may_save ?\n+\tmy $options = $may_save ?\n \t \"(R)eject, accept (t)emporarily or accept (p)ermanently? \" :\n \t \"(R)eject or accept (t)emporarily? \";\n-\tSTDERR->flush;\n-\t$choice = lc(substr( || 'R', 0, 1));\n+\t$choice = substr(Git->prompt(\"Certificate unknown. \" . $options) || 'R', 0, 1);\n \tif ($choice =~ /^t$/i) {\n \t\t$cred->may_save(undef);\n \t} elsif ($choice =~ /^r$/i) {\n@@ -4378,10 +4377,7 @@ prompt:\n sub ssl_client_cert {\n \tmy ($cred, $realm, $may_save, $pool) = @_;\n \t$may_save = undef if $_no_auth_cache;\n-\tprint STDERR \"Client certificate filename: \";\n-\tSTDERR->flush;\n-\tchomp(my $filename = );\n-\t$cred->cert_file($filename);\n+\t$cred->cert_file(Git->prompt(\"Client certificate filename: \"));\n \t$cred->may_save($may_save);\n \t$SVN::_Core::SVN_NO_ERROR;\n }\n@@ -4404,9 +4400,7 @@ sub username {\n \tif (defined $_username) {\n \t\t$username = $_username;\n \t} else {\n-\t\tprint STDERR \"Username: \";\n-\t\tSTDERR->flush;\n-\t\tchomp($username = );\n+\t\t$username = Git->prompt(\"Username: \");\n \t}\n \t$cred->username($username);\n \t$cred->may_save($may_save);\n@@ -4415,7 +4409,7 @@ sub username {\n\n sub _read_password {\n \tmy ($prompt, $realm) = @_;\n-\tmy $password = Git->prompt($prompt);\n+\tmy $password = Git->prompt($prompt, 1);\n \t$password;\n }\n\ndiff --git a/perl/Git.pm b/perl/Git.pm\nindex b1c7c50..62b824c 100644\n--- a/perl/Git.pm\n+++ b/perl/Git.pm\n@@ -512,18 +512,19 @@ C). Useful mostly only internally.\n sub html_path { command_oneline('--html-path') }\n\n\n-=item prompt ( PROMPT )\n+=item prompt ( PROMPT , ISPASSWORD )\n\n Query user C and return answer from user.\n\n Check if GIT_ASKPASS or SSH_ASKPASS is set, use first matching for querying\n user and return answer. If no *_ASKPASS variable is set, the variable is\n empty or an error occoured, the terminal is tried as a fallback.\n+If C is set and true, the terminal disables echo.\n\n =cut\n\n sub prompt {\n-\tmy ($self, $prompt) = _maybe_self(@_);\n+\tmy ($self, $prompt, $isPassword) = _maybe_self(@_);\n \tmy $ret;\n \tif (exists $ENV{'GIT_ASKPASS'}) {\n \t\t$ret = _prompt($ENV{'GIT_ASKPASS'}, $prompt);\n@@ -534,15 +535,19 @@ sub prompt {\n \tif (!defined $ret) {\n \t\tprint STDERR $prompt;\n \t\tSTDERR->flush;\n-\t\trequire Term::ReadKey;\n-\t\tTerm::ReadKey::ReadMode('noecho');\n-\t\twhile (defined(my $key = Term::ReadKey::ReadKey(0))) {\n-\t\t\tlast if $key =~ /[\\012\\015]/; # \\n\\r\n-\t\t\t$ret .= $key;\n+\t\tif (defined $isPassword && $isPassword) {\n+\t\t\trequire Term::ReadKey;\n+\t\t\tTerm::ReadKey::ReadMode('noecho');\n+\t\t\twhile (defined(my $key = Term::ReadKey::ReadKey(0))) {\n+\t\t\t\tlast if $key =~ /[\\012\\015]/; # \\n\\r\n+\t\t\t\t$ret .= $key;\n+\t\t\t}\n+\t\t\tTerm::ReadKey::ReadMode('restore');\n+\t\t\tprint STDERR \"\\n\";\n+\t\t\tSTDERR->flush;\n+\t\t} else {\n+\t\t\tchomp($ret = );\n \t\t}\n-\t\tTerm::ReadKey::ReadMode('restore');\n-\t\tprint STDERR \"\\n\";\n-\t\tSTDERR->flush;\n \t}\n \treturn $ret;\n }\n-- \nBest regards,\n Sven Strickroth\n ClamAV, a GPL anti-virus toolkit http://www.clamav.net\n PGP key id F5A9D4C4 @ any key-server\n"},{"id":"181744","messageId":"7vboqt2zm4.fsf@alter.siamese.dyndns.org","threadId":"28961","inReplyTo":"4EFA5EB3.4000802@tu-clausthal.de","subject":"Re: [PATCH 1/2] git-svn, perl/Git.pm: add central method for prompting passwords honoring GIT_ASKPASS and SSH_ASKPASS","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-12-28T02:34:27Z","receivedAt":"2011-12-28T02:34:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sven Strickroth writes:\n\n> git-svn reads passwords from an interactive terminal or by using\n> GIT_ASKPASS helper tool. But if GIT_ASKPASS environment variable is not\n> set, git-svn does not try to use SSH_ASKPASS as git-core does. This\n> cause GUIs (w/o STDIN connected) to hang waiting forever for git-svn to\n> complete (http://code.google.com/p/tortoisegit/issues/detail?id=967).\n>\n> Commit 56a853b62c0ae7ebaad0a7a0a704f5ef561eb795 tried to solve this\n> issue, but was incomplete as described above.\n>\n> Instead of using hand-rolled prompt-response code that only works with\n> the interactive terminal, a reusable prompt() method is introduced in\n> this commit. This change also adds a fallback to SSH_ASKPASS.\n>\n> Signed-off-by: Sven Strickroth \n> ---\n\nThanks. Vastly more readable ;-)\n\nI only have a few minor nits, and request for extra set of eyeballs from\nPerl-y people.\n\n> sub _read_password {\n> \tmy ($prompt, $realm) = @_;\n> -\tmy $password = '';\n> -\tif (exists $ENV{GIT_ASKPASS}) {\n> -\t\topen(PH, \"-|\", $ENV{GIT_ASKPASS}, $prompt);\n> -\t\t$password = ;\n> -\t\t$password =~ s/[\\012\\015]//; # \\n\\r\n> - ...\n> -\t\twhile (defined(my $key = Term::ReadKey::ReadKey(0))) {\n> -\t\t\tlast if $key =~ /[\\012\\015]/; # \\n\\r\n> -\t\t\t$password .= $key;\n> -\t\t}\n> - ...\n> +\tmy $password = Git->prompt($prompt);\n> \t$password;\n> }\n> ...\n> +Check if GIT_ASKPASS or SSH_ASKPASS is set, use first matching for querying\n> +user and return answer. If no *_ASKPASS variable is set, the variable is\n> +empty or an error occoured, the terminal is tried as a fallback.\n\nLooks like a description that is correct, but I feel a slight hiccup when\ntrying to read the first sentence aloud. Perhaps other reviewers on the\nlist can offer an easier to read alternative?\n\n> +sub prompt {\n> +\tmy ($self, $prompt) = _maybe_self(@_);\n> +\tmy $ret;\n> +\tif (exists $ENV{'GIT_ASKPASS'}) {\n> +\t\t$ret = _prompt($ENV{'GIT_ASKPASS'}, $prompt);\n> +\t}\n> +\tif (!defined $ret && exists $ENV{'SSH_ASKPASS'}) {\n> +\t\t$ret = _prompt($ENV{'SSH_ASKPASS'}, $prompt);\n> +\t}\n> +\tif (!defined $ret) {\n> +\t\tprint STDERR $prompt;\n> +\t\tSTDERR->flush;\n> +\t\trequire Term::ReadKey;\n> +\t\tTerm::ReadKey::ReadMode('noecho');\n> +\t\twhile (defined(my $key = Term::ReadKey::ReadKey(0))) {\n> +\t\t\tlast if $key =~ /[\\012\\015]/; # \\n\\r\n> +\t\t\t$ret .= $key;\n\nUnlike the original in _read_password, $ret ($password over there) is left\n\"undef\" here; I am wondering if \"$ret .= $key\" might trigger a warning and\nif that is the case, probably we should have an explicit \"$ret = '';\"\nbefore going into the while loop.\n\n> +sub _prompt {\n> +\tmy ($askpass, $prompt) = @_;\n> +\tunless ($askpass) {\n> +\t\treturn undef;\n> +\t}\n\nPerl gurus on the list might prefer to rewrite this with statement\nmodifier as \"return undef unless (...);\" but I am not one of them.\n\n> +\tmy $ret;\n> +\topen my $fh, \"-|\", $askpass, $prompt || return undef;\n\nI am so used see this spelled with the lower-precedence \"or\" like this\n\n\topen my $fh, \"-|\", $askpass, $prompt\n \tor return undef;\n\nthat I am no longer sure if the use of \"||\" is Ok here. Help from Perl\ngurus on the list?\n\n> +\t$ret = <$fh>;\n> +\t$ret =~ s/[\\012\\015]//g; # strip \\n\\r, chomp does not work on all systems (i.e. windows) as expected\n\nThe original reads one line from the helper process, removes the first \\n\nor \\r (expecting there is only one), and returns the result. The new code\nreads one line, removes all \\n and \\r everywhere, and returns the result.\n\nI do not think it makes any difference in practice, but shouldn't this\nlogically be more like \"s/\\r?\\n$//\", that is \"remove the CRLF or LF at the\nend\"?\n\n> +\tclose ($fh);\n\nIt seems that we aquired a SP after \"close\" compared to the\noriginal. What's the prevailing coding style in our Perl code?\n\nThis close() of pipe to the subprocess is where a lot of error checking\nhappens, no? Can this return an error?\n\nI can see the original ignored an error condition, but do we care, or not\ncare?\n"},{"id":"181745","messageId":"7vpqf91kqo.fsf@alter.siamese.dyndns.org","threadId":"28961","inReplyTo":"4EFA5F08.2060705@tu-clausthal.de","subject":"Re: [PATCH 2/2] git-svn, perl/Git.pm: extend and use Git->prompt method for querying users","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-12-28T02:41:03Z","receivedAt":"2011-12-28T02:41:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sven Strickroth writes:\n\n> @@ -4357,11 +4357,10 @@ sub ssl_server_trust {\n> \t issuer_dname fingerprint);\n> \tmy $choice;\n> prompt:\n> -\tprint STDERR $may_save ?\n> +\tmy $options = $may_save ?\n> \t \"(R)eject, accept (t)emporarily or accept (p)ermanently? \" :\n> \t \"(R)eject or accept (t)emporarily? \";\n> -\tSTDERR->flush;\n> -\t$choice = lc(substr( || 'R', 0, 1));\n> +\t$choice = substr(Git->prompt(\"Certificate unknown. \" . $options) || 'R', 0, 1);\n\nI am afraid the extra \"Certificate unknown. \" prefix may make the prompt\nway too long to fit on a line on the terminal or in the GUI. Would it be\nOk to perhaps add LF to make it a multi-line prompt? Do GUI based helpers\nmake that into a dialog box with multi-line prompt, or do they just barf?\n\n> \tif ($choice =~ /^t$/i) {\n> \t\t$cred->may_save(undef);\n\nWe seem to have lost lc() there, but it probably is deliberate and\nharmless, as the value is checked with /^x$/i later.\n\nAs we are making sure $choice has a single character anyway, I think that\nchecking with \"=~ /^x$/i\" is unnecessarily ugly and wrong, even though it\nis obviously not the fault of this patch.\n\n> @@ -4378,10 +4377,7 @@ prompt:\n> sub ssl_client_cert {\n> \tmy ($cred, $realm, $may_save, $pool) = @_;\n> \t$may_save = undef if $_no_auth_cache;\n> -\tprint STDERR \"Client certificate filename: \";\n> -\tSTDERR->flush;\n> -\tchomp(my $filename = );\n> -\t$cred->cert_file($filename);\n> +\t$cred->cert_file(Git->prompt(\"Client certificate filename: \"));\n> \t$cred->may_save($may_save);\n> \t$SVN::_Core::SVN_NO_ERROR;\n> }\n\nWe may later add an option to \"prompt\" method to allow the caller to say\nthat we are asking for a filename, and let GUI prompt helper to run a file\npicker, but I think that is outside the immediate scope of this patch.\nJust a thing for the future to keep in mind.\n\nThis patch itself looks almost perfect to me (modulo the above minor\nnits), and except that it textually depends on 1/2 that may need to be\nupdated.\n\nThanks.\n"},{"id":"181750","messageId":"4EFAF241.9050806@tu-clausthal.de","threadId":"28961","inReplyTo":"7vpqf91kqo.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/2] git-svn, perl/Git.pm: extend and use Git->prompt method for querying users","fromName":"Sven Strickroth","fromEmail":"sven.strickroth@tu-clausthal.de","sentAt":"2011-12-28T10:41:05Z","receivedAt":"2011-12-28T10:41:05Z","isPatch":true,"sender":{"key":"sven.strickroth@tu-clausthal.de","avatar":null},"body":"Am 28.12.2011 03:41 schrieb Junio C Hamano:\n> I am afraid the extra \"Certificate unknown. \" prefix may make the prompt\n> way too long to fit on a line on the terminal or in the GUI. Would it be\n> Ok to perhaps add LF to make it a multi-line prompt? Do GUI based helpers\n> make that into a dialog box with multi-line prompt, or do they just barf?\n\nLF is problematic. But we could do $prompt =~ s/\\n/ /g; in _prompt()-method.\n\n-- \nBest regards,\n Sven Strickroth\n ClamAV, a GPL anti-virus toolkit http://www.clamav.net\n PGP key id F5A9D4C4 @ any key-server\n"},{"id":"181753","messageId":"4EFB4108.5040704@tu-clausthal.de","threadId":"28961","inReplyTo":"7vboqt2zm4.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/2] git-svn, perl/Git.pm: add central method for prompting passwords honoring GIT_ASKPASS and SSH_ASKPASS","fromName":"Sven Strickroth","fromEmail":"sven.strickroth@tu-clausthal.de","sentAt":"2011-12-28T16:17:12Z","receivedAt":"2011-12-28T16:17:12Z","isPatch":true,"sender":{"key":"sven.strickroth@tu-clausthal.de","avatar":null},"body":"Am 28.12.2011 03:34 schrieb Junio C Hamano:\n>> +\tclose ($fh);\n> \n> It seems that we aquired a SP after \"close\" compared to the\n> original. What's the prevailing coding style in our Perl code?\n> \n> This close() of pipe to the subprocess is where a lot of error checking\n> happens, no? Can this return an error?\n> \n> I can see the original ignored an error condition, but do we care, or not\n> care?\n\nclose() can return a number in case of an error, but we already got our\nresponse/line, so why care?\n\n-- \nBest regards,\n Sven Strickroth\n ClamAV, a GPL anti-virus toolkit http://www.clamav.net\n PGP key id F5A9D4C4 @ any key-server\n"},{"id":"181755","messageId":"201112281956.30289.jnareb@gmail.com","threadId":"28961","inReplyTo":"7vboqt2zm4.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/2] git-svn, perl/Git.pm: add central method for prompting passwords honoring GIT_ASKPASS and SSH_ASKPASS","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2011-12-28T18:56:29Z","receivedAt":"2011-12-28T18:56:29Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Junio C Hamano wrote:\n> Sven Strickroth writes:\n\n> I only have a few minor nits, and request for extra set of eyeballs from\n> Perl-y people.\n> \n> > sub _read_password {\n> > \tmy ($prompt, $realm) = @_;\n> > -\tmy $password = '';\n> > -\tif (exists $ENV{GIT_ASKPASS}) {\n> > -\t\topen(PH, \"-|\", $ENV{GIT_ASKPASS}, $prompt);\n> > -\t\t$password = ;\n> > -\t\t$password =~ s/[\\012\\015]//; # \\n\\r\n> > - ...\n> > -\t\twhile (defined(my $key = Term::ReadKey::ReadKey(0))) {\n> > -\t\t\tlast if $key =~ /[\\012\\015]/; # \\n\\r\n> > -\t\t\t$password .= $key;\n> > -\t\t}\n> > - ...\n> > +\tmy $password = Git->prompt($prompt);\n> > \t$password;\n> > }\n> > ...\n> > +Check if GIT_ASKPASS or SSH_ASKPASS is set, use first matching for querying\n> > +user and return answer. If no *_ASKPASS variable is set, the variable is\n> > +empty or an error occoured, the terminal is tried as a fallback.\n> \n> Looks like a description that is correct, but I feel a slight hiccup when\n> trying to read the first sentence aloud. Perhaps other reviewers on the\n> list can offer an easier to read alternative?\n\nPerhaps\n\n Query user for password with given PROMPT and return answer. It respects\n GIT_ASKPASS and SSH_ASKPASS environment variables, with terminal in a\n password mode (no echo) as a fallback. Returns undef if it cannot ask\n for password. \n\n> > +sub prompt {\n> > +\tmy ($self, $prompt) = _maybe_self(@_);\n> > +\tmy $ret;\n> > +\tif (exists $ENV{'GIT_ASKPASS'}) {\n\nWouldn't it be simpler and more resilent to just check for $ENV{'GIT_ASKPASS'}?\nAssuming that nobody uses command named '0' it would cover both GIT_ASKPASS\nnot being set (!exists) and being set to empty value (eq '').\n\n> > +\t\t$ret = _prompt($ENV{'GIT_ASKPASS'}, $prompt);\n> > +\t}\n> > +\tif (!defined $ret && exists $ENV{'SSH_ASKPASS'}) {\n> > +\t\t$ret = _prompt($ENV{'SSH_ASKPASS'}, $prompt);\n> > +\t}\n> > +\tif (!defined $ret) {\n> > +\t\tprint STDERR $prompt;\n> > +\t\tSTDERR->flush;\n> > +\t\trequire Term::ReadKey;\n> > +\t\tTerm::ReadKey::ReadMode('noecho');\n> > +\t\twhile (defined(my $key = Term::ReadKey::ReadKey(0))) {\n> > +\t\t\tlast if $key =~ /[\\012\\015]/; # \\n\\r\n> > +\t\t\t$ret .= $key;\n\nI wonder if the last part wouldn't be better to be refactored into\na separate subroutine, e.g. _prompt_readkey.\n\n> \n> Unlike the original in _read_password, $ret ($password over there) is left\n> \"undef\" here; I am wondering if \"$ret .= $key\" might trigger a warning and\n> if that is the case, probably we should have an explicit \"$ret = '';\"\n> before going into the while loop.\n\nNo that is not a problem. In Perl undefined variable functions as 0 in\nnumeric context ($foo++), as '' in string context ($foo .= $key), and []\nin arrayref context (push @$foo, $key).\n\n> > +sub _prompt {\n> > +\tmy ($askpass, $prompt) = @_;\n> > +\tunless ($askpass) {\n> > +\t\treturn undef;\n> > +\t}\n> \n> Perl gurus on the list might prefer to rewrite this with statement\n> modifier as \"return undef unless (...);\" but I am not one of them.\n> \n> > +\tmy $ret;\n> > +\topen my $fh, \"-|\", $askpass, $prompt || return undef;\n> \n> I am so used see this spelled with the lower-precedence \"or\" like this\n> \n> \topen my $fh, \"-|\", $askpass, $prompt\n> \tor return undef;\n> \n> that I am no longer sure if the use of \"||\" is Ok here. Help from Perl\n> gurus on the list?\n\nIt is incorrect, which you can check with B::Deparse.\n\n$ perl -MO=Deparse,-p -e 'open my $fh, \"-|\", $askpass, $prompt || return undef;'\n\n open(my $fh, '-|', $askpass, ($prompt || return(undef)));\n \nAnyway, wouldn't it be simpler and better to use command_oneline or its\nbackend here?\n\n> > +\t$ret = <$fh>;\n> > +\t$ret =~ s/[\\012\\015]//g; # strip \\n\\r, chomp does not work on all systems (i.e. windows) as expected\n> \n> The original reads one line from the helper process, removes the first \\n\n> or \\r (expecting there is only one), and returns the result. The new code\n> reads one line, removes all \\n and \\r everywhere, and returns the result.\n> \n> I do not think it makes any difference in practice, but shouldn't this\n> logically be more like \"s/\\r?\\n$//\", that is \"remove the CRLF or LF at the\n> end\"?\n> \n> > +\tclose ($fh);\n> \n> It seems that we aquired a SP after \"close\" compared to the\n> original. What's the prevailing coding style in our Perl code?\n> \n> This close() of pipe to the subprocess is where a lot of error checking\n> happens, no? Can this return an error?\n> \n> I can see the original ignored an error condition, but do we care, or not\n> care?\n \nIf we use command_oneline or its backend we wouldn't have to worry\nabout this.\n\n-- \nJakub Narebski\nPoland\n"},{"id":"181760","messageId":"7v39c41keo.fsf@alter.siamese.dyndns.org","threadId":"28961","inReplyTo":"4EFAF241.9050806@tu-clausthal.de","subject":"Re: [PATCH 2/2] git-svn, perl/Git.pm: extend and use Git->prompt method for querying users","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-12-28T21:00:31Z","receivedAt":"2011-12-28T21:00:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sven Strickroth writes:\n\n> Am 28.12.2011 03:41 schrieb Junio C Hamano:\n>> I am afraid the extra \"Certificate unknown. \" prefix may make the prompt\n>> way too long to fit on a line on the terminal or in the GUI. Would it be\n>> Ok to perhaps add LF to make it a multi-line prompt? Do GUI based helpers\n>> make that into a dialog box with multi-line prompt, or do they just barf?\n>\n> LF is problematic. But we could do $prompt =~ s/\\n/ /g; in _prompt()-method.\n\nI actually was hoping that the answer is \"it depends on the helper\nspecified by *_ASKPASS\".\n\nIn any case, let's not add that extra \"Certificate unknown. \" prefix at\nall to avoid regressions and queue this patch series for real.\n\nAfter somebody comes up with a way to deal with overlong prompt, building\non top of this series, we can work on making this particular prompt longer\nand more descriptive.\n"},{"id":"181762","messageId":"7vpqf8z8a6.fsf@alter.siamese.dyndns.org","threadId":"28961","inReplyTo":"7v39c41keo.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/2] git-svn, perl/Git.pm: extend and use Git->prompt method for querying users","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-12-28T21:38:25Z","receivedAt":"2011-12-28T21:38:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> I actually was hoping that the answer is \"it depends on the helper\n> specified by *_ASKPASS\".\n>\n> In any case, let's not add that extra \"Certificate unknown. \" prefix at\n> all to avoid regressions and queue this patch series for real.\n>\n> After somebody comes up with a way to deal with overlong prompt, building\n> on top of this series, we can work on making this particular prompt longer\n> and more descriptive.\n\nI've queued the two patches with minor tweaks.\n\nI think the first patch is a definite improvement for both GUI users and\nterminal users who use the *_ASKPASS environment variable. Other parts of\ngit already asks the latter their password using *_ASKPASS anyway, so I do\nnot foresee complaints from them saying that git-svn suddenly stopped\nreading the password from the terminal.\n\nI am however not sure if the second patch in this series is a good thing\nin the current shape. For GUI users who do not have a terminal, earlier\nthey couldn't respond to these questions but now they can, so in that\nnarrow sense we are not going backwards.\n\nBut for people who use *_ASKPASS and are working from the terminal, it is\na regression to ask these non-password questions using *_ASKPASS. Most\nlikely, these helpers that are designed for password entry will hide what\nis typed, and I also wouldn't be surprised if some of them have fairly low\ninput-length restriction that may be shorter than a long-ish pathname that\nusers might want to give as an answer, which they could do in the terminal\nbased interaction but will become impossible with this patch.\n\nI suspect that we would need to enhance *_ASKPASS interface first, so that\nwe can ask things other than passwords. Until that happens, I do not think\nwe should apply the second patch to use *_ASKPASS for non-passwords.\n"},{"id":"181763","messageId":"4EFB8E78.4090205@tu-clausthal.de","threadId":"28961","inReplyTo":"7vpqf8z8a6.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/2] git-svn, perl/Git.pm: extend and use Git->prompt method for querying users","fromName":"Sven Strickroth","fromEmail":"sven.strickroth@tu-clausthal.de","sentAt":"2011-12-28T21:47:36Z","receivedAt":"2011-12-28T21:47:36Z","isPatch":true,"sender":{"key":"sven.strickroth@tu-clausthal.de","avatar":null},"body":"Am 28.12.2011 22:38 schrieb Junio C Hamano:\n> I am however not sure if the second patch in this series is a good thing\n> in the current shape. For GUI users who do not have a terminal, earlier\n> they couldn't respond to these questions but now they can, so in that\n> narrow sense we are not going backwards.\n> \n> But for people who use *_ASKPASS and are working from the terminal, it is\n> a regression to ask these non-password questions using *_ASKPASS. Most\n> likely, these helpers that are designed for password entry will hide what\n> is typed, and I also wouldn't be surprised if some of them have fairly low\n> input-length restriction that may be shorter than a long-ish pathname that\n> users might want to give as an answer, which they could do in the terminal\n> based interaction but will become impossible with this patch.\n> \n> I suspect that we would need to enhance *_ASKPASS interface first, so that\n> we can ask things other than passwords. Until that happens, I do not think\n> we should apply the second patch to use *_ASKPASS for non-passwords.\n\ngit-core also asks for username using *_ASKPASS, this is the reason why\nI implemented it this way. I noticed it when I tried to push to google\ncode (using https).\n\n-- \nBest regards,\n Sven Strickroth\n ClamAV, a GPL anti-virus toolkit http://www.clamav.net\n PGP key id F5A9D4C4 @ any key-server\n"},{"id":"181766","messageId":"7vlipwz5xs.fsf@alter.siamese.dyndns.org","threadId":"28961","inReplyTo":"4EFB8E78.4090205@tu-clausthal.de","subject":"Re: [PATCH 2/2] git-svn, perl/Git.pm: extend and use Git->prompt method for querying users","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-12-28T22:29:03Z","receivedAt":"2011-12-28T22:29:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sven Strickroth writes:\n\n>> I suspect that we would need to enhance *_ASKPASS interface first, so that\n>> we can ask things other than passwords. Until that happens, I do not think\n>> we should apply the second patch to use *_ASKPASS for non-passwords.\n>\n> git-core also asks for username using *_ASKPASS, this is the reason why\n> I implemented it this way. I noticed it when I tried to push to google\n> code (using https).\n\nI thought that was updated with Peff's series recently?\n\nIn any case, your username has a lot minor annoyance factor if we force\nyou to type in blind, but the second patch in your series ask things other\nthan that using the same mechanism, so it is not a good excuse for this\nusability regression in git-svn, I would think.\n"},{"id":"181797","messageId":"4EFD40CF.8000801@tu-clausthal.de","threadId":"28961","inReplyTo":"7vlipwz5xs.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/2] git-svn, perl/Git.pm: extend and use Git->prompt method for querying users","fromName":"Sven Strickroth","fromEmail":"sven.strickroth@tu-clausthal.de","sentAt":"2011-12-30T04:40:47Z","receivedAt":"2011-12-30T04:40:47Z","isPatch":true,"sender":{"key":"sven.strickroth@tu-clausthal.de","avatar":null},"body":"Am 28.12.2011 23:29 schrieb Junio C Hamano:\n>>> I suspect that we would need to enhance *_ASKPASS interface first, so that\n>>> we can ask things other than passwords. Until that happens, I do not think\n>>> we should apply the second patch to use *_ASKPASS for non-passwords.\n>>\n>> git-core also asks for username using *_ASKPASS, this is the reason why\n>> I implemented it this way. I noticed it when I tried to push to google\n>> code (using https).\n> \n> I thought that was updated with Peff's series recently?\n\nSo, was this changed? git-core doesn't ask for a username using\n*_ASKPASS helpers anymore?\n\n> In any case, your username has a lot minor annoyance factor if we force\n> you to type in blind, but the second patch in your series ask things other\n> than that using the same mechanism, so it is not a good excuse for this\n> usability regression in git-svn, I would think.\n\nYeah, typing a whole path is more annoying. But not being able to clone\n(push, ...) w/o being able to type in a username or accept an unknown\ncertificate is also problematic.\n\nI talked off-list with Junio and he proposed to use another environment\nvariable (e.g. GIT_DIALOG for a different tool) to solve these issues.\n\nA good way could be to define the GIT_DIALOG-tools to have two\nparameters. First (pass|text|filename|...) with fallback to text, this\nway one can implement a password field, a text field, a file chooser (on\ntype filename) and it is still extendable for e.g. directory choosers\n(if we might need that)...\n\nTo make it even more complicated one could also say that we propose more\nparameters (e.g. two additional parameters for working copy directory\nand repository), so that answers can be stored by the dialog-helper in\nthe .git-directory.\n\n-- \nBest regards,\n Sven Strickroth\n ClamAV, a GPL anti-virus toolkit http://www.clamav.net\n PGP key id F5A9D4C4 @ any key-server\n"},{"id":"181804","messageId":"20111230135423.GA1684@sigill.intra.peff.net","threadId":"28961","inReplyTo":"4EFD40CF.8000801@tu-clausthal.de","subject":"Re: [PATCH 2/2] git-svn, perl/Git.pm: extend and use Git->prompt method for querying users","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-12-30T13:54:23Z","receivedAt":"2011-12-30T13:54:23Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Dec 30, 2011 at 05:40:47AM +0100, Sven Strickroth wrote:\n\n> >> git-core also asks for username using *_ASKPASS, this is the reason why\n> >> I implemented it this way. I noticed it when I tried to push to google\n> >> code (using https).\n> > \n> > I thought that was updated with Peff's series recently?\n> \n> So, was this changed? git-core doesn't ask for a username using\n> *_ASKPASS helpers anymore?\n\nNo, it will. It's only that we will echo characters when using the\nterminal prompt. In theory we could have an ASKPASS-style interface that\nwould would echo characters, but there's no such interface in common\nuse (i.e., we would have to invent it).\n\n> I talked off-list with Junio and he proposed to use another environment\n> variable (e.g. GIT_DIALOG for a different tool) to solve these issues.\n> \n> A good way could be to define the GIT_DIALOG-tools to have two\n> parameters. First (pass|text|filename|...) with fallback to text, this\n> way one can implement a password field, a text field, a file chooser (on\n> type filename) and it is still extendable for e.g. directory choosers\n> (if we might need that)...\n\nYes, like that. I think some windowing toolkits already have programs to\nprovide dialogs from shell scripts. You might look at them for\ninspiration on the interface.\n\nFor credentials, it would be nice to be able to create a multi-field\ndialog, like:\n\n Username: \n Password: \n Remember password? [checkbox]\n\nI was planning to do something custom for credentials as an extension to\nthe credential helper protocol, but this could also fall under the\nheading of a general prompt helper.\n\n-Peff\n"},{"id":"181806","messageId":"4EFDD06A.3010708@tu-clausthal.de","threadId":"28961","inReplyTo":"20111230135423.GA1684@sigill.intra.peff.net","subject":"Re: [PATCH 2/2] git-svn, perl/Git.pm: extend and use Git->prompt method for querying users","fromName":"Sven Strickroth","fromEmail":"sven.strickroth@tu-clausthal.de","sentAt":"2011-12-30T14:53:30Z","receivedAt":"2011-12-30T14:53:30Z","isPatch":true,"sender":{"key":"sven.strickroth@tu-clausthal.de","avatar":null},"body":"Am 30.12.2011 14:54 schrieb Jeff King:\n>>> I thought that was updated with Peff's series recently?\n>>\n>> So, was this changed? git-core doesn't ask for a username using\n>> *_ASKPASS helpers anymore?\n> \n> No, it will. It's only that we will echo characters when using the\n> terminal prompt. In theory we could have an ASKPASS-style interface that\n> would would echo characters, but there's no such interface in common\n> use (i.e., we would have to invent it).\n\nI also updated the _ASKPASS helper of TortoiseGit so that it only shows\nasterisks if \"pass\" is not contained in the prompt.\n\n> For credentials, it would be nice to be able to create a multi-field\n> dialog, like:\n> \n> Username: \n> Password: \n> Remember password? [checkbox]\n> \n> I was planning to do something custom for credentials as an extension to\n> the credential helper protocol, but this could also fall under the\n> heading of a general prompt helper.\n\nThis might be problematic, because (for git-svn) username and password\nare not requested together.\n\n-- \nBest regards,\n Sven Strickroth\n ClamAV, a GPL anti-virus toolkit http://www.clamav.net\n PGP key id F5A9D4C4 @ any key-server\n"},{"id":"181821","messageId":"7v8vlrwzw9.fsf@alter.siamese.dyndns.org","threadId":"28961","inReplyTo":"4EFDD06A.3010708@tu-clausthal.de","subject":"Re: [PATCH 2/2] git-svn, perl/Git.pm: extend and use Git->prompt method for querying users","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-01-01T09:11:34Z","receivedAt":"2012-01-01T09:11:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sven Strickroth writes:\n\n> Am 30.12.2011 14:54 schrieb Jeff King:\n> ...\n>> Username: \n>> Password: \n>> Remember password? [checkbox]\n>> \n>> I was planning to do something custom for credentials as an extension to\n>> the credential helper protocol, but this could also fall under the\n>> heading of a general prompt helper.\n>\n> This might be problematic, because (for git-svn) username and password\n> are not requested together.\n\nI do not think Peff means the dialog must ask these three items at the\nsame time. The point is that other codepaths know they need to ask them\nand would benefit if they can instruct the dialog external helper to ask\nthem in a single interaction. So if your callsite does not ask them\ntogether, it is OK. You can keep asking them separately in two dialog\ninteractions.\n"},{"id":"181829","messageId":"4F00B7F3.1060105@tu-clausthal.de","threadId":"28961","inReplyTo":"7vpqf8z8a6.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/2] git-svn, perl/Git.pm: extend and use Git->prompt method for querying users","fromName":"Sven Strickroth","fromEmail":"sven.strickroth@tu-clausthal.de","sentAt":"2012-01-01T19:45:55Z","receivedAt":"2012-01-01T19:45:55Z","isPatch":true,"sender":{"key":"sven.strickroth@tu-clausthal.de","avatar":null},"body":"Am 28.12.2011 22:38 schrieb Junio C Hamano:\n> I am however not sure if the second patch in this series is a good thing\n> in the current shape. For GUI users who do not have a terminal, earlier\n> they couldn't respond to these questions but now they can, so in that\n> narrow sense we are not going backwards.\n\n> But for people who use *_ASKPASS and are working from the terminal, it is\n> a regression to ask these non-password questions using *_ASKPASS. Most\n> likely, these helpers that are designed for password entry will hide what\n> is typed, and I also wouldn't be surprised if some of them have fairly low\n> input-length restriction that may be shorter than a long-ish pathname that\n> users might want to give as an answer, which they could do in the terminal\n> based interaction but will become impossible with this patch.\n\nI'm still for the second patch to be applied (maybe w/o the certificate\nfilename prompt), too, because this makes git-svn behave the save way as\ngit-core does (especially asking for username).\n\nDo you think that ppl. mainly using the terminal have *_ASKPASS set?\nMost GUIs I know do set it automatically.\n\nI agree that a new interface is needed (working on a patch), but before\nwe hurry, we should make git-core and git-svn behave the same way.\n\nBtw. git-svn also does not honour git-credentials.\n\n-- \nBest regards,\n Sven Strickroth\n ClamAV, a GPL anti-virus toolkit http://www.clamav.net\n PGP key id F5A9D4C4 @ any key-server\n"},{"id":"181830","messageId":"4F00BAA6.9050104@tu-clausthal.de","threadId":"28961","inReplyTo":"7v8vlrwzw9.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/2] git-svn, perl/Git.pm: extend and use Git->prompt method for querying users","fromName":"Sven Strickroth","fromEmail":"sven.strickroth@tu-clausthal.de","sentAt":"2012-01-01T19:57:26Z","receivedAt":"2012-01-01T19:57:26Z","isPatch":true,"sender":{"key":"sven.strickroth@tu-clausthal.de","avatar":null},"body":"Am 01.01.2012 10:11 schrieb Junio C Hamano:\n> I do not think Peff means the dialog must ask these three items at the\n> same time. The point is that other codepaths know they need to ask them\n> and would benefit if they can instruct the dialog external helper to ask\n> them in a single interaction. So if your callsite does not ask them\n> together, it is OK. You can keep asking them separately in two dialog\n> interactions.\n\nSure. This is possible with my proposed interface.\n\nTwo parameters should be sufficient, since we get the path to the\nrepository from the CWD.\n\nTYPE: 'text' (default), 'pass', 'userpass' (username + password in one\ndialog), 'filename'\nPROMPT\n\nAm I missing something?\n\n-- \nBest regards,\n Sven Strickroth\n ClamAV, a GPL anti-virus toolkit http://www.clamav.net\n PGP key id F5A9D4C4 @ any key-server\n"},{"id":"181832","messageId":"4F00C85F.7060401@tu-clausthal.de","threadId":"28961","inReplyTo":"4F00BAA6.9050104@tu-clausthal.de","subject":"Re: [PATCH 2/2] git-svn, perl/Git.pm: extend and use Git->prompt method for querying users","fromName":"Sven Strickroth","fromEmail":"sven.strickroth@tu-clausthal.de","sentAt":"2012-01-01T20:55:59Z","receivedAt":"2012-01-01T20:55:59Z","isPatch":true,"sender":{"key":"sven.strickroth@tu-clausthal.de","avatar":null},"body":"Following you can find my first proposal, based upon patch 1. It's only the\nperl part, because I haven't checked out Peffs git_prompt patch(es) and to\navoid double work.\n\nATM I'm unsure about the 'username' type, but I think it's quite necessary\nto make git-svn behave like git-core in case of asking for a username. A type\n'userpass' (username and password in one dialog) isn't mentioned here, because\nit's not necessary for the git-svn part, but we should also specify/document it\nif we want to use it in the future.\n\nBtw. happy new year! ;)\n\n---\n git-svn.perl | 24 +++++++------------\n perl/Git.pm | 70 ++++++++++++++++++++++++++++++++++++++++-----------------\n 2 files changed, 58 insertions(+), 36 deletions(-)\n\ndiff --git a/git-svn.perl b/git-svn.perl\nindex 26d3559..54cf77f 100755\n--- a/git-svn.perl\n+++ b/git-svn.perl\n@@ -4356,17 +4356,16 @@ sub ssl_server_trust {\n \t map $cert_info->$_, qw(hostname valid_from valid_until\n \t issuer_dname fingerprint);\n \tmy $choice;\n-prompt:\n-\tprint STDERR $may_save ?\n+\tmy $prompt = $may_save ?\n \t \"(R)eject, accept (t)emporarily or accept (p)ermanently? \" :\n \t \"(R)eject or accept (t)emporarily? \";\n-\tSTDERR->flush;\n-\t$choice = lc(substr( || 'R', 0, 1));\n-\tif ($choice =~ /^t$/i) {\n+prompt:\n+\t$choice = lc(substr(Git->prompt($prompt) || 'R', 0, 1));\n+\tif ($choice eq \"t\") {\n \t\t$cred->may_save(undef);\n-\t} elsif ($choice =~ /^r$/i) {\n+\t} elsif ($choice eq \"r\") {\n \t\treturn -1;\n-\t} elsif ($may_save && $choice =~ /^p$/i) {\n+\t} elsif ($may_save && $choice eq \"p\") {\n \t\t$cred->may_save($may_save);\n \t} else {\n \t\tgoto prompt;\n@@ -4378,10 +4377,7 @@ prompt:\n sub ssl_client_cert {\n \tmy ($cred, $realm, $may_save, $pool) = @_;\n \t$may_save = undef if $_no_auth_cache;\n-\tprint STDERR \"Client certificate filename: \";\n-\tSTDERR->flush;\n-\tchomp(my $filename = );\n-\t$cred->cert_file($filename);\n+\t$cred->cert_file(Git->prompt(\"Client certificate filename: \", 'filename'));\n \t$cred->may_save($may_save);\n \t$SVN::_Core::SVN_NO_ERROR;\n }\n@@ -4404,9 +4400,7 @@ sub username {\n \tif (defined $_username) {\n \t\t$username = $_username;\n \t} else {\n-\t\tprint STDERR \"Username: \";\n-\t\tSTDERR->flush;\n-\t\tchomp($username = );\n+\t\t$username = Git->prompt(\"Username: \", 'username');\n \t}\n \t$cred->username($username);\n \t$cred->may_save($may_save);\n@@ -4415,7 +4409,7 @@ sub username {\n\n sub _read_password {\n \tmy ($prompt, $realm) = @_;\n-\tmy $password = Git->prompt($prompt);\n+\tmy $password = Git->prompt($prompt, 'pass');\n \t$password;\n }\n\ndiff --git a/perl/Git.pm b/perl/Git.pm\nindex ba9a5f2..17ddf40 100644\n--- a/perl/Git.pm\n+++ b/perl/Git.pm\n@@ -512,52 +512,80 @@ C). Useful mostly only internally.\n sub html_path { command_oneline('--html-path') }\n\n\n-=item prompt ( PROMPT )\n+=item prompt ( PROMPT , TYPE )\n\n Query user C and return answer from user.\n\n-If an external helper is specified via GIT_ASKPASS or SSH_ASKPASS, it\n-is used to interact with the user; otherwise the prompt is given to\n-and the answer is read from the terminal.\n+If an external helper is specified via GIT_DIALOG, GIT_ASKPASS or\n+SSH_ASKPASS, it is used to interact with the user; otherwise the\n+prompt is given to and the answer is read from the terminal.\n+\n+Possible values for C:\n+- '' or 'text': prompt for normal text (only GIT_DIALOG)\n+- 'username': prompt for username (behaves exactly as 'text', but also\n+ uses *_ASKPASS)\n+- 'pass': prompt for password, echoing of what is typed is disabled on\n+ the terminal, GUI tool shows asterisks.\n+- 'filename': prompt for a filename, GUI tool migh provide file chooser\n+ (only GIT_DIALOG)\n\n =cut\n\n sub prompt {\n-\tmy ($self, $prompt) = _maybe_self(@_);\n+\tmy ($self, $prompt, $type) = _maybe_self(@_);\n+\t$type = 'text' unless ($type);\n+\tmy $useAskPass = ($type eq 'pass' || $type eq 'username');\n \tmy $ret;\n \tif (!defined $ret) {\n-\t\t$ret = _prompt($ENV{'GIT_ASKPASS'}, $prompt);\n+\t\t$ret = _promptgitdialog($ENV{'GIT_DIALOG'}, $prompt, $type);\n \t}\n-\tif (!defined $ret) {\n-\t\t$ret = _prompt($ENV{'SSH_ASKPASS'}, $prompt);\n+\tif ($useAskPass && !defined $ret) {\n+\t\t$ret = _promptaskpass($ENV{'GIT_ASKPASS'}, $prompt);\n+\t}\n+\tif ($useAskPass && !defined $ret) {\n+\t\t$ret = _promptaskpass($ENV{'SSH_ASKPASS'}, $prompt);\n \t}\n \tif (!defined $ret) {\n \t\t$ret = '';\n \t\tprint STDERR $prompt;\n \t\tSTDERR->flush;\n-\t\trequire Term::ReadKey;\n-\t\tTerm::ReadKey::ReadMode('noecho');\n-\t\twhile (defined(my $key = Term::ReadKey::ReadKey(0))) {\n-\t\t\tlast if $key =~ /[\\012\\015]/; # \\n\\r\n-\t\t\t$ret .= $key;\n+\t\tif ($type eq 'pass') {\n+\t\t\trequire Term::ReadKey;\n+\t\t\tTerm::ReadKey::ReadMode('noecho');\n+\t\t\twhile (defined(my $key = Term::ReadKey::ReadKey(0))) {\n+\t\t\t\tlast if $key =~ /[\\012\\015]/; # \\n\\r\n+\t\t\t\t$ret .= $key;\n+\t\t\t}\n+\t\t\tTerm::ReadKey::ReadMode('restore');\n+\t\t\tprint STDERR \"\\n\";\n+\t\t\tSTDERR->flush;\n+\t\t} else {\n+\t\t\tchomp($ret = );\n \t\t}\n-\t\tTerm::ReadKey::ReadMode('restore');\n-\t\tprint STDERR \"\\n\";\n-\t\tSTDERR->flush;\n \t}\n \treturn $ret;\n }\n\n-sub _prompt {\n+sub _promptgitdialog {\n+\tmy ($gitdialog, $prompt, $type) = @_;\n+\treturn undef unless ($askpass);\n+\tmy $ret;\n+\topen my $fh, \"-|\", $gitdialog, $type, $prompt\n+\t\tor return undef;\n+\t$ret = <$fh>;\n+\t$ret =~ s/\\r?\\n$//; # strip \\r\\n, chomp does not work on all systems (i.e. windows) as expected\n+\tclose ($fh);\n+\treturn $ret;\n+}\n+\n+sub _promptaskpass {\n \tmy ($askpass, $prompt) = @_;\n-\tunless ($askpass) {\n-\t\treturn undef;\n-\t}\n+\treturn undef unless ($askpass);\n \tmy $ret;\n \topen my $fh, \"-|\", $askpass, $prompt\n \t\tor return undef;\n \t$ret = <$fh>;\n-\t$ret =~ s/[\\012\\015]//g; # \\n\\r\n+\t$ret =~ s/\\r?\\n$//; # strip \\r\\n, chomp does not work on all systems (i.e. windows) as expected\n \tclose ($fh);\n \treturn $ret;\n }\n-- \n\n-- \nBest regards,\n Sven Strickroth\n ClamAV, a GPL anti-virus toolkit http://www.clamav.net\n PGP key id F5A9D4C4 @ any key-server\n"},{"id":"181855","messageId":"CACBZZX7P9PEq0wZp0d3dSwDjF6J6Z3cO4VtWc9_frBengtqPLw@mail.gmail.com","threadId":"28961","inReplyTo":"4EFA5EB3.4000802@tu-clausthal.de","subject":"Re: [PATCH 1/2] git-svn, perl/Git.pm: add central method for prompting passwords honoring GIT_ASKPASS and SSH_ASKPASS","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2012-01-03T10:17:10Z","receivedAt":"2012-01-03T10:17:10Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Wed, Dec 28, 2011 at 01:11, Sven Strickroth\n wrote:\n\nNom nom, some Perl. Thanks for tackling this. Reviewing it as\nrequested by Junio.\n\n> diff --git a/git-svn.perl b/git-svn.perl\n> index eeb83d3..bcee8aa 100755\n> --- a/git-svn.perl\n> +++ b/git-svn.perl\n> @@ -4415,25 +4415,7 @@ sub username {\n>\n>  sub _read_password {\n>        my ($prompt, $realm) = @_;\n> -       my $password = '';\n> -       if (exists $ENV{GIT_ASKPASS}) {\n> -               open(PH, \"-|\", $ENV{GIT_ASKPASS}, $prompt);\n> -               $password = ;\n> -               $password =~ s/[\\012\\015]//; # \\n\\r\n> -               close(PH);\n> -       } else {\n> -               print STDERR $prompt;\n> -               STDERR->flush;\n> -               require Term::ReadKey;\n> -               Term::ReadKey::ReadMode('noecho');\n> -               while (defined(my $key = Term::ReadKey::ReadKey(0))) {\n> -                       last if $key =~ /[\\012\\015]/; # \\n\\r\n> -                       $password .= $key;\n> -               }\n> -               Term::ReadKey::ReadMode('restore');\n> -               print STDERR \"\\n\";\n> -               STDERR->flush;\n> -       }\n> +       my $password = Git->prompt($prompt);\n>        $password;\n>  }\n>\n> diff --git a/perl/Git.pm b/perl/Git.pm\n> index f7ce511..b1c7c50 100644\n> --- a/perl/Git.pm\n> +++ b/perl/Git.pm\n> @@ -58,7 +58,7 @@ require Exporter;\n>                 command_output_pipe command_input_pipe command_close_pipe\n>                 command_bidi_pipe command_close_bidi_pipe\n>                 version exec_path html_path hash_object git_cmd_try\n> -                remote_refs\n> +                remote_refs prompt\n>                 temp_acquire temp_release temp_reset temp_path);\n>\n>\n> @@ -512,6 +512,55 @@ C). Useful mostly only internally.\n>  sub html_path { command_oneline('--html-path') }\n>\n>\n> +=item prompt ( PROMPT )\n> +\n> +Query user C and return answer from user.\n> +\n> +Check if GIT_ASKPASS or SSH_ASKPASS is set, use first matching for querying\n> +user and return answer. If no *_ASKPASS variable is set, the variable is\n> +empty or an error occoured, the terminal is tried as a fallback.\n> +\n> +=cut\n> +\n> +sub prompt {\n> +       my ($self, $prompt) = _maybe_self(@_);\n> +       my $ret;\n> +       if (exists $ENV{'GIT_ASKPASS'}) {\n> +               $ret = _prompt($ENV{'GIT_ASKPASS'}, $prompt);\n> +       }\n> +       if (!defined $ret && exists $ENV{'SSH_ASKPASS'}) {\n> +               $ret = _prompt($ENV{'SSH_ASKPASS'}, $prompt);\n> +       }\n> +       if (!defined $ret) {\n> +               print STDERR $prompt;\n> +               STDERR->flush;\n> +               require Term::ReadKey;\n> +               Term::ReadKey::ReadMode('noecho');\n> +               while (defined(my $key = Term::ReadKey::ReadKey(0))) {\n> +                       last if $key =~ /[\\012\\015]/; # \\n\\r\n> +                       $ret .= $key;\n> +               }\n> +               Term::ReadKey::ReadMode('restore');\n> +               print STDERR \"\\n\";\n> +               STDERR->flush;\n> +       }\n> +       return $ret;\n> +}\n\nPersonally I prefer not to write code I don't have to write. Here\nyou're doing:\n\n my ($self, $prompt) = _maybe_self(@_);\n\nAnd then never using $self, so there's actually no reason for this to\nbe an object/class function. It could just be called as:\n\n my $password = Git::prompt($prompt);\n\nInstead of:\n\n my $password = Git->prompt($prompt);\n\nWhich means you could change the first line to just:\n\n my ($prompt) = @_;\n\nWhich wouldn't leave the reader wondering why this needs to maybe\nconstruct an object just to throw it away.\n\n> +sub _prompt {\n> +       my ($askpass, $prompt) = @_;\n> +       unless ($askpass) {\n> +               return undef;\n> +       }\n\nThe empty list is false in Perl, so explicitly returning undef is\nusually the wrong thing. It means that in list context you'll return a\ntrue list (consisting of one undef element), instead of an empty false\none.\n\n $ perl -MData::Dump=dump -wle 'sub a { return } sub b { return\nundef }; my @lists = ([a()], [b()]); for (@lists) { print @$_ ? \"true\"\n: \"false\" }'\n false\n true\n\nYou're not running into that error here since you always use scalar\ncontext. But it's better just to do:\n\n if (xyz) {\n return;\n }\n\nUnless you want this behavior.\n\nBut aside from that I find this code a bit bizarre. The caller is\ndoing:\n\n if (exists $ENV{'GIT_ASKPASS'}) {\n         $ret = _prompt($ENV{'GIT_ASKPASS'}, $prompt);\n }\n if (!defined $ret && exists $ENV{'SSH_ASKPASS'}) {\n         $ret = _prompt($ENV{'SSH_ASKPASS'}, $prompt);\n }\n\nWhich means we check whether *_ASKPASS *exists* in the env hash, and\nthen the function checks whether that value is true or not.\n\nWhich means the code implicitly depends on Perl's idea of true/false\nvalues for the names of those commands. E.g. you can't create a\ncommand called \"0\" and put it in your $PATH.\n\nI suppose the case you actually care about is someone being able to do:\n\n export GIT_ASKPASS=\n\nInstead of the better:\n\n unset GIT_ASKPASS\n\n> +       my $ret;\n> +       open my $fh, \"-|\", $askpass, $prompt || return undef;\n\nAs pointed out already this is a logic error due to the ||.\n\nBut even if that worked I think this behavior is a bit\nquestionable. Why don't we just throw an error at this point? The way\nI'd write this would be something like:\n\n my $pass = _prompt('GIT_ASKPASS', $prompt);\n\nAnd then:\n\n sub _prompt {\n my ($env_var, $prompt) = @_;\n\n # or exists(), depending on whether we insist on \"unset\"\n return unless length $ENV{$env_var};\n\n my $command = $ENV{$env_var};\n open my $fh, \"-|\", $command or die \"We can't get your password\nfrom `$command' given in $env_var: $!\";\n chomp(my $password = <$fh>);\n close $fh or die \"We can't close() `$command' given in $env_var: $!\";\n\n return $password;\n }\n\nWhich would give the user an error if his GIT_ASKPASS command\nfailed. Check the return value of close() too, and re-structure the\npassing around of the env variable so we could give a sensible error\nmessage.\n\n> +       $ret = <$fh>;\n> +       $ret =~ s/[\\012\\015]//g; # strip \\n\\r, chomp does not work on all systems (i.e. windows) as expected\n\nUrm yes it does. \\n in Perl is magical and doesn't mean \\012 like it\ndoes in some languages. It means \"The Platform's Native\nNewline\".\n\nWhich is \\012 on Unix, \\015\\012 on Windows, and was \\015 on Mac OS\nuntil support for it was removed. This is covered in the second\nsection of \"perldoc perlport\".\n\nCan you show me a case where it fails, and under what environment\nexactly? Maybe it's e.g.s some Cygwin-specific peculiarity, in which\ncase we could check for that platform specifically.\n"},{"id":"181856","messageId":"4F02D79B.1020002@tu-clausthal.de","threadId":"28961","inReplyTo":"CACBZZX7P9PEq0wZp0d3dSwDjF6J6Z3cO4VtWc9_frBengtqPLw@mail.gmail.com","subject":"Re: [PATCH 1/2] git-svn, perl/Git.pm: add central method for prompting passwords honoring GIT_ASKPASS and SSH_ASKPASS","fromName":"Sven Strickroth","fromEmail":"sven.strickroth@tu-clausthal.de","sentAt":"2012-01-03T10:25:31Z","receivedAt":"2012-01-03T10:25:31Z","isPatch":true,"sender":{"key":"sven.strickroth@tu-clausthal.de","avatar":null},"body":"Am 03.01.2012 11:17 schrieb Ævar Arnfjörð Bjarmason:\n>> + $ret = <$fh>;\n>> + $ret =~ s/[\\012\\015]//g; # strip \\n\\r, chomp does not work on all systems (i.e. windows) as expected\n> \n> Urm yes it does. \\n in Perl is magical and doesn't mean \\012 like it\n> does in some languages. It means \"The Platform's Native\n> Newline\".\n> \n> Which is \\012 on Unix, \\015\\012 on Windows, and was \\015 on Mac OS\n> until support for it was removed. This is covered in the second\n> section of \"perldoc perlport\".\n> \n> Can you show me a case where it fails, and under what environment\n> exactly? Maybe it's e.g.s some Cygwin-specific peculiarity, in which\n> case we could check for that platform specifically.\n\nI'm using msys perl (shipped with msysgit) and there just using chomp()\ndid not work.\n\n-- \nBest regards,\n Sven Strickroth\n ClamAV, a GPL anti-virus toolkit http://www.clamav.net\n PGP key id F5A9D4C4 @ any key-server\n"},{"id":"181857","messageId":"CACBZZX6iMobuU90skpbNPaGQFxYNOAjmZ6ceO4PGqfZSMkgePQ@mail.gmail.com","threadId":"28961","inReplyTo":"4F02D79B.1020002@tu-clausthal.de","subject":"Re: [PATCH 1/2] git-svn, perl/Git.pm: add central method for prompting passwords honoring GIT_ASKPASS and SSH_ASKPASS","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2012-01-03T12:03:48Z","receivedAt":"2012-01-03T12:03:48Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Tue, Jan 3, 2012 at 11:25, Sven Strickroth\n wrote:\n> Am 03.01.2012 11:17 schrieb Ævar Arnfjörð Bjarmason:\n>>> +       $ret = <$fh>;\n>>> +       $ret =~ s/[\\012\\015]//g; # strip \\n\\r, chomp does not work on all systems (i.e. windows) as expected\n>>\n>> Urm yes it does. \\n in Perl is magical and doesn't mean \\012 like it\n>> does in some languages. It means \"The Platform's Native\n>> Newline\".\n>>\n>> Which is \\012 on Unix, \\015\\012 on Windows, and was \\015 on Mac OS\n>> until support for it was removed. This is covered in the second\n>> section of \"perldoc perlport\".\n>>\n>> Can you show me a case where it fails, and under what environment\n>> exactly? Maybe it's e.g.s some Cygwin-specific peculiarity, in which\n>> case we could check for that platform specifically.\n>\n> I'm using msys perl (shipped with msysgit) and there just using chomp()\n> did not work.\n\nThat's odd, what does this print:\n\n perl -MData::Dumper -MFile::Temp=tempfile -we 'my $str =\n\"moo\\015\\012\"; my ($fh, $name) = tempfile(); print $fh $str; close\n$fh; open my $in, \"<\", $name or die $!; my $in_str = <$in>; chomp(my\n$cin_str = $in_str); print \"in_str:<$in_str> cin_str:<$cin_str>\nEND\\n\"'\n\nAnd how about this:\n\n perl -MData::Dumper -MFile::Temp=tempfile -we 'my $str =\n\"moo\\015\\012\"; my ($fh, $name) = tempfile(); print $fh $str; close\n$fh; open my $in, \"<:crlf\", $name or die $!; my $in_str = <$in>;\nchomp(my $cin_str = $in_str); print \"in_str:<$in_str>\ncin_str:<$cin_str> END\\n\"'\n\nIt could be that there's some bug in either perl or mingw's build of\nperl where it won't turn on the :crlf IO layer by default.\n"},{"id":"181858","messageId":"CACBZZX4HdrfPZzEL4=YV0_Tt6inYpZjSRy-HnUdGJ6YFOrE0Ag@mail.gmail.com","threadId":"28961","inReplyTo":"CACBZZX6iMobuU90skpbNPaGQFxYNOAjmZ6ceO4PGqfZSMkgePQ@mail.gmail.com","subject":"Re: [PATCH 1/2] git-svn, perl/Git.pm: add central method for prompting passwords honoring GIT_ASKPASS and SSH_ASKPASS","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2012-01-03T12:06:30Z","receivedAt":"2012-01-03T12:06:30Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Tue, Jan 3, 2012 at 13:03, Ævar Arnfjörð Bjarmason wrote:\n> On Tue, Jan 3, 2012 at 11:25, Sven Strickroth\n> wrote:\n>> Am 03.01.2012 11:17 schrieb Ævar Arnfjörð Bjarmason:\n>>>> +       $ret = <$fh>;\n>>>> +       $ret =~ s/[\\012\\015]//g; # strip \\n\\r, chomp does not work on all systems (i.e. windows) as expected\n>>>\n>>> Urm yes it does. \\n in Perl is magical and doesn't mean \\012 like it\n>>> does in some languages. It means \"The Platform's Native\n>>> Newline\".\n>>>\n>>> Which is \\012 on Unix, \\015\\012 on Windows, and was \\015 on Mac OS\n>>> until support for it was removed. This is covered in the second\n>>> section of \"perldoc perlport\".\n>>>\n>>> Can you show me a case where it fails, and under what environment\n>>> exactly? Maybe it's e.g.s some Cygwin-specific peculiarity, in which\n>>> case we could check for that platform specifically.\n>>\n>> I'm using msys perl (shipped with msysgit) and there just using chomp()\n>> did not work.\n>\n> That's odd, what does this print:\n>\n>    perl -MData::Dumper -MFile::Temp=tempfile -we 'my $str =\n> \"moo\\015\\012\"; my ($fh, $name) = tempfile(); print $fh $str; close\n> $fh; open my $in, \"<\", $name or die $!; my $in_str = <$in>; chomp(my\n> $cin_str = $in_str); print \"in_str:<$in_str> cin_str:<$cin_str>\n> END\\n\"'\n>\n> And how about this:\n>\n>    perl -MData::Dumper -MFile::Temp=tempfile -we 'my $str =\n> \"moo\\015\\012\"; my ($fh, $name) = tempfile(); print $fh $str; close\n> $fh; open my $in, \"<:crlf\", $name or die $!; my $in_str = <$in>;\n> chomp(my $cin_str = $in_str); print \"in_str:<$in_str>\n> cin_str:<$cin_str> END\\n\"'\n>\n> It could be that there's some bug in either perl or mingw's build of\n> perl where it won't turn on the :crlf IO layer by default.\n\nOr actually you could do this before running your patch, except you\nhave to change the code to use chomp() instead of your regex hack:\n\n export PERLIO=crlf\n\nIf that makes it work we should just do that somewhere else (e.g. at\nthe top of Git.pm) if we detect Windows, which'll make chomp() work as\nintended everywhere.\n"},{"id":"181860","messageId":"4F030029.40006@tu-clausthal.de","threadId":"28961","inReplyTo":"CACBZZX6iMobuU90skpbNPaGQFxYNOAjmZ6ceO4PGqfZSMkgePQ@mail.gmail.com","subject":"Re: [PATCH 1/2] git-svn, perl/Git.pm: add central method for prompting passwords honoring GIT_ASKPASS and SSH_ASKPASS","fromName":"Sven Strickroth","fromEmail":"sven.strickroth@tu-clausthal.de","sentAt":"2012-01-03T13:18:33Z","receivedAt":"2012-01-03T13:18:33Z","isPatch":true,"sender":{"key":"sven.strickroth@tu-clausthal.de","avatar":null},"body":"Am 03.01.2012 13:03 schrieb Ævar Arnfjörð Bjarmason:\n>> I'm using msys perl (shipped with msysgit) and there just using chomp()\n>> did not work.\n\nBut why not drop all \\n and \\r, since we only accept and wait for a\nsingle line?\n\nchomp($str = ) works as expected, but for some reason the line I\ngot from the ASKPASS tool isn't. Btw. the askpass tool provides a string\nfollowed with \\n (c++).\n\n> That's odd, what does this print:\n> \n> perl -MData::Dumper -MFile::Temp=tempfile -we 'my $str =\n> \"moo\\015\\012\"; my ($fh, $name) = tempfile(); print $fh $str; close\n> $fh; open my $in, \"<\", $name or die $!; my $in_str = <$in>; chomp(my\n> $cin_str = $in_str); print \"in_str:<$in_str> cin_str:<$cin_str>\n> END\\n\"'\n> \n> And how about this:\n> \n> perl -MData::Dumper -MFile::Temp=tempfile -we 'my $str =\n> \"moo\\015\\012\"; my ($fh, $name) = tempfile(); print $fh $str; close\n> $fh; open my $in, \"<:crlf\", $name or die $!; my $in_str = <$in>;\n> chomp(my $cin_str = $in_str); print \"in_str:<$in_str>\n> cin_str:<$cin_str> END\\n\"'\n> \n> It could be that there's some bug in either perl or mingw's build of\n> perl where it won't turn on the :crlf IO layer by default.\n\nI get an error in both cases \"Das System kann die angegebene Datei nicht\nfinden\".\n\n-- \nBest regards,\n Sven Strickroth\n ClamAV, a GPL anti-virus toolkit http://www.clamav.net\n PGP key id F5A9D4C4 @ any key-server\n"},{"id":"181868","messageId":"7vzke4vebl.fsf@alter.siamese.dyndns.org","threadId":"28961","inReplyTo":"4F00B7F3.1060105@tu-clausthal.de","subject":"Re: [PATCH 2/2] git-svn, perl/Git.pm: extend and use Git->prompt method for querying users","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-01-03T18:19:42Z","receivedAt":"2012-01-03T18:19:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sven Strickroth writes:\n\n> I'm still for the second patch to be applied (maybe w/o the certificate\n> filename prompt), too, because this makes git-svn behave the save way as\n> git-core does (especially asking for username).\n\nI do not have much issues against the patch if the filename thing is\nexcluded as a short-term workaround to avoid regression.\n\nFor people who have been used to interact with git-svn from the terminal\nbut has *_ASKPASS for reasons other than their use of git-svn in their\nenvironment, the change to the username codepath is technically a\nregression, as they used to be able to see and correct typo while giving\ntheir username but with the patch *_ASKPASS will kick in and they have to\ntype in bline, but I am not particularly worried about it. It is something\nyou type very often and committed to your muscle memory anyway.\n"},{"id":"181870","messageId":"20120103184022.GA20926@sigill.intra.peff.net","threadId":"28961","inReplyTo":"7vzke4vebl.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/2] git-svn, perl/Git.pm: extend and use Git->prompt method for querying users","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-01-03T18:40:22Z","receivedAt":"2012-01-03T18:40:22Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jan 03, 2012 at 10:19:42AM -0800, Junio C Hamano wrote:\n\n> For people who have been used to interact with git-svn from the terminal\n> but has *_ASKPASS for reasons other than their use of git-svn in their\n> environment, the change to the username codepath is technically a\n> regression, as they used to be able to see and correct typo while giving\n> their username but with the patch *_ASKPASS will kick in and they have to\n> type in bline, but I am not particularly worried about it. It is something\n> you type very often and committed to your muscle memory anyway.\n\nThere is one difference between how git and ssh use the ASKPASS\nvariable. In git, we try it _first_, and fall back to asking on the\nterminal. For ssh, they first try the terminal, and fall back to\naskpass only when the terminal cannot be opened.\n\nIf we tried the terminal first, then it wouldn't be a big deal to use\n*_ASKPASS more frequently, since it's a fallback. Of course, that in\nitself might be a regression for some people.\n\nI wonder if we should make the order:\n\n 1. GIT_ASKPASS\n\n 2. terminal\n\n 3. SSH_ASKPASS\n\nto help make our use SSH_ASKPASS better match that of ssh. I dunno. I am\nnot an askpass user these days, so I don't know what people expect or\nwant.\n\n-Peff\n"},{"id":"181879","messageId":"7v4nwcvaie.fsf@alter.siamese.dyndns.org","threadId":"28961","inReplyTo":"CACBZZX6iMobuU90skpbNPaGQFxYNOAjmZ6ceO4PGqfZSMkgePQ@mail.gmail.com","subject":"Re: [PATCH 1/2] git-svn, perl/Git.pm: add central method for prompting passwords honoring GIT_ASKPASS and SSH_ASKPASS","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-01-03T19:42:01Z","receivedAt":"2012-01-03T19:42:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> And how about this:\n>\n> perl -MData::Dumper -MFile::Temp=tempfile -we 'my $str =\n> \"moo\\015\\012\"; my ($fh, $name) = tempfile(); print $fh $str; close\n> $fh; open my $in, \"<:crlf\", $name or die $!; my $in_str = <$in>;\n> chomp(my $cin_str = $in_str); print \"in_str:<$in_str>\n> cin_str:<$cin_str> END\\n\"'\n>\n> It could be that there's some bug in either perl or mingw's build of\n> perl where it won't turn on the :crlf IO layer by default.\n\nI agree that PERLIO may be a solution that is supposed to work, but let's\nnot go there, at least not yet in this patch. The stripping of \\015\\012\n(not \\r\\n) was copied from the original code and I would prefer to see the\npatch focus on one primary thing it wants to enhance at a time.\n\nThanks.\n"},{"id":"181899","messageId":"7vboqks8la.fsf@alter.siamese.dyndns.org","threadId":"28961","inReplyTo":"CACBZZX7P9PEq0wZp0d3dSwDjF6J6Z3cO4VtWc9_frBengtqPLw@mail.gmail.com","subject":"Re: [PATCH 1/2] git-svn, perl/Git.pm: add central method for prompting passwords honoring GIT_ASKPASS and SSH_ASKPASS","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-01-03T22:51:45Z","receivedAt":"2012-01-03T22:51:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> On Wed, Dec 28, 2011 at 01:11, Sven Strickroth\n> wrote:\n>\n> Nom nom, some Perl. Thanks for tackling this. Reviewing it as\n> requested by Junio.\n>\n\nAs I'd like to have this in 1.7.9-rc-something, here is my attempt to\nrewrite the patch based on your comments, modulo the \"chomp()\" bit, to\nexpedite the cycle. Major fixes are:\n\n * make \"prompt\" just a helper subroutine. It does not have to be tied to\n any particular repository object anyway.\n\n * Move the \"ah, the environment is there but the value is not set to\n anything sensible\" logic from the caller \"prompt\" to the helper\n \"_prompt\". For now, forget about an executable whose name is \"0\" for\n simplicity.\n\n * Do so using Perl-ish idiom \"return unless ($foo)\" to avoid potential\n issues with callers who might call it in the list context (I do not\n think it is reasonable to call \"prompt\" in the list context to begin\n with, though. It is not like the function is to return a list of zero\n or more answers, in which case \"@answers = function()\" makes\n sense. This is \"ask to get a single answer\" interface).\n\n * \"open ... or return\", not \"||\".\n\nSven, does it look agreeable? And more importantly, does it still work? ;-)\n\nThanks.\n\n git-svn.perl | 20 +-------------------\n perl/Git.pm | 51 ++++++++++++++++++++++++++++++++++++++++++++++++++-\n 2 files changed, 51 insertions(+), 20 deletions(-)\n\ndiff --git a/git-svn.perl b/git-svn.perl\nindex e30df22..6a01176 100755\n--- a/git-svn.perl\n+++ b/git-svn.perl\n@@ -4415,25 +4415,7 @@ sub username {\n \n sub _read_password {\n \tmy ($prompt, $realm) = @_;\n-\tmy $password = '';\n-\tif (exists $ENV{GIT_ASKPASS}) {\n-\t\topen(PH, \"-|\", $ENV{GIT_ASKPASS}, $prompt);\n-\t\t$password = ;\n-\t\t$password =~ s/[\\012\\015]//; # \\n\\r\n-\t\tclose(PH);\n-\t} else {\n-\t\tprint STDERR $prompt;\n-\t\tSTDERR->flush;\n-\t\trequire Term::ReadKey;\n-\t\tTerm::ReadKey::ReadMode('noecho');\n-\t\twhile (defined(my $key = Term::ReadKey::ReadKey(0))) {\n-\t\t\tlast if $key =~ /[\\012\\015]/; # \\n\\r\n-\t\t\t$password .= $key;\n-\t\t}\n-\t\tTerm::ReadKey::ReadMode('restore');\n-\t\tprint STDERR \"\\n\";\n-\t\tSTDERR->flush;\n-\t}\n+\tmy $password = Git::prompt($prompt);\n \t$password;\n }\n \ndiff --git a/perl/Git.pm b/perl/Git.pm\nindex f7ce511..abf9de9 100644\n--- a/perl/Git.pm\n+++ b/perl/Git.pm\n@@ -58,7 +58,7 @@ require Exporter;\n command_output_pipe command_input_pipe command_close_pipe\n command_bidi_pipe command_close_bidi_pipe\n version exec_path html_path hash_object git_cmd_try\n- remote_refs\n+ remote_refs prompt\n temp_acquire temp_release temp_reset temp_path);\n \n \n@@ -512,6 +512,55 @@ C). Useful mostly only internally.\n sub html_path { command_oneline('--html-path') }\n \n \n+=item prompt ( PROMPT )\n+\n+Query user C and return answer from user.\n+\n+If an external helper is specified via GIT_ASKPASS or SSH_ASKPASS, it\n+is used to interact with the user; otherwise the prompt is given to\n+and the answer is read from the terminal.\n+\n+=cut\n+\n+sub prompt {\n+\tmy ($prompt) = @_;\n+\tmy $ret;\n+\tif (!defined $ret) {\n+\t\t$ret = _prompt($ENV{'GIT_ASKPASS'}, $prompt);\n+\t}\n+\tif (!defined $ret) {\n+\t\t$ret = _prompt($ENV{'SSH_ASKPASS'}, $prompt);\n+\t}\n+\tif (!defined $ret) {\n+\t\t$ret = '';\n+\t\tprint STDERR $prompt;\n+\t\tSTDERR->flush;\n+\t\trequire Term::ReadKey;\n+\t\tTerm::ReadKey::ReadMode('noecho');\n+\t\twhile (defined(my $key = Term::ReadKey::ReadKey(0))) {\n+\t\t\tlast if $key =~ /[\\012\\015]/; # \\n\\r\n+\t\t\t$ret .= $key;\n+\t\t}\n+\t\tTerm::ReadKey::ReadMode('restore');\n+\t\tprint STDERR \"\\n\";\n+\t\tSTDERR->flush;\n+\t}\n+\treturn $ret;\n+}\n+\n+sub _prompt {\n+\tmy ($askpass, $prompt) = @_;\n+\treturn unless ($askpass);\n+\n+\topen my $fh, \"-|\", $askpass, $prompt\n+\t\tor return;\n+\tmy $ret = <$fh>;\n+\t$ret =~ s/[\\012\\015]//g; # \\n\\r\n+\tclose ($fh);\n+\treturn $ret;\n+}\n+\n+\n =item repo_path ()\n \n Return path to the git repository. Must be called on a repository instance.\n"},{"id":"181900","messageId":"4F038E49.9080809@tu-clausthal.de","threadId":"28961","inReplyTo":"7vzke4vebl.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/2] git-svn, perl/Git.pm: extend and use Git->prompt method for querying users","fromName":"Sven Strickroth","fromEmail":"sven.strickroth@tu-clausthal.de","sentAt":"2012-01-03T23:24:57Z","receivedAt":"2012-01-03T23:24:57Z","isPatch":true,"sender":{"key":"sven.strickroth@tu-clausthal.de","avatar":null},"body":"git-svn reads usernames and other user queries from an interactive\nterminal. This cause GUIs (w/o STDIN connected) to hang waiting forever\nfor git-svn to complete (http://code.google.com/p/tortoisegit/issues/detail?id=967).\ngit-core already asks for username using *_ASKPASS tools, this commit\nalso enables git-svn to do so.\n\nThis change extends the Git::prompt method, so that it can also be used\nfor non password queries (e.g. usernames), and makes use of it instead\nof using hand-rolled prompt-response code that only works with the\ninteractive terminal.\n\nSigned-off-by: Sven Strickroth \n---\n git-svn.perl | 19 ++++++++-----------\n perl/Git.pm | 27 ++++++++++++++++-----------\n 2 files changed, 24 insertions(+), 22 deletions(-)\n\ndiff --git a/git-svn.perl b/git-svn.perl\nindex ade88ae..be713f5 100755\n--- a/git-svn.perl\n+++ b/git-svn.perl\n@@ -4356,17 +4356,16 @@ sub ssl_server_trust {\n \t map $cert_info->$_, qw(hostname valid_from valid_until\n \t issuer_dname fingerprint);\n \tmy $choice;\n-prompt:\n-\tprint STDERR $may_save ?\n+\tmy $options = $may_save ?\n \t \"(R)eject, accept (t)emporarily or accept (p)ermanently? \" :\n \t \"(R)eject or accept (t)emporarily? \";\n-\tSTDERR->flush;\n-\t$choice = lc(substr( || 'R', 0, 1));\n-\tif ($choice =~ /^t$/i) {\n+prompt:\n+\t$choice = lc(substr(Git::prompt($options) || 'R', 0, 1));\n+\tif ($choice eq 't') {\n \t\t$cred->may_save(undef);\n-\t} elsif ($choice =~ /^r$/i) {\n+\t} elsif ($choice eq 'r') {\n \t\treturn -1;\n-\t} elsif ($may_save && $choice =~ /^p$/i) {\n+\t} elsif ($may_save && $choice eq 'p') {\n \t\t$cred->may_save($may_save);\n \t} else {\n \t\tgoto prompt;\n@@ -4404,9 +4403,7 @@ sub username {\n \tif (defined $_username) {\n \t\t$username = $_username;\n \t} else {\n-\t\tprint STDERR \"Username: \";\n-\t\tSTDERR->flush;\n-\t\tchomp($username = );\n+\t\t$username = Git::prompt(\"Username: \");\n \t}\n \t$cred->username($username);\n \t$cred->may_save($may_save);\n@@ -4415,7 +4412,7 @@ sub username {\n\n sub _read_password {\n \tmy ($prompt, $realm) = @_;\n-\tmy $password = Git::prompt($prompt);\n+\tmy $password = Git::prompt($prompt, 1);\n \t$password;\n }\n\ndiff --git a/perl/Git.pm b/perl/Git.pm\nindex 46f11a8..33e68c4 100644\n--- a/perl/Git.pm\n+++ b/perl/Git.pm\n@@ -512,18 +512,19 @@ C). Useful mostly only internally.\n sub html_path { command_oneline('--html-path') }\n\n\n-=item prompt ( PROMPT )\n+=item prompt ( PROMPT , ISPASSWORD )\n\n Query user C and return answer from user.\n\n If an external helper is specified via GIT_ASKPASS or SSH_ASKPASS, it\n is used to interact with the user; otherwise the prompt is given to\n and the answer is read from the terminal.\n+If C is true, the terminal disables echo.\n\n =cut\n\n sub prompt {\n-\tmy ($prompt) = @_;\n+\tmy ($prompt, $isPassword) = @_;\n \tmy $ret;\n \tif (!defined $ret) {\n \t\t$ret = _prompt($ENV{'GIT_ASKPASS'}, $prompt);\n@@ -532,18 +533,22 @@ sub prompt {\n \t\t$ret = _prompt($ENV{'SSH_ASKPASS'}, $prompt);\n \t}\n \tif (!defined $ret) {\n-\t\t$ret = '';\n \t\tprint STDERR $prompt;\n \t\tSTDERR->flush;\n-\t\trequire Term::ReadKey;\n-\t\tTerm::ReadKey::ReadMode('noecho');\n-\t\twhile (defined(my $key = Term::ReadKey::ReadKey(0))) {\n-\t\t\tlast if $key =~ /[\\012\\015]/; # \\n\\r\n-\t\t\t$ret .= $key;\n+\t\tif ($isPassword) {\n+\t\t\t$ret = '';\n+\t\t\trequire Term::ReadKey;\n+\t\t\tTerm::ReadKey::ReadMode('noecho');\n+\t\t\twhile (defined(my $key = Term::ReadKey::ReadKey(0))) {\n+\t\t\t\tlast if $key =~ /[\\012\\015]/; # \\n\\r\n+\t\t\t\t$ret .= $key;\n+\t\t\t}\n+\t\t\tTerm::ReadKey::ReadMode('restore');\n+\t\t\tprint STDERR \"\\n\";\n+\t\t\tSTDERR->flush;\n+\t\t} else {\n+\t\t\tchomp($ret = );\n \t\t}\n-\t\tTerm::ReadKey::ReadMode('restore');\n-\t\tprint STDERR \"\\n\";\n-\t\tSTDERR->flush;\n \t}\n \treturn $ret;\n }\n-- \n1.7.8.msysgit.0\n"},{"id":"181901","messageId":"4F038EC8.505@tu-clausthal.de","threadId":"28961","inReplyTo":"7vboqks8la.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/2] git-svn, perl/Git.pm: add central method for prompting passwords honoring GIT_ASKPASS and SSH_ASKPASS","fromName":"Sven Strickroth","fromEmail":"sven.strickroth@tu-clausthal.de","sentAt":"2012-01-03T23:27:04Z","receivedAt":"2012-01-03T23:27:04Z","isPatch":true,"sender":{"key":"sven.strickroth@tu-clausthal.de","avatar":null},"body":"Am 03.01.2012 23:51 schrieb Junio C Hamano:\n> Sven, does it look agreeable? And more importantly, does it still work? ;-)\n\nWorks for me :)\n\nI also updated my second patch minutes ago to fit onto the new patch\n(w/o the filename stuff).\n\n-- \nBest regards,\n Sven Strickroth\n ClamAV, a GPL anti-virus toolkit http://www.clamav.net\n PGP key id F5A9D4C4 @ any key-server\n"},{"id":"181904","messageId":"7v39bws4xi.fsf@alter.siamese.dyndns.org","threadId":"28961","inReplyTo":"4F038EC8.505@tu-clausthal.de","subject":"Re: [PATCH 1/2] git-svn, perl/Git.pm: add central method for prompting passwords honoring GIT_ASKPASS and SSH_ASKPASS","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-01-04T00:10:49Z","receivedAt":"2012-01-04T00:10:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sven Strickroth writes:\n\n> Am 03.01.2012 23:51 schrieb Junio C Hamano:\n>> Sven, does it look agreeable? And more importantly, does it still work? ;-)\n>\n> Works for me :)\n>\n> I also updated my second patch minutes ago to fit onto the new patch\n> (w/o the filename stuff).\n\nThanks.\n\nFor the second patch, I have a feeling that Peff's earlier suggestion to\ngive precedence to the terminal interaction over SSH_ASKPASS iff we can\nopen terminal, but I think the first one is OK for 1.7.9.\n\nI'll queue both of them in 'pu' for now just in case others spot silly\nmistakes I made while rewriting the first one, though.\n"},{"id":"181905","messageId":"7vy5toqqab.fsf@alter.siamese.dyndns.org","threadId":"28961","inReplyTo":"4F038E49.9080809@tu-clausthal.de","subject":"Re: [PATCH 2/2] git-svn, perl/Git.pm: extend and use Git->prompt method for querying users","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-01-04T00:12:28Z","receivedAt":"2012-01-04T00:12:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sven Strickroth writes:\n\n> git-svn reads usernames and other user queries from an interactive\n> terminal. This cause GUIs (w/o STDIN connected) to hang waiting forever\n> for git-svn to complete (http://code.google.com/p/tortoisegit/issues/detail?id=967).\n> git-core already asks for username using *_ASKPASS tools, this commit\n> also enables git-svn to do so.\n>\n> This change extends the Git::prompt method, so that it can also be used\n> for non password queries (e.g. usernames), and makes use of it instead\n> of using hand-rolled prompt-response code that only works with the\n> interactive terminal.\n\nNow \"prompt\" is no longer a method but is merely a helper function, so\nI've queued this (and 1/2 rewrite we discussed in a separate thread) to\n'pu' after rewording the commit log message.\n\nThanks.\n"},{"id":"181909","messageId":"4F0405D4.9090102@tu-clausthal.de","threadId":"28961","inReplyTo":"7v39bws4xi.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/2] git-svn, perl/Git.pm: add central method for prompting passwords honoring GIT_ASKPASS and SSH_ASKPASS","fromName":"Sven Strickroth","fromEmail":"sven.strickroth@tu-clausthal.de","sentAt":"2012-01-04T07:55:00Z","receivedAt":"2012-01-04T07:55:00Z","isPatch":true,"sender":{"key":"sven.strickroth@tu-clausthal.de","avatar":null},"body":"Am 04.01.2012 01:10 schrieb Junio C Hamano:\n> I'll queue both of them in 'pu' for now just in case others spot silly\n> mistakes I made while rewriting the first one, though.\n\nI just hit another issue (I created another patch, but we might want to\nintegrate it into the first one). This is especially needed if we want\nto apply my second patch in this mail.\nFrom: Sven Strickroth \nDate: Wed, 4 Jan 2012 08:32:13 +0100\nSubject: [PATCH] Git.pm: check if value is defined before accessing it\n\nSome perl versions, like the one from msys, crash sometimes\nif reading from STDIN wasn't successful and chomp is applied\nto the variable into which was read.\n\nErrormessage:\nUsername: Use of uninitialized value in chomp at C:\\Program\nFiles\\Git/libexec/git-core\\git-svn line 4321.\n0 [main] perl.exe\" 1916 handle_exceptions: Exception:\nSTATUS_ACCESS_VIOLATION\n1297 [main] perl.exe\" 1916 open_stackdumpfile: Dumping stack trace to\nperl.exe.stackdump\n\nSigned-off-by: Sven Strickroth \n---\n perl/Git.pm | 7 ++++++-\n 1 files changed, 6 insertions(+), 1 deletions(-)\n\ndiff --git a/perl/Git.pm b/perl/Git.pm\nindex 33e68c4..1c96a20 100644\n--- a/perl/Git.pm\n+++ b/perl/Git.pm\n@@ -547,7 +547,12 @@ sub prompt {\n \t\t\tprint STDERR \"\\n\";\n \t\t\tSTDERR->flush;\n \t\t} else {\n-\t\t\tchomp($ret = );\n+\t\t\t$ret = ;\n+\t\t\tif (defined $ret) {\n+\t\t\t\tchomp($ret);\n+\t\t\t} else {\n+\t\t\t\t$ret = '';\n+\t\t\t}\n \t\t}\n \t}\n \treturn $ret;\n\n> For the second patch, I have a feeling that Peff's earlier suggestion to\n> give precedence to the terminal interaction over SSH_ASKPASS iff we can\n> open terminal, but I think the first one is OK for 1.7.9.\n\nWe also do the wrong order for querying the password. if we want to\nadopt this, we should also update prompt.c, the make both prompt methods\nbehave the same way again.\n\nThe Git.pm part is easy, but I also tried to update prompt.c (untested).\nFrom: Sven Strickroth \nDate: Wed, 4 Jan 2012 08:44:48 +0100\nSubject: [PATCH] Git.pm, prompt: try reading from interactive terminal\nbefore\n using SSH_ASKPASS\n\nSVN tries to read reading from interactive terminal before using\nSSH_ASKPASS helper. This change adjust git to behave the same way.\n\nSigned-off-by: Sven Strickroth \n---\n perl/Git.pm | 6 +++---\n prompt.c | 14 +++++++++++---\n 2 files changed, 14 insertions(+), 6 deletions(-)\n\ndiff --git a/perl/Git.pm b/perl/Git.pm\nindex 1c96a20..6ce193e 100644\n--- a/perl/Git.pm\n+++ b/perl/Git.pm\n@@ -530,9 +530,6 @@ sub prompt {\n \t\t$ret = _prompt($ENV{'GIT_ASKPASS'}, $prompt);\n \t}\n \tif (!defined $ret) {\n-\t\t$ret = _prompt($ENV{'SSH_ASKPASS'}, $prompt);\n-\t}\n-\tif (!defined $ret) {\n \t\tprint STDERR $prompt;\n \t\tSTDERR->flush;\n \t\tif ($isPassword) {\n@@ -555,5 +552,8 @@ sub prompt {\n \t\t}\n \t}\n+\tif (!defined $ret) {\n+\t\t$ret = _prompt($ENV{'SSH_ASKPASS'}, $prompt);\n+\t}\n \treturn $ret;\n }\n\n\ndiff --git a/prompt.c b/prompt.c\nindex 72ab9de..e791619 100644\n--- a/prompt.c\n+++ b/prompt.c\n@@ -52,9 +52,17 @@ char *git_prompt(const char *prompt, int flags)\n \t}\n\n \tr = git_terminal_prompt(prompt, flags & PROMPT_ECHO);\n-\tif (!r)\n-\t\tdie_errno(\"could not read '%s'\", prompt);\n-\treturn r;\n+\tif (r)\n+\t\treturn r;\n+\n+\tif (flags & PROMPT_ASKPASS) {\n+\t\tconst char *askpass;\n+\t\taskpass = getenv(\"SSH_ASKPASS\");\n+\t\tif (askpass && *askpass)\n+\t\t\treturn do_askpass(askpass, prompt);\n+\t}\n+\n+\tdie_errno(\"could not read '%s'\", prompt);\n }\n\n char *git_getpass(const char *prompt)\n-- \nBest regards,\n Sven Strickroth\n ClamAV, a GPL anti-virus toolkit http://www.clamav.net\n PGP key id F5A9D4C4 @ any key-server\n"},{"id":"181910","messageId":"4F040E46.5030001@tu-clausthal.de","threadId":"28961","inReplyTo":"4F0405D4.9090102@tu-clausthal.de","subject":"Re: [PATCH 1/2] git-svn, perl/Git.pm: add central method for prompting passwords honoring GIT_ASKPASS and SSH_ASKPASS","fromName":"Sven Strickroth","fromEmail":"sven.strickroth@tu-clausthal.de","sentAt":"2012-01-04T08:31:02Z","receivedAt":"2012-01-04T08:31:02Z","isPatch":true,"sender":{"key":"sven.strickroth@tu-clausthal.de","avatar":null},"body":"Am 04.01.2012 08:55 schrieb Sven Strickroth:\n> The Git.pm part is easy, but I also tried to update prompt.c (untested).\n\nI said \"easy\" and then I mailed the wrong/outdated patch :(\nI'm sorry for the noise.\n\nFrom: Sven Strickroth \nDate: Wed, 4 Jan 2012 08:44:48 +0100\nSubject: [PATCH] Git.pm, prompt: try reading from interactive terminal\n before using SSH_ASKPASS\n\nSVN tries to read reading from interactive terminal before using\nSSH_ASKPASS helper. This change adjust git to behave the same way.\n\nSigned-off-by: Sven Strickroth \n---\n perl/Git.pm | 9 ++++-----\n prompt.c | 14 +++++++++++---\n 2 files changed, 15 insertions(+), 8 deletions(-)\n\ndiff --git a/perl/Git.pm b/perl/Git.pm\nindex 1c96a20..721aef7 100644\n--- a/perl/Git.pm\n+++ b/perl/Git.pm\n@@ -530,13 +530,9 @@ sub prompt {\n \t\t$ret = _prompt($ENV{'GIT_ASKPASS'}, $prompt);\n \t}\n \tif (!defined $ret) {\n-\t\t$ret = _prompt($ENV{'SSH_ASKPASS'}, $prompt);\n-\t}\n-\tif (!defined $ret) {\n \t\tprint STDERR $prompt;\n \t\tSTDERR->flush;\n \t\tif ($isPassword) {\n-\t\t\t$ret = '';\n \t\t\trequire Term::ReadKey;\n \t\t\tTerm::ReadKey::ReadMode('noecho');\n \t\t\twhile (defined(my $key = Term::ReadKey::ReadKey(0))) {\n@@ -551,10 +547,13 @@ sub prompt {\n \t\t\tif (defined $ret) {\n \t\t\t\tchomp($ret);\n \t\t\t} else {\n-\t\t\t\t$ret = '';\n+\t\t\t\tundef $ret;\n \t\t\t}\n \t\t}\n \t}\n+\tif (!defined $ret) {\n+\t\t$ret = _prompt($ENV{'SSH_ASKPASS'}, $prompt);\n+\t}\n \treturn $ret;\n }\n\ndiff --git a/prompt.c b/prompt.c\nindex 72ab9de..e791619 100644\n--- a/prompt.c\n+++ b/prompt.c\n@@ -52,9 +52,17 @@ char *git_prompt(const char *prompt, int flags)\n \t}\n\n \tr = git_terminal_prompt(prompt, flags & PROMPT_ECHO);\n-\tif (!r)\n-\t\tdie_errno(\"could not read '%s'\", prompt);\n-\treturn r;\n+\tif (r)\n+\t\treturn r;\n+\n+\tif (flags & PROMPT_ASKPASS) {\n+\t\tconst char *askpass;\n+\t\taskpass = getenv(\"SSH_ASKPASS\");\n+\t\tif (askpass && *askpass)\n+\t\t\treturn do_askpass(askpass, prompt);\n+\t}\n+\n+\tdie_errno(\"could not read '%s'\", prompt);\n }\n\n char *git_getpass(const char *prompt)\n-- \nBest regards,\n Sven Strickroth\n ClamAV, a GPL anti-virus toolkit http://www.clamav.net\n PGP key id F5A9D4C4 @ any key-server\n"},{"id":"181918","messageId":"20120104133459.GA6564@sigill.intra.peff.net","threadId":"28961","inReplyTo":"4F040E46.5030001@tu-clausthal.de","subject":"Re: [PATCH 1/2] git-svn, perl/Git.pm: add central method for prompting passwords honoring GIT_ASKPASS and SSH_ASKPASS","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-01-04T13:34:59Z","receivedAt":"2012-01-04T13:34:59Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jan 04, 2012 at 09:31:02AM +0100, Sven Strickroth wrote:\n\n> diff --git a/prompt.c b/prompt.c\n> index 72ab9de..e791619 100644\n> --- a/prompt.c\n> +++ b/prompt.c\n> @@ -52,9 +52,17 @@ char *git_prompt(const char *prompt, int flags)\n> \t}\n> \n> \tr = git_terminal_prompt(prompt, flags & PROMPT_ECHO);\n> -\tif (!r)\n> -\t\tdie_errno(\"could not read '%s'\", prompt);\n> -\treturn r;\n> +\tif (r)\n> +\t\treturn r;\n> +\n> +\tif (flags & PROMPT_ASKPASS) {\n> +\t\tconst char *askpass;\n> +\t\taskpass = getenv(\"SSH_ASKPASS\");\n> +\t\tif (askpass && *askpass)\n> +\t\t\treturn do_askpass(askpass, prompt);\n> +\t}\n> +\n> +\tdie_errno(\"could not read '%s'\", prompt);\n> }\n\nWouldn't you also have to drop checking of SSH_ASKPASS in the block\nright before calling git_terminal_prompt (right before the context in\nyour patch)?\n\n-Peff\n"},{"id":"181919","messageId":"4F045E8B.4060200@tu-clausthal.de","threadId":"28961","inReplyTo":"20120104133459.GA6564@sigill.intra.peff.net","subject":"Re: [PATCH 1/2] git-svn, perl/Git.pm: add central method for prompting passwords honoring GIT_ASKPASS and SSH_ASKPASS","fromName":"Sven Strickroth","fromEmail":"sven.strickroth@tu-clausthal.de","sentAt":"2012-01-04T14:13:31Z","receivedAt":"2012-01-04T14:13:31Z","isPatch":true,"sender":{"key":"sven.strickroth@tu-clausthal.de","avatar":null},"body":"Am 04.01.2012 14:34 schrieb Jeff King:\n> Wouldn't you also have to drop checking of SSH_ASKPASS in the block\n> right before calling git_terminal_prompt (right before the context in\n> your patch)?\n\nof course :( Thanks for spotting this...\n\ndiff --git a/prompt.c b/prompt.c\nindex 72ab9de..230ac3c 100644\n--- a/prompt.c\n+++ b/prompt.c\n@@ -45,16 +45,23 @@ char *git_prompt(const char *prompt, int flags)\n \t\taskpass = getenv(\"GIT_ASKPASS\");\n \t\tif (!askpass)\n \t\t\taskpass = askpass_program;\n-\t\tif (!askpass)\n-\t\t\taskpass = getenv(\"SSH_ASKPASS\");\n \t\tif (askpass && *askpass)\n \t\t\treturn do_askpass(askpass, prompt);\n \t}\n\n \tr = git_terminal_prompt(prompt, flags & PROMPT_ECHO);\n-\tif (!r)\n-\t\tdie_errno(\"could not read '%s'\", prompt);\n-\treturn r;\n+\tif (r)\n+\t\treturn r;\n+\n+\tif (flags & PROMPT_ASKPASS) {\n+\t\tconst char *askpass;\n+\n+\t\taskpass = getenv(\"SSH_ASKPASS\");\n+\t\tif (askpass && *askpass)\n+\t\t\treturn do_askpass(askpass, prompt);\n+\t}\n+\n+\tdie_errno(\"could not read '%s'\", prompt);\n }\n\n char *git_getpass(const char *prompt)\n-- \nBest regards,\n Sven Strickroth\n ClamAV, a GPL anti-virus toolkit http://www.clamav.net\n PGP key id F5A9D4C4 @ any key-server\n"},{"id":"181935","messageId":"7vmxa3pa6e.fsf@alter.siamese.dyndns.org","threadId":"28961","inReplyTo":"4F0405D4.9090102@tu-clausthal.de","subject":"Re: [PATCH 1/2] git-svn, perl/Git.pm: add central method for prompting passwords honoring GIT_ASKPASS and SSH_ASKPASS","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-01-04T18:58:01Z","receivedAt":"2012-01-04T18:58:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sven Strickroth writes:\n\n> Some perl versions, like the one from msys, crash sometimes\n> if reading from STDIN wasn't successful and chomp is applied\n> to the variable into which was read.\n\nDepending on how widespread such implementations of Perl are, a patch to\nfix other uses of chomps might deserve to be on the maintenance track\nindependent from this patch series. I seem to find many hits to:\n\n $ git grep -e 'chomp *([^)@]*='\n\nalready in our codebase.\n"},{"id":"181936","messageId":"7vipkrp9pq.fsf@alter.siamese.dyndns.org","threadId":"28961","inReplyTo":"4F040E46.5030001@tu-clausthal.de","subject":"Re: [PATCH 1/2] git-svn, perl/Git.pm: add central method for prompting passwords honoring GIT_ASKPASS and SSH_ASKPASS","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-01-04T19:08:01Z","receivedAt":"2012-01-04T19:08:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sven Strickroth writes:\n\n> Am 04.01.2012 08:55 schrieb Sven Strickroth:\n>> The Git.pm part is easy, but I also tried to update prompt.c (untested).\n>\n> I said \"easy\" and then I mailed the wrong/outdated patch :(\n> I'm sorry for the noise.\n>\n> From: Sven Strickroth \n> Date: Wed, 4 Jan 2012 08:44:48 +0100\n> Subject: [PATCH] Git.pm, prompt: try reading from interactive terminal\n> before using SSH_ASKPASS\n>\n> SVN tries to read reading from interactive terminal before using\n> SSH_ASKPASS helper. This change adjust git to behave the same way.\n\nIt might be an accurate description of what Subversion does (\"tries to\" is\nthe key phrase). I however do not know if it is equivalent to what your\npatch does.\n\nWhen GIT_ASKPASS is not set, the $prompt is given to the standard error\nstream unconditionally and then the \"require Term::ReadKey\" codepath is\nused. When the terminal is unavailable, you might get undef in $ret and be\nable to fall back on SSH_ASKPASS part, but you cannot take back the noise\nyou have given to the standard error stream.\n\nIs there a way to ask Term::ReadKey (or possibly some other module) if we\nwill be able to interact with the terminal _before_ we give that prompt?\n\nThe simplest would be to do this, I would think, but I didn't test it.\n\n\tif (!defined $ret && -t) {\n\t\tprint STDERR $prompt;\n\t\tif ($isPassword) {\n \t...\n\t}\n if (!defined $ret) {\n\t\t$ret = _prompt($ENV{'SSH_ASKPASS'}, $prompt);\n\t}\n"},{"id":"181937","messageId":"4F04A660.4020000@tu-clausthal.de","threadId":"28961","inReplyTo":"7vmxa3pa6e.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/2] git-svn, perl/Git.pm: add central method for prompting passwords honoring GIT_ASKPASS and SSH_ASKPASS","fromName":"Sven Strickroth","fromEmail":"sven.strickroth@tu-clausthal.de","sentAt":"2012-01-04T19:20:00Z","receivedAt":"2012-01-04T19:20:00Z","isPatch":true,"sender":{"key":"sven.strickroth@tu-clausthal.de","avatar":null},"body":"Am 04.01.2012 19:58 schrieb Junio C Hamano:\n> Depending on how widespread such implementations of Perl are, a patch to\n> fix other uses of chomps might deserve to be on the maintenance track\n> independent from this patch series. I seem to find many hits to:\n> \n> $ git grep -e 'chomp *([^)@]*='\n> \n> already in our codebase.\n\nI'm not sure if this is a general chomp problem. I think that this is\nmore related to a variable which is accessed after a readfailure on STDIN.\n\nReported to msys team.\n\n-- \nBest regards,\n Sven Strickroth\n ClamAV, a GPL anti-virus toolkit http://www.clamav.net\n PGP key id F5A9D4C4 @ any key-server\n"},{"id":"182055","messageId":"4F07C9CE.30905@tu-clausthal.de","threadId":"28961","inReplyTo":"7vipkrp9pq.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/2] git-svn, perl/Git.pm: add central method for prompting passwords honoring GIT_ASKPASS and SSH_ASKPASS","fromName":"Sven Strickroth","fromEmail":"sven.strickroth@tu-clausthal.de","sentAt":"2012-01-07T04:27:58Z","receivedAt":"2012-01-07T04:27:58Z","isPatch":true,"sender":{"key":"sven.strickroth@tu-clausthal.de","avatar":null},"body":"Hi,\n\nAm 04.01.2012 20:08 schrieb Junio C Hamano:\n> Is there a way to ask Term::ReadKey (or possibly some other module) if we\n> will be able to interact with the terminal _before_ we give that prompt?\n> \n> The simplest would be to do this, I would think, but I didn't test it.\n> \n> \tif (!defined $ret && -t) {\n> \t\tprint STDERR $prompt;\n> \t\tif ($isPassword) {\n> \t...\n> \t}\n\n-t does not help, but I think it's not a big deal if the prompt is printed on\nthe terminal and also on the ASKPASS-helper.\n\nUsing Term::ReadLine seems to help:\n...\n\t\t$ret = _prompt($ENV{'SSH_ASKPASS'}, $prompt);\n\t}\n\tif (!defined $ret) {\n\t\tuse Term::Readline;\n\t\tmy $term = Term::ReadLine->new(\"Git.pm\");\n\t\tif ($isPassword) {\n\t\t\trequire Term::ReadKey;\n\t\t\tTerm::ReadKey::ReadMode('noecho');\n\t\t}\n\t\t$ret = $term->readline($prompt);\n\t\tif ($isPassword) {\n\t\t\tTerm::ReadKey::ReadMode('restore');\n\t\t\tprint STDERR \"\\n\";\n\t\t\t\n\t\t}\n\t}\n\tif (!defined $ret) {\n\t\t$ret = _prompt($ENV{'SSH_ASKPASS'}, $prompt);\n...\n\nBut I'm not sure if this is what we want, because you can go with the cursor\nover the whole terminal.\n\nA better (working) alternative might be:\n...\n\t\t$ret = _prompt($ENV{'SSH_ASKPASS'}, $prompt);\n\t}\n\tuse Term::Readline;\n\tmy $term = Term::ReadLine->new(\"Git.pm\");\n\tif (!defined $ret && fileno($term->IN)) {\n \t\tprint STDERR $prompt;\n \t\tif ($isPassword) {\n \t...\n \t}\n...\n\n-- \nBest regards,\n Sven Strickroth\n ClamAV, a GPL anti-virus toolkit http://www.clamav.net\n PGP key id F5A9D4C4 @ any key-server\n"},{"id":"184518","messageId":"4F37E2B0.9060007@tu-clausthal.de","threadId":"28961","inReplyTo":"20120103184022.GA20926@sigill.intra.peff.net","subject":"Re: [PATCH 2/2] git-svn, perl/Git.pm: extend and use Git->prompt method for querying users","fromName":"Sven Strickroth","fromEmail":"sven.strickroth@tu-clausthal.de","sentAt":"2012-02-12T16:02:56Z","receivedAt":"2012-02-12T16:02:56Z","isPatch":true,"sender":{"key":"sven.strickroth@tu-clausthal.de","avatar":null},"body":"Hi,\n\nAm 03.01.2012 19:40 schrieb Jeff King:\n> There is one difference between how git and ssh use the ASKPASS\n> variable. In git, we try it _first_, and fall back to asking on the\n> terminal. For ssh, they first try the terminal, and fall back to\n> askpass only when the terminal cannot be opened.\n\nI checked out subversion (svn co\nhttp://svn.apache.org/repos/asf/subversion/trunk subversion) and\nperformed a \"grep ASKPASS * -R\": Only match in\n\"contrib\\client-side\\emacs\\psvn.el\". So I doubt if subversion really\nsupports SSH_ASKPASS.\n\n-- \nBest regards,\n Sven Strickroth\n"},{"id":"184517","messageId":"201202121711.45920.jnareb@gmail.com","threadId":"28961","inReplyTo":"4F37E2B0.9060007@tu-clausthal.de","subject":"Re: [PATCH 2/2] git-svn, perl/Git.pm: extend and use Git->prompt method for querying users","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2012-02-12T16:11:44Z","receivedAt":"2012-02-12T16:11:44Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Sven Strickroth wrote:\n> Am 03.01.2012 19:40 schrieb Jeff King:\n\n> > There is one difference between how git and ssh use the ASKPASS\n> > variable. In git, we try it _first_, and fall back to asking on the\n> > terminal. For ssh, they first try the terminal, and fall back to\n> > askpass only when the terminal cannot be opened.\n> \n> I checked out subversion (svn co\n> http://svn.apache.org/repos/asf/subversion/trunk subversion) and\n> performed a \"grep ASKPASS * -R\": Only match in\n> \"contrib\\client-side\\emacs\\psvn.el\". So I doubt if subversion really\n> supports SSH_ASKPASS.\n\nDoesn't Subversion use SSH directly? If it is so, the question is\nabout how SSH itself supports SSH_ASKPASS.\n\n-- \nJakub Narebski\nPoland\n"},{"id":"184520","messageId":"4F37E843.6070107@tu-clausthal.de","threadId":"28961","inReplyTo":"201202121711.45920.jnareb@gmail.com","subject":"Re: [PATCH 2/2] git-svn, perl/Git.pm: extend and use Git->prompt method for querying users","fromName":"Sven Strickroth","fromEmail":"sven.strickroth@tu-clausthal.de","sentAt":"2012-02-12T16:26:43Z","receivedAt":"2012-02-12T16:26:43Z","isPatch":true,"sender":{"key":"sven.strickroth@tu-clausthal.de","avatar":null},"body":"Am 12.02.2012 17:11 schrieb Jakub Narebski:\n> Doesn't Subversion use SSH directly? If it is so, the question is\n> about how SSH itself supports SSH_ASKPASS.\n\nOh sorry, I mixed up SSH and SVN_ASKPASS. :( Of couse SSH_ASKPASS is\nprovided by the ssh-client itself.\n\n-- \nBest regards,\n Sven Strickroth\n"},{"id":"184713","messageId":"20120214222055.GE24802@sigill.intra.peff.net","threadId":"28961","inReplyTo":"4F37E843.6070107@tu-clausthal.de","subject":"Re: [PATCH 2/2] git-svn, perl/Git.pm: extend and use Git->prompt method for querying users","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-02-14T22:20:55Z","receivedAt":"2012-02-14T22:20:55Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Feb 12, 2012 at 05:26:43PM +0100, Sven Strickroth wrote:\n\n> Am 12.02.2012 17:11 schrieb Jakub Narebski:\n> > Doesn't Subversion use SSH directly? If it is so, the question is\n> > about how SSH itself supports SSH_ASKPASS.\n> \n> Oh sorry, I mixed up SSH and SVN_ASKPASS. :( Of couse SSH_ASKPASS is\n> provided by the ssh-client itself.\n\nThat raises an interesting point for git (I don't remember seeing this\nin the previous discussion, so apologies if I'm repeating). We sometimes\nuse SSH_ASKPASS for internal prompting, and sometimes via calling out to\nssh. So forgetting about git being consistent with the rest of the\nworld for a moment, I think we are inconsistent with ourselves. E.g.:\n\n export SSH_ASKPASS=whatever\n\n # this will try the terminal first, then SSH_ASKPASS, because it is\n # ssh doing the asking\n git push ssh://example.com/repo.git\n\n # this will try SSH_ASKPASS first, then the terminal, because git is\n # doing the asking\n git push https://example.com/repo.git\n\nSo now I'm more convinced than ever that the order should be\nGIT_ASKPASS, terminal, SSH_ASKPASS.\n\n-Peff\n"},{"id":"184720","messageId":"7v4nut59hw.fsf@alter.siamese.dyndns.org","threadId":"28961","inReplyTo":"20120214222055.GE24802@sigill.intra.peff.net","subject":"Re: [PATCH 2/2] git-svn, perl/Git.pm: extend and use Git->prompt method for querying users","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-02-14T22:35:23Z","receivedAt":"2012-02-14T22:35:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> export SSH_ASKPASS=whatever\n>\n> # this will try the terminal first, then SSH_ASKPASS, because it is\n> # ssh doing the asking\n> git push ssh://example.com/repo.git\n\nSorry, you lost me here. Does \"ssh example.com\" consult the terminal\nfirst and then fall back to SSH_ASKPASS environment variable?\n\nI was under the impression that SSH_ASKPASS was to either give hands-free\naccess to the keychain or give GUI experience so that people do not have\nto type from their terminals...\n\n> # this will try SSH_ASKPASS first, then the terminal, because git is\n> # doing the asking\n> git push https://example.com/repo.git\n>\n> So now I'm more convinced than ever that the order should be\n> GIT_ASKPASS, terminal, SSH_ASKPASS.\n"},{"id":"184722","messageId":"20120214224741.GH24802@sigill.intra.peff.net","threadId":"28961","inReplyTo":"7v4nut59hw.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/2] git-svn, perl/Git.pm: extend and use Git->prompt method for querying users","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-02-14T22:47:41Z","receivedAt":"2012-02-14T22:47:41Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Feb 14, 2012 at 02:35:23PM -0800, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > export SSH_ASKPASS=whatever\n> >\n> > # this will try the terminal first, then SSH_ASKPASS, because it is\n> > # ssh doing the asking\n> > git push ssh://example.com/repo.git\n> \n> Sorry, you lost me here. Does \"ssh example.com\" consult the terminal\n> first and then fall back to SSH_ASKPASS environment variable?\n\nYes. Try:\n\n SSH_ASKPASS=cat ssh example.com\n\nwhere cat is any program whose running you could detect (in this case,\nbecause cat will complain to stderr about not being able to open its\nargument). And \"example.com\" must be something that you can actually\nmake an ssh connection to.\n\nCompare with:\n\n SSH_ASKPASS=cat setsid ssh example.com\n\nwhich will realize it has no controlling tty, and fallback to\nSSH_ASKPASS.\n\nI actually find the behavior slightly annoying (because sometimes you do\nhave a controlling terminal, but it is not accessible or obvious to the\nuser). But I think it's important to be consistent, and provide\nGIT_ASKPASS for people who really want to say \"no, don't even bother\nwith the terminal\".\n\n> I was under the impression that SSH_ASKPASS was to either give hands-free\n> access to the keychain or give GUI experience so that people do not have\n> to type from their terminals...\n\nNot exactly. It's useful in two situations (in my experience):\n\n 1. A GUI program spawns an ssh tunnel, and there is no tty on which to\n prompt the user.\n\n 2. Populating an ssh-agent via ssh-add during the user's login\n sequence.\n\nIt doesn't work to give a GUI experience to creating a remote terminal\nsession, since if you have a terminal, it will always prefer to prompt\non the terminal.\n\n-Peff\n"},{"id":"200667","messageId":"50704BB8.1020603@tu-clausthal.de","threadId":"28961","inReplyTo":"7vy5toqqab.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/2] git-svn, perl/Git.pm: extend and use Git->prompt method for querying users","fromName":"Sven Strickroth","fromEmail":"sven.strickroth@tu-clausthal.de","sentAt":"2012-10-06T15:18:16Z","receivedAt":"2012-10-06T15:18:16Z","isPatch":true,"sender":{"key":"sven.strickroth@tu-clausthal.de","avatar":null},"body":"Am 04.01.2012 01:12 schrieb Junio C Hamano:\n> Now \"prompt\" is no longer a method but is merely a helper function, so\n> I've queued this (and 1/2 rewrite we discussed in a separate thread) to\n> 'pu' after rewording the commit log message.\n> \n> Thanks.\n\nIs there a reason why these changes did not get merged? The issues are\nstill there.\n\n-- \nBest regards,\n Sven Strickroth\n PGP key id F5A9D4C4 @ any key-server\n"},{"id":"200679","messageId":"7vmwzzqwud.fsf@alter.siamese.dyndns.org","threadId":"28961","inReplyTo":"50704BB8.1020603@tu-clausthal.de","subject":"Re: [PATCH 2/2] git-svn, perl/Git.pm: extend and use Git->prompt method for querying users","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-10-06T18:28:10Z","receivedAt":"2012-10-06T18:28:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sven Strickroth writes:\n\n> Am 04.01.2012 01:12 schrieb Junio C Hamano:\n>> Now \"prompt\" is no longer a method but is merely a helper function, so\n>> I've queued this (and 1/2 rewrite we discussed in a separate thread) to\n>> 'pu' after rewording the commit log message.\n>> \n>> Thanks.\n>\n> Is there a reason why these changes did not get merged? The issues are\n> still there.\n\nIt is either that it was simply forgotten, or after I wrote the part\nyou quoted early in January there were discussions later that showed\nthe patch was not desirable for some reason. I do not recall which.\n"},{"id":"202875","messageId":"509FD4F6.5050606@gym-oha.de","threadId":"28961","inReplyTo":"7vmwzzqwud.fsf@alter.siamese.dyndns.org","subject":"[PATCH 0/2] second try","fromName":"Sven Strickroth","fromEmail":"sstrickroth@gym-oha.de","sentAt":"2012-11-11T16:40:22Z","receivedAt":"2012-11-11T16:40:22Z","isPatch":true,"sender":{"key":"sstrickroth@gym-oha.de","avatar":null},"body":"Hi,\n\nAm 06.10.2012 20:28 schrieb Junio C Hamano:\n> It is either that it was simply forgotten, or after I wrote the part\n> you quoted early in January there were discussions later that showed\n> the patch was not desirable for some reason. I do not recall which.\n\nI noticed no threads about possible problems, so I try again.\n\n-- \nBest regards,\n Sven Strickroth\n PGP key id F5A9D4C4 @ any key-server\n"},{"id":"202876","messageId":"509FD513.4070206@tu-clausthal.de","threadId":"28961","inReplyTo":"7vmwzzqwud.fsf@alter.siamese.dyndns.org","subject":"[PATCH 1/2] git-svn, perl/Git.pm: add central method for prompting passwords honoring GIT_ASKPASS and SSH_ASKPASS","fromName":"Sven Strickroth","fromEmail":"sven.strickroth@tu-clausthal.de","sentAt":"2012-11-11T16:40:51Z","receivedAt":"2012-11-11T16:40:51Z","isPatch":true,"sender":{"key":"sven.strickroth@tu-clausthal.de","avatar":null},"body":"git-svn reads passwords from an interactive terminal or by using\nGIT_ASKPASS helper tool. But if GIT_ASKPASS environment variable is not\nset, git-svn does not try to use SSH_ASKPASS as git-core does. This\ncause GUIs (w/o STDIN connected) to hang waiting forever for git-svn to\ncomplete (http://code.google.com/p/tortoisegit/issues/detail?id=967).\n\nCommit 56a853b62c0ae7ebaad0a7a0a704f5ef561eb795 also tried to solve\nthis issue, but was incomplete as described above.\n\nInstead of using hand-rolled prompt-response code that only works with the\ninteractive terminal, a reusable prompt() method is introduced in this commit.\nThis change also adds a fallback to SSH_ASKPASS.\n\nSigned-off-by: Sven Strickroth \n---\n perl/Git.pm | 48 +++++++++++++++++++++++++++++++++++++++++++++++-\n perl/Git/SVN/Prompt.pm | 20 +-------------------\n 2 files changed, 48 insertions(+), 20 deletions(-)\n\ndiff --git a/perl/Git.pm b/perl/Git.pm\nindex 497f420..0a0fe91 100644\n--- a/perl/Git.pm\n+++ b/perl/Git.pm\n@@ -58,7 +58,7 @@ require Exporter;\n command_output_pipe command_input_pipe command_close_pipe\n command_bidi_pipe command_close_bidi_pipe\n version exec_path html_path hash_object git_cmd_try\n- remote_refs\n+ remote_refs prompt\n temp_acquire temp_release temp_reset temp_path);\n \n \n@@ -511,6 +511,52 @@ C). Useful mostly only internally.\n \n sub html_path { command_oneline('--html-path') }\n \n+=item prompt ( PROMPT )\n+\n+Query user C and return answer from user.\n+\n+Honours GIT_ASKPASS, SSH_ASKPASS environment variables for querying\n+the user. If no *_ASKPASS variable is set or an error occoured,\n+the terminal is tried as a fallback.\n+\n+=cut\n+\n+sub prompt {\n+\tmy ($prompt) = @_;\n+\tmy $ret;\n+\tif (exists $ENV{'GIT_ASKPASS'}) {\n+\t\t$ret = _prompt($ENV{'GIT_ASKPASS'}, $prompt);\n+\t}\n+\tif (!defined $ret && exists $ENV{'SSH_ASKPASS'}) {\n+\t\t$ret = _prompt($ENV{'SSH_ASKPASS'}, $prompt);\n+\t}\n+\tif (!defined $ret) {\n+\t\tprint STDERR $prompt;\n+\t\tSTDERR->flush;\n+\t\trequire Term::ReadKey;\n+\t\tTerm::ReadKey::ReadMode('noecho');\n+\t\t$ret = '';\n+\t\twhile (defined(my $key = Term::ReadKey::ReadKey(0))) {\n+\t\t\tlast if $key =~ /[\\012\\015]/; # \\n\\r\n+\t\t\t$ret .= $key;\n+\t\t}\n+\t\tTerm::ReadKey::ReadMode('restore');\n+\t\tprint STDERR \"\\n\";\n+\t\tSTDERR->flush;\n+\t}\n+\treturn $ret;\n+}\n+\n+sub _prompt {\n+\tmy ($askpass, $prompt) = @_;\n+\treturn unless length $askpass;\n+\tmy $ret;\n+\topen my $fh, \"-|\", $askpass, $prompt or return;\n+\t$ret = <$fh>;\n+\t$ret =~ s/[\\015\\012]//g; # strip \\r\\n, chomp does not work on all systems (i.e. windows) as expected\n+\tclose ($fh);\n+\treturn $ret;\n+}\n \n =item repo_path ()\n \ndiff --git a/perl/Git/SVN/Prompt.pm b/perl/Git/SVN/Prompt.pm\nindex 3a6f8af..a2cbcc8 100644\n--- a/perl/Git/SVN/Prompt.pm\n+++ b/perl/Git/SVN/Prompt.pm\n@@ -120,25 +120,7 @@ sub username {\n \n sub _read_password {\n \tmy ($prompt, $realm) = @_;\n-\tmy $password = '';\n-\tif (exists $ENV{GIT_ASKPASS}) {\n-\t\topen(PH, \"-|\", $ENV{GIT_ASKPASS}, $prompt);\n-\t\t$password = ;\n-\t\t$password =~ s/[\\012\\015]//; # \\n\\r\n-\t\tclose(PH);\n-\t} else {\n-\t\tprint STDERR $prompt;\n-\t\tSTDERR->flush;\n-\t\trequire Term::ReadKey;\n-\t\tTerm::ReadKey::ReadMode('noecho');\n-\t\twhile (defined(my $key = Term::ReadKey::ReadKey(0))) {\n-\t\t\tlast if $key =~ /[\\012\\015]/; # \\n\\r\n-\t\t\t$password .= $key;\n-\t\t}\n-\t\tTerm::ReadKey::ReadMode('restore');\n-\t\tprint STDERR \"\\n\";\n-\t\tSTDERR->flush;\n-\t}\n+\tmy $password = Git::prompt($prompt);\n \t$password;\n }\n \n-- \nBest regards,\n Sven Strickroth\n PGP key id F5A9D4C4 @ any key-server\n"},{"id":"202880","messageId":"509FD527.70605@tu-clausthal.de","threadId":"28961","inReplyTo":"7vmwzzqwud.fsf@alter.siamese.dyndns.org","subject":"[PATCH 2/2] git-svn, perl/Git.pm: extend and use Git->prompt method for querying users","fromName":"Sven Strickroth","fromEmail":"sven.strickroth@tu-clausthal.de","sentAt":"2012-11-11T16:41:11Z","receivedAt":"2012-11-11T16:41:11Z","isPatch":true,"sender":{"key":"sven.strickroth@tu-clausthal.de","avatar":null},"body":"git-svn reads usernames and other user queries from an interactive\nterminal. This cause GUIs (w/o STDIN connected) to hang waiting forever\nfor git-svn to complete (http://code.google.com/p/tortoisegit/issues/detail?id=967).\n\nThis change extends the Git::prompt helper, so that it can also be used\nfor non password queries, and makes use of it instead of using\nhand-rolled prompt-response code that only works with the interactive\nterminal.\n\nSigned-off-by: Sven Strickroth \n---\n perl/Git.pm | 28 +++++++++++++++++-----------\n perl/Git/SVN/Prompt.pm | 16 +++++++---------\n 2 files changed, 24 insertions(+), 20 deletions(-)\n\ndiff --git a/perl/Git.pm b/perl/Git.pm\nindex 0a0fe91..3200f4d 100644\n--- a/perl/Git.pm\n+++ b/perl/Git.pm\n@@ -511,18 +511,19 @@ C). Useful mostly only internally.\n \n sub html_path { command_oneline('--html-path') }\n \n-=item prompt ( PROMPT )\n+=item prompt ( PROMPT , ISPASSWORD )\n \n Query user C and return answer from user.\n \n Honours GIT_ASKPASS, SSH_ASKPASS environment variables for querying\n the user. If no *_ASKPASS variable is set or an error occoured,\n the terminal is tried as a fallback.\n+If C is set and true, the terminal disables echo.\n \n =cut\n \n sub prompt {\n-\tmy ($prompt) = @_;\n+\tmy ($prompt, $isPassword) = @_;\n \tmy $ret;\n \tif (exists $ENV{'GIT_ASKPASS'}) {\n \t\t$ret = _prompt($ENV{'GIT_ASKPASS'}, $prompt);\n@@ -533,16 +534,20 @@ sub prompt {\n \tif (!defined $ret) {\n \t\tprint STDERR $prompt;\n \t\tSTDERR->flush;\n-\t\trequire Term::ReadKey;\n-\t\tTerm::ReadKey::ReadMode('noecho');\n-\t\t$ret = '';\n-\t\twhile (defined(my $key = Term::ReadKey::ReadKey(0))) {\n-\t\t\tlast if $key =~ /[\\012\\015]/; # \\n\\r\n-\t\t\t$ret .= $key;\n+\t\tif (defined $isPassword && $isPassword) {\n+\t\t\trequire Term::ReadKey;\n+\t\t\tTerm::ReadKey::ReadMode('noecho');\n+\t\t\t$ret = '';\n+\t\t\twhile (defined(my $key = Term::ReadKey::ReadKey(0))) {\n+\t\t\t\tlast if $key =~ /[\\012\\015]/; # \\n\\r\n+\t\t\t\t$ret .= $key;\n+\t\t\t}\n+\t\t\tTerm::ReadKey::ReadMode('restore');\n+\t\t\tprint STDERR \"\\n\";\n+\t\t\tSTDERR->flush;\n+\t\t} else {\n+\t\t\tchomp($ret = );\n \t\t}\n-\t\tTerm::ReadKey::ReadMode('restore');\n-\t\tprint STDERR \"\\n\";\n-\t\tSTDERR->flush;\n \t}\n \treturn $ret;\n }\n@@ -550,6 +555,7 @@ sub prompt {\n sub _prompt {\n \tmy ($askpass, $prompt) = @_;\n \treturn unless length $askpass;\n+\t$prompt =~ s/\\n/ /g;\n \tmy $ret;\n \topen my $fh, \"-|\", $askpass, $prompt or return;\n \t$ret = <$fh>;\ndiff --git a/perl/Git/SVN/Prompt.pm b/perl/Git/SVN/Prompt.pm\nindex a2cbcc8..74daa7a 100644\n--- a/perl/Git/SVN/Prompt.pm\n+++ b/perl/Git/SVN/Prompt.pm\n@@ -62,16 +62,16 @@ sub ssl_server_trust {\n \t issuer_dname fingerprint);\n \tmy $choice;\n prompt:\n-\tprint STDERR $may_save ?\n+\tmy $options = $may_save ?\n \t \"(R)eject, accept (t)emporarily or accept (p)ermanently? \" :\n \t \"(R)eject or accept (t)emporarily? \";\n \tSTDERR->flush;\n-\t$choice = lc(substr( || 'R', 0, 1));\n-\tif ($choice =~ /^t$/i) {\n+\t$choice = lc(substr(Git::prompt(\"Certificate problem.\\n\" . $options) || 'R', 0, 1));\n+\tif ($choice eq 't') {\n \t\t$cred->may_save(undef);\n-\t} elsif ($choice =~ /^r$/i) {\n+\t} elsif ($choice eq 'r') {\n \t\treturn -1;\n-\t} elsif ($may_save && $choice =~ /^p$/i) {\n+\t} elsif ($may_save && $choice eq 'p') {\n \t\t$cred->may_save($may_save);\n \t} else {\n \t\tgoto prompt;\n@@ -109,9 +109,7 @@ sub username {\n \tif (defined $_username) {\n \t\t$username = $_username;\n \t} else {\n-\t\tprint STDERR \"Username: \";\n-\t\tSTDERR->flush;\n-\t\tchomp($username = );\n+\t\t$username = Git::prompt(\"Username: \");\n \t}\n \t$cred->username($username);\n \t$cred->may_save($may_save);\n@@ -120,7 +118,7 @@ sub username {\n \n sub _read_password {\n \tmy ($prompt, $realm) = @_;\n-\tmy $password = Git::prompt($prompt);\n+\tmy $password = Git::prompt($prompt, 1);\n \t$password;\n }\n \n-- \nBest regards,\n Sven Strickroth\n PGP key id F5A9D4C4 @ any key-server\n"},{"id":"203770","messageId":"50B11AF5.2090701@tu-clausthal.de","threadId":"28961","inReplyTo":"509FD4F6.5050606@gym-oha.de","subject":"Re: [PATCH 0/2] second try","fromName":"Sven Strickroth","fromEmail":"sven.strickroth@tu-clausthal.de","sentAt":"2012-11-24T19:07:33Z","receivedAt":"2012-11-24T19:07:33Z","isPatch":true,"sender":{"key":"sven.strickroth@tu-clausthal.de","avatar":null},"body":"Hi,\n\nAm 11.11.2012 17:40 schrieb Sven Strickroth:\n> Am 06.10.2012 20:28 schrieb Junio C Hamano:\n>> It is either that it was simply forgotten, or after I wrote the part\n>> you quoted early in January there were discussions later that showed\n>> the patch was not desirable for some reason. I do not recall which.\n> \n> I noticed no threads about possible problems, so I try again.\n\nOn November 11th I submitted the updated patches again, however, without\nany reaction or comments.\n\ngit pull git://github.com/csware/git.git gitsvn-askpass\n\nMaybe the reason that this was forgotten was, that the git-svn code was\nrearranged and splitted into different files in the past and the code\ndid not apply cleanly any more.\n\nSo, what's the status of this issue? It's now open for nearly one year.\n\n-- \nBest regards,\n Sven Strickroth\n PGP key id F5A9D4C4 @ any key-server\n"},{"id":"203850","messageId":"7vtxsdvug3.fsf@alter.siamese.dyndns.org","threadId":"28961","inReplyTo":"50B11AF5.2090701@tu-clausthal.de","subject":"Re: [PATCH 0/2] second try","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-11-26T04:50:36Z","receivedAt":"2012-11-26T04:50:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sven Strickroth writes:\n\n> Am 11.11.2012 17:40 schrieb Sven Strickroth:\n>> Am 06.10.2012 20:28 schrieb Junio C Hamano:\n>>> It is either that it was simply forgotten, or after I wrote the part\n>>> you quoted early in January there were discussions later that showed\n>>> the patch was not desirable for some reason. I do not recall which.\n>> \n>> I noticed no threads about possible problems, so I try again.\n>\n> On November 11th I submitted the updated patches again, however, without\n> any reaction or comments.\n\nI think between Peff and me it fell in the cracks during the\nhand-off; I do not know about the others, probably people did not\nfind it interesting perhaps?\n\nI'll add Eric Wong (git-svn submaintainer) to Cc.\n"},{"id":"205060","messageId":"50CF4020.4090901@tu-clausthal.de","threadId":"28961","inReplyTo":"7vtxsdvug3.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 0/2] second try","fromName":"Sven Strickroth","fromEmail":"sven.strickroth@tu-clausthal.de","sentAt":"2012-12-17T15:54:08Z","receivedAt":"2012-12-17T15:54:08Z","isPatch":true,"sender":{"key":"sven.strickroth@tu-clausthal.de","avatar":null},"body":"Hi,\n\nAm 26.11.2012 05:50 schrieb Junio C Hamano:\n> Sven Strickroth writes:\n> \n>> Am 11.11.2012 17:40 schrieb Sven Strickroth:\n>>> Am 06.10.2012 20:28 schrieb Junio C Hamano:\n>>>> It is either that it was simply forgotten, or after I wrote the part\n>>>> you quoted early in January there were discussions later that showed\n>>>> the patch was not desirable for some reason. I do not recall which.\n>>>\n>>> I noticed no threads about possible problems, so I try again.\n>>\n>> On November 11th I submitted the updated patches again, however, without\n>> any reaction or comments.\n> \n> I think between Peff and me it fell in the cracks during the\n> hand-off; I do not know about the others, probably people did not\n> find it interesting perhaps?\n> \n> I'll add Eric Wong (git-svn submaintainer) to Cc.\n\nI received no feedback, so is there any progress on this issue?\n\nI'd really appreciate if we could fix it soon.\n\n-- \nBest regards,\n Sven Strickroth\n PGP key id F5A9D4C4 @ any key-server\n"},{"id":"205071","messageId":"7vehiol9w2.fsf@alter.siamese.dyndns.org","threadId":"28961","inReplyTo":"50CF4020.4090901@tu-clausthal.de","subject":"Re: [PATCH 0/2] second try","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-12-17T20:08:13Z","receivedAt":"2012-12-17T20:08:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sven Strickroth writes:\n\n> Am 26.11.2012 05:50 schrieb Junio C Hamano:\n>> I think between Peff and me it fell in the cracks during the\n>> hand-off; I do not know about the others, probably people did not\n>> find it interesting perhaps?\n>> \n>> I'll add Eric Wong (git-svn submaintainer) to Cc.\n>\n> I received no feedback, so is there any progress on this issue?\n\nI took a look at it, and from the code-cleanness point of view, I\nthink it loos more or less right, even though I'd prefer to see the\n\"fall back on SSH_ASKPASS\" bit as a separate patch, either before or\nafter [1/2] that moves the logic to a separate helper function.\n"},{"id":"205093","messageId":"50CFB8BD.5040006@tu-clausthal.de","threadId":"28961","inReplyTo":"7vehiol9w2.fsf@alter.siamese.dyndns.org","subject":"[PATCH 1/3] git-svn, perl/Git.pm: add central method for prompting passwords","fromName":"Sven Strickroth","fromEmail":"sven.strickroth@tu-clausthal.de","sentAt":"2012-12-18T00:28:45Z","receivedAt":"2012-12-18T00:28:45Z","isPatch":true,"sender":{"key":"sven.strickroth@tu-clausthal.de","avatar":null},"body":"git-svn reads passwords from an interactive terminal or by using\nGIT_ASKPASS helper tool. This cause GUIs (w/o STDIN connected) to hang\nwaiting forever for git-svn to complete\n(http://code.google.com/p/tortoisegit/issues/detail?id=967).\n\nCommit 56a853b62c0ae7ebaad0a7a0a704f5ef561eb795 also tried to solve\nthis issue, but was incomplete as described above.\n\nInstead of using hand-rolled prompt-response code that only works with the\ninteractive terminal, a reusable prompt() method is introduced in this commit.\n\nSigned-off-by: Sven Strickroth \n---\n perl/Git.pm | 45 ++++++++++++++++++++++++++++++++++++++++++++-\n perl/Git/SVN/Prompt.pm | 20 +-------------------\n 2 files changed, 45 insertions(+), 20 deletions(-)\n\ndiff --git a/perl/Git.pm b/perl/Git.pm\nindex 497f420..72e93c7 100644\n--- a/perl/Git.pm\n+++ b/perl/Git.pm\n@@ -58,7 +58,7 @@ require Exporter;\n command_output_pipe command_input_pipe command_close_pipe\n command_bidi_pipe command_close_bidi_pipe\n version exec_path html_path hash_object git_cmd_try\n- remote_refs\n+ remote_refs prompt\n temp_acquire temp_release temp_reset temp_path);\n \n \n@@ -511,6 +511,49 @@ C). Useful mostly only internally.\n \n sub html_path { command_oneline('--html-path') }\n \n+=item prompt ( PROMPT )\n+\n+Query user C and return answer from user.\n+\n+Honours GIT_ASKPASS environment variable for querying\n+the user. If no GIT_ASKPASS variable is set or an error occoured,\n+the terminal is tried as a fallback.\n+\n+=cut\n+\n+sub prompt {\n+\tmy ($prompt) = @_;\n+\tmy $ret;\n+\tif (exists $ENV{'GIT_ASKPASS'}) {\n+\t\t$ret = _prompt($ENV{'GIT_ASKPASS'}, $prompt);\n+\t}\n+\tif (!defined $ret) {\n+\t\tprint STDERR $prompt;\n+\t\tSTDERR->flush;\n+\t\trequire Term::ReadKey;\n+\t\tTerm::ReadKey::ReadMode('noecho');\n+\t\t$ret = '';\n+\t\twhile (defined(my $key = Term::ReadKey::ReadKey(0))) {\n+\t\t\tlast if $key =~ /[\\012\\015]/; # \\n\\r\n+\t\t\t$ret .= $key;\n+\t\t}\n+\t\tTerm::ReadKey::ReadMode('restore');\n+\t\tprint STDERR \"\\n\";\n+\t\tSTDERR->flush;\n+\t}\n+\treturn $ret;\n+}\n+\n+sub _prompt {\n+\tmy ($askpass, $prompt) = @_;\n+\treturn unless length $askpass;\n+\tmy $ret;\n+\topen my $fh, \"-|\", $askpass, $prompt or return;\n+\t$ret = <$fh>;\n+\t$ret =~ s/[\\015\\012]//g; # strip \\r\\n, chomp does not work on all systems (i.e. windows) as expected\n+\tclose ($fh);\n+\treturn $ret;\n+}\n \n =item repo_path ()\n \ndiff --git a/perl/Git/SVN/Prompt.pm b/perl/Git/SVN/Prompt.pm\nindex 3a6f8af..a2cbcc8 100644\n--- a/perl/Git/SVN/Prompt.pm\n+++ b/perl/Git/SVN/Prompt.pm\n@@ -120,25 +120,7 @@ sub username {\n \n sub _read_password {\n \tmy ($prompt, $realm) = @_;\n-\tmy $password = '';\n-\tif (exists $ENV{GIT_ASKPASS}) {\n-\t\topen(PH, \"-|\", $ENV{GIT_ASKPASS}, $prompt);\n-\t\t$password = ;\n-\t\t$password =~ s/[\\012\\015]//; # \\n\\r\n-\t\tclose(PH);\n-\t} else {\n-\t\tprint STDERR $prompt;\n-\t\tSTDERR->flush;\n-\t\trequire Term::ReadKey;\n-\t\tTerm::ReadKey::ReadMode('noecho');\n-\t\twhile (defined(my $key = Term::ReadKey::ReadKey(0))) {\n-\t\t\tlast if $key =~ /[\\012\\015]/; # \\n\\r\n-\t\t\t$password .= $key;\n-\t\t}\n-\t\tTerm::ReadKey::ReadMode('restore');\n-\t\tprint STDERR \"\\n\";\n-\t\tSTDERR->flush;\n-\t}\n+\tmy $password = Git::prompt($prompt);\n \t$password;\n }\n \n-- \nBest regards,\n Sven Strickroth\n PGP key id F5A9D4C4 @ any key-server\n"},{"id":"205092","messageId":"50CFB8BF.4000405@tu-clausthal.de","threadId":"28961","inReplyTo":"7vehiol9w2.fsf@alter.siamese.dyndns.org","subject":"[PATCH 2/3] perl/Git.pm: Honor SSH_ASKPASS as fallback if GIT_ASKPASS is not set","fromName":"Sven Strickroth","fromEmail":"sven.strickroth@tu-clausthal.de","sentAt":"2012-12-18T00:28:47Z","receivedAt":"2012-12-18T00:28:47Z","isPatch":true,"sender":{"key":"sven.strickroth@tu-clausthal.de","avatar":null},"body":"If GIT_ASKPASS environment variable is not set, git-svn does not try to use\nSSH_ASKPASS as git-core does. This change adds a fallback to SSH_ASKPASS.\n\nSigned-off-by: Sven Strickroth \n---\n perl/Git.pm | 7 +++++--\n 1 file changed, 5 insertions(+), 2 deletions(-)\n\ndiff --git a/perl/Git.pm b/perl/Git.pm\nindex 72e93c7..8dfca65 100644\n--- a/perl/Git.pm\n+++ b/perl/Git.pm\n@@ -515,8 +515,8 @@ sub html_path { command_oneline('--html-path') }\n\n Query user C and return answer from user.\n\n-Honours GIT_ASKPASS environment variable for querying\n-the user. If no GIT_ASKPASS variable is set or an error occoured,\n+Honours GIT_ASKPASS and SSH_ASKPASS environment variables for querying\n+the user. If no *_ASKPASS variable is set or an error occoured,\n the terminal is tried as a fallback.\n\n =cut\n@@ -527,6 +527,9 @@ sub prompt {\n \tif (exists $ENV{'GIT_ASKPASS'}) {\n \t\t$ret = _prompt($ENV{'GIT_ASKPASS'}, $prompt);\n \t}\n+\tif (!defined $ret && exists $ENV{'SSH_ASKPASS'}) {\n+\t\t$ret = _prompt($ENV{'SSH_ASKPASS'}, $prompt);\n+\t}\n \tif (!defined $ret) {\n \t\tprint STDERR $prompt;\n \t\tSTDERR->flush;\n-- \nBest regards,\n Sven Strickroth\n PGP key id F5A9D4C4 @ any key-server\n"},{"id":"205094","messageId":"50CFB8C0.5040107@tu-clausthal.de","threadId":"28961","inReplyTo":"7vehiol9w2.fsf@alter.siamese.dyndns.org","subject":"[PATCH 3/3] git-svn, perl/Git.pm: extend and use Git->prompt method for querying users","fromName":"Sven Strickroth","fromEmail":"sven.strickroth@tu-clausthal.de","sentAt":"2012-12-18T00:28:48Z","receivedAt":"2012-12-18T00:28:48Z","isPatch":true,"sender":{"key":"sven.strickroth@tu-clausthal.de","avatar":null},"body":"git-svn reads usernames and other user queries from an interactive\nterminal. This cause GUIs (w/o STDIN connected) to hang waiting forever\nfor git-svn to complete (http://code.google.com/p/tortoisegit/issues/detail?id=967).\n\nThis change extends the Git::prompt helper, so that it can also be used\nfor non password queries, and makes use of it instead of using\nhand-rolled prompt-response code that only works with the interactive\nterminal.\n\nSigned-off-by: Sven Strickroth \n---\n perl/Git.pm | 28 +++++++++++++++++-----------\n perl/Git/SVN/Prompt.pm | 16 +++++++---------\n 2 files changed, 24 insertions(+), 20 deletions(-)\n\ndiff --git a/perl/Git.pm b/perl/Git.pm\nindex 8dfca65..931047c 100644\n--- a/perl/Git.pm\n+++ b/perl/Git.pm\n@@ -511,18 +511,19 @@ C). Useful mostly only internally.\n \n sub html_path { command_oneline('--html-path') }\n \n-=item prompt ( PROMPT )\n+=item prompt ( PROMPT , ISPASSWORD )\n \n Query user C and return answer from user.\n \n Honours GIT_ASKPASS and SSH_ASKPASS environment variables for querying\n the user. If no *_ASKPASS variable is set or an error occoured,\n the terminal is tried as a fallback.\n+If C is set and true, the terminal disables echo.\n \n =cut\n \n sub prompt {\n-\tmy ($prompt) = @_;\n+\tmy ($prompt, $isPassword) = @_;\n \tmy $ret;\n \tif (exists $ENV{'GIT_ASKPASS'}) {\n \t\t$ret = _prompt($ENV{'GIT_ASKPASS'}, $prompt);\n@@ -533,16 +534,20 @@ sub prompt {\n \tif (!defined $ret) {\n \t\tprint STDERR $prompt;\n \t\tSTDERR->flush;\n-\t\trequire Term::ReadKey;\n-\t\tTerm::ReadKey::ReadMode('noecho');\n-\t\t$ret = '';\n-\t\twhile (defined(my $key = Term::ReadKey::ReadKey(0))) {\n-\t\t\tlast if $key =~ /[\\012\\015]/; # \\n\\r\n-\t\t\t$ret .= $key;\n+\t\tif (defined $isPassword && $isPassword) {\n+\t\t\trequire Term::ReadKey;\n+\t\t\tTerm::ReadKey::ReadMode('noecho');\n+\t\t\t$ret = '';\n+\t\t\twhile (defined(my $key = Term::ReadKey::ReadKey(0))) {\n+\t\t\t\tlast if $key =~ /[\\012\\015]/; # \\n\\r\n+\t\t\t\t$ret .= $key;\n+\t\t\t}\n+\t\t\tTerm::ReadKey::ReadMode('restore');\n+\t\t\tprint STDERR \"\\n\";\n+\t\t\tSTDERR->flush;\n+\t\t} else {\n+\t\t\tchomp($ret = );\n \t\t}\n-\t\tTerm::ReadKey::ReadMode('restore');\n-\t\tprint STDERR \"\\n\";\n-\t\tSTDERR->flush;\n \t}\n \treturn $ret;\n }\n@@ -550,6 +555,7 @@ sub prompt {\n sub _prompt {\n \tmy ($askpass, $prompt) = @_;\n \treturn unless length $askpass;\n+\t$prompt =~ s/\\n/ /g;\n \tmy $ret;\n \topen my $fh, \"-|\", $askpass, $prompt or return;\n \t$ret = <$fh>;\ndiff --git a/perl/Git/SVN/Prompt.pm b/perl/Git/SVN/Prompt.pm\nindex a2cbcc8..74daa7a 100644\n--- a/perl/Git/SVN/Prompt.pm\n+++ b/perl/Git/SVN/Prompt.pm\n@@ -62,16 +62,16 @@ sub ssl_server_trust {\n \t issuer_dname fingerprint);\n \tmy $choice;\n prompt:\n-\tprint STDERR $may_save ?\n+\tmy $options = $may_save ?\n \t \"(R)eject, accept (t)emporarily or accept (p)ermanently? \" :\n \t \"(R)eject or accept (t)emporarily? \";\n \tSTDERR->flush;\n-\t$choice = lc(substr( || 'R', 0, 1));\n-\tif ($choice =~ /^t$/i) {\n+\t$choice = lc(substr(Git::prompt(\"Certificate problem.\\n\" . $options) || 'R', 0, 1));\n+\tif ($choice eq 't') {\n \t\t$cred->may_save(undef);\n-\t} elsif ($choice =~ /^r$/i) {\n+\t} elsif ($choice eq 'r') {\n \t\treturn -1;\n-\t} elsif ($may_save && $choice =~ /^p$/i) {\n+\t} elsif ($may_save && $choice eq 'p') {\n \t\t$cred->may_save($may_save);\n \t} else {\n \t\tgoto prompt;\n@@ -109,9 +109,7 @@ sub username {\n \tif (defined $_username) {\n \t\t$username = $_username;\n \t} else {\n-\t\tprint STDERR \"Username: \";\n-\t\tSTDERR->flush;\n-\t\tchomp($username = );\n+\t\t$username = Git::prompt(\"Username: \");\n \t}\n \t$cred->username($username);\n \t$cred->may_save($may_save);\n@@ -120,7 +118,7 @@ sub username {\n \n sub _read_password {\n \tmy ($prompt, $realm) = @_;\n-\tmy $password = Git::prompt($prompt);\n+\tmy $password = Git::prompt($prompt, 1);\n \t$password;\n }\n \n-- \nBest regards,\n Sven Strickroth\n PGP key id F5A9D4C4 @ any key-server\n"},{"id":"205096","messageId":"20121218005725.GA4125@sigill.intra.peff.net","threadId":"28961","inReplyTo":"50CFB8BF.4000405@tu-clausthal.de","subject":"Re: [PATCH 2/3] perl/Git.pm: Honor SSH_ASKPASS as fallback if GIT_ASKPASS is not set","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-12-18T00:57:25Z","receivedAt":"2012-12-18T00:57:25Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Dec 18, 2012 at 01:28:47AM +0100, Sven Strickroth wrote:\n\n> If GIT_ASKPASS environment variable is not set, git-svn does not try to use\n> SSH_ASKPASS as git-core does. This change adds a fallback to SSH_ASKPASS.\n> \n> Signed-off-by: Sven Strickroth \n> ---\n\nThanks, this series looks fine to me.\n\nI skimmed through the original thread. It looks like we got bogged down\nin discussion of whether GIT_ASKPASS should fall back on error, and\nwhether SSH_ASKPASS should come _after_ the terminal has been tried and\nfailed. This version of the series brings the behavior of git-svn in\nline with the rest of git, which I think is a good first step.\n\nI don't know whether we want to take a second step and tweak the order\nin both perl and C code. Since nobody has mentioned it in the interim\nmonths, I'm assuming it's not a big deal, and people are happy enough\nwith the current ordering. So unless somebody feels strongly about it,\nit's not worth bothering.\n\n-Peff\n"}]}