[PATCH] git-send-email: add ~/.authinfo parsing
From: Michal Nazarewicz
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
---
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 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 port login password
++
+Contrary to other tools, 'git-send-email' does not support symbolic
+port names like 'imap' thus `` must be a number.
--smtp-server=::
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 = ;
+ 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: ";
- $_ = ;
- print "\n";
- } while (!defined $_);
-
- chomp($smtp_authpass = $_);
-
- system "stty echo";
+ $smtp_authpass = read_password
}
$auth ||= $smtp->auth( $smtp_authuser, $smtp_authpass )
or die $smtp->mes
Re: [PATCH] git-send-email: add ~/.authinfo parsing
Michal Nazarewicz writes: > From: Michal Nazarewicz > > 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. Makes one wonder why .authinfo and not .netrc; http://www.gnu.org/software/emacs/manual/html_node/auth/Help-for-users.html phrases it amusingly: “Netrc” files are usually called .authinfo or .netr nowadays .authinfo seems to be more popular and the auth-source library encourages this confusion by accepting 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. Again the Help-for-users has this amusing bit: You could just say (but we don't recommend it, we're just showing that it's possible) password mypassword to use the same password everywhere. Again, DO NOT DO THIS or you will be pwned as the kids say. > +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 port login password It is rather strange to require a comma-separated-values parser to read a file format this simple, isn't it? > ++ > +Contrary to other tools, 'git-send-email' does not support symbolic > +port names like 'imap' thus `` must be a number. 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? -- 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
On Tue, Jan 29, 2013 at 11:53:19AM -0800, Junio C Hamano wrote: > 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. Again the Help-for-users has this > amusing bit: 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. -Peff -- 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
On Tue, 29 Jan 2013 11:53:19 -0800 Junio C Hamano wrote: JCH> Makes one wonder why .authinfo and not .netrc; JCH> http://www.gnu.org/software/emacs/manual/html_node/auth/Help-for-users.html JCH> phrases it amusingly: JCH> “Netrc” files are usually called .authinfo or .netr JCH> nowadays .authinfo seems to be more popular and the JCH> auth-source library encourages this confusion by accepting JCH> both I wrote this and the auth-source.el library in Emacs (I'm glad it was amusing :). The confusion is further perpetuated by our (in Emacs) encouragement to use a .authinfo.gpg file, which is then decrypted on the fly by Emacs through GPG. The format is the same; by the time auth-source.el sees the contents, they are plain text since the decoding happens at the file handler level. I think it makes sense to write the code to support both `git-send-email' and credentials. I have had it in my TODO list for almost 2 years now to work on credential support, and to support the ~/.authinfo.gpg decoding specifically. Ideally this would also support the other formats... Michal, would you be interested in that feature? I promise to get off my rear and help out. >> +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 port login password JCH> It is rather strange to require a comma-separated-values parser to JCH> read a file format this simple, isn't it? I'd recommend a hand-crafted parser. Among other things, you should accept both "strings" and 'strings' if possible (I've seen both formats in the wild), and the format is simple enough to avoid the module dependency. >> ++ >> +Contrary to other tools, 'git-send-email' does not support symbolic >> +port names like 'imap' thus `` must be a number. JCH> Perhaps you can convert at least some popular ones yourself? After JCH> all, the user may be using an _existing_ .authinfo/.netrc that she JCH> has been using with other programs that do understand symbolic port JCH> names. Rather than forcing all such users to update their files, JCH> the patch can work a bit harder for them and the world will be a JCH> better place, no? I agree, "port imap" is a nice self-documenting token. Maybe it can be interpreted by the program that requests the token with a services lookup, where supported. Ted -- 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
Jeff King writes: > 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. Yeah, that sounds like a neat idea. -- 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
On Wed, 30 Jan 2013 07:57:29 -0800 Junio C Hamano wrote: JCH> Jeff King writes: >> 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. JCH> Yeah, that sounds like a neat idea. Jeff, is there a way for git-credential to currently support authinfo/netrc parsing? I assume that's the right way, instead of using Michal's proposal to parse internally? I'd like to add that, plus support for the 'string' and "string" formats, and authinfo.gpg decoding through GPG. I'd write it in Perl, if there's a choice. Ted -- 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
On Thu, Jan 31, 2013 at 10:23:51AM -0500, Ted Zlatanov wrote: > Jeff, is there a way for git-credential to currently support > authinfo/netrc parsing? I assume that's the right way, instead of using > Michal's proposal to parse internally? > > I'd like to add that, plus support for the 'string' and "string" > formats, and authinfo.gpg decoding through GPG. I'd write it in Perl, > if there's a choice. Yes, you could write a credential helper that understands netrc and friends; git talks to the helpers over a socket, so there is no problem with writing it in Perl. See Documentation/technical/api-credentials.txt for an overview, or the sample implementation in credential-store.c for a simple example. -Peff -- 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
On Thu, 31 Jan 2013 14:38:45 -0500 Jeff King wrote:
JK> On Thu, Jan 31, 2013 at 10:23:51AM -0500, Ted Zlatanov wrote:
>> Jeff, is there a way for git-credential to currently support
>> authinfo/netrc parsing? I assume that's the right way, instead of using
>> Michal's proposal to parse internally?
>>
>> I'd like to add that, plus support for the 'string' and "string"
>> formats, and authinfo.gpg decoding through GPG. I'd write it in Perl,
>> if there's a choice.
JK> Yes, you could write a credential helper that understands netrc and
JK> friends; git talks to the helpers over a socket, so there is no problem
JK> with writing it in Perl. See Documentation/technical/api-credentials.txt
JK> for an overview, or the sample implementation in credential-store.c for a
JK> simple example.
I wrote a Perl credential helper for netrc parsing which is pretty
robust, has built-in docs with -h, and doesn't depend on external
modules. The netrc parser regex was stolen from Net::Netrc.
It will by default use ~/.authinfo.gpg, ~/.netrc.gpg, ~/.authinfo, and
~/.netrc (whichever is found first) and this can be overridden with -f.
If the file name ends with ".gpg", it will run "gpg --decrypt FILE" and
use the output. So non-interactively, that could hang if GPG was
waiting for input. Does Git handle that, or should I check for a TTY?
Take a look at the proposed patch and let me know if it's usable, if you
need a formal copyright assignment, etc.
Thanks
Ted
commit 3d28bc2a610ebcc988eba5443d82d0ded92c24bc
Author: Ted Zlatanov
Date: Sat Feb 2 06:42:13 2013 -0500
Add contrib/credentials/netrc with GPG support
diff --git a/contrib/credential/netrc/git-credential-netrc b/contrib/credential/netrc/git-credential-netrc
new file mode 100755
index 000..92fc306
--- /dev/null
+++ b/contrib/credential/netrc/git-credential-netrc
@@ -0,0 +1,242 @@
+#!/usr/bin/perl
+
+use strict;
+use warnings;
+
+use Data::Dumper;
+
+use Getopt::Long;
+use File::Basename;
+
+my $VERSION = "0.1";
+
+my %options = (
+ help => 0,
+ debug => 0,
+
+ # identical token maps, e.g. host -> host, will be inserted later
+ tmap => {
+port => 'protocol',
+machine => 'host',
+path => 'path',
+login => 'username',
+user => 'username',
+password => 'password',
+ }
+ );
+
+foreach my $v (values %{$options{tmap}})
+{
+ $options{tmap}->{$v} = $v;
+}
+
+foreach my $suffix ('.gpg', '')
+{
+ foreach my $base (qw/authinfo netrc/)
+ {
+ my $file = glob("~/.$base$suffix");
+ next unless (defined $file && -f $file);
+ $options{file} = $file ;
+ }
+}
+
+Getopt::Long::Configure("bundling");
+
+# TODO: maybe allow the token map $options{tmap} to be configurable.
+GetOptions(\%options,
+ "help|h",
+ "debug|d",
+ "file|f=s",
+ );
+
+if ($options{help})
+{
+ my $shortname = basename($0);
+ $shortname =~ s/git-credential-//;
+
+ print <)
+{
+ next unless m/([a-z]+)=(.+)/;
+
+ my ($token, $value) = ($1, $2);
+ die "Unknown search token $1" unless exists $q{$token};
+ $q{$token} = $value;
+}
+
+# build reverse token map
+my %rmap;
+foreach my $k (keys %{$options{tmap}})
+{
+ push @{$rmap{$options{tmap}->{$k}}}, $k;
+}
+
+# there are CPAN modules to do this better, but we want to avoid
+# dependencies and generally, complex netrc-style files are rare
+
+if ($debug)
+{
+ foreach (sort keys %q)
+ {
+ printf STDERR "searching for %s = %s\n",
+ $_, $q{$_} || '(any value)';
+ }
+}
+
+LINE: foreach my $line (@data)
+{
+
+ print STDERR "line [$line]\n" if $debug;
+ my @tok;
+ # gratefully stolen from Net::Netrc
+ while (length $line &&
+$line =~ s/^("((?:[^"]+|\\.)*)"|((?:[^\\\s]+|\\.)*))\s*//)
+ {
+ (my $tok = $+) =~ s/\\(.)/$1/g;
+ push(@tok, $tok);
+ }
+
+ my %tokens;
+ while (@tok)
+ {
+ my ($k, $v) = (shift @tok, shift @tok);
+ next unless defined $v;
+ next unless exists $options{tmap}->{$k};
+ $tokens{$options{tmap}->{$k}} = $v;
+ }
+
+ foreach my $check (sort keys %q)
+ {
+ if (exists $tokens{$check} && defined $q{$check})
+ {
+ print STDERR "comparing [$tokens{$check}] to [$q{$check}] in line [$line]\n" if $debug;
+ next LINE unless $tokens{$check} eq $q{$check};
+ }
+ else
+ {
+ print STDERR "we could not find [$check] but it's OK\n" if $debug;
+ }
+ }
+
+ print STDERR "line has passed all the search checks\n" if $debug;
+ foreach my $token (sort keys %rmap)
+ {
+ print STDERR "looking for useful token $token\n" if $debug;
+ next unless exists $tokens{$token}; # did we match?
+
+ foreach my $rctoken (@{$rmap{$token}})
+ {
+ next if defined $q{$rctoken}; # don't re-print given tokens
+ }
+
+ print STDERR "FOUND: $token=$tokens{$token}\n" if $debug;
+ printf "%s=%s\n", $token, $tokens{$token};
+ }
+
+ last;
+}
+
+sub load
+{
+ my $file = shift;
+ #
Re: [PATCH] git-send-email: add ~/.authinfo parsing
On Sat, Feb 02, 2013 at 06:57:29AM -0500, Ted Zlatanov wrote:
> I wrote a Perl credential helper for netrc parsing which is pretty
> robust, has built-in docs with -h, and doesn't depend on external
> modules. The netrc parser regex was stolen from Net::Netrc.
>
> It will by default use ~/.authinfo.gpg, ~/.netrc.gpg, ~/.authinfo, and
> ~/.netrc (whichever is found first) and this can be overridden with -f.
Cool, thanks for working on this.
> If the file name ends with ".gpg", it will run "gpg --decrypt FILE" and
> use the output. So non-interactively, that could hang if GPG was
> waiting for input. Does Git handle that, or should I check for a TTY?
No, git does not do anything special with respect to credential helpers
and ttys (nor should it, since one use of helpers is to get credentials
when there is no tty). I think it is GPG's problem to deal with, though.
We will invoke it, and it is up to it to decide whether it can acquire
the passphrase or not (either through the tty, or possibly from
gpg-agent). So it would be wrong to do the tty check yourself.
I haven't tested GPG, but I assume it properly tries to read from
/dev/tty and not stdin. Your helper's stdio is connected to git and
speaking the credential-helper protocol, so GPG reading from stdin would
either steal your input (if run before you read it), or just get EOF (if
you have read all of the pipe content already). If GPG isn't well
behaved, it may be worth redirecting its stdin from /dev/null as a
safety measure.
> Take a look at the proposed patch and let me know if it's usable, if you
> need a formal copyright assignment, etc.
Overall looks sane to me, though my knowledge of .netrc is not
especially good. Usually we try to send patches inline in the email
(i.e., as generated by git-format-patch), and include a "Signed-off-by"
line indicating that content is released to the project; see
Documentation/SubmittingPatches.
> +use Data::Dumper;
I don't see it used here. Leftover from debugging?
> + print < +$0 [-f AUTHFILE] [-d] get
> +
> +Version $VERSION by tzz\@lifelogs.com. License: any use is OK.
I don't know if we have a particular policy for items in contrib/, but
this license may be too vague. In particular, it does not explicitly
allow redistribution, which would make Junio shipping a release with it
a copyright violation.
Any objection to just putting it under some well-known simple license
(GPL, BSD, or whatever)?
> +if ($file =~ m/\.gpg$/)
> +{
> + $file = "gpg --decrypt $file|";
> +}
Does this need to quote $file, since the result will get passed to the
shell? It might be easier to just use the list form of open(), like:
my @data = $file =~ /\.gpg$/ ?
load('-|', qw(gpg --decrypt), $file) :
load('<', $file);
(and then obviously update load to just dump all of @_ to open()).
> +die "Sorry, we could not load data from [$file]"
> + unless (scalar @data);
Probably not that interesting a corner case, but this means we die on an
empty .netrc, whereas it might be more sensible for it to behave as "no
match".
For the same reason, it might be worth silently exiting when we don't
find a .netrc (or any of its variants). That lets people who share their
dot-files across machines configure git globally, even if they don't
necessarily have a netrc on every machine.
> +# the query
> +my %q;
> +
> +foreach my $v (values %{$options{tmap}})
> +{
> + undef $q{$v};
> +}
Just my personal style, but I find the intent more obvious with "map" (I
know some people find it unreadable, though):
my %q = map { $_ => undef } values(%{$options{tmap}});
> +while ()
> +{
> + next unless m/([a-z]+)=(.+)/;
We don't currently have any exotic tokens that this would not match, nor
do I plan to add them, but the credential documentation defines a valid
line as /^([^=]+)=(.+)/.
It's also possible for the value to be empty, but I do not think
off-hand that current git will ever send such an empty value.
> [...]
The rest of it looks fine to me. I don't think any of my comments are
show-stoppers. Tests would be nice, but integrating contrib/ stuff with
the test harness is kind of a pain.
-Peff
--
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
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 +--ooO--(_)--Ooo-- pgpjkQvER45u7.pgp Description: PGP signature
Re: [PATCH] git-send-email: add ~/.authinfo parsing
On Sun, 3 Feb 2013 14:41:49 -0500 Jeff King wrote:
JK> On Sat, Feb 02, 2013 at 06:57:29AM -0500, Ted Zlatanov wrote:
>> If the file name ends with ".gpg", it will run "gpg --decrypt FILE" and
>> use the output. So non-interactively, that could hang if GPG was
>> waiting for input. Does Git handle that, or should I check for a TTY?
JK> No, git does not do anything special with respect to credential helpers
JK> and ttys (nor should it, since one use of helpers is to get credentials
JK> when there is no tty). I think it is GPG's problem to deal with, though.
JK> We will invoke it, and it is up to it to decide whether it can acquire
JK> the passphrase or not (either through the tty, or possibly from
JK> gpg-agent). So it would be wrong to do the tty check yourself.
JK> I haven't tested GPG, but I assume it properly tries to read from
JK> /dev/tty and not stdin. Your helper's stdio is connected to git and
JK> speaking the credential-helper protocol, so GPG reading from stdin would
JK> either steal your input (if run before you read it), or just get EOF (if
JK> you have read all of the pipe content already). If GPG isn't well
JK> behaved, it may be worth redirecting its stdin from /dev/null as a
JK> safety measure.
In my testing GPG did the right thing, so I think this is OK.
>> Take a look at the proposed patch and let me know if it's usable, if you
>> need a formal copyright assignment, etc.
JK> Overall looks sane to me, though my knowledge of .netrc is not
JK> especially good. Usually we try to send patches inline in the email
JK> (i.e., as generated by git-format-patch), and include a "Signed-off-by"
JK> line indicating that content is released to the project; see
JK> Documentation/SubmittingPatches.
OK, thanks. I will fire that off.
>> +use Data::Dumper;
JK> I don't see it used here. Leftover from debugging?
It's part of my Perl new script skeleton, sorry.
>> + print < Cute, I haven't seen that one before.
Heh heh. I've had to explain that one in code review many times. "See,
it's the precursor to the modern horse..."
>> +$0 [-f AUTHFILE] [-d] get
>> +
>> +Version $VERSION by tzz\@lifelogs.com. License: any use is OK.
JK> I don't know if we have a particular policy for items in contrib/, but
JK> this license may be too vague. In particular, it does not explicitly
JK> allow redistribution, which would make Junio shipping a release with it
JK> a copyright violation.
JK> Any objection to just putting it under some well-known simple license
JK> (GPL, BSD, or whatever)?
No, I didn't know what Git requires, and I'd like it to be the least
restrictive, so BSD is OK. Stated in -h now.
>> +if ($file =~ m/\.gpg$/)
>> +{
>> + $file = "gpg --decrypt $file|";
>> +}
JK> Does this need to quote $file, since the result will get passed to the
JK> shell? It might be easier to just use the list form of open(), like:
JK> my @data = $file =~ /\.gpg$/ ?
JK> load('-|', qw(gpg --decrypt), $file) :
JK> load('<', $file);
JK> (and then obviously update load to just dump all of @_ to open()).
Yes, thanks. Done.
>> +die "Sorry, we could not load data from [$file]"
>> + unless (scalar @data);
JK> Probably not that interesting a corner case, but this means we die on an
JK> empty .netrc, whereas it might be more sensible for it to behave as "no
JK> match".
JK> For the same reason, it might be worth silently exiting when we don't
JK> find a .netrc (or any of its variants). That lets people who share their
JK> dot-files across machines configure git globally, even if they don't
JK> necessarily have a netrc on every machine.
OK; done.
>> +# the query
>> +my %q;
>> +
>> +foreach my $v (values %{$options{tmap}})
>> +{
>> + undef $q{$v};
>> +}
JK> Just my personal style, but I find the intent more obvious with "map" (I
JK> know some people find it unreadable, though):
JK> my %q = map { $_ => undef } values(%{$options{tmap}});
Yes, changed.
>> +while ()
>> +{
>> + next unless m/([a-z]+)=(.+)/;
JK> We don't currently have any exotic tokens that this would not match, nor
JK> do I plan to add them, but the credential documentation defines a valid
JK> line as /^([^=]+)=(.+)/.
JK> It's also possible for the value to be empty, but I do not think
JK> off-hand that current git will ever send such an empty value.
Yes, changed.
JK> The rest of it looks fine to me. I don't think any of my comments are
JK> show-stoppers. Tests would be nice, but integrating contrib/ stuff with
JK> the test harness is kind of a pain.
"I tested it on AIX, it works great!" :)
It's pretty easy to write a local Makefile with a test target, if you
think it worthwhile.
Ted
--
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
On Mon, 04 Feb 2013 17:33:58 +0100 Michal Nazarewicz wrote: MN> As far as I understand, there could be a git-credential helper that MN> reads ~/.authinfo and than git-send-email would just call “git MN> credential fill”, right? MN> I've noticed though, that git-credential does not support port argument, MN> which makes it slightly incompatible with ~/.authinfo. My proposed netrc credential helper does this :) The token mapping I use: port, protocol=> protocol machine, host => host path => path login, username, user => username password => password I think that's sensible. Ted -- 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
On Mon, Feb 04, 2013 at 12:00:34PM -0500, Ted Zlatanov wrote: > On Mon, 04 Feb 2013 17:33:58 +0100 Michal Nazarewicz > wrote: > > MN> As far as I understand, there could be a git-credential helper that > MN> reads ~/.authinfo and than git-send-email would just call “git > MN> credential fill”, right? > > MN> I've noticed though, that git-credential does not support port argument, > MN> which makes it slightly incompatible with ~/.authinfo. > > My proposed netrc credential helper does this :) > > The token mapping I use: > > port, protocol=> protocol > machine, host => host > path => path > login, username, user => username > password => password > > I think that's sensible. Technically you can speak a particular protocol on an alternate port: https://example.com:31337/repo.git In this case, git will send you the host as: example.com:31337 You might want to map this to "port" in .autoinfo separately if it's available. -Peff -- 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
On Mon, 4 Feb 2013 15:10:40 -0500 Jeff King wrote: JK> Technically you can speak a particular protocol on an alternate port: JK> https://example.com:31337/repo.git JK> In this case, git will send you the host as: JK> example.com:31337 JK> You might want to map this to "port" in .autoinfo separately if it's JK> available. That would create the following possibilities: * host example.com:31337, protocol https * host example.com:31337, protocol unspecified * host example.com, protocol https * host example.com, protocol unspecified How would you like each one to be handled? My preference would be to make the user say "host example.com:31337" in the netrc file (the current situation); that's what we do in Emacs and it lets applications request credentials for a logical service no matter what the port is. It means that example.com credentials won't be used for example.com:31337. In practice, that has not been a problem for us. Ted -- 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
On Mon, Feb 04, 2013 at 03:28:52PM -0500, Ted Zlatanov wrote: > JK> You might want to map this to "port" in .autoinfo separately if it's > JK> available. > > That would create the following possibilities: > > * host example.com:31337, protocol https > * host example.com:31337, protocol unspecified > * host example.com, protocol https > * host example.com, protocol unspecified Possibilities for .netrc, or for git? Git will always specify the protocol. > How would you like each one to be handled? My preference would be to > make the user say "host example.com:31337" in the netrc file (the > current situation); that's what we do in Emacs and it lets applications > request credentials for a logical service no matter what the port is. > > It means that example.com credentials won't be used for > example.com:31337. In practice, that has not been a problem for us. Yeah, I think that is a good thing. The credentials used for example.com:31337 are not necessarily the same as for the main site. It's less convenient, but a more secure default. What I was more wondering (and I know very little about .netrc, so this might not be a possibility at all) is a line like: host example.com port 5001 protocol https username foo password bar To match git's representation on a token-by-token basis, you would have to either split out git's "host:port" pair, or combine the .netrc's representation to "example.com:5001". -Peff -- 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
On Mon, 4 Feb 2013 15:59:11 -0500 Jeff King wrote: JK> On Mon, Feb 04, 2013 at 03:28:52PM -0500, Ted Zlatanov wrote: JK> You might want to map this to "port" in .autoinfo separately if it's JK> available. >> >> That would create the following possibilities: >> >> * host example.com:31337, protocol https >> * host example.com:31337, protocol unspecified >> * host example.com, protocol https >> * host example.com, protocol unspecified JK> Possibilities for .netrc, or for git? Git will always specify the JK> protocol. Possibilities for the netrc data. How clever do we want to be with taking 31337 and mapping it to the "protocol"? My preference is to be very simple here. JK> What I was more wondering (and I know very little about .netrc, so this JK> might not be a possibility at all) is a line like: JK> host example.com port 5001 protocol https username foo password bar JK> To match git's representation on a token-by-token basis, you would have JK> to either split out git's "host:port" pair, or combine the .netrc's JK> representation to "example.com:5001". Currently, we map both the "port" and "protocol" netrc tokens to the credential helper protocol's "protocol". So this will have undefined results. To do what you specify could be pretty simple: we could do a preliminary scan of the tokens, looking for "host X port Y" where Y is an integer, and rewriting the host to be "X:Y". That would be clean and simple, unless the user breaks it with "host x:23 port 22". Let me know if you agree and I'll do. Ted -- 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
On Mon, Feb 04, 2013 at 04:08:52PM -0500, Ted Zlatanov wrote: > >> That would create the following possibilities: > >> > >> * host example.com:31337, protocol https > >> * host example.com:31337, protocol unspecified > >> * host example.com, protocol https > >> * host example.com, protocol unspecified > > JK> Possibilities for .netrc, or for git? Git will always specify the > JK> protocol. > > Possibilities for the netrc data. How clever do we want to be with > taking 31337 and mapping it to the "protocol"? My preference is to be > very simple here. I think simple is OK, as we can iterate on it as specific use-cases come up. The important thing is to make sure we err on the side of "does not match" and not "oops, we accidentally sent your plaintext credentials to the wrong server". > Currently, we map both the "port" and "protocol" netrc tokens to the > credential helper protocol's "protocol". So this will have undefined > results. To do what you specify could be pretty simple: we could do a > preliminary scan of the tokens, looking for "host X port Y" where Y is > an integer, and rewriting the host to be "X:Y". That would be clean and > simple, unless the user breaks it with "host x:23 port 22". Let me know > if you agree and I'll do. Yeah, I think that is simple and obvious. If the user is saying "host x:23 port 22", that is nonsensical. -Peff -- 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
On Mon, 4 Feb 2013 16:22:03 -0500 Jeff King wrote: >> Currently, we map both the "port" and "protocol" netrc tokens to the >> credential helper protocol's "protocol". So this will have undefined >> results. To do what you specify could be pretty simple: we could do a >> preliminary scan of the tokens, looking for "host X port Y" where Y is >> an integer, and rewriting the host to be "X:Y". That would be clean and >> simple, unless the user breaks it with "host x:23 port 22". Let me know >> if you agree and I'll do. JK> Yeah, I think that is simple and obvious. If the user is saying "host JK> x:23 port 22", that is nonsensical. OK; added. Ted -- 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
Jeff King writes: > On Tue, Jan 29, 2013 at 11:53:19AM -0800, Junio C Hamano wrote: > >> 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. Again the Help-for-users has this >> amusing bit: > > 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. I see a lot of rerolls on the credential helper front, but is there anybody working on hooking send-email to the credential framework? -- 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
Junio C Hamano writes: > I see a lot of rerolls on the credential helper front, but is there > anybody working on hooking send-email to the credential framework? Not answering the question, but git-remote-mediawiki supports the credential framework. It is written in perl, and the credential support is rather cleanly written and doesn't have dependencies on the wiki part, so the way to go for send-email is probably to libify the credential support in git-remote-mediawiki, and to use it in send-email. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- 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
On Wed, Feb 06 2013, Junio C Hamano 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=: 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 +--ooO--(_)--Ooo-- pgpyBEG11AUhI.pgp Description: PGP signature
Re: [PATCH] git-send-email: add ~/.authinfo parsing
On Wed, 06 Feb 2013 14:26:46 +0100 Michal Nazarewicz wrote: MN> On Wed, Feb 06 2013, Junio C Hamano 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? MN> I assumed someone had, but if not I can take a stab at it. I'm not sure MN> however how should I map server, server-port, and user to credential MN> key-value pairs. I'm leaning towards MN> protocol=smtp MN> host=: MN> user= MN> and than netrc/authinfo helper splitting host to host name and port MN> number, unless port is not in host in which case protocol is assumed as MN> port. That would work (with my PATCHv6 of the netrc credential helper) as follows: 1) just host host=H maps to machine H login Y password Z 2) host + protocol smtp host=H protocol=smtp maps to any of: machine H port smtp login Y password Z machine H protocol smtp login Y password Z 3) host:port + protocol smtp host=H:25 protocol=smtp maps to any of: machine H port 25 protocol smtp login Y password Z machine H:25 port smtp login Y password Z machine H:25 protocol smtp login Y password Z That's my understanding of what we discussed with Peff and Junio about token mapping. Note we don't split the input host, but instead say "if token 'port' is numeric, append it to the host token" on the netrc side. Does that sound reasonable? If yes, I can add it to the testing Makefile for the netrc credential helper, to make sure it's clearly stated and tested. Ted -- 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
On Wed, 06 Feb 2013 09:11:17 +0100 Matthieu Moy wrote: MM> Junio C Hamano writes: >> I see a lot of rerolls on the credential helper front, but is there >> anybody working on hooking send-email to the credential framework? MM> Not answering the question, but git-remote-mediawiki supports the MM> credential framework. It is written in perl, and the credential support MM> is rather cleanly written and doesn't have dependencies on the wiki MM> part, so the way to go for send-email is probably to libify the MM> credential support in git-remote-mediawiki, and to use it in send-email. I looked and that's indeed very useful. If it's put in a library, I'd use credential_read() and credential_write() in my netrc credential helper. But I would formalize it a little more about the token names and output, and I wouldn't necessarily die() on error. Maybe this can be merged with the netrc credential helper's read_credential_data_from_stdin() and print_credential_data()? Let me know if you'd like me to libify this... I'm happy to leave it to Matthieu or Michal, or anyone else interested. Ted -- 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
Ted Zlatanov writes: > MM> [...] so the way to go for send-email is probably to libify the > MM> credential support in git-remote-mediawiki, and to use it in send-email. > > I looked and that's indeed very useful. If it's put in a library, I'd > use credential_read() and credential_write() in my netrc credential > helper. But I would formalize it a little more about the token names > and output, Can you elaborate on this? The idea of the Perl code was to mimick a call to the C API, keeping essentially the same names. > and I wouldn't necessarily die() on error. Sure, die()ing in a library is bad. > Maybe this can be merged with the netrc credential helper's > read_credential_data_from_stdin() and print_credential_data()? I don't know about the netrc credential helper, but I guess that's another layer. The git-remote-mediawiki code is the code to call the credential C API, that in turn may (or may not) call a credential helper. > Let me know if you'd like me to libify this... I'm happy to leave it to > Matthieu or Michal, or anyone else interested. I'd happily let you do the job, but I can help if needed. One thing to be careful about: git-remote-mediawiki is currently a standalone script, so it can be installed with a plain "cp git-remote-mediawiki $somewhere/". One consequence of libification is that it adds a dependency on the library (e.g. Git.pm). We should be carefull to keep it easy for the user to install it (e.g. some kind of "make install", or update the doc). -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- 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
On Wed, 06 Feb 2013 16:10:12 +0100 Matthieu Moy wrote: MM> Ted Zlatanov writes: MM> [...] so the way to go for send-email is probably to libify the MM> credential support in git-remote-mediawiki, and to use it in send-email. >> >> I looked and that's indeed very useful. If it's put in a library, I'd >> use credential_read() and credential_write() in my netrc credential >> helper. But I would formalize it a little more about the token names >> and output, MM> Can you elaborate on this? The idea of the Perl code was to mimick a MM> call to the C API, keeping essentially the same names. None of these are a big deal, and Michal said he's working on libifying this anyhow: - making 'fill' a special operation is weird - anchor the key regex to beginning of line (not strictly necessary) - sort the output tokens (after 'url' is extracted) so the output is consistent and testable >> and I wouldn't necessarily die() on error. MM> Sure, die()ing in a library is bad. >> Maybe this can be merged with the netrc credential helper's >> read_credential_data_from_stdin() and print_credential_data()? MM> I don't know about the netrc credential helper, but I guess that's MM> another layer. The git-remote-mediawiki code is the code to call the MM> credential C API, that in turn may (or may not) call a credential MM> helper. Yup. But what you call "read" and "write" are, to the credential helper, "write" and "read" but it's the same protocol :) So maybe the names should be changed to reflect that, e.g. "query" and "response." MM> One thing to be careful about: git-remote-mediawiki is currently a MM> standalone script, so it can be installed with a plain "cp MM> git-remote-mediawiki $somewhere/". One consequence of libification MM> is that it adds a dependency on the library (e.g. Git.pm). We should MM> be carefull to keep it easy for the user to install it (e.g. some MM> kind of "make install", or update the doc). I don't know--it's up to the `git-remote-mediawiki' maintainers... But I think anywhere you have Git, you also have Git.pm, right? Maybe? But then you also have to look at whether Git.pm has the functionality you need... so I better go quiet :) Ted -- 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
Ted Zlatanov writes:
> None of these are a big deal, and Michal said he's working on libifying
> this anyhow:
>
> - making 'fill' a special operation is weird
Well, 'fill' is the only operation that mutates the credential structure
(i.e. the only one for which "git credential" emits an output to be
parsed), so you don't have much choice.
> - anchor the key regex to beginning of line (not strictly necessary)
Right. The greedyness of * ensures correction, but I like explicit
anchors ^...$ too.
> - sort the output tokens (after 'url' is extracted) so the output is
> consistent and testable
Why not, if you want to use the output of credential_write in tests. But
credential_write is essentially used to talk to "git credential", so the
important information is the content of the hash before credential_write
and after credential_read. They are unordered, but consistent and
testable.
>>> Maybe this can be merged with the netrc credential helper's
>>> read_credential_data_from_stdin() and print_credential_data()?
>
> MM> I don't know about the netrc credential helper, but I guess that's
> MM> another layer. The git-remote-mediawiki code is the code to call the
> MM> credential C API, that in turn may (or may not) call a credential
> MM> helper.
>
> Yup. But what you call "read" and "write" are, to the credential
> helper, "write" and "read" but it's the same protocol :) So maybe the
> names should be changed to reflect that, e.g. "query" and "response."
I don't think that would be a better naming. Maybe "serialize" and
"parse" would be better, but "query" would sound like it establishes the
connection and possibly reads the response to me.
> MM> One thing to be careful about: git-remote-mediawiki is currently a
> MM> standalone script, so it can be installed with a plain "cp
> MM> git-remote-mediawiki $somewhere/". One consequence of libification
> MM> is that it adds a dependency on the library (e.g. Git.pm). We should
> MM> be carefull to keep it easy for the user to install it (e.g. some
> MM> kind of "make install", or update the doc).
>
> I don't know--it's up to the `git-remote-mediawiki' maintainers...
That is, me ;-).
> But I think anywhere you have Git, you also have Git.pm, right?
Yes, but you have to find out where it is installed. Git's Makefile
hardcodes the path to Git.pm at build time, inserting one line in the
perl script:
use lib (split(/:/, $ENV{GITPERLLIB} || "$INSTLIBDIR"));
The same needs to be done for git-remote-mediawiki. As much as possible,
I'd rather avoid copy-pasting from Git's Makefile, so this means
extracting the perl part of Git's Makefile and make it available in
contrib/.
I'll try a patch in this direction.
> Maybe? But then you also have to look at whether Git.pm has the
> functionality you need...
If git-remote-mediawiki is installed from Git's source, I think it's OK
to assume that Git.pm will be up to date, but that would be even better
if we can issue a clean error message when the functions to be called do
not exist.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
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
On Wed, 06 Feb 2013 17:41:01 +0100 Matthieu Moy wrote: MM> Ted Zlatanov writes: >> - sort the output tokens (after 'url' is extracted) so the output is >> consistent and testable MM> Why not, if you want to use the output of credential_write in tests. But MM> credential_write is essentially used to talk to "git credential", so the MM> important information is the content of the hash before credential_write MM> and after credential_read. They are unordered, but consistent and MM> testable. I like testing output (especially when it's part of an API), so we should make the externally observable output consistent and testable. The change is tiny, just sort the keys instead of calling each(), so I hope it makes it in the final version. >> Yup. But what you call "read" and "write" are, to the credential >> helper, "write" and "read" but it's the same protocol :) So maybe the >> names should be changed to reflect that, e.g. "query" and "response." MM> I don't think that would be a better naming. Maybe "serialize" and MM> "parse" would be better, but "query" would sound like it establishes the MM> connection and possibly reads the response to me. I'm OK with anything unambiguous. Thanks! Ted -- 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
On Wed, Feb 06, 2013 at 10:58:13AM -0500, Ted Zlatanov wrote: > MM> I don't know about the netrc credential helper, but I guess that's > MM> another layer. The git-remote-mediawiki code is the code to call the > MM> credential C API, that in turn may (or may not) call a credential > MM> helper. > > Yup. But what you call "read" and "write" are, to the credential > helper, "write" and "read" but it's the same protocol :) So maybe the > names should be changed to reflect that, e.g. "query" and "response." Is that true? As a user of the credential system, git-remote-mediawiki would want to "write" to git-credential, then "read" the response. As a helper, git-credential-netrc would want to "read" the query then "write" the response. The order is different, but the operations should be the same in both cases. The big difference is that mediawiki would want an additional function to open a pipe to "git credential" and operate on that, whereas the helper will be reading/writing stdio. -Peff -- 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
On Wed, 6 Feb 2013 16:57:24 -0500 Jeff King wrote: JK> On Wed, Feb 06, 2013 at 10:58:13AM -0500, Ted Zlatanov wrote: MM> I don't know about the netrc credential helper, but I guess that's MM> another layer. The git-remote-mediawiki code is the code to call the MM> credential C API, that in turn may (or may not) call a credential MM> helper. >> >> Yup. But what you call "read" and "write" are, to the credential >> helper, "write" and "read" but it's the same protocol :) So maybe the >> names should be changed to reflect that, e.g. "query" and "response." JK> Is that true? As a user of the credential system, git-remote-mediawiki JK> would want to "write" to git-credential, then "read" the response. As a JK> helper, git-credential-netrc would want to "read" the query then JK> "write" the response. The order is different, but the operations should JK> be the same in both cases. Logically they are different steps (query and response), even though the data protocol is the same. But it's really not a big deal, I know what it means either way. JK> The big difference is that mediawiki would want an additional function JK> to open a pipe to "git credential" and operate on that, whereas the JK> helper will be reading/writing stdio. Yup. Ted -- 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
Ted Zlatanov writes: > Logically they are different steps (query and response), even though the > data protocol is the same. But it's really not a big deal, I know what > it means either way. Yes, but if you rename write() to query(), then on the helper side, you'll have to call query() to send the response, and response() to read the query. Much worse than keeping read/write. Plus, read/write has already been used for a while in the C API, so I'd rather keep the same names for the Perl equivalent. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- 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
On Thu, 07 Feb 2013 08:08:59 +0100 Matthieu Moy wrote: MM> Plus, read/write has already been used for a while in the C API, so I'd MM> rather keep the same names for the Perl equivalent. That makes perfect sense. Ted -- 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
