* Re: [PATCH 3/5] honour *_ASKPASS for querying username and for querying further actions like unknown certificates
From: Junio C Hamano @ 2011-12-27 20:56 UTC (permalink / raw)
To: Sven Strickroth; +Cc: git, Jakub Narebski, Jeff King
In-Reply-To: <4EF9ED24.2040902@tu-clausthal.de>
Sven Strickroth <sven.strickroth@tu-clausthal.de> writes:
> git-svn reads usernames (and other stuff) from an interactive terminal.
> This behavior cause GUIs to hang waiting for git-svn to complete (http://code.google.com/p/tortoisegit/issues/detail?id=967).
(style) Wrap the line perhaps after "to complete".
Does the above mean "GUIs hang, until the user goes back to the terminal
and authenticates"? Where is the "interactive terminal" connected when
running the GUI?
With that bit information, I think the above is a decent problem
description (i.e. "what problem is this change trying to solve? is it
worth solving?").
The second paragraph (missing) should then discuss what approach is taken
by the proposed patch to solve that problem. Something like
Instead of using hand-rolled prompt-response code that only works with
the interactive terminal, use the git_prompt() method introduced in
the earlier commit.
would suffice (I didn't check what method name you used, though).
> Also see commit 56a853b62c0ae7ebaad0a7a0a704f5ef561eb795.
I checked that commit, and what you wanted to say is unclear. Are you
saying this patch attempts to fix the breakage by that commit? That commit
tried to go in a right direction but did not go far enough and you are
trying to enhance it? Somerthing else?
Which means that you shouldn't have said "Also see..." at all and instead
directly said what you wanted to say here.
^ permalink raw reply
* Re: [PATCH 2/5] switch to central prompt method
From: Junio C Hamano @ 2011-12-27 20:47 UTC (permalink / raw)
To: Sven Strickroth; +Cc: git, Jakub Narebski, Jeff King
In-Reply-To: <4EF9ECEF.6020403@tu-clausthal.de>
Sven Strickroth <sven.strickroth@tu-clausthal.de> writes:
> Signed-off-by: Sven Strickroth <email@cs-ware.de>
> ---
It would be better to have this and the previous patch squashed into one
commit, for three reasons:
- It is far easier to review the patch that way, because it will show the
old way to get the password from the user as the removed code and new
way to do so as the added helper function, enabling reviewers to
compare the two in a single review session, to see if the change keeps
the feature bug-to-bug compatible, introduces any regression, and/or
offers improvements over existing code.
- If there were a bug in the implementation of askpass_prompt method
introduced in patch 1 without being used, and the calling codepath
introduced in patch 2 is bug-free (i.e. it correctly follows the
calling convention of the new method, only the implementation of the
method is buggy), bisection will still point at patch 2 that dumped the
old proven working way and started using the buggy new implementation.
Of course, it is possible that patch 1 perfectly implements the new
method and a bug exists in the way the caller introduced in patch 2
calls the method and in such a case bisection will correctly point out
that the caller is at fault, but the point of this refactoring is to
make it harder for callers to make such mistakes.
- As we can see above the three-dash line, even the author of the series
could not come up with any justification why the proposed change is a
good thing in the proposed commit log message for this patch alone (or
the previous patch alone for that matter). Combining these patches
together would make it clearer why it may be a good thing, which would
make it easier to come up with a better log message.
> git-svn.perl | 9 ++-------
> 1 files changed, 2 insertions(+), 7 deletions(-)
>
> diff --git a/git-svn.perl b/git-svn.perl
> index eeb83d3..25d5da7 100755
> --- a/git-svn.perl
> +++ b/git-svn.perl
> @@ -4415,13 +4415,8 @@ sub username {
>
> sub _read_password {
> my ($prompt, $realm) = @_;
> - my $password = '';
> - if (exists $ENV{GIT_ASKPASS}) {
> - open(PH, "-|", $ENV{GIT_ASKPASS}, $prompt);
> - $password = <PH>;
> - $password =~ s/[\012\015]//; # \n\r
> - close(PH);
> - } else {
> + my $password = Git->askpass_prompt($prompt);;
> + if (!defined $password) {
> print STDERR $prompt;
> STDERR->flush;
> require Term::ReadKey;
^ permalink raw reply
* Re: [PATCH 1/5] add central method for prompting a user using GIT_ASKPASS or SSH_ASKPASS
From: Junio C Hamano @ 2011-12-27 20:47 UTC (permalink / raw)
To: Sven Strickroth; +Cc: git, Jakub Narebski, Jeff King
In-Reply-To: <4EF9ECC0.606@tu-clausthal.de>
Sven Strickroth <sven.strickroth@tu-clausthal.de> writes:
> Signed-off-by: Sven Strickroth <email@cs-ware.de>
> ---
Thanks.
Please indicate what area you are touching on the Subject, e.g.
Subject: [PATCH 1/5] perl/Git.pm: add askpass_prompt() method
and explain what problem it tries to solve, how it tries to solve it, and
why this particular way of solving that problem is a good thing to have in
the proposed commit log message (but see the comments to 2/5 before doing
so).
> perl/Git.pm | 36 +++++++++++++++++++++++++++++++++++-
> 1 files changed, 35 insertions(+), 1 deletions(-)
>
> diff --git a/perl/Git.pm b/perl/Git.pm
> index f7ce511..7fdf805 100644
> --- a/perl/Git.pm
> +++ b/perl/Git.pm
> @@ -58,7 +58,7 @@ require Exporter;
> command_output_pipe command_input_pipe command_close_pipe
> command_bidi_pipe command_close_bidi_pipe
> version exec_path html_path hash_object git_cmd_try
> - remote_refs
> + remote_refs askpass_prompt
> temp_acquire temp_release temp_reset temp_path);
>
>
> @@ -512,6 +512,40 @@ C<git --html-path>). Useful mostly only internally.
> sub html_path { command_oneline('--html-path') }
>
>
> +=item askpass_prompt ( PROMPT)
Is the unbalanced spacing around parentheses your finger slippage?
> +
> +Asks user using *_ASKPASS programs and return answer from user.
Other =item entries in this file (I only checked a couple of earlier ones)
begin with "Construct a new ...", "Execute the given git command ...",
i.e. in imperative mood.
Also I agree with Jakub that it would be more appropriate to start the
description with the purpose and the effect, i.e. what the function is
for, than the internal implementation, i.e. what the function does.
> +Checks if GIT_ASKPASS or SSH_ASKPASS is set, and use first matching for querying
> +user and returns answer.
> +
> +If no *_ASKPASS variable is set, the variable is empty or an error occours,
> +it returns undef and the caller has to ask the user (e.g. on terminal).
> +
> +=cut
> +
> +sub askpass_prompt {
> + my ($self, $prompt) = _maybe_self(@_);
> + if (exists $ENV{'GIT_ASKPASS'}) {
> + return _askpass_prompt($ENV{'GIT_ASKPASS'}, $prompt);
> + } elsif (exists $ENV{'SSH_ASKPASS'}) {
> + return _askpass_prompt($ENV{'SSH_ASKPASS'}, $prompt);
> + } else {
> + return undef;
Two problems with this if/elsif/else cascade.
- If _askpass_prompt() fails to open the pipe to ENV{'GIT_ASKPASS'}, it
will return 'undef' to us. Don't we want to fall back to SSH_ASKPASS in
such a case?
- The last "return undef" makes all callers of this method to implement a
fall-back way somehow. I find it very likely that they will want to use
the fall-back code that uses Term::ReadKey found in _read_password, and
they will hate the above askpass_prompt implementation for focing them
to duplicate the code more than they will appreciate the flexibility
that they could implement a different fall-back.
> +}
> +
> +sub _askpass_prompt {
> + my ($self, $askpass, $prompt) = _maybe_self(@_);
I am not sure why you would want _maybe_self() here for this internal
helper function _askpass_prompt that is not even called as a method of
anything.
> + my $ret;
> + open my $fh, "-|", $askpass, $prompt || return undef;
> + $ret = <$fh>;
> + $ret =~ s/[\012\015]//g; # strip \n\r, chomp does not work on all systems (i.e. windows) as expected
> + close ($fh);
> + return $ret;
> +}
> +
> +
> =item repo_path ()
>
> Return path to the git repository. Must be called on a repository instance.
^ permalink raw reply
* Re: Git beginner - Need help understanding
From: Junio C Hamano @ 2011-12-27 19:50 UTC (permalink / raw)
To: chirin; +Cc: git
In-Reply-To: <1324969372444-7129429.post@n2.nabble.com>
chirin <takonatto@gmail.com> writes:
> From what I understand, your scenario is exactly what I expect. Which is why
> when I asked around my colleagues, no one was able to explain why I'm having
> this issue.
>
> As per your scenario:
>
> # A changes hello.txt
>
> # Going into B (who has not done anything to hello.txt)
> git pull --> merge conflict on hello.txt
>
> git commit
> git pull --> OK
This is quite different from what Dov showed as "the right way to
report". Dob's recipe begins from absolutely empty state and creates three
repositories involved with exact sequence and anybody can reproduce how it
is supposed to work (or break). It was written in such a way that you can
try to follow the sequence to see if you see a behaviour that is different
from Dob expects (in which case your machine, filesystem or the Git binary
you installed might be the culprit). Have you tried it?
Compared to that, your version above does not say anything about what the
state of A, B and the repository A and B interact with were in before the
problem started, so even if Dob wanted to help you by trying to reproduce
your situation, there is not enough information to do so.
>> In this sequence, which fulfills the scenario that you described,
>> there are no conflicts. So I suggest that you try to change the
>> command sequence to illustrate what you don't understand and ask
>> again.
Exactly.
^ permalink raw reply
* Possible submodule or submodule documentation issue
From: Bill Zaumen @ 2011-12-27 19:24 UTC (permalink / raw)
To: git
For the 'add' command, the man page for get-submodule states
"<repository> is the URL of the new submodule’s origin repository. This
may be either an absolute URL, or (if it begins with ./ or ../), the
location relative to the superproject’s origin repository."
and
"In either case, the given URL is recorded into .gitmodules for use by
subsequent users cloning the superproject. If the URL is given relative
to the superproject’s repository, the presumption is the superproject
and submodule repositories will be kept together in the same relative
location, and only the superproject’s URL needs to be provided:
git-submodule will correctly locate the submodule using the relative URL
in .gitmodules."
Based on that documentation, I tried the following sequence of commands:
git init --bare library.git
git init --bare library-pkg.git
git clone library.git
cd library;
echo hello > hello
git add hello
git commit -m "initial"
git push origin master
cd ..
git clone library-pkg.git
echo goodbye > goodbye
git add goodbye
git submodule add ./library.git src <FAILS>
git submodule add ../library.git src <WORKS>
git commit -a -m "initial pkg"
git push origin master
The documentation as written suggests that the first use of
"git submodule add" is the one that should have worked because
library.git is in the same directory as the origin repository
library-pgk.git . I didn't try moving the two .git directories
to a server to see if the behavior is the same in that case (my
test was using the local file system).
git --version reports 1.7.1 (current version for my linux
distribution).
^ permalink raw reply
* Re: [PATCH] write first for-merge ref to FETCH_HEAD first
From: Junio C Hamano @ 2011-12-27 18:44 UTC (permalink / raw)
To: Joey Hess; +Cc: git
In-Reply-To: <20111226161656.GB29582@gnu.kitenet.net>
Joey Hess <joey@kitenet.net> writes:
> The FETCH_HEAD refname is supposed to refer to the ref that was fetched
> and should be merged. However all fetched refs are written to
> .git/FETCH_HEAD in an arbitrary order, and resolve_ref_unsafe simply
> takes the first ref as the FETCH_HEAD, which is often the wrong one,
> when other branches were also fetched.
>
> The solution is to write the for-merge ref(s) to FETCH_HEAD first.
> Then, unless --append is used, the FETCH_HEAD refname behaves as intended.
> If the user uses --append, they presumably are doing so in order to
> preserve the old FETCH_HEAD.
>
> Also included a fix to documentation that assumes FETCH_HEAD contains
> only a single ref.
That "single ref" assumption is perfectly fine for the part of the
documentation you patched, actually. The "fetch" command-line the example
shows explicitly fetches a single ref.
It is a good idea to use rev-parse anyway, so the patch itself is good. A
potential problem of that example (I haven't re-tried these examples for
eons since they were written) comes from the fact that FETCH_HEAD contains
not just the object name of what we fetched, but also other information to
describe what happened to that fetched object.
> ---
Sign-off?
> Documentation/git-read-tree.txt | 2 +-
> builtin/fetch.c | 158 +++++++++++++++++++++------------------
> 2 files changed, 85 insertions(+), 75 deletions(-)
^ permalink raw reply
* Re: [PATCH] Use Python's "print" as a function, not as a keyword
From: Pete Wyckoff @ 2011-12-27 16:41 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason; +Cc: Sebastian Morr, git, srabbelier
In-Reply-To: <CACBZZX7PVyCFfHTJN_QZfyt5wAcr4UAiJSmo54PSi=8pgv3sYA@mail.gmail.com>
avarab@gmail.com wrote on Wed, 21 Dec 2011 03:48 +0100:
> On Wed, Dec 21, 2011 at 03:19, Sebastian Morr <sebastian@morr.cc> wrote:
>
> > But, as nobody seems to have cared before: Is Git designed to be
> > compatible only with versions prior 3.0?
>
> I'm running Debian unstable and it has Python 2.7. Most people are
> still using Python 2.x as their default system Python since 3.x breaks
> backwards compatibility for common constructs like print.
>
> Does this only break Python 2.6, or all 2.x versions of Python?
>
> What's our currently supported Python version for the Python code in
> Git? It's 5.8.0 for Perl, do we have any particular aim for a
> supported Python version?
I test contrib/fast-import/git-p4 on python 2.5 and 2.7. I'm
hesitant to convert print() now, without committing to testing on
post-3.0 too. The work to support 3.x doesn't buy us much.
-- Pete
^ permalink raw reply
* [PATCH 5/5] make askpass_prompt a global prompt method for asking users
From: Sven Strickroth @ 2011-12-27 16:07 UTC (permalink / raw)
To: git; +Cc: Jakub Narebski, Jeff King, gitster
In-Reply-To: <4EF9EBF4.7070200@tu-clausthal.de>
Signed-off-by: Sven Strickroth <email@cs-ware.de>
---
git-svn.perl | 37 ++++---------------------------------
perl/Git.pm | 43 +++++++++++++++++++++++++++++++------------
2 files changed, 35 insertions(+), 45 deletions(-)
diff --git a/git-svn.perl b/git-svn.perl
index 2c99aaa..1f30dc2 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -4360,12 +4360,7 @@ prompt:
my $options = $may_save ?
"(R)eject, accept (t)emporarily or accept (p)ermanently? " :
"(R)eject or accept (t)emporarily? ";
- $choice = Git->askpass_prompt("Certificate unknown. " . $options);
- if (!defined $choice) {
- print STDERR $options;
- STDERR->flush;
- $choice = lc(substr(<STDIN> || 'R', 0, 1));
- }
+ $choice = substr(Git->prompt("Certificate unknown. " . $options) || 'R', 0, 1);
if ($choice =~ /^t$/i) {
$cred->may_save(undef);
} elsif ($choice =~ /^r$/i) {
@@ -4382,13 +4377,7 @@ prompt:
sub ssl_client_cert {
my ($cred, $realm, $may_save, $pool) = @_;
$may_save = undef if $_no_auth_cache;
- my $filename = Git->askpass_prompt("Client certificate filename:");
- if (!defined $filename) {
- print STDERR "Client certificate filename: ";
- STDERR->flush;
- chomp($filename = <STDIN>);
- }
- $cred->cert_file($filename);
+ $cred->cert_file(Git->prompt("Client certificate filename: "));
$cred->may_save($may_save);
$SVN::_Core::SVN_NO_ERROR;
}
@@ -4411,12 +4400,7 @@ sub username {
if (defined $_username) {
$username = $_username;
} else {
- $username = Git->askpass_prompt("Username");
- }
- if (!defined $username) {
- print STDERR "Username: ";
- STDERR->flush;
- chomp($username = <STDIN>);
+ $username = Git->prompt("Username: ");
}
$cred->username($username);
$cred->may_save($may_save);
@@ -4425,20 +4409,7 @@ sub username {
sub _read_password {
my ($prompt, $realm) = @_;
- my $password = Git->askpass_prompt($prompt);;
- if (!defined $password) {
- print STDERR $prompt;
- STDERR->flush;
- require Term::ReadKey;
- Term::ReadKey::ReadMode('noecho');
- while (defined(my $key = Term::ReadKey::ReadKey(0))) {
- last if $key =~ /[\012\015]/; # \n\r
- $password .= $key;
- }
- Term::ReadKey::ReadMode('restore');
- print STDERR "\n";
- STDERR->flush;
- }
+ my $password = Git->prompt($prompt, 1);
$password;
}
diff --git a/perl/Git.pm b/perl/Git.pm
index c6b3e11..acc00b4 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -58,7 +58,7 @@ require Exporter;
command_output_pipe command_input_pipe command_close_pipe
command_bidi_pipe command_close_bidi_pipe
version exec_path html_path hash_object git_cmd_try
- remote_refs askpass_prompt
+ remote_refs prompt
temp_acquire temp_release temp_reset temp_path);
@@ -512,30 +512,49 @@ C<git --html-path>). Useful mostly only internally.
sub html_path { command_oneline('--html-path') }
-=item askpass_prompt ( PROMPT)
+=item prompt ( PROMPT , ISPASSWORD )
-Asks user using *_ASKPASS programs and return answer from user.
+Asks user using *_ASKPASS programs or terminal the prompt C<PROMPT> and return answer from user.
Checks if GIT_ASKPASS or SSH_ASKPASS is set, and use first matching for querying
user and returns answer.
If no *_ASKPASS variable is set, the variable is empty or an error occours,
-it returns undef and the caller has to ask the user (e.g. on terminal).
+the querying the user using terminal is tried. If C<ISPASSWORD> is set and true,
+the terminal disables echo.
=cut
-sub askpass_prompt {
- my ($self, $prompt) = _maybe_self(@_);
+sub prompt {
+ my ($self, $prompt, $isPassword) = _maybe_self(@_);
+ my $ret;
if (exists $ENV{'GIT_ASKPASS'}) {
- return _askpass_prompt($ENV{'GIT_ASKPASS'}, $prompt);
- } elsif (exists $ENV{'SSH_ASKPASS'}) {
- return _askpass_prompt($ENV{'SSH_ASKPASS'}, $prompt);
- } else {
- return undef;
+ $ret = _prompt($ENV{'GIT_ASKPASS'}, $prompt);
}
+ if (!defined $ret && exists $ENV{'SSH_ASKPASS'}) {
+ $ret = _prompt($ENV{'SSH_ASKPASS'}, $prompt);
+ }
+ if (!defined $ret) {
+ print STDERR $prompt;
+ STDERR->flush;
+ if (defined $isPassword && $isPassword) {
+ require Term::ReadKey;
+ Term::ReadKey::ReadMode('noecho');
+ while (defined(my $key = Term::ReadKey::ReadKey(0))) {
+ last if $key =~ /[\012\015]/; # \n\r
+ $ret .= $key;
+ }
+ Term::ReadKey::ReadMode('restore');
+ print STDERR "\n";
+ STDERR->flush;
+ } else {
+ chomp($ret = <STDIN>);
+ }
+ }
+ return $ret;
}
-sub _askpass_prompt {
+sub _prompt {
my ($self, $askpass, $prompt) = _maybe_self(@_);
unless ($askpass) {
return undef;
--
Best regards,
Sven Strickroth
ClamAV, a GPL anti-virus toolkit http://www.clamav.net
PGP key id F5A9D4C4 @ any key-server
^ permalink raw reply related
* [PATCH 4/5] ignore empty *_ASKPASS variables
From: Sven Strickroth @ 2011-12-27 16:07 UTC (permalink / raw)
To: git; +Cc: Jakub Narebski, Jeff King, gitster
In-Reply-To: <4EF9EBF4.7070200@tu-clausthal.de>
Signed-off-by: Sven Strickroth <email@cs-ware.de>
---
perl/Git.pm | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/perl/Git.pm b/perl/Git.pm
index 7fdf805..c6b3e11 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -537,6 +537,9 @@ sub askpass_prompt {
sub _askpass_prompt {
my ($self, $askpass, $prompt) = _maybe_self(@_);
+ unless ($askpass) {
+ return undef;
+ }
my $ret;
open my $fh, "-|", $askpass, $prompt || return undef;
$ret = <$fh>;
--
Best regards,
Sven Strickroth
ClamAV, a GPL anti-virus toolkit http://www.clamav.net
PGP key id F5A9D4C4 @ any key-server
^ permalink raw reply related
* [PATCH 3/5] honour *_ASKPASS for querying username and for querying further actions like unknown certificates
From: Sven Strickroth @ 2011-12-27 16:07 UTC (permalink / raw)
To: git; +Cc: Jakub Narebski, Jeff King, gitster
In-Reply-To: <4EF9EBF4.7070200@tu-clausthal.de>
git-svn reads usernames (and other stuff) from an interactive terminal.
This behavior cause GUIs to hang waiting for git-svn to complete (http://code.google.com/p/tortoisegit/issues/detail?id=967).
Also see commit 56a853b62c0ae7ebaad0a7a0a704f5ef561eb795.
Signed-off-by: Sven Strickroth <email@cs-ware.de>
---
git-svn.perl | 22 ++++++++++++++++------
1 files changed, 16 insertions(+), 6 deletions(-)
diff --git a/git-svn.perl b/git-svn.perl
index 25d5da7..2c99aaa 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -4357,11 +4357,15 @@ sub ssl_server_trust {
issuer_dname fingerprint);
my $choice;
prompt:
- print STDERR $may_save ?
+ my $options = $may_save ?
"(R)eject, accept (t)emporarily or accept (p)ermanently? " :
"(R)eject or accept (t)emporarily? ";
- STDERR->flush;
- $choice = lc(substr(<STDIN> || 'R', 0, 1));
+ $choice = Git->askpass_prompt("Certificate unknown. " . $options);
+ if (!defined $choice) {
+ print STDERR $options;
+ STDERR->flush;
+ $choice = lc(substr(<STDIN> || 'R', 0, 1));
+ }
if ($choice =~ /^t$/i) {
$cred->may_save(undef);
} elsif ($choice =~ /^r$/i) {
@@ -4378,9 +4382,12 @@ prompt:
sub ssl_client_cert {
my ($cred, $realm, $may_save, $pool) = @_;
$may_save = undef if $_no_auth_cache;
- print STDERR "Client certificate filename: ";
- STDERR->flush;
- chomp(my $filename = <STDIN>);
+ my $filename = Git->askpass_prompt("Client certificate filename:");
+ if (!defined $filename) {
+ print STDERR "Client certificate filename: ";
+ STDERR->flush;
+ chomp($filename = <STDIN>);
+ }
$cred->cert_file($filename);
$cred->may_save($may_save);
$SVN::_Core::SVN_NO_ERROR;
@@ -4404,6 +4411,9 @@ sub username {
if (defined $_username) {
$username = $_username;
} else {
+ $username = Git->askpass_prompt("Username");
+ }
+ if (!defined $username) {
print STDERR "Username: ";
STDERR->flush;
chomp($username = <STDIN>);
--
Best regards,
Sven Strickroth
ClamAV, a GPL anti-virus toolkit http://www.clamav.net
PGP key id F5A9D4C4 @ any key-server
^ permalink raw reply related
* [PATCH 2/5] switch to central prompt method
From: Sven Strickroth @ 2011-12-27 16:06 UTC (permalink / raw)
To: git; +Cc: Jakub Narebski, Jeff King, gitster
In-Reply-To: <4EF9EBF4.7070200@tu-clausthal.de>
Signed-off-by: Sven Strickroth <email@cs-ware.de>
---
git-svn.perl | 9 ++-------
1 files changed, 2 insertions(+), 7 deletions(-)
diff --git a/git-svn.perl b/git-svn.perl
index eeb83d3..25d5da7 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -4415,13 +4415,8 @@ sub username {
sub _read_password {
my ($prompt, $realm) = @_;
- my $password = '';
- if (exists $ENV{GIT_ASKPASS}) {
- open(PH, "-|", $ENV{GIT_ASKPASS}, $prompt);
- $password = <PH>;
- $password =~ s/[\012\015]//; # \n\r
- close(PH);
- } else {
+ my $password = Git->askpass_prompt($prompt);;
+ if (!defined $password) {
print STDERR $prompt;
STDERR->flush;
require Term::ReadKey;
--
Best regards,
Sven Strickroth
ClamAV, a GPL anti-virus toolkit http://www.clamav.net
PGP key id F5A9D4C4 @ any key-server
^ permalink raw reply related
* [PATCH 1/5] add central method for prompting a user using GIT_ASKPASS or SSH_ASKPASS
From: Sven Strickroth @ 2011-12-27 16:05 UTC (permalink / raw)
To: git; +Cc: Jakub Narebski, Jeff King, gitster
In-Reply-To: <4EF9EBF4.7070200@tu-clausthal.de>
Signed-off-by: Sven Strickroth <email@cs-ware.de>
---
perl/Git.pm | 36 +++++++++++++++++++++++++++++++++++-
1 files changed, 35 insertions(+), 1 deletions(-)
diff --git a/perl/Git.pm b/perl/Git.pm
index f7ce511..7fdf805 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -58,7 +58,7 @@ require Exporter;
command_output_pipe command_input_pipe command_close_pipe
command_bidi_pipe command_close_bidi_pipe
version exec_path html_path hash_object git_cmd_try
- remote_refs
+ remote_refs askpass_prompt
temp_acquire temp_release temp_reset temp_path);
@@ -512,6 +512,40 @@ C<git --html-path>). Useful mostly only internally.
sub html_path { command_oneline('--html-path') }
+=item askpass_prompt ( PROMPT)
+
+Asks user using *_ASKPASS programs and return answer from user.
+
+Checks if GIT_ASKPASS or SSH_ASKPASS is set, and use first matching for querying
+user and returns answer.
+
+If no *_ASKPASS variable is set, the variable is empty or an error occours,
+it returns undef and the caller has to ask the user (e.g. on terminal).
+
+=cut
+
+sub askpass_prompt {
+ my ($self, $prompt) = _maybe_self(@_);
+ if (exists $ENV{'GIT_ASKPASS'}) {
+ return _askpass_prompt($ENV{'GIT_ASKPASS'}, $prompt);
+ } elsif (exists $ENV{'SSH_ASKPASS'}) {
+ return _askpass_prompt($ENV{'SSH_ASKPASS'}, $prompt);
+ } else {
+ return undef;
+ }
+}
+
+sub _askpass_prompt {
+ my ($self, $askpass, $prompt) = _maybe_self(@_);
+ my $ret;
+ open my $fh, "-|", $askpass, $prompt || return undef;
+ $ret = <$fh>;
+ $ret =~ s/[\012\015]//g; # strip \n\r, chomp does not work on all systems (i.e. windows) as expected
+ close ($fh);
+ return $ret;
+}
+
+
=item repo_path ()
Return path to the git repository. Must be called on a repository instance.
--
Best regards,
Sven Strickroth
ClamAV, a GPL anti-virus toolkit http://www.clamav.net
PGP key id F5A9D4C4 @ any key-server
^ permalink raw reply related
* [PATCH 0/5] honour *_ASKPASS for querying user in git-svn
From: Sven Strickroth @ 2011-12-27 16:01 UTC (permalink / raw)
To: git; +Cc: Jakub Narebski, Jeff King, gitster
In-Reply-To: <4EF9D8B9.9060106@tu-clausthal.de>
Hi,
git-svn reads usernames (and other stuff) from an interactive terminal.
This behavior cause GUIs to hang waiting for git-svn to complete
(http://code.google.com/p/tortoisegit/issues/detail?id=967).
Again I honoured all your ideas. Hopefully the patches can be applied
now. The new patches follow in the next mails (you can also pull from
git://github.com/csware/git.git askpass-prompt).
The first four patches implement the raw functionality. The last and
fifth patch extends the prompt-method so that it can be used for all
purposes in order to query users.
--
Best regards,
Sven Strickroth
ClamAV, a GPL anti-virus toolkit http://www.clamav.net
PGP key id F5A9D4C4 @ any key-server
^ permalink raw reply
* Re: [PATCH] honour GIT_ASKPASS for querying username in git-svn
From: Jakub Narebski @ 2011-12-27 16:00 UTC (permalink / raw)
To: Sven Strickroth; +Cc: git, Jeff King
In-Reply-To: <4EF9D8B9.9060106@tu-clausthal.de>
On Tue, 27 Dec 2011, Sven Strickroth wrote:
> Am 27.12.2011 15:33 schrieb Jakub Narebski:
>>> +sub prompt {
>>> + my ($self, $prompt) = _maybe_self(@_);
>>> + if (exists $ENV{'GIT_ASKPASS'}) {
>>> + return _prompt($ENV{'GIT_ASKPASS'}, $prompt);
>>> + } elsif (exists $ENV{'SSH_ASKPASS'}) {
>>> + return _prompt($ENV{'SSH_ASKPASS'}, $prompt);
>>> + } else {
>>> + return undef;
>>> + }
>>> +}
>>
>> ...and provide some kind of fallback even if neither of GIT_ASKPASS
>> nor SSH_ASKPASS are set (perhaps assuming that some Perl packages from
>> CPAN are installed).
>
> If neither of GIT_ASKPASS nor SSH_ASKPASS are set the caller has to
> handle the request. This has to be done this way, because of lots of
> different needs (username, password (no echo) and so on).
I think that Git.pm and therefore git commands written in Perl should
behave the same as git command written in C; and I think builtins do
use common gitprompt fallback.
>>> +sub _prompt {
>>> + my ($self, $askpass, $prompt) = _maybe_self(@_);
>>> + my $ret;
>>> + open(PH, "-|", $askpass, $prompt);
>>> + $ret = <PH>;
>>> + $ret =~ s/[\012\015]//g; # strip \n\r
>>> + close(PH);
>>> + return $ret;
>>> +}
>>
>> Please, use modern Perl, in particula use lexical filehandles instead
>> of typeglobs (which are global variables), i.e.
>
> I used the same style as I found in Git.pm (see lines I removed in patch 2).
Yes, that should be fixed (together with host of other issues), but
one should use modern and _better_ way (no possibility of action at
distance).
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: [PATCH] add post-fetch hook
From: Joey Hess @ 2011-12-27 15:49 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7v8vly8qqx.fsf@alter.siamese.dyndns.org>
[-- Attachment #1: Type: text/plain, Size: 925 bytes --]
Junio C Hamano wrote:
> Joey Hess <joey@kitenet.net> writes:
>
> > .... And other code in git uses an async feeder similarly,
> > see for example convert.c's apply_filter(). So I think this is ok..?
>
> Yeah, I didn't look at your patch (sorry) but if it uses async like the
> filtering codepath does, it should be perfectly fine (please forget about
> the select(2) based kludge I alluded to; the async interface is the right
> thing to use here).
No problem, I was surprised to be getting responses at all over the
holidays. :)
Then async also seems the right thing to use for the hook refactoring. A
caller can provide two function pointers; a feeder function that is
called async, and a reader that is *not* called async (which would allow
it to modify program state), and the refactored hook function handles
running the hook(s) and connecting them to the feeder and/or reader.
--
see shy jo
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 828 bytes --]
^ permalink raw reply
* Re: [PATCH] honour GIT_ASKPASS for querying username in git-svn
From: Sven Strickroth @ 2011-12-27 14:39 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git, Jeff King
In-Reply-To: <m3d3baf5kd.fsf@localhost.localdomain>
Am 27.12.2011 15:33 schrieb Jakub Narebski:
>> +sub prompt {
>> + my ($self, $prompt) = _maybe_self(@_);
>> + if (exists $ENV{'GIT_ASKPASS'}) {
>> + return _prompt($ENV{'GIT_ASKPASS'}, $prompt);
>> + } elsif (exists $ENV{'SSH_ASKPASS'}) {
>> + return _prompt($ENV{'SSH_ASKPASS'}, $prompt);
>> + } else {
>> + return undef;
>> + }
>> +}
>
> ...and provide some kind of fallback even if neither of GIT_ASKPASS
> nor SSH_ASKPASS are set (perhaps assuming that some Perl packages from
> CPAN are installed).
If neither of GIT_ASKPASS nor SSH_ASKPASS are set the caller has to
handle the request. This has to be done this way, because of lots of
different needs (username, password (no echo) and so on).
>> +sub _prompt {
>> + my ($self, $askpass, $prompt) = _maybe_self(@_);
>> + my $ret;
>> + open(PH, "-|", $askpass, $prompt);
>> + $ret = <PH>;
>> + $ret =~ s/[\012\015]//g; # strip \n\r
>> + close(PH);
>> + return $ret;
>> +}
>
> Please, use modern Perl, in particula use lexical filehandles instead
> of typeglobs (which are global variables), i.e.
I used the same style as I found in Git.pm (see lines I removed in patch 2).
--
Best regards,
Sven Strickroth
ClamAV, a GPL anti-virus toolkit http://www.clamav.net
PGP key id F5A9D4C4 @ any key-server
^ permalink raw reply
* Re: [PATCH] honour GIT_ASKPASS for querying username in git-svn
From: Jakub Narebski @ 2011-12-27 14:33 UTC (permalink / raw)
To: Sven Strickroth; +Cc: git, Jeff King
In-Reply-To: <4EF907F1.1030801@tu-clausthal.de>
Sven Strickroth <sven.strickroth@tu-clausthal.de> writes:
> +=item prompt ( PROMPT)
> +
> +Checks if GIT_ASKPASS or SSH_ASKPASS is set, and if yes
> +use it and return answer from user.
> +
> +=cut
I think it would be good idea to describe what this function is for...
> +sub prompt {
> + my ($self, $prompt) = _maybe_self(@_);
> + if (exists $ENV{'GIT_ASKPASS'}) {
> + return _prompt($ENV{'GIT_ASKPASS'}, $prompt);
> + } elsif (exists $ENV{'SSH_ASKPASS'}) {
> + return _prompt($ENV{'SSH_ASKPASS'}, $prompt);
> + } else {
> + return undef;
> + }
> +}
...and provide some kind of fallback even if neither of GIT_ASKPASS
nor SSH_ASKPASS are set (perhaps assuming that some Perl packages from
CPAN are installed).
> +sub _prompt {
> + my ($self, $askpass, $prompt) = _maybe_self(@_);
> + my $ret;
> + open(PH, "-|", $askpass, $prompt);
> + $ret = <PH>;
> + $ret =~ s/[\012\015]//g; # strip \n\r
> + close(PH);
> + return $ret;
> +}
Please, use modern Perl, in particula use lexical filehandles instead
of typeglobs (which are global variables), i.e.
+ open my $fh, "-|", $askpass, $prompt
+ or die "...";
+ $ret = <$fh>;
+ chomp($ret);
+ close($fh)
+ or die "...";
> --
> 1.7.7.1.msysgit.0
>
> From ef4c6557d1b0e33440d13c64742d44b2a22143f3 Mon Sep 17 00:00:00 2001
> From: Sven Strickroth <email@cs-ware.de>
> Date: Tue, 27 Dec 2011 00:34:09 +0100
> Subject: [PATCH 2/4] switch to central prompt method
>
> Signed-off-by: Sven Strickroth <email@cs-ware.de>
Please send those patches as a separate emails, not concatenated in a
single email (perhaps even with cover letter).
See Documentation/SubmittingPatches
[...]
--
Jakub Narebski
^ permalink raw reply
* [PATCH] gc --auto: warn garbage collection happens soon
From: Nguyễn Thái Ngọc Duy @ 2011-12-27 13:45 UTC (permalink / raw)
To: git; +Cc: Nguyễn Thái Ngọc Duy
This gives users a chance to run gc explicitly elsewhere if they do not
want gc to run suddenly in current terminal.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
v2 of a patch posted a few months ago. The warning limits are in
percentage and configurable. I could have set the default limits to
100% (i.e. no warnings) to keep current behavior. However I think
warning is better.
May need rewording inn config.txt, I'm not sure I state it clearly.
Documentation/config.txt | 12 ++++++++++++
Documentation/git-gc.txt | 4 ++++
builtin/gc.c | 41 +++++++++++++++++++++++++++++++++++++++--
3 files changed, 55 insertions(+), 2 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 5a841da..c263496 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -965,12 +965,24 @@ gc.auto::
light-weight garbage collection from time to time. The
default value is 6700. Setting this to 0 disables it.
+gc.autowarn::
+ The percentage of loose objects specified in `gc.auto`. If the
+ number of loose objects exceeds this limit, `git gc --auto`
+ will warn users garbage collection will happen soon. Default
+ value is 90. Setting this to 100 disables it.
+
gc.autopacklimit::
When there are more than this many packs that are not
marked with `*.keep` file in the repository, `git gc
--auto` consolidates them into one larger pack. The
default value is 50. Setting this to 0 disables it.
+gc.autopackwarn::
+ The percentage of packs specified in `gc.autopacklimit`. If
+ the number of packs exceeds this limit, `git gc --auto` will
+ warn users garbage collection will happen soon. Default value
+ is 90. Setting this to 100 disables it.
+
gc.packrefs::
Running `git pack-refs` in a repository renders it
unclonable by Git versions prior to 1.5.1.2 over dumb
diff --git a/Documentation/git-gc.txt b/Documentation/git-gc.txt
index 815afcb..937b3d6 100644
--- a/Documentation/git-gc.txt
+++ b/Documentation/git-gc.txt
@@ -59,6 +59,10 @@ then existing packs (except those marked with a `.keep` file)
are consolidated into a single pack by using the `-A` option of
'git repack'. Setting `gc.autopacklimit` to 0 disables
automatic consolidation of packs.
++
+`git gc --auto` will warn users when the number of loose objects or
+packs is close to the limits. See `gc.autowarn` and `gc.autopackwarn`
+for details.
--prune=<date>::
Prune loose objects older than date (default is 2 weeks ago,
diff --git a/builtin/gc.c b/builtin/gc.c
index 0498094..f3fa46d 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -28,6 +28,10 @@ static int gc_auto_threshold = 6700;
static int gc_auto_pack_limit = 50;
static const char *prune_expire = "2.weeks.ago";
+/* numbers are in percent, to be converted to absolute later */
+static int gc_warn_auto_threshold = 90;
+static int gc_warn_auto_pack_limit = 90;
+
#define MAX_ADD 10
static const char *argv_pack_refs[] = {"pack-refs", "--all", "--prune", NULL};
static const char *argv_reflog[] = {"reflog", "expire", "--all", NULL};
@@ -52,10 +56,26 @@ static int gc_config(const char *var, const char *value, void *cb)
gc_auto_threshold = git_config_int(var, value);
return 0;
}
+ if (!strcmp(var, "gc.autowarn")) {
+ int percent = percent = git_config_int(var, value);
+ if (percent <= 0 || percent > 100)
+ die(_("gc.autowarn %d%% does not make sense"),
+ percent);
+ gc_warn_auto_threshold = percent;
+ return 0;
+ }
if (!strcmp(var, "gc.autopacklimit")) {
gc_auto_pack_limit = git_config_int(var, value);
return 0;
}
+ if (!strcmp(var, "gc.autopackwarn")) {
+ int percent = percent = git_config_int(var, value);
+ if (percent <= 0 || percent > 100)
+ die(_("gc.autopackwarn %d%% does not make sense"),
+ percent);
+ gc_warn_auto_pack_limit = percent;
+ return 0;
+ }
if (!strcmp(var, "gc.pruneexpire")) {
if (value && strcmp(value, "now")) {
unsigned long now = approxidate("now");
@@ -118,7 +138,15 @@ static int too_many_loose_objects(void)
}
}
closedir(dir);
- return needed;
+ if (needed)
+ return 1;
+
+ auto_threshold = (gc_warn_auto_threshold + 255) / 256;
+ if (num_loose >= auto_threshold)
+ warning(_("Too many loose objects (current approx. %d, limit %d).\n"
+ "\"git gc\" will soon run automatically"),
+ num_loose * 256, gc_auto_threshold);
+ return 0;
}
static int too_many_packs(void)
@@ -141,7 +169,14 @@ static int too_many_packs(void)
*/
cnt++;
}
- return gc_auto_pack_limit <= cnt;
+ if (gc_auto_pack_limit <= cnt)
+ return 1;
+
+ if (gc_warn_auto_pack_limit <= cnt)
+ warning(_("Too many packs (current %d, limit %d)\n"
+ "\"git gc\" will soon run automatically."),
+ cnt, gc_auto_pack_limit);
+ return 0;
}
static int need_to_gc(void)
@@ -193,6 +228,8 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
usage_with_options(builtin_gc_usage, builtin_gc_options);
git_config(gc_config, NULL);
+ gc_warn_auto_threshold = 0.01 * gc_auto_threshold * gc_warn_auto_threshold;
+ gc_warn_auto_pack_limit = 0.01 * gc_auto_pack_limit * gc_auto_pack_limit;
if (pack_refs < 0)
pack_refs = !is_bare_repository();
--
1.7.8.36.g69ee2
^ permalink raw reply related
* Re: Git beginner - Need help understanding
From: chirin @ 2011-12-27 7:02 UTC (permalink / raw)
To: git
In-Reply-To: <CA++fsGHPKhzfd7-KohOZ4WpYatx_-EW0bjq46zwswbu8TomHCg@mail.gmail.com>
From what I understand, your scenario is exactly what I expect. Which is why
when I asked around my colleagues, no one was able to explain why I'm having
this issue.
As per your scenario:
# A changes hello.txt
# Going into B (who has not done anything to hello.txt)
git pull --> merge conflict on hello.txt
git commit
git pull --> OK
Would going through Gerrit have anything to do with it? I've compared git
config -l with the others but they are the same (aside from
remote.origin.url which has our own gerrit userid).
Dov Grobgeld wrote
>
> The best way of understanding and also of asking questions, is if you
> can reproduce the steps of exactly what you want and don't understand
> by a sequence of commands like so:
>
> # First create a bare repository
> mkdir R
> cd R
> git init --bare .
>
> # Clone it into A
> git clone R A
>
> # Clone it into B
> git clone R B
>
> # Now start doing changes for A and B, pulling and pushing into R
> cd A
> echo "Change #1" > hello.txt
> git add hello.txt
> git commit -m 'Commit #1'
> git push origin master
>
> # Get into B
> cd ../B
> git pull
> echo "Change #2" >> hello.txt
> git commit -a -m 'Commit #2'
> git push
>
> # Get into A and pull the changes done by B
> cd ../A
> git pull
>
> In this sequence, which fulfills the scenario that you described,
> there are no conflicts. So I suggest that you try to change the
> command sequence to illustrate what you don't understand and ask
> again.
>
> Regards,
> Dov
>
--
View this message in context: http://git.661346.n2.nabble.com/Git-beginner-Need-help-understanding-tp7129186p7129429.html
Sent from the git mailing list archive at Nabble.com.
^ permalink raw reply
* Re: [PATCH] add post-fetch hook
From: Junio C Hamano @ 2011-12-27 6:37 UTC (permalink / raw)
To: Joey Hess; +Cc: git
In-Reply-To: <20111226155152.GA29582@gnu.kitenet.net>
Joey Hess <joey@kitenet.net> writes:
> .... And other code in git uses an async feeder similarly,
> see for example convert.c's apply_filter(). So I think this is ok..?
Yeah, I didn't look at your patch (sorry) but if it uses async like the
filtering codepath does, it should be perfectly fine (please forget about
the select(2) based kludge I alluded to; the async interface is the right
thing to use here).
Thanks.
^ permalink raw reply
* Re: Git beginner - Need help understanding
From: Dov Grobgeld @ 2011-12-27 6:34 UTC (permalink / raw)
To: chirin; +Cc: git
In-Reply-To: <CA++fsGGEv=jS4YNEUCxTwZ3pZc7HbbmoPbDH+MamrqamxrsADA@mail.gmail.com>
The best way of understanding and also of asking questions, is if you
can reproduce the steps of exactly what you want and don't understand
by a sequence of commands like so:
# First create a bare repository
mkdir R
cd R
git init --bare .
# Clone it into A
git clone R A
# Clone it into B
git clone R B
# Now start doing changes for A and B, pulling and pushing into R
cd A
echo "Change #1" > hello.txt
git add hello.txt
git commit -m 'Commit #1'
git push origin master
# Get into B
cd ../B
git pull
echo "Change #2" >> hello.txt
git commit -a -m 'Commit #2'
git push
# Get into A and pull the changes done by B
cd ../A
git pull
In this sequence, which fulfills the scenario that you described,
there are no conflicts. So I suggest that you try to change the
command sequence to illustrate what you don't understand and ask
again.
Regards,
Dov
On Tue, Dec 27, 2011 at 05:12, chirin <takonatto@gmail.com> wrote:
>
> I'm just beginning to use Git in my workplace, and (rather shamefully) have
> never heard of Git until now. While I pore over the stacks of documentations
> for beginners, could someone help me understand this issue I've been having?
>
> Every time a colleague updates a file, I would not be able to pull due to
> merge conflicts - even though I have never made any changes to the
> repository. I'd try and then again, but it would still give the same merge
> conflict. The only way I could pull currently is to do a git commit (even
> without changes), and then git pull.
>
> If this was SVN, I could simply resolve this with Update. What am I missing,
> and what should I be looking for?
>
> Thanks in advance..
>
> --
> View this message in context: http://git.661346.n2.nabble.com/Git-beginner-Need-help-understanding-tp7129186p7129186.html
> Sent from the git mailing list archive at Nabble.com.
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* I can never finish a push
From: Bill Zaumen @ 2011-12-27 4:18 UTC (permalink / raw)
To: mresnick, git
Aside from the other comments, you said, "someone else on-site
pushes and adds new commits before mine can finish." If I
interpreted that correctly, the "someone else" should also be
using a link as slow as yours. Why isn't he having the same
problem you are having?
If you can track that person down (his name should show up in
the new commits you pulled), compare your config files to see
what is different.
Bill
^ permalink raw reply
* Git beginner - Need help understanding
From: chirin @ 2011-12-27 3:12 UTC (permalink / raw)
To: git
I'm just beginning to use Git in my workplace, and (rather shamefully) have
never heard of Git until now. While I pore over the stacks of documentations
for beginners, could someone help me understand this issue I've been having?
Every time a colleague updates a file, I would not be able to pull due to
merge conflicts - even though I have never made any changes to the
repository. I'd try and then again, but it would still give the same merge
conflict. The only way I could pull currently is to do a git commit (even
without changes), and then git pull.
If this was SVN, I could simply resolve this with Update. What am I missing,
and what should I be looking for?
Thanks in advance..
--
View this message in context: http://git.661346.n2.nabble.com/Git-beginner-Need-help-understanding-tp7129186p7129186.html
Sent from the git mailing list archive at Nabble.com.
^ permalink raw reply
* [PATCH] Fix an incorrect reference to --set-all.
From: Jelmer Vernooij @ 2011-12-27 2:03 UTC (permalink / raw)
To: git
Signed-off-by: Jelmer Vernooij <jelmer@samba.org>
---
builtin/config.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/builtin/config.c b/builtin/config.c
index 0315ad7..d35c06a 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -444,7 +444,7 @@ int cmd_config(int argc, const char **argv, const char *prefix)
ret = git_config_set(argv[0], value);
if (ret == CONFIG_NOTHING_SET)
error("cannot overwrite multiple values with a single value\n"
- " Use a regexp, --add or --set-all to change %s.", argv[0]);
+ " Use a regexp, --add or --replace-all to change %s.", argv[0]);
return ret;
}
else if (actions == ACTION_SET_ALL) {
--
1.7.7.3
^ permalink raw reply related
* Re: [PATCH] honour GIT_ASKPASS for querying username in git-svn
From: Sven Strickroth @ 2011-12-26 23:49 UTC (permalink / raw)
To: git; +Cc: Jeff King, gitster
In-Reply-To: <20111130064401.GC5317@sigill.intra.peff.net>
Hi,
Am 30.11.2011 07:44 schrieb Jeff King:
> That aside, I think this is an improvement over the current code.
> 1. Regular git will also respect SSH_ASKPASS
> 2. Regular git will ignore an askpass variable that is set but empty.
> Perhaps git-svn should be refactored to have a reusable "prompt"
> function that respects askpass and tries to behave like C git? It could
> even go into the Git perl module.
I honoured all your ideas. Hopefully the patches can be applied now. The new patches
follow (you can also pull from git://github.com/csware/git.git askpass-prompt):
>From b760546c59d1b9982296c19f8eaea6dc225b5a4f Mon Sep 17 00:00:00 2001
From: Sven Strickroth <email@cs-ware.de>
Date: Tue, 27 Dec 2011 00:33:46 +0100
Subject: [PATCH 1/4] add central method for prompting a user using
GIT_ASKPASS or SSH_ASKPASS
Signed-off-by: Sven Strickroth <email@cs-ware.de>
---
perl/Git.pm | 31 ++++++++++++++++++++++++++++++-
1 files changed, 30 insertions(+), 1 deletions(-)
diff --git a/perl/Git.pm b/perl/Git.pm
index f7ce511..8176d47 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -58,7 +58,7 @@ require Exporter;
command_output_pipe command_input_pipe command_close_pipe
command_bidi_pipe command_close_bidi_pipe
version exec_path html_path hash_object git_cmd_try
- remote_refs
+ remote_refs prompt
temp_acquire temp_release temp_reset temp_path);
@@ -512,6 +512,35 @@ C<git --html-path>). Useful mostly only internally.
sub html_path { command_oneline('--html-path') }
+=item prompt ( PROMPT)
+
+Checks if GIT_ASKPASS or SSH_ASKPASS is set, and if yes
+use it and return answer from user.
+
+=cut
+
+sub prompt {
+ my ($self, $prompt) = _maybe_self(@_);
+ if (exists $ENV{'GIT_ASKPASS'}) {
+ return _prompt($ENV{'GIT_ASKPASS'}, $prompt);
+ } elsif (exists $ENV{'SSH_ASKPASS'}) {
+ return _prompt($ENV{'SSH_ASKPASS'}, $prompt);
+ } else {
+ return undef;
+ }
+}
+
+sub _prompt {
+ my ($self, $askpass, $prompt) = _maybe_self(@_);
+ my $ret;
+ open(PH, "-|", $askpass, $prompt);
+ $ret = <PH>;
+ $ret =~ s/[\012\015]//g; # strip \n\r
+ close(PH);
+ return $ret;
+}
+
+
=item repo_path ()
Return path to the git repository. Must be called on a repository instance.
--
1.7.7.1.msysgit.0
>From ef4c6557d1b0e33440d13c64742d44b2a22143f3 Mon Sep 17 00:00:00 2001
From: Sven Strickroth <email@cs-ware.de>
Date: Tue, 27 Dec 2011 00:34:09 +0100
Subject: [PATCH 2/4] switch to central prompt method
Signed-off-by: Sven Strickroth <email@cs-ware.de>
---
git-svn.perl | 9 ++-------
1 files changed, 2 insertions(+), 7 deletions(-)
diff --git a/git-svn.perl b/git-svn.perl
index eeb83d3..4fd4eca 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -4415,13 +4415,8 @@ sub username {
sub _read_password {
my ($prompt, $realm) = @_;
- my $password = '';
- if (exists $ENV{GIT_ASKPASS}) {
- open(PH, "-|", $ENV{GIT_ASKPASS}, $prompt);
- $password = <PH>;
- $password =~ s/[\012\015]//; # \n\r
- close(PH);
- } else {
+ my $password = Git->prompt($prompt);;
+ if (!defined $password) {
print STDERR $prompt;
STDERR->flush;
require Term::ReadKey;
--
1.7.7.1.msysgit.0
>From d58f41d7b9b8e690c9839f6f7539774da88aa3a4 Mon Sep 17 00:00:00 2001
From: Sven Strickroth <email@cs-ware.de>
Date: Tue, 27 Dec 2011 00:37:43 +0100
Subject: [PATCH 3/4] honour *_ASKPASS for querying username and for querying
further actions on unknown certificates
git-svn reads usernames (and answers for certificate errors) from an interactive terminal.
This behavior cause GUIs to hang waiting for git-svn to complete (http://code.google.com/p/tortoisegit/issues/detail?id=967).
Also see commit 56a853b62c0ae7ebaad0a7a0a704f5ef561eb795.
Signed-off-by: Sven Strickroth <email@cs-ware.de>
---
git-svn.perl | 13 ++++++++++---
1 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/git-svn.perl b/git-svn.perl
index 4fd4eca..b85a7de 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -4357,11 +4357,15 @@ sub ssl_server_trust {
issuer_dname fingerprint);
my $choice;
prompt:
- print STDERR $may_save ?
+ my $options = $may_save ?
"(R)eject, accept (t)emporarily or accept (p)ermanently? " :
"(R)eject or accept (t)emporarily? ";
- STDERR->flush;
- $choice = lc(substr(<STDIN> || 'R', 0, 1));
+ $choice = Git->prompt("Certificate unknown. " . $options);
+ if (!defined $choice) {
+ print STDERR $options;
+ STDERR->flush;
+ $choice = lc(substr(<STDIN> || 'R', 0, 1));
+ }
if ($choice =~ /^t$/i) {
$cred->may_save(undef);
} elsif ($choice =~ /^r$/i) {
@@ -4404,6 +4408,9 @@ sub username {
if (defined $_username) {
$username = $_username;
} else {
+ $username = Git->prompt("Username");
+ }
+ if (!defined $username) {
print STDERR "Username: ";
STDERR->flush;
chomp($username = <STDIN>);
--
1.7.7.1.msysgit.0
>From 2c1dbdae8024f28d17abfbdc7e45865a1277151a Mon Sep 17 00:00:00 2001
From: Sven Strickroth <email@cs-ware.de>
Date: Tue, 27 Dec 2011 00:42:07 +0100
Subject: [PATCH 4/4] ignore empty *_ASKPASS variables
Signed-off-by: Sven Strickroth <email@cs-ware.de>
---
perl/Git.pm | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/perl/Git.pm b/perl/Git.pm
index 8176d47..fade617 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -532,6 +532,9 @@ sub prompt {
sub _prompt {
my ($self, $askpass, $prompt) = _maybe_self(@_);
+ unless ($askpass) {
+ return undef;
+ }
my $ret;
open(PH, "-|", $askpass, $prompt);
$ret = <PH>;
--
1.7.7.1.msysgit.0
--
Best regards,
Sven Strickroth
ClamAV, a GPL anti-virus toolkit http://www.clamav.net
PGP key id F5A9D4C4 @ any key-server
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox