[PATCH] send-email: simplify Gmail example in the documentation

[Michal Nazarewicz]

There is no need for use to manually call ‘git credential’ especially
as the interface isn’t super user-friendly and a bit confusing.  ‘git
send-email’ will do that for them at the first execution and if the
password matches, it will be saved in the store.

Simplify the documentaion so it dosn’t include the ‘git credential’
invocation (which was incorrect anyway as it should use ‘approve’
instead of ‘fill’) and instead just mentions that credentials helper
must be set up.

Signed-off-by: Michał Nazarewicz 
---
 Documentation/git-send-email.txt | 16 ++--
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 71ef97ba9..af07840b4 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -473,16 +473,7 @@ edit ~/.gitconfig to specify your account settings:

 If you have multifactor authentication setup on your gmail account, you will
 need to generate an app-specific password for use with 'git send-email'. Visit
-https://security.google.com/settings/security/apppasswords to setup an
-app-specific password.  Once setup, you can store it with the credentials
-helper:
-
-   $ git credential fill
-   protocol=smtp
-   host=smtp.gmail.com
-   username=youn...@gmail.com
-   password=app-password
-
+https://security.google.com/settings/security/apppasswords to create it.

 Once your commits are ready to be sent to the mailing list, run the
 following commands:
@@ -491,7 +482,11 @@ following commands:
$ edit outgoing/-*
$ git send-email outgoing/*

+The first time you run it, you will be prompted for your credentials.  Enter 
the
+app-specific or your regular password as appropriate.  If you have credential
+helper configured (see linkgit:git-credential[1]), the password will be saved 
in
+the credential store so you won't have to type it the next time.
+
 Note: the following perl modules are required
   Net::SMTP::SSL, MIME::Base64 and Authen::SASL

-- 
2.16.2

[PATCH] send-email: fix docs regarding storing password with git credential

[Michal Nazarewicz]

First of all, ‘git credential fill’ does not store credentials
but is used to *read* them.  The command which adds credentials
to the helper’s store is ‘git credential approve’.

Second of all, git-send-email will include port number in host
parameter when getting the password so it has to be set when
storing the password as well.

Apply the two above to fix the Gmail example in git-send-email
documentation.

Signed-off-by: Michał Nazarewicz 
---
 Documentation/git-send-email.txt | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 71ef97ba9..172c7b344 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -477,13 +477,12 @@ 
https://security.google.com/settings/security/apppasswords to setup an
 app-specific password.  Once setup, you can store it with the credentials
 helper:
 
-   $ git credential fill
+   $ git credential approve
protocol=smtp
-   host=smtp.gmail.com
+   host=smtp.gmail.com:587
username=youn...@gmail.com
password=app-password
 
-
 Once your commits are ready to be sent to the mailing list, run the
 following commands:
 
-- 
2.16.2

[PATCH] send-email: report host and port separately when calling git credential

[Michal Nazarewicz]

When git-send-email uses git-credential to get SMTP password, it will
communicate SMTP host and port (if both are provided) as a single entry
‘host=:’.  This trips the ‘git-credential-store’ helper
which expects those values as separate keys (‘host’ and ‘port’).

Send the values as separate pieces of information so things work
smoothly.

Signed-off-by: Michał Nazarewicz 
---
 git-send-email.perl | 11 ++-
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index 2fa7818ca..2a9f89a58 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1229,14 +1229,6 @@ sub maildomain {
return maildomain_net() || maildomain_mta() || 'localhost.localdomain';
 }
 
-sub smtp_host_string {
-   if (defined $smtp_server_port) {
-   return "$smtp_server:$smtp_server_port";
-   } else {
-   return $smtp_server;
-   }
-}
-
 # Returns 1 if authentication succeeded or was not necessary
 # (smtp_user was not specified), and 0 otherwise.
 
@@ -1263,7 +1255,8 @@ sub smtp_auth_maybe {
# reject credentials.
$auth = Git::credential({
'protocol' => 'smtp',
-   'host' => smtp_host_string(),
+   'host' => $smtp_server,
+   'port' => $smtp_server_port,
'username' => $smtp_authuser,
# if there's no password, "git credential fill" will
# give us one, otherwise it'll just pass this one.
-- 
2.16.2

[PATCH] path: for clarity, rename get_pathname to get_path_buffer

[Michal Nazarewicz]

The get_pathname function does not really return path name but rather
a buffer to store pathname in.  As such, current name is a bit
confusing.  Change the name as to make it clearer what the function is
doing.

Signed-off-by: Michal Nazarewicz min...@mina86.com
---
 path.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/path.c b/path.c
index bc804a3..70e2f85 100644
--- a/path.c
+++ b/path.c
@@ -16,7 +16,7 @@ static int get_st_mode_bits(const char *path, int *mode)
 
 static char bad_path[] = /bad-path/;
 
-static char *get_pathname(void)
+static char *get_path_buffer(void)
 {
static char pathname_array[4][PATH_MAX];
static int index;
@@ -108,7 +108,7 @@ char *mkpath(const char *fmt, ...)
 {
va_list args;
unsigned len;
-   char *pathname = get_pathname();
+   char *pathname = get_path_buffer();
 
va_start(args, fmt);
len = vsnprintf(pathname, PATH_MAX, fmt, args);
@@ -120,7 +120,7 @@ char *mkpath(const char *fmt, ...)
 
 char *git_path(const char *fmt, ...)
 {
-   char *pathname = get_pathname();
+   char *pathname = get_path_buffer();
va_list args;
char *ret;
 
@@ -158,7 +158,7 @@ void home_config_paths(char **global, char **xdg, char 
*file)
 
 char *git_path_submodule(const char *path, const char *fmt, ...)
 {
-   char *pathname = get_pathname();
+   char *pathname = get_path_buffer();
struct strbuf buf = STRBUF_INIT;
const char *git_dir;
va_list args;
-- 
2.0.0.526.g5318336
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] path: for clarity, rename get_pathname to get_path_buffer

[Michal Nazarewicz]

The get_pathname function does not really return path name but rather
a buffer to store pathname in.  As such, current name is a bit
confusing.  Change the name as to make it clearer what the function is
doing.

Signed-off-by: Michal Nazarewicz min...@mina86.com
---
 path.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

 This time sent with Junio's correct address.

diff --git a/path.c b/path.c
index bc804a3..70e2f85 100644
--- a/path.c
+++ b/path.c
@@ -16,7 +16,7 @@ static int get_st_mode_bits(const char *path, int *mode)
 
 static char bad_path[] = /bad-path/;
 
-static char *get_pathname(void)
+static char *get_path_buffer(void)
 {
static char pathname_array[4][PATH_MAX];
static int index;
@@ -108,7 +108,7 @@ char *mkpath(const char *fmt, ...)
 {
va_list args;
unsigned len;
-   char *pathname = get_pathname();
+   char *pathname = get_path_buffer();
 
va_start(args, fmt);
len = vsnprintf(pathname, PATH_MAX, fmt, args);
@@ -120,7 +120,7 @@ char *mkpath(const char *fmt, ...)
 
 char *git_path(const char *fmt, ...)
 {
-   char *pathname = get_pathname();
+   char *pathname = get_path_buffer();
va_list args;
char *ret;
 
@@ -158,7 +158,7 @@ void home_config_paths(char **global, char **xdg, char 
*file)
 
 char *git_path_submodule(const char *path, const char *fmt, ...)
 {
-   char *pathname = get_pathname();
+   char *pathname = get_path_buffer();
struct strbuf buf = STRBUF_INIT;
const char *git_dir;
va_list args;
-- 
2.0.0.526.g5318336
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] Remove dead link

[Michal Nazarewicz]

From: Michal Nazarewicz min...@mina86.com

Signed-off-by: Michal Nazarewicz min...@mina86.com
---
 Documentation/technical/pack-heuristics.txt | 6 --
 1 file changed, 6 deletions(-)

diff --git a/Documentation/technical/pack-heuristics.txt 
b/Documentation/technical/pack-heuristics.txt
index 8b7ae1c..b7bd951 100644
--- a/Documentation/technical/pack-heuristics.txt
+++ b/Documentation/technical/pack-heuristics.txt
@@ -366,12 +366,6 @@ been detailed!
 
 linus Yes, we always write out most recent first
 
-For the other record:
-
-pasky njs`: http://pastebin.com/547965
-
-The 'net never forgets, so that should be good until the end of time.
-
 njs` And, yeah, I got the part about deeper-in-history stuff
 having worse IO characteristics, one sort of doesn't care.
 
-- 
1.8.4.rc1

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Remove dead link

[Michal Nazarewicz]

On Fri, Aug 23 2013, Junio C Hamano wrote:
 That is unfortunate, especially given the last line that the patch
 removes.  Has anybody asked pastebin folks why it is gone and if it
 can be resurrected?

Way Back Machine has nothing.

What was under that link?  If IRC logs, then those appear to be
available: http://pasky.or.cz/cp/%23git/2006-02-10.log

-- 
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of  o' \,=./ `o
..o | Computer Science,  Michał “mina86” Nazarewicz(o o)
ooo +--m...@google.com--xmpp:min...@jabber.org--ooO--(_)--Ooo--


signature.asc
Description: PGP signature

Re: [PATCHv4 6/6] git-send-email: use git credential to obtain password

[Michal Nazarewicz]

On Wed, Feb 27 2013, Junio C Hamano gits...@pobox.com wrote:
 Matthieu Moy matthieu@grenoble-inp.fr writes:

 Michal Nazarewicz m...@google.com writes:

 +   $auth = Git::credential({
 +   'protocol' = 'smtp',
 +   'host' = join(':', $smtp_server, $smtp_server_port),

 At this point, $smtp_server_port is not always defined. I just tested
 and got

 Use of uninitialized value $smtp_server_port in join or string at
 git-send-email line 1077.

 Other than that, the whole series looks good.

 Given that there is another place that conditionally append :$port
 to the host string, I think we should follow suit here.  Perhaps
 like the attached diff?

Damn meetings, you beat me to it…  I was just about to send a patch. ;)

 Thanks for a review.


  git-send-email.perl | 14 ++
  1 file changed, 10 insertions(+), 4 deletions(-)

 diff --git a/git-send-email.perl b/git-send-email.perl
 index 76bbfc3..c3501d9 100755
 --- a/git-send-email.perl
 +++ b/git-send-email.perl
 @@ -1045,6 +1045,14 @@ sub maildomain {
   return maildomain_net() || maildomain_mta() || 'localhost.localdomain';
  }
  
 +sub smtp_host_string {
 + if (defined $smtp_server_port) {
 + return $smtp_server:$smtp_server_port;
 + } else {
 + return $smtp_server;
 + }
 +}
 +
  # Returns 1 if authentication succeeded or was not necessary
  # (smtp_user was not specified), and 0 otherwise.
  
 @@ -1065,7 +1073,7 @@ sub smtp_auth_maybe {
   # reject credentials.
   $auth = Git::credential({
   'protocol' = 'smtp',
 - 'host' = join(':', $smtp_server, $smtp_server_port),
 + 'host' = smtp_host_string(),
   'username' = $smtp_authuser,
   # if there's no password, git credential fill will
   # give us one, otherwise it'll just pass this one.
 @@ -1188,9 +1196,7 @@ sub send_message {
   else {
   require Net::SMTP;
   $smtp_domain ||= maildomain();
 - $smtp ||= Net::SMTP-new((defined $smtp_server_port)
 -  ? 
 $smtp_server:$smtp_server_port
 -  : $smtp_server,
 + $smtp ||= Net::SMTP-new(smtp_host_string(),
Hello = $smtp_domain,
Debug = $debug_net_smtp);
   if ($smtp_encryption eq 'tls'  $smtp) {

From reading of SMTP.pm, it seems that this could be changed to:

-   $smtp ||= Net::SMTP-new((defined $smtp_server_port)
-? 
$smtp_server:$smtp_server_port
-: $smtp_server,
+   $smtp ||= Net::SMTP-new($smtp_server,
+Port = $smtp_server_port,

and than the other part would become:

@@ -1060,12 +1060,17 @@ sub smtp_auth_maybe {
Authen::SASL-import(qw(Perl));
};
 
+   my $host = $smtp_server;
+   if (defined $smtp_server_port) {
+   $host .= ':' . $smtp_server_port;
+   }
+
# TODO: Authentication may fail not because credentials were
# invalid but due to other reasons, in which we should not
# reject credentials.
$auth = Git::credential({
'protocol' = 'smtp',
-   'host' = join(':', $smtp_server, $smtp_server_port),
+   'host' = $host,
'username' = $smtp_authuser,
# if there's no password, git credential fill will
# give us one, otherwise it'll just pass this one.

Either way, looks good to me.

-- 
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of  o' \,=./ `o
..o | Computer Science,  Michał “mina86” Nazarewicz(o o)
ooo +email/xmpp: m...@google.com--ooO--(_)--Ooo--

pgpjon85cv7ZQ.pgp
Description: PGP signature

[PATCHv4 4/6] Git.pm: allow pipes to be closed prior to calling command_close_bidi_pipe

[Michal Nazarewicz]

From: Michal Nazarewicz min...@mina86.com

The command_close_bidi_pipe() function will insist on closing both
input and output pipes returned by command_bidi_pipe().  With this
change it is possible to close one of the pipes in advance and
pass undef as an argument.

Signed-off-by: Michal Nazarewicz min...@mina86.com
---
 perl/Git.pm | 15 ++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/perl/Git.pm b/perl/Git.pm
index 6bc9a3c..d6e6c9e 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -426,12 +426,25 @@ Note that you should not rely on whatever actually is in 
CCTX;
 currently it is simply the command name but in future the context might
 have more complicated structure.
 
+CPIPE_IN and CPIPE_OUT may be Cundef if they have been closed prior to
+calling this function.  This may be useful in a query-response type of
+commands where caller first writes a query and later reads response, eg:
+
+   my ($pid, $in, $out, $ctx) = $r-command_bidi_pipe('cat-file 
--batch-check');
+   print $out 0\n;
+   close $out;
+   while ($in) { ... }
+   $r-command_close_bidi_pipe($pid, $in, undef, $ctx);
+
+This idiom may prevent potential dead locks caused by data sent to the output
+pipe not being flushed and thus not reaching the executed command.
+
 =cut
 
 sub command_close_bidi_pipe {
local $?;
my ($self, $pid, $in, $out, $ctx) = _maybe_self(@_);
-   _cmd_close($ctx, $in, $out);
+   _cmd_close($ctx, grep defined, $in, $out);
waitpid $pid, 0;
if ($?  8) {
throw Git::Error::Command($ctx, $? 8);
-- 
1.8.1.3.572.g32bae1f

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv4 6/6] git-send-email: use git credential to obtain password

[Michal Nazarewicz]

From: Michal Nazarewicz min...@mina86.com

If smtp_user is provided but smtp_pass is not, instead of
prompting for password, make git-send-email use git
credential command instead.

Signed-off-by: Michal Nazarewicz min...@mina86.com
---
 Documentation/git-send-email.txt |  4 +--
 git-send-email.perl  | 59 +++-
 2 files changed, 36 insertions(+), 27 deletions(-)

diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 44a1f7c..0cffef8 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -164,8 +164,8 @@ Sending
 Furthermore, passwords need not be specified in configuration files
 or on the command line. If a username has been specified (with
 '--smtp-user' or a 'sendemail.smtpuser'), but no password has been
-specified (with '--smtp-pass' or 'sendemail.smtppass'), then the
-user is prompted for a password while the input is masked for privacy.
+specified (with '--smtp-pass' or 'sendemail.smtppass'), then
+a password is obtained using 'git-credential'.
 
 --smtp-server=host::
If set, specifies the outgoing SMTP server to use (e.g.
diff --git a/git-send-email.perl b/git-send-email.perl
index be809e5..76bbfc3 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1045,6 +1045,39 @@ sub maildomain {
return maildomain_net() || maildomain_mta() || 'localhost.localdomain';
 }
 
+# Returns 1 if authentication succeeded or was not necessary
+# (smtp_user was not specified), and 0 otherwise.
+
+sub smtp_auth_maybe {
+   if (!defined $smtp_authuser || $auth) {
+   return 1;
+   }
+
+   # Workaround AUTH PLAIN/LOGIN interaction defect
+   # with Authen::SASL::Cyrus
+   eval {
+   require Authen::SASL;
+   Authen::SASL-import(qw(Perl));
+   };
+
+   # TODO: Authentication may fail not because credentials were
+   # invalid but due to other reasons, in which we should not
+   # reject credentials.
+   $auth = Git::credential({
+   'protocol' = 'smtp',
+   'host' = join(':', $smtp_server, $smtp_server_port),
+   'username' = $smtp_authuser,
+   # if there's no password, git credential fill will
+   # give us one, otherwise it'll just pass this one.
+   'password' = $smtp_authpass
+   }, sub {
+   my $cred = shift;
+   return !!$smtp-auth($cred-{'username'}, $cred-{'password'});
+   });
+
+   return $auth;
+}
+
 # Returns 1 if the message was sent, and 0 otherwise.
 # In actuality, the whole program dies when there
 # is an error sending a message.
@@ -1185,31 +1218,7 @@ X-Mailer: git-send-email $gitversion
defined $smtp_server_port ?  
port=$smtp_server_port : ;
}
 
-   if (defined $smtp_authuser) {
-   # Workaround AUTH PLAIN/LOGIN interaction defect
-   # with Authen::SASL::Cyrus
-   eval {
-   require Authen::SASL;
-   Authen::SASL-import(qw(Perl));
-   };
-
-   if (!defined $smtp_authpass) {
-
-   system stty -echo;
-
-   do {
-   print Password: ;
-   $_ = STDIN;
-   print \n;
-   } while (!defined $_);
-
-   chomp($smtp_authpass = $_);
-
-   system stty echo;
-   }
-
-   $auth ||= $smtp-auth( $smtp_authuser, $smtp_authpass ) 
or die $smtp-message;
-   }
+   smtp_auth_maybe or die $smtp-message;
 
$smtp-mail( $raw_from ) or die $smtp-message;
$smtp-to( @recipients ) or die $smtp-message;
-- 
1.8.1.3.572.g32bae1f

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv4 0/6] git-credential support in git-send-email

[Michal Nazarewicz]

From: Michal Nazarewicz min...@mina86.com

Besids git-credential support in git-send-email, there are some other
minor improvements to Git.pm in this patchset.  Patch 3/6 is new
compared to the previous patchset.

Michal Nazarewicz (6):
  Git.pm: allow command_close_bidi_pipe to be called as method
  Git.pm: fix example in command_close_bidi_pipe documentation
  Git.pm: refactor command_close_bidi_pipe to use _cmd_close
  Git.pm: allow pipes to be closed prior to calling
command_close_bidi_pipe
  Git.pm: add interface for git credential command
  git-send-email: use git credential to obtain password

 Documentation/git-send-email.txt |   4 +-
 git-send-email.perl  |  59 +++-
 perl/Git.pm  | 198 ++-
 3 files changed, 213 insertions(+), 48 deletions(-)

-- 
1.8.1.3.572.g32bae1f

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv4 1/6] Git.pm: allow command_close_bidi_pipe to be called as method

[Michal Nazarewicz]

From: Michal Nazarewicz min...@mina86.com

The documentation of command_close_bidi_pipe() claims that it can
be called as a method, but it does not check whether the first
argument is $self or not assuming the latter.  Using _maybe_self()
fixes this.

Signed-off-by: Michal Nazarewicz min...@mina86.com
---
 perl/Git.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/perl/Git.pm b/perl/Git.pm
index 931047c..bbb753a 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -430,7 +430,7 @@ have more complicated structure.
 
 sub command_close_bidi_pipe {
local $?;
-   my ($pid, $in, $out, $ctx) = @_;
+   my ($self, $pid, $in, $out, $ctx) = _maybe_self(@_);
foreach my $fh ($in, $out) {
unless (close $fh) {
if ($!) {
-- 
1.8.1.3.572.g32bae1f

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv4 5/6] Git.pm: add interface for git credential command

[Michal Nazarewicz]

From: Michal Nazarewicz min...@mina86.com

Add a credential() function which is an interface to the git
credential command.  The code is heavily based on credential_*
functions in contrib/mw-to-git/git-remote-mediawiki.

Signed-off-by: Michal Nazarewicz min...@mina86.com
---
 perl/Git.pm | 151 
 1 file changed, 151 insertions(+)

diff --git a/perl/Git.pm b/perl/Git.pm
index d6e6c9e..a24458c 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -59,6 +59,7 @@ require Exporter;
 command_bidi_pipe command_close_bidi_pipe
 version exec_path html_path hash_object git_cmd_try
 remote_refs prompt
+credential credential_read credential_write
 temp_acquire temp_release temp_reset temp_path);
 
 
@@ -1003,6 +1004,156 @@ sub _close_cat_blob {
 }
 
 
+=item credential_read( FILEHANDLE )
+
+Reads credential key-value pairs from CFILEHANDLE.  Reading stops at EOF or
+when an empty line is encountered.  Each line must be of the form Ckey=value
+with a non-empty key.  Function returns hash with all read values.  Any white
+space (other then new-line character) is preserved.
+
+=cut
+
+sub credential_read {
+   my ($self, $reader) = _maybe_self(@_);
+   my %credential;
+   while ($reader) {
+   chomp;
+   if ($_ eq '') {
+   last;
+   } elsif (!/^([^=]+)=(.*)$/) {
+   throw Error::Simple(unable to parse git credential 
data:\n$_);
+   }
+   $credential{$1} = $2;
+   }
+   return %credential;
+}
+
+=item credential_write( FILEHANDLE, CREDENTIAL_HASHREF )
+
+Writes credential key-value pairs from hash referenced by
+CCREDENTIAL_HASHREF to CFILEHANDLE.  Keys and values cannot contain
+new-lines or NUL bytes characters, and key cannot contain equal signs nor be
+empty (if they do Error::Simple is thrown).  Any white space is preserved.  If
+value for a key is Cundef, it will be skipped.
+
+If C'url' key exists it will be written first.  (All the other key-value
+pairs are written in sorted order but you should not depend on that).  Once
+all lines are written, an empty line is printed.
+
+=cut
+
+sub credential_write {
+   my ($self, $writer, $credential) = _maybe_self(@_);
+   my ($key, $value);
+
+   # Check if $credential is valid prior to writing anything
+   while (($key, $value) = each %$credential) {
+   if (!defined $key || !length $key) {
+   throw Error::Simple(credential key empty or 
undefined);
+   } elsif ($key =~ /[=\n\0]/) {
+   throw Error::Simple(credential key contains invalid 
characters: $key);
+   } elsif (defined $value  $value =~ /[\n\0]/) {
+   throw Error::Simple(credential value for key=$key 
contains invalid characters: $value);
+   }
+   }
+
+   for $key (sort {
+   # url overwrites other fields, so it must come first
+   return -1 if $a eq 'url';
+   return  1 if $b eq 'url';
+   return $a cmp $b;
+   } keys %$credential) {
+   if (defined $credential-{$key}) {
+   print $writer $key, '=', $credential-{$key}, \n;
+   }
+   }
+   print $writer \n;
+}
+
+sub _credential_run {
+   my ($self, $credential, $op) = _maybe_self(@_);
+   my ($pid, $reader, $writer, $ctx) = command_bidi_pipe('credential', 
$op);
+
+   credential_write $writer, $credential;
+   close $writer;
+
+   if ($op eq fill) {
+   %$credential = credential_read $reader;
+   }
+   if ($reader) {
+   throw Error::Simple(unexpected output from git credential $op 
response:\n$_\n);
+   }
+
+   command_close_bidi_pipe($pid, $reader, undef, $ctx);
+}
+
+=item credential( CREDENTIAL_HASHREF [, OPERATION ] )
+
+=item credential( CREDENTIAL_HASHREF, CODE )
+
+Executes Cgit credential for a given set of credentials and specified
+operation.  In both forms CCREDENTIAL_HASHREF needs to be a reference to
+a hash which stores credentials.  Under certain conditions the hash can
+change.
+
+In the first form, COPERATION can be C'fill', C'approve' or C'reject',
+and function will execute corresponding Cgit credential sub-command.  If
+it's omitted C'fill' is assumed.  In case of C'fill' the values stored in
+CCREDENTIAL_HASHREF will be changed to the ones returned by the Cgit
+credential fill command.  The usual usage would look something like:
+
+   my %cred = (
+   'protocol' = 'https',
+   'host' = 'example.com',
+   'username' = 'bob'
+   );
+   Git::credential \%cred;
+   if (try_to_authenticate($cred{'username'}, $cred{'password'})) {
+   Git::credential \%cred, 'approve';
+   ... do more stuff ...
+   } else

[PATCHv4 3/6] Git.pm: refactor command_close_bidi_pipe to use _cmd_close

[Michal Nazarewicz]

From: Michal Nazarewicz min...@mina86.com

The body of the loop in command_close_bidi_pipe function is identical to
what _cmd_close function does so instead of duplicating, refactor change
_cmd_close so that it accepts list of file handlers to be closed, which
makes it usable with command_close_bidi_pipe.

Signed-off-by: Michal Nazarewicz min...@mina86.com
---
 perl/Git.pm | 30 +++---
 1 file changed, 11 insertions(+), 19 deletions(-)

diff --git a/perl/Git.pm b/perl/Git.pm
index 11f310a..6bc9a3c 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -267,13 +267,13 @@ sub command {
 
if (not defined wantarray) {
# Nothing to pepper the possible exception with.
-   _cmd_close($fh, $ctx);
+   _cmd_close($ctx, $fh);
 
} elsif (not wantarray) {
local $/;
my $text = $fh;
try {
-   _cmd_close($fh, $ctx);
+   _cmd_close($ctx, $fh);
} catch Git::Error::Command with {
# Pepper with the output:
my $E = shift;
@@ -286,7 +286,7 @@ sub command {
my @lines = $fh;
defined and chomp for @lines;
try {
-   _cmd_close($fh, $ctx);
+   _cmd_close($ctx, $fh);
} catch Git::Error::Command with {
my $E = shift;
$E-{'-outputref'} = \@lines;
@@ -313,7 +313,7 @@ sub command_oneline {
my $line = $fh;
defined $line and chomp $line;
try {
-   _cmd_close($fh, $ctx);
+   _cmd_close($ctx, $fh);
} catch Git::Error::Command with {
# Pepper with the output:
my $E = shift;
@@ -381,7 +381,7 @@ have more complicated structure.
 sub command_close_pipe {
my ($self, $fh, $ctx) = _maybe_self(@_);
$ctx ||= 'unknown';
-   _cmd_close($fh, $ctx);
+   _cmd_close($ctx, $fh);
 }
 
 =item command_bidi_pipe ( COMMAND [, ARGUMENTS... ] )
@@ -431,18 +431,8 @@ have more complicated structure.
 sub command_close_bidi_pipe {
local $?;
my ($self, $pid, $in, $out, $ctx) = _maybe_self(@_);
-   foreach my $fh ($in, $out) {
-   unless (close $fh) {
-   if ($!) {
-   carp error closing pipe: $!;
-   } elsif ($?  8) {
-   throw Git::Error::Command($ctx, $? 8);
-   }
-   }
-   }
-
+   _cmd_close($ctx, $in, $out);
waitpid $pid, 0;
-
if ($?  8) {
throw Git::Error::Command($ctx, $? 8);
}
@@ -1355,9 +1345,11 @@ sub _execv_git_cmd { exec('git', @_); }
 
 # Close pipe to a subprocess.
 sub _cmd_close {
-   my ($fh, $ctx) = @_;
-   if (not close $fh) {
-   if ($!) {
+   my $ctx = shift @_;
+   foreach my $fh (@_) {
+   if (close $fh) {
+   # nop;
+   } elsif ($!) {
# It's just close, no point in fatalities
carp error closing pipe: $!;
} elsif ($?  8) {
-- 
1.8.1.3.572.g32bae1f

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv4 2/6] Git.pm: fix example in command_close_bidi_pipe documentation

[Michal Nazarewicz]

From: Michal Nazarewicz min...@mina86.com


Signed-off-by: Michal Nazarewicz min...@mina86.com
---
 perl/Git.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/perl/Git.pm b/perl/Git.pm
index bbb753a..11f310a 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -418,7 +418,7 @@ and it is the fourth value returned by 
Ccommand_bidi_pipe().  The call idiom
 is:
 
my ($pid, $in, $out, $ctx) = $r-command_bidi_pipe('cat-file 
--batch-check');
-   print 0\n $out;
+   print $out 0\n;
while ($in) { ... }
$r-command_close_bidi_pipe($pid, $in, $out, $ctx);
 
-- 
1.8.1.3.572.g32bae1f

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv4 3/6] Git.pm: refactor command_close_bidi_pipe to use _cmd_close

[Michal Nazarewicz]

 Michal Nazarewicz m...@google.com writes:
  The body of the loop in command_close_bidi_pipe function is identical to
  what _cmd_close function does so instead of duplicating, refactor change
  _cmd_close so that it accepts list of file handlers to be closed, which

 On Tue, Feb 12, 2013 at 10:55:05AM -0800, Junio C Hamano wrote:
 s/file handlers/file handles/, I think.

On Tue, Feb 12 2013, Jeff King wrote:
 And s/refactor change/refactor/.

 Other than that, I think the series looks OK. I have one style micro-nit
 on patch 4 which I'll reply in-line. But it is either fix while
 applying or ignore, I don't think it will be worth a re-roll.

All fixed.

Junio, do you want me to resend or would you be fine with just pulling:

git://github.com/mina86/git.git master

-- 
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of  o' \,=./ `o
..o | Computer Science,  Michał “mina86” Nazarewicz(o o)
ooo +email/xmpp: m...@google.com--ooO--(_)--Ooo--

pgpLiQYC6X4hj.pgp
Description: PGP signature

Re: [PATCHv4 4/6] Git.pm: allow pipes to be closed prior to calling command_close_bidi_pipe

[Michal Nazarewicz]

On Tue, Feb 12 2013, Jeff King wrote:
 On Tue, Feb 12, 2013 at 03:02:31PM +0100, Michal Nazarewicz wrote:

  sub command_close_bidi_pipe {
  local $?;
  my ($self, $pid, $in, $out, $ctx) = _maybe_self(@_);
 -_cmd_close($ctx, $in, $out);
 +_cmd_close($ctx, grep defined, $in, $out);

 Maybe it is just me, but I find the grep EXPR form a little subtle
 inside an argument list. Either:

   _cmd_close($ctx, grep { defined } $in, $out);

 or

   _cmd_close($ctx, grep(defined, $in, $out));

 is a little more obvious to me.

I personally avoid parens whenever possible in Perl, but Git.pm seem to
favour them so I went with the second option.

-- 
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of  o' \,=./ `o
..o | Computer Science,  Michał “mina86” Nazarewicz(o o)
ooo +email/xmpp: m...@google.com--ooO--(_)--Ooo--

pgpkczeNsdtRQ.pgp
Description: PGP signature

Re: [PATCHv4 4/6] Git.pm: allow pipes to be closed prior to calling command_close_bidi_pipe

[Michal Nazarewicz]

On Tue, Feb 12 2013, Junio C Hamano wrote:
 I would actually vote for the most explicit:

   _cmd_close($ctx, (grep { defined } ($in, $out)));

To me that looks weird at best, but I don't have strong opinions on that
matter.

-- 
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of  o' \,=./ `o
..o | Computer Science,  Michał “mina86” Nazarewicz(o o)
ooo +email/xmpp: m...@google.com--ooO--(_)--Ooo--

pgp4dBTjMexcA.pgp
Description: PGP signature

[PATCHv3 0/5] Add git-credential support to git-send-email

[Michal Nazarewicz]

From: Michal Nazarewicz min...@mina86.com

The third version of the patch with changes suggested by Jeff in the
4/5 patch.  Also credential_read and credential_write are now public
functions in case someone wants to write a helper in perl.

Michal Nazarewicz (5):
  Git.pm: allow command_close_bidi_pipe to be called as method
  Git.pm: fix example in command_close_bidi_pipe documentation
  Git.pm: allow pipes to be closed prior to calling
command_close_bidi_pipe
  Git.pm: add interface for git credential command
  git-send-email: use git credential to obtain password

 Documentation/git-send-email.txt |   4 +-
 git-send-email.perl  |  59 --
 perl/Git.pm  | 166 ++-
 3 files changed, 198 insertions(+), 31 deletions(-)

-- 
1.8.1.3.571.g3f8bed7

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv3 1/5] Git.pm: allow command_close_bidi_pipe to be called as method

[Michal Nazarewicz]

From: Michal Nazarewicz min...@mina86.com

The documentation of command_close_bidi_pipe() claims that it can
be called as a method, but it does not check whether the first
argument is $self or not assuming the latter.  Using _maybe_self()
fixes this.

Signed-off-by: Michal Nazarewicz min...@mina86.com
---
 perl/Git.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/perl/Git.pm b/perl/Git.pm
index 931047c..bbb753a 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -430,7 +430,7 @@ have more complicated structure.
 
 sub command_close_bidi_pipe {
local $?;
-   my ($pid, $in, $out, $ctx) = @_;
+   my ($self, $pid, $in, $out, $ctx) = _maybe_self(@_);
foreach my $fh ($in, $out) {
unless (close $fh) {
if ($!) {
-- 
1.8.1.3.571.g3f8bed7.dirty

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv3 3/5] Git.pm: allow pipes to be closed prior to calling command_close_bidi_pipe

[Michal Nazarewicz]

From: Michal Nazarewicz min...@mina86.com

The command_close_bidi_pipe() function will insist on closing both
input and output pipes returned by command_bidi_pipe().  With this
change it is possible to close one of the pipes in advance and
pass undef as an argument.

Signed-off-by: Michal Nazarewicz min...@mina86.com
---
 perl/Git.pm | 15 ++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/perl/Git.pm b/perl/Git.pm
index 11f310a..9dded54 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -426,13 +426,26 @@ Note that you should not rely on whatever actually is in 
CCTX;
 currently it is simply the command name but in future the context might
 have more complicated structure.
 
+CPIPE_IN and CPIPE_OUT may be Cundef if they have been closed prior to
+calling this function.  This may be useful in a query-response type of
+commands where caller first writes a query and later reads response, eg:
+
+   my ($pid, $in, $out, $ctx) = $r-command_bidi_pipe('cat-file 
--batch-check');
+   print $out 0\n;
+   close $out;
+   while ($in) { ... }
+   $r-command_close_bidi_pipe($pid, $in, undef, $ctx);
+
+This idiom may prevent potential dead locks caused by data sent to the output
+pipe not being flushed and thus not reaching the executed command.
+
 =cut
 
 sub command_close_bidi_pipe {
local $?;
my ($self, $pid, $in, $out, $ctx) = _maybe_self(@_);
foreach my $fh ($in, $out) {
-   unless (close $fh) {
+   if (defined $fh  !close $fh) {
if ($!) {
carp error closing pipe: $!;
} elsif ($?  8) {
-- 
1.8.1.3.571.g3f8bed7.dirty

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv3 2/5] Git.pm: fix example in command_close_bidi_pipe documentation

[Michal Nazarewicz]

From: Michal Nazarewicz min...@mina86.com


Signed-off-by: Michal Nazarewicz min...@mina86.com
---
 perl/Git.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/perl/Git.pm b/perl/Git.pm
index bbb753a..11f310a 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -418,7 +418,7 @@ and it is the fourth value returned by 
Ccommand_bidi_pipe().  The call idiom
 is:
 
my ($pid, $in, $out, $ctx) = $r-command_bidi_pipe('cat-file 
--batch-check');
-   print 0\n $out;
+   print $out 0\n;
while ($in) { ... }
$r-command_close_bidi_pipe($pid, $in, $out, $ctx);
 
-- 
1.8.1.3.571.g3f8bed7.dirty

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv3 4/5] Git.pm: add interface for git credential command

[Michal Nazarewicz]

On Mon, Feb 11 2013, Jeff King wrote:
 On Mon, Feb 11, 2013 at 05:23:38PM +0100, Michal Nazarewicz wrote:

 +=item credential_read( FILE_HANDLE )
 +
 +Reads credential key-value pairs from CFILE_HANDLE.  Reading stops at EOF 
 or
 +when an empty line is encountered.  Each line must be of the form 
 Ckey=value
 +with a non-empty key.  Function returns a hash with all read values.  Any
 +white space (other then new-line character) is preserved.
 +
 +=cut
 +
 +sub credential_read {
 +my ($self, $reader) = _maybe_self(@_);
 +my %credential;
 +while ($reader) {
 +chomp;
 +if ($_ eq '') {
 +last;
 +} elsif (!/^([^=]+)=(.*)$/) {
 +throw Error::Simple(unable to parse git credential 
 data:\n$_);
 +}
 +$credential{$1} = $2;
 +}
 +return %credential;
 +}

 Should this return a hash reference? It seems like that is how we end up
 using and passing it elsewhere (since we have to anyway when passing it
 as a parameter).

Admittedly I mostly just copied what git-remote-mediawiki did here and
don't really have any preference either way, even though with this
function returning a reference the call site would have to become:

%$credential = %{ credential_read $reader };

Another alternative would be for it to take a reference as an argument,
possibly an optional one:

+sub credential_read {
+   my ($self, $reader, $ret) = (_maybe_self(@_), {});
+   my %credential;
+   while ($reader) {
+   # ...
+   }
+   %$ret = %credential;
+   $ret;
+}

I'd avoid modifying the hash while reading though since I think it's
best if it's left intact in case of an error.

And of course, if we want to get even more crazy, credential_write could
accept either reference or a hash, like so:

+sub credential_write {
+   my ($self, $writer, @rest) = _maybe_self(@_);
+   my $credential = @rest == 1 ? $rest[0] : { @rest };
+   my ($key, $value);
+   # ...
+}

Bottom line is, anything can be coded, but a question is whether it
makes sense to do so. ;)

-- 
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of  o' \,=./ `o
..o | Computer Science,  Michał “mina86” Nazarewicz(o o)
ooo +email/xmpp: m...@google.com--ooO--(_)--Ooo--

pgpzSk9PC_DIu.pgp
Description: PGP signature

Re: [PATCHv3 0/5] Add git-credential support to git-send-email

[Michal Nazarewicz]

On Mon, Feb 11 2013, Jeff King wrote:
 I have two minor comments, which I'll reply inline with. But even with
 those comments, I think this would be OK to merge.

I'll send a new patchset tomorrow with.

-- 
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of  o' \,=./ `o
..o | Computer Science,  Michał “mina86” Nazarewicz(o o)
ooo +email/xmpp: m...@google.com--ooO--(_)--Ooo--

pgpaRPCA6Kr3d.pgp
Description: PGP signature

[PATCHv2 0/5] Make git-send-email use git-credential

[Michal Nazarewicz]

From: Michal Nazarewicz min...@mina86.com

Minor fixes as suggested in emails.

Michal Nazarewicz (5):
  Git.pm: allow command_close_bidi_pipe to be called as method
  Git.pm: fix example in command_close_bidi_pipe documentation
  Git.pm: allow pipes to be closed prior to calling
command_close_bidi_pipe
  Git.pm: add interface for git credential command
  git-send-email: use git credential to obtain password

 Documentation/git-send-email.txt |   4 +-
 git-send-email.perl  |  59 ++
 perl/Git.pm  | 129 +--
 3 files changed, 161 insertions(+), 31 deletions(-)

-- 
1.8.1.2.549.g1d13f9f

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv2 1/5] Git.pm: allow command_close_bidi_pipe to be called as method

[Michal Nazarewicz]

From: Michal Nazarewicz min...@mina86.com

The documentation of command_close_bidi_pipe() claims that it can
be called as a method, but it does not check whether the first
argument is $self or not assuming the latter.  Using _maybe_self()
fixes this.

Signed-off-by: Michal Nazarewicz min...@mina86.com
---
 perl/Git.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/perl/Git.pm b/perl/Git.pm
index 931047c..bbb753a 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -430,7 +430,7 @@ have more complicated structure.
 
 sub command_close_bidi_pipe {
local $?;
-   my ($pid, $in, $out, $ctx) = @_;
+   my ($self, $pid, $in, $out, $ctx) = _maybe_self(@_);
foreach my $fh ($in, $out) {
unless (close $fh) {
if ($!) {
-- 
1.8.1.2.549.g1d13f9f

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv2 2/5] Git.pm: fix example in command_close_bidi_pipe documentation

[Michal Nazarewicz]

From: Michal Nazarewicz min...@mina86.com

File handle goes as the first argument when calling print on it.

Signed-off-by: Michal Nazarewicz min...@mina86.com
---
 perl/Git.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/perl/Git.pm b/perl/Git.pm
index bbb753a..11f310a 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -418,7 +418,7 @@ and it is the fourth value returned by 
Ccommand_bidi_pipe().  The call idiom
 is:
 
my ($pid, $in, $out, $ctx) = $r-command_bidi_pipe('cat-file 
--batch-check');
-   print 0\n $out;
+   print $out 0\n;
while ($in) { ... }
$r-command_close_bidi_pipe($pid, $in, $out, $ctx);
 
-- 
1.8.1.2.549.g1d13f9f

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv2 4/5] Git.pm: add interface for git credential command

[Michal Nazarewicz]

From: Michal Nazarewicz min...@mina86.com

Add a credential() function which is an interface to the git
credential command.  The code is heavily based on credential_*
functions in contrib/mw-to-git/git-remote-mediawiki.

Signed-off-by: Michal Nazarewicz min...@mina86.com
---
 perl/Git.pm | 110 +++-
 1 file changed, 109 insertions(+), 1 deletion(-)

 On Thu, Feb 07 2013, Jeff King p...@peff.net wrote:
  On Wed, Feb 06, 2013 at 09:47:05PM +0100, Michal Nazarewicz wrote:
 
  +sub _credential_read {
  +   my %credential;
  +   my ($reader, $op) = (@_);
  +   while ($reader) {
  +   chomp;
  +   my ($key, $value) = /([^=]*)=(.*)/;
 
  Empty keys are not valid. Can we make this:
 
/^([^=]+)=(.*)/
 
  to fail the regex? Otherwise, I think this check:
 
  +   if (not defined $key) {
  +   throw Error::Simple(unable to parse git credential $op 
  response:\n$_\n);
  +   }
 
  would not pass because $key would be the empty string.

 Right, fixed.  

  +sub _credential_write {
  +   my ($credential, $writer) = @_;
  +
  +   for my $key (sort {
  +   # url overwrites other fields, so it must come first
  +   return -1 if $a eq 'url';
  +   return  1 if $b eq 'url';
  +   return $a cmp $b;
  +   } keys %$credential) {
  +   if (defined $credential-{$key}  length $credential-{$key}) {
  +   print $writer $key, '=', $credential-{$key}, \n;
  +   }
  +   }
 
  There are a few disallowed characters, like \n in key or value, and
  = in a key. They should never happen unless the caller is buggy, but
  should we check and catch them here?

 I left it as is for now since it's not entairly clear to me what to
 do in all cases.  In particular:
 
 - when reading, what to do if the line is  foo = bar ,
 - when reading, what to do if the line is foo= (ie. empty value),
 - when writing, what to do if value is a single space,
 - when writing, what to do if value ends with a new line,
 - when writing, what to do if value is empty (currently not printed at all),

 On Thu, Feb 07 2013, Matthieu Moy matthieu@grenoble-inp.fr wrote:
  I think you should credit git-remote-mediawiki for the code in the
  commit message. Perhaps have a first copy/paste commit, and then an
  adaptation commit to add sort, ^ anchor in regexp, doc and your
  callback mechanism, but I won't insist on that.

 Good point.  Creating additional commit is a bit too much for my
 licking, but added note in commit message.

diff --git a/perl/Git.pm b/perl/Git.pm
index 9dded54..b4adead 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -59,7 +59,8 @@ require Exporter;
 command_bidi_pipe command_close_bidi_pipe
 version exec_path html_path hash_object git_cmd_try
 remote_refs prompt
-temp_acquire temp_release temp_reset temp_path);
+temp_acquire temp_release temp_reset temp_path
+credential);
 
 
 =head1 DESCRIPTION
@@ -1013,6 +1014,113 @@ sub _close_cat_blob {
 }
 
 
+sub _credential_read {
+   my %credential;
+   my ($reader, $op) = (@_);
+   while ($reader) {
+   if (!/^([^=\s]+)=(.*?)\s*$/) {
+   throw Error::Simple(unable to parse git credential $op 
response:\n$_);
+   }
+   $credential{$1} = $2;
+   }
+   return %credential;
+}
+
+sub _credential_write {
+   my ($credential, $writer) = @_;
+
+   for my $key (sort {
+   # url overwrites other fields, so it must come first
+   return -1 if $a eq 'url';
+   return  1 if $b eq 'url';
+   return $a cmp $b;
+   } keys %$credential) {
+   if (defined $credential-{$key}  length $credential-{$key}) {
+   print $writer $key, '=', $credential-{$key}, \n;
+   }
+   }
+   print $writer \n;
+}
+
+sub _credential_run {
+   my ($self, $credential, $op) = _maybe_self(@_);
+
+   my ($pid, $reader, $writer, $ctx) = command_bidi_pipe('credential', 
$op);
+
+   _credential_write $credential, $writer;
+   close $writer;
+
+   if ($op eq fill) {
+   %$credential = _credential_read $reader, $op;
+   } elsif ($reader) {
+   throw Error::Simple(unexpected output from git credential $op 
response:\n$_\n);
+   }
+
+   command_close_bidi_pipe($pid, $reader, undef, $ctx);
+}
+
+=item credential( CREDENTIAL_HASH [, OPERATION ] )
+
+=item credential( CREDENTIAL_HASH, CODE )
+
+Executes Cgit credential for a given set of credentials and
+specified operation.  In both form CCREDENTIAL_HASH needs to be
+a reference to a hash which stores credentials.  Under certain
+conditions the hash can change.
+
+In the first form, COPERATION can be C'fill' (or omitted),
+C'approve' or C'reject', and function will execute corresponding
+Cgit credential sub

[PATCHv2 3/5] Git.pm: allow pipes to be closed prior to calling command_close_bidi_pipe

[Michal Nazarewicz]

From: Michal Nazarewicz min...@mina86.com

The command_close_bidi_pipe() function will insist on closing both
input and output pipes returned by command_bidi_pipe().  With this
change it is possible to close one of the pipes in advance and
pass undef as an argument.

Signed-off-by: Michal Nazarewicz min...@mina86.com
---
 perl/Git.pm | 15 ++-
 1 file changed, 14 insertions(+), 1 deletion(-)

  On Wed, Feb 06, 2013 at 09:47:04PM +0100, Michal Nazarewicz wrote:
  This allows for something like:
  
my ($pid, $in, $out, $ctx) = command_bidi_pipe(...);
print $out write data;
close $out;
# ... do stuff with $in
command_close_bidi_pipe($pid, $in, undef, $ctx);

 On Thu, Feb 07 2013, Jeff King p...@peff.net wrote:
  Should this part go into the documentation for command_close_bidi_pipe
  in Git.pm?

 Done.

diff --git a/perl/Git.pm b/perl/Git.pm
index 11f310a..9dded54 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -426,13 +426,26 @@ Note that you should not rely on whatever actually is in 
CCTX;
 currently it is simply the command name but in future the context might
 have more complicated structure.
 
+CPIPE_IN and CPIPE_OUT may be Cundef if they have been closed prior to
+calling this function.  This may be useful in a query-response type of
+commands where caller first writes a query and later reads response, eg:
+
+   my ($pid, $in, $out, $ctx) = $r-command_bidi_pipe('cat-file 
--batch-check');
+   print $out 0\n;
+   close $out;
+   while ($in) { ... }
+   $r-command_close_bidi_pipe($pid, $in, undef, $ctx);
+
+This idiom may prevent potential dead locks caused by data sent to the output
+pipe not being flushed and thus not reaching the executed command.
+
 =cut
 
 sub command_close_bidi_pipe {
local $?;
my ($self, $pid, $in, $out, $ctx) = _maybe_self(@_);
foreach my $fh ($in, $out) {
-   unless (close $fh) {
+   if (defined $fh  !close $fh) {
if ($!) {
carp error closing pipe: $!;
} elsif ($?  8) {
-- 
1.8.1.2.549.g1d13f9f

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv2 5/5] git-send-email: use git credential to obtain password

[Michal Nazarewicz]

From: Michal Nazarewicz min...@mina86.com

If smtp_user is provided but smtp_pass is not, instead of
prompting for password, make git-send-email use git
credential command instead.

Signed-off-by: Michal Nazarewicz min...@mina86.com
---
 Documentation/git-send-email.txt |  4 +--
 git-send-email.perl  | 59 +++-
 2 files changed, 36 insertions(+), 27 deletions(-)

diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 44a1f7c..0cffef8 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -164,8 +164,8 @@ Sending
 Furthermore, passwords need not be specified in configuration files
 or on the command line. If a username has been specified (with
 '--smtp-user' or a 'sendemail.smtpuser'), but no password has been
-specified (with '--smtp-pass' or 'sendemail.smtppass'), then the
-user is prompted for a password while the input is masked for privacy.
+specified (with '--smtp-pass' or 'sendemail.smtppass'), then
+a password is obtained using 'git-credential'.
 
 --smtp-server=host::
If set, specifies the outgoing SMTP server to use (e.g.
diff --git a/git-send-email.perl b/git-send-email.perl
index be809e5..76bbfc3 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1045,6 +1045,39 @@ sub maildomain {
return maildomain_net() || maildomain_mta() || 'localhost.localdomain';
 }
 
+# Returns 1 if authentication succeeded or was not necessary
+# (smtp_user was not specified), and 0 otherwise.
+
+sub smtp_auth_maybe {
+   if (!defined $smtp_authuser || $auth) {
+   return 1;
+   }
+
+   # Workaround AUTH PLAIN/LOGIN interaction defect
+   # with Authen::SASL::Cyrus
+   eval {
+   require Authen::SASL;
+   Authen::SASL-import(qw(Perl));
+   };
+
+   # TODO: Authentication may fail not because credentials were
+   # invalid but due to other reasons, in which we should not
+   # reject credentials.
+   $auth = Git::credential({
+   'protocol' = 'smtp',
+   'host' = join(':', $smtp_server, $smtp_server_port),
+   'username' = $smtp_authuser,
+   # if there's no password, git credential fill will
+   # give us one, otherwise it'll just pass this one.
+   'password' = $smtp_authpass
+   }, sub {
+   my $cred = shift;
+   return !!$smtp-auth($cred-{'username'}, $cred-{'password'});
+   });
+
+   return $auth;
+}
+
 # Returns 1 if the message was sent, and 0 otherwise.
 # In actuality, the whole program dies when there
 # is an error sending a message.
@@ -1185,31 +1218,7 @@ X-Mailer: git-send-email $gitversion
defined $smtp_server_port ?  
port=$smtp_server_port : ;
}
 
-   if (defined $smtp_authuser) {
-   # Workaround AUTH PLAIN/LOGIN interaction defect
-   # with Authen::SASL::Cyrus
-   eval {
-   require Authen::SASL;
-   Authen::SASL-import(qw(Perl));
-   };
-
-   if (!defined $smtp_authpass) {
-
-   system stty -echo;
-
-   do {
-   print Password: ;
-   $_ = STDIN;
-   print \n;
-   } while (!defined $_);
-
-   chomp($smtp_authpass = $_);
-
-   system stty echo;
-   }
-
-   $auth ||= $smtp-auth( $smtp_authuser, $smtp_authpass ) 
or die $smtp-message;
-   }
+   smtp_auth_maybe or die $smtp-message;
 
$smtp-mail( $raw_from ) or die $smtp-message;
$smtp-to( @recipients ) or die $smtp-message;
-- 
1.8.1.2.549.g1d13f9f

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv2 4/5] Git.pm: add interface for git credential command

[Michal Nazarewicz]

On Fri, Feb 08 2013, Junio C Hamano wrote:
 I'd actually be more worried about the error checking issue
 Peff raised during his review.  I have a feeling that when in doubt,
 do not cause harm is a more prudent way to go than I do not know,
 so I'll let anything pass.

I can implement whatever checking you wish, just tell me what to do in
corner cases I've listed. ;)

-- 
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of  o' \,=./ `o
..o | Computer Science,  Michał “mina86” Nazarewicz(o o)
ooo +email/xmpp: m...@google.com--ooO--(_)--Ooo--

pgpkH3LocTxhw.pgp
Description: PGP signature

Re: [PATCH] git-send-email: add ~/.authinfo parsing

[Michal Nazarewicz]

On Wed, Feb 06 2013, Junio C Hamano gits...@pobox.com wrote:
 I see a lot of rerolls on the credential helper front, but is there
 anybody working on hooking send-email to the credential framework?

I assumed someone had, but if not I can take a stab at it.  I'm not sure
however how should I map server, server-port, and user to credential
key-value pairs.  I'm leaning towards

protocol=smtp
host=smtp-server:smtp-port
user=user

and than netrc/authinfo helper splitting host to host name and port
number, unless port is not in host in which case protocol is assumed as
port.

-- 
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of  o' \,=./ `o
..o | Computer Science,  Michał “mina86” Nazarewicz(o o)
ooo +email/xmpp: m...@google.com--ooO--(_)--Ooo--

pgpyBEG11AUhI.pgp
Description: PGP signature

[PATCH 0/4] Make git-send-email git-credential

[Michal Nazarewicz]

From: Michal Nazarewicz min...@mina86.com

As discussed on the list, adding git-credential interface to Git.pm
(sort of copied from git-remote-mediawiki) and making git-send-email
use it.

I see git-remote-mediawiki does not have “use Git” so I did not touch
it.  On top of that I'd have no way to tests the changes anyway.

Michal Nazarewicz (4):
  Git.pm: Allow command_close_bidi_pipe() to be called as method
  Git.pm: Allow pipes to be closed prior to calling
command_close_bidi_pipe
  Git.pm: Add interface for git credential command.
  git-send-email: Use git credential to obtain password.

 Documentation/git-send-email.txt |   4 +-
 git-send-email.perl  |  60 +++-
 perl/Git.pm  | 116 ++-
 3 files changed, 149 insertions(+), 31 deletions(-)

-- 
1.8.1.2.550.g0d3a9c0.dirty

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/4] Make git-send-email git-credential

[Michal Nazarewicz]

On Wed, Feb 06 2013, Michal Nazarewicz wrote:
 As discussed on the list, adding git-credential interface to Git.pm
 (sort of copied from git-remote-mediawiki) and making git-send-email
 use it.

 I see git-remote-mediawiki does not have “use Git” so I did not touch
 it.  On top of that I'd have no way to tests the changes anyway.

 Michal Nazarewicz (4):
   Git.pm: Allow command_close_bidi_pipe() to be called as method
   Git.pm: Allow pipes to be closed prior to calling
 command_close_bidi_pipe
   Git.pm: Add interface for git credential command.
   git-send-email: Use git credential to obtain password.

  Documentation/git-send-email.txt |   4 +-
  git-send-email.perl  |  60 +++-
  perl/Git.pm  | 116 
 ++-
  3 files changed, 149 insertions(+), 31 deletions(-)

On second thought, give me a moment, ;) I've just discovered a bug
preventing git-send-email from mailing a patchset.

-- 
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of  o' \,=./ `o
..o | Computer Science,  Michał “mina86” Nazarewicz(o o)
ooo +email/xmpp: m...@google.com--ooO--(_)--Ooo--

pgpHpgySqvgfA.pgp
Description: PGP signature

[PATCH 0/4] Make git-send-email git-credential

[Michal Nazarewicz]

From: Michal Nazarewicz min...@mina86.com

As discussed on the list, adding git-credential interface to Git.pm
(sort of copied from git-remote-mediawiki) and making git-send-email
use it.

I see git-remote-mediawiki does not have “use Git” so I did not touch
it.  On top of that I'd have no way to tests the changes anyway.

Michal Nazarewicz (4):
  Git.pm: Allow command_close_bidi_pipe() to be called as method
  Git.pm: Allow pipes to be closed prior to calling
command_close_bidi_pipe
  Git.pm: Add interface for git credential command.
  git-send-email: Use git credential to obtain password.

 Documentation/git-send-email.txt |   4 +-
 git-send-email.perl  |  59 +++-
 perl/Git.pm  | 116 ++-
 3 files changed, 149 insertions(+), 30 deletions(-)

-- 
1.8.1.2.549.g4fa355e

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/4] Git.pm: Allow command_close_bidi_pipe() to be called as method

[Michal Nazarewicz]

From: Michal Nazarewicz min...@mina86.com

The documentation of command_close_bidi_pipe() claims that it can
be called as a method, but it does not check whether the first
argument is $self or not assuming the latter.  Using _maybe_self()
fixes this.

Signed-off-by: Michal Nazarewicz min...@mina86.com
---
 perl/Git.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/perl/Git.pm b/perl/Git.pm
index 931047c..bbb753a 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -430,7 +430,7 @@ have more complicated structure.
 
 sub command_close_bidi_pipe {
local $?;
-   my ($pid, $in, $out, $ctx) = @_;
+   my ($self, $pid, $in, $out, $ctx) = _maybe_self(@_);
foreach my $fh ($in, $out) {
unless (close $fh) {
if ($!) {
-- 
1.8.1.2.549.g4fa355e

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/4] Git.pm: Allow pipes to be closed prior to calling command_close_bidi_pipe

[Michal Nazarewicz]

From: Michal Nazarewicz min...@mina86.com

The command_close_bidi_pipe() function will insist on closing both
input and output pipes returned by command_bidi_pipe().  With this
change it is possible to close one of the pipes in advance and
pass undef as an argument.

This allows for something like:

  my ($pid, $in, $out, $ctx) = command_bidi_pipe(...);
  print $out write data;
  close $out;
  # ... do stuff with $in
  command_close_bidi_pipe($pid, $in, undef, $ctx);

Signed-off-by: Michal Nazarewicz min...@mina86.com
---
 perl/Git.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/perl/Git.pm b/perl/Git.pm
index bbb753a..6a2d52d 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -432,7 +432,7 @@ sub command_close_bidi_pipe {
local $?;
my ($self, $pid, $in, $out, $ctx) = _maybe_self(@_);
foreach my $fh ($in, $out) {
-   unless (close $fh) {
+   if (defined $fh  !close $fh) {
if ($!) {
carp error closing pipe: $!;
} elsif ($?  8) {
-- 
1.8.1.2.549.g4fa355e

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 4/4] git-send-email: Use git credential to obtain password.

[Michal Nazarewicz]

From: Michal Nazarewicz min...@mina86.com

If smtp_user is provided but smtp_pass is not, instead of prompting
for password, make git-send-email use git credential command
instead.

Signed-off-by: Michal Nazarewicz min...@mina86.com
---
 Documentation/git-send-email.txt |  4 +--
 git-send-email.perl  | 59 +++-
 2 files changed, 36 insertions(+), 27 deletions(-)

diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 44a1f7c..0cffef8 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -164,8 +164,8 @@ Sending
 Furthermore, passwords need not be specified in configuration files
 or on the command line. If a username has been specified (with
 '--smtp-user' or a 'sendemail.smtpuser'), but no password has been
-specified (with '--smtp-pass' or 'sendemail.smtppass'), then the
-user is prompted for a password while the input is masked for privacy.
+specified (with '--smtp-pass' or 'sendemail.smtppass'), then
+a password is obtained using 'git-credential'.
 
 --smtp-server=host::
If set, specifies the outgoing SMTP server to use (e.g.
diff --git a/git-send-email.perl b/git-send-email.perl
index be809e5..76bbfc3 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1045,6 +1045,39 @@ sub maildomain {
return maildomain_net() || maildomain_mta() || 'localhost.localdomain';
 }
 
+# Returns 1 if authentication succeeded or was not necessary
+# (smtp_user was not specified), and 0 otherwise.
+
+sub smtp_auth_maybe {
+   if (!defined $smtp_authuser || $auth) {
+   return 1;
+   }
+
+   # Workaround AUTH PLAIN/LOGIN interaction defect
+   # with Authen::SASL::Cyrus
+   eval {
+   require Authen::SASL;
+   Authen::SASL-import(qw(Perl));
+   };
+
+   # TODO: Authentication may fail not because credentials were
+   # invalid but due to other reasons, in which we should not
+   # reject credentials.
+   $auth = Git::credential({
+   'protocol' = 'smtp',
+   'host' = join(':', $smtp_server, $smtp_server_port),
+   'username' = $smtp_authuser,
+   # if there's no password, git credential fill will
+   # give us one, otherwise it'll just pass this one.
+   'password' = $smtp_authpass
+   }, sub {
+   my $cred = shift;
+   return !!$smtp-auth($cred-{'username'}, $cred-{'password'});
+   });
+
+   return $auth;
+}
+
 # Returns 1 if the message was sent, and 0 otherwise.
 # In actuality, the whole program dies when there
 # is an error sending a message.
@@ -1185,31 +1218,7 @@ X-Mailer: git-send-email $gitversion
defined $smtp_server_port ?  
port=$smtp_server_port : ;
}
 
-   if (defined $smtp_authuser) {
-   # Workaround AUTH PLAIN/LOGIN interaction defect
-   # with Authen::SASL::Cyrus
-   eval {
-   require Authen::SASL;
-   Authen::SASL-import(qw(Perl));
-   };
-
-   if (!defined $smtp_authpass) {
-
-   system stty -echo;
-
-   do {
-   print Password: ;
-   $_ = STDIN;
-   print \n;
-   } while (!defined $_);
-
-   chomp($smtp_authpass = $_);
-
-   system stty echo;
-   }
-
-   $auth ||= $smtp-auth( $smtp_authuser, $smtp_authpass ) 
or die $smtp-message;
-   }
+   smtp_auth_maybe or die $smtp-message;
 
$smtp-mail( $raw_from ) or die $smtp-message;
$smtp-to( @recipients ) or die $smtp-message;
-- 
1.8.1.2.549.g4fa355e

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] git-send-email: add ~/.authinfo parsing

[Michal Nazarewicz]

On Wed, Jan 30 2013, Jeff King wrote:
 I do not mind a .netrc or .authinfo parser, because while those formats
 do have security problems, they are standard files that may already be
 in use. So as long as we are not encouraging their use, I do not see a
 problem in supporting them (and we already do the same with curl's netrc
 support).

 But it would probably make sense for send-email to support the existing
 git-credential subsystem, so that it can take advantage of secure
 system-specific storage. And that is where we should be pointing new
 users. I think contrib/mw-to-git even has credential support written in
 perl, so it would just need to be factored out to Git.pm.

As far as I understand, there could be a git-credential helper that
reads ~/.authinfo and than git-send-email would just call “git
credential fill”, right?

I've noticed though, that git-credential does not support port argument,
which makes it slightly incompatible with ~/.authinfo.

-- 
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of  o' \,=./ `o
..o | Computer Science,  Michał “mina86” Nazarewicz(o o)
ooo +email/xmpp: m...@google.com--ooO--(_)--Ooo--

pgpjkQvER45u7.pgp
Description: PGP signature

[PATCH] git-send-email: add ~/.authinfo parsing

[Michal Nazarewicz]

From: Michal Nazarewicz min...@mina86.com

Make git-send-email read password from a ~/.authinfo file instead of
requiring it to be stored in git configuration, passed as command line
argument or typed in.

There are various other applications that use this file for
authentication information so letting users use it for git-send-email
is convinient.  Furthermore, some users store their ~/.gitconfig file
in a public repository and having to store password there makes it
easy to publish the password.

Not to introduce any new dependencies, ~/.authinfo file is parsed only
if Text::CSV Perl module is installed.  If it's not, a notification is
printed and the file is ignored.

Signed-off-by: Michal Nazarewicz min...@mina86.com
---
 Documentation/git-send-email.txt | 15 +++--
 git-send-email.perl  | 69 +---
 2 files changed, 70 insertions(+), 14 deletions(-)

diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index eeb561c..b83576e 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -158,14 +158,25 @@ Sending
 --smtp-pass[=password]::
Password for SMTP-AUTH. The argument is optional: If no
argument is specified, then the empty string is used as
-   the password. Default is the value of 'sendemail.smtppass',
-   however '--smtp-pass' always overrides this value.
+   the password. Default is the value of 'sendemail.smtppass'
+   or value read from '~/.authinfo' file, however '--smtp-pass'
+   always overrides this value.
 +
 Furthermore, passwords need not be specified in configuration files
 or on the command line. If a username has been specified (with
 '--smtp-user' or a 'sendemail.smtpuser'), but no password has been
 specified (with '--smtp-pass' or 'sendemail.smtppass'), then the
 user is prompted for a password while the input is masked for privacy.
++
+The '~/.authinfo' file is read if Text::CSV Perl module is installed
+on the system; if it's missing, a notification message will be printed
+and the file ignored altogether.  The file should contain a line with
+the following format:
++
+  machine domain port port login user password pass
++
+Contrary to other tools, 'git-send-email' does not support symbolic
+port names like 'imap' thus `port` must be a number.
 
 --smtp-server=host::
If set, specifies the outgoing SMTP server to use (e.g.
diff --git a/git-send-email.perl b/git-send-email.perl
index be809e5..d824098 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1045,6 +1045,62 @@ sub maildomain {
return maildomain_net() || maildomain_mta() || 'localhost.localdomain';
 }
 
+
+sub read_password_from_stdin {
+   my $line;
+
+   system stty -echo;
+
+   do {
+   print Password: ;
+   $line = STDIN;
+   print \n;
+   } while (!defined $line);
+
+   system stty echo;
+
+   chomp $line;
+   return $line;
+}
+
+sub read_password_from_authinfo {
+   my $fd;
+   if (!open $fd, '', $ENV{'HOME'} . '/.authinfo') {
+   return;
+   }
+
+   if (!eval { require Text::CSV; 1 }) {
+   print STDERR Text::CSV missing, won't read ~/.authinfo\n;
+   close $fd;
+   return;
+   }
+
+   my $csv = Text::CSV-new( { sep_char = ' ' } );
+   my $password;
+   while (my $line = $fd) {
+   chomp $line;
+   $csv-parse($line);
+   my %row = $csv-fields();
+   if (defined $row{'machine'} 
+   defined $row{'login'} 
+   defined $row{'port'} 
+   defined $row{'password'} 
+   $row{'machine'} eq $smtp_server 
+   $row{'login'} eq $smtp_authuser 
+   $row{'port'} eq $smtp_server_port) {
+   $password = $row{'password'};
+   last;
+   }
+   }
+
+   close $fd;
+   return $password;
+}
+
+sub read_password {
+   return read_password_from_authinfo || read_password_from_stdin;
+}
+
 # Returns 1 if the message was sent, and 0 otherwise.
 # In actuality, the whole program dies when there
 # is an error sending a message.
@@ -1194,18 +1250,7 @@ X-Mailer: git-send-email $gitversion
};
 
if (!defined $smtp_authpass) {
-
-   system stty -echo;
-
-   do {
-   print Password: ;
-   $_ = STDIN;
-   print \n;
-   } while (!defined $_);
-
-   chomp($smtp_authpass = $_);
-
-   system stty echo;
+   $smtp_authpass = read_password
}
 
$auth ||= $smtp-auth

[PATCHv2] git-send-email: add ~/.authinfo parsing

[Michal Nazarewicz]

From: Michal Nazarewicz min...@mina86.com

Make git-send-email read password from a ~/.authinfo or a ~/.netrc
file instead of requiring it to be stored in git configuration, passed
as command line argument or typed in.

There are various other applications that use this file for
authentication information so letting users use it for git-send-email
is convinient.  Furthermore, some users store their ~/.gitconfig file
in a public repository and having to store password there makes it
easy to publish the password.

Signed-off-by: Michal Nazarewicz min...@mina86.com
---
 Documentation/git-send-email.txt |  34 +--
 git-send-email.perl  | 124 +++
 2 files changed, 140 insertions(+), 18 deletions(-)

On Tue, Jan 29 2013, Junio C Hamano wrote:
 Makes one wonder why .authinfo and not .netrc; 

Fine… Let's parse both. ;)

 Either way it still encourages a plaintext password to be on disk,
 which may not be what we want, even though it may be slight if not
 really much of an improvement.

Well… Users store passwords on disks in a lot of places.  I wager that
most have mail clients configured not to ask for password but instead
store it on hard drive.  I don't see that changing any time soon, so
at least we can try and minimise number of places where a password is
stored.

 It is rather strange to require a comma-separated-values parser to
 read a file format this simple, isn't it?

I was worried about spaces in password.  CVS should handle such case
nicely, whereas simple split won't.  Nonetheless, I guess that in the
end this is not likely enough to add the dependency.

 Perhaps you can convert at least some popular ones yourself?  After
 all, the user may be using an _existing_ .authinfo/.netrc that she
 has been using with other programs that do understand symbolic port
 names.  Rather than forcing all such users to update their files,
 the patch can work a bit harder for them and the world will be a
 better place, no?

Parsing /etc/services added.

diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index eeb561c..ee20714 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -158,14 +158,36 @@ Sending
 --smtp-pass[=password]::
Password for SMTP-AUTH. The argument is optional: If no
argument is specified, then the empty string is used as
-   the password. Default is the value of 'sendemail.smtppass',
-   however '--smtp-pass' always overrides this value.
+   the password. Default is the value of 'sendemail.smtppass'
+   or value read from ~/.authinfo file, however '--smtp-pass'
+   always overrides this value.
 +
-Furthermore, passwords need not be specified in configuration files
-or on the command line. If a username has been specified (with
+Furthermore, passwords need not be specified in configuration files or
+on the command line. If a username has been specified (with
 '--smtp-user' or a 'sendemail.smtpuser'), but no password has been
-specified (with '--smtp-pass' or 'sendemail.smtppass'), then the
-user is prompted for a password while the input is masked for privacy.
+specified (with '--smtp-pass', 'sendemail.smtppass' or via
+~/.authinfo file), then the user is prompted for a password while
+the input is masked for privacy.
++
+The ~/.authinfo file should contain a line with the following
+format:
++
+  machine domain port port login user password pass
++
+Each pair (expect for `password pass`) can be omitted which will
+skip matching of the given value.  Lines are interpreted in order and
+password from the first line that matches will be used.  `port` can
+be either an integer or a symbolic name.  In the latter case, it is
+looked up in `/etc/services` file (if it exists).  For instance, you
+can put
++
+  machine example.com login testuser port ssmtp password smtppassword
+  machine example.com login testuserpassword testpassword
++
+if you want to use `smtppassword` for authenticating to a service at
+port 465 (SSMTP) and `testpassword` for all other services.  As shown
+in the example, `port` can use   If ~/.authinfo file is
+missing, 'git-send-email' will also try ~/.netrc file.
 
 --smtp-server=host::
If set, specifies the outgoing SMTP server to use (e.g.
diff --git a/git-send-email.perl b/git-send-email.perl
index be809e5..2d8fd1b 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1045,6 +1045,117 @@ sub maildomain {
return maildomain_net() || maildomain_mta() || 'localhost.localdomain';
 }
 
+
+sub read_password_from_stdin {
+   my $line;
+
+   system stty -echo;
+
+   do {
+   print Password: ;
+   $line = STDIN;
+   print \n;
+   } while (!defined $line);
+
+   system stty echo;
+
+   chomp $line;
+   return $line;
+}
+
+sub read_etc_services {
+   my $fd;
+   if (!open $fd, '', '/etc/services') {
+   return

[PATCHv3] git-send-email: add ~/.authinfo parsing

[Michal Nazarewicz]

From: Michal Nazarewicz min...@mina86.com

Make git-send-email read password from a ~/.authinfo or ~/.netrc file
instead of requiring it to be stored in git configuration, passed as
command line argument or typed in.

There are various other applications that use this file for
authentication information so letting users use it for git-send-email
is convinient.  Furthermore, some users store their ~/.gitconfig file
in a public repository and having to store password there makes it
easy to publish the password.

Signed-off-by: Michal Nazarewicz min...@mina86.com
---
 Documentation/git-send-email.txt | 47 +---
 git-send-email.perl  | 93 ++--
 2 files changed, 122 insertions(+), 18 deletions(-)

On Tue, Jan 29 2013, Junio C Hamano wrote:
 But .netrc/.authinfo format separates its entries with SP, HT, or
 LF.  An entry begins with machine remote-hostname token pair.

 split(/\s+/) will not work for an entry that span multiple lines but
 CSV will not help, either.

 Is it bad to use Net::Netrc instead?  This looks like exactly the
 use case that module was written for, no?

I don't think that's the case.  For one, Net::Netrc does not seem to
process port number.

There is a Text::Authinfo module but it just uses Text::CSV.

I can change the code to use Net::Netrc, but I dunno if that's really
the best option, since I feel people would expect parsing to be
somehow compatible with
http://www.gnu.org/software/emacs/manual/html_node/gnus/NNTP.html
rather than the original .netrc file format.

 Hmph.  I would have expected to see getservbyname.

Ha!  Even better. :]

diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index eeb561c..ac020d1 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -158,14 +158,49 @@ Sending
 --smtp-pass[=password]::
Password for SMTP-AUTH. The argument is optional: If no
argument is specified, then the empty string is used as
-   the password. Default is the value of 'sendemail.smtppass',
-   however '--smtp-pass' always overrides this value.
+   the password. Default is the value of 'sendemail.smtppass'
+   or value read from ~/.authinfo file, however '--smtp-pass'
+   always overrides this value.
 +
-Furthermore, passwords need not be specified in configuration files
-or on the command line. If a username has been specified (with
+Furthermore, passwords need not be specified in configuration files or
+on the command line. If a username has been specified (with
 '--smtp-user' or a 'sendemail.smtpuser'), but no password has been
-specified (with '--smtp-pass' or 'sendemail.smtppass'), then the
-user is prompted for a password while the input is masked for privacy.
+specified (with '--smtp-pass', 'sendemail.smtppass' or via
+~/.authinfo file), then the user is prompted for a password while
+the input is masked for privacy.
++
+The ~/.authinfo file should contain a line with the following
+format:
++
+  machine domain port port login user password pass
++
+Instead of `machine domain` pair a `default` token can be used
+instead in which case all domains will match.  Similarly, `port
+port` and `login user` pairs can be omitted in which case matching
+of the given value will be skipped.  `port` can be either an integer
+or a symbolic name.  Lines are interpreted in order and password from
+the first line that matches will be used.  For instance, one may end
+up with:
++
+  machine example.com login jane port ssmtp password smtppassword
+  machine example.com login janepassword janepassword
+  default login janedoe password doepassword
++
+if she wants to use `smtppassword` for authenticating as `jane` to
+a service at example.com:465 (SSMTP), `janepassword` for all other
+services at example.com; and `doepassword` when authonticating as
+`janedoe` to any service.  If ~/.authinfo file is missing,
+'git-send-email' will also try ~/.netrc file (even though parsing is
+not fully compatible with ftp's .netrc file format).
++
+Note that you should never make ~/.authinfo file world-readable.  To
+help guarantee that, you might want to create the file with the
+following command:
++
+  ( umask 077; cat ~/.authinfo EOF
+  ... file contents ...
+  EOF
+  )
 
 --smtp-server=host::
If set, specifies the outgoing SMTP server to use (e.g.
diff --git a/git-send-email.perl b/git-send-email.perl
index be809e5..a62dfa4 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1045,6 +1045,86 @@ sub maildomain {
return maildomain_net() || maildomain_mta() || 'localhost.localdomain';
 }
 
+
+sub read_password_from_stdin {
+   my $line;
+
+   system stty -echo;
+
+   do {
+   print Password: ;
+   $line = STDIN;
+   print \n;
+   } while (!defined $line);
+
+   system stty echo;
+
+   chomp $line;
+   return $line;
+}
+
+sub