* Re: [PATCH] connect.c: Tell *PLink to always use ssh protocol
From: Sven Strickroth @ 2013-02-07 9:40 UTC (permalink / raw)
To: Jeff King; +Cc: msysgit, git, gitster
In-Reply-To: <20130206232214.GN27507@sigill.intra.peff.net>
Am 07.02.2013 00:22 schrieb Jeff King:
> On Wed, Feb 06, 2013 at 10:58:49PM +0100, Sven Strickroth wrote:
>
>> Default values for *plink can be set using PuTTY. If a user makes
>> telnet the default in PuTTY this breaks ssh clones in git.
>>
>> Since git clones of the type user@host:path use ssh, tell *plink
>> to use ssh and override PuTTY defaults for the protocol to use.
>> ---
>> connect.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/connect.c b/connect.c
>> index 49e56ba..d337b6f 100644
>> --- a/connect.c
>> +++ b/connect.c
>> @@ -625,6 +625,8 @@ struct child_process *git_connect(int fd[2], const char *url_orig,
>> if (!ssh) ssh = "ssh";
>>
>> *arg++ = ssh;
>> + if (putty)
>> + *arg++ = "-ssh";
>> if (putty && !strcasestr(ssh, "tortoiseplink"))
>> *arg++ = "-batch";
>> if (port) {
Just for the completeness: This might have an unwanted side effect...
Using the -ssh parameter sets the protocol to ssh AND the port number to
22. This might break a setting where a user stores a PuTTY default for
ssh, but with a different port number (e.g. because a user always pushes
to a remote ssh repository which resides on a different port).
PuTTY settings for a named session still work, it only affects the
"Default Settings" session - so users can set up specific sessons in
PuTTY if he wants to change the default port.
--
Best regards,
Sven Strickroth
PGP key id F5A9D4C4 @ any key-server
--
--
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.
You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en
---
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.
^ permalink raw reply
* Re: [RFC/PATCH 1/4] show: obey --textconv for blobs
From: Jeff King @ 2013-02-07 9:43 UTC (permalink / raw)
To: Michael J Gruber; +Cc: git, Junio C Hamano
In-Reply-To: <51137522.3010005@drmicha.warpmail.net>
On Thu, Feb 07, 2013 at 10:34:26AM +0100, Michael J Gruber wrote:
> > Just as we record the path from the surrounding tree, we record the
> > mode. It's that mode which gets put into the pending object list by the
> > revision parser (see the very end of handle_revision_arg). Storing an
> > object_context instead of the mode would be a strict superset of what we
> > store now (right now we just throw the rest away).
>
> Sure. But why does object_context have a mode member at all? Maybe it is
> not alway used together with another struct which has the mode already,
> then that's a reason.
Exactly. It's purely for pulling information out of
get_sha1_with_context, and does not know that you are going to put its
output into an object_array_entry (and many call sites do not).
-Peff
^ permalink raw reply
* Re: [RFC/PATCH 4/4] grep: obey --textconv for the case rev:path
From: Michael J Gruber @ 2013-02-07 9:47 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, Git Mailing List
In-Reply-To: <20130207092640.GC15727@sigill.intra.peff.net>
Jeff King venit, vidit, dixit 07.02.2013 10:26:
> On Thu, Feb 07, 2013 at 10:05:57AM +0100, Michael J Gruber wrote:
>
>>>> @@ -265,9 +260,28 @@ void add_object_array_with_mode(struct object *obj, const char *name, struct obj
>>>> objects[nr].item = obj;
>>>> objects[nr].name = name;
>>>> objects[nr].mode = mode;
>>>> + objects[nr].context = context;
>>>> array->nr = ++nr;
>>>> }
>>>
>>> This seems a little gross. Who is responsible for allocating the
>>> context? Who frees it? It looks like we duplicate it in cmd_grep. Which
>>
>> Well, who is responsible for allocating and freeing name and item? I
>> didn't want to introduce a new member which is a struct when all other
>> complex members are pointers. Wouldn't that be confusing?
>
> We cheat on those two. "item" is always a pointer to a "struct object",
> which lasts forever and never gets freed. When "name" is set by
> setup_revisions, it comes from the argv list, which is assumed to last
> forever (and when we add pending blobs for a "--objects" traversal, it
> is the empty string (literal).
I see, so they are really different.
> I'd be OK if we had an exterior object_context that could be handled
> in the same way. But how do we tell setup_revisions that we are
> interested in seeing the object_context from each parsed item, where
> does the allocation come from (is it malloc'd by setup_revisions?), and
> who is responsible for freeing it when we pop pending objects in
> get_revisions and similar?
Do we really need all of tree, path and mode in object_context (I mean
not just here, but other users), or only the path? I'd try and resurrect
the virtual path name objects then, they would be just like "item"
storage-wise.
> I don't think it's as clear cut.
>
> I wonder, though...what we really care about here is just the pathname.
> But if it is a pending object that comes from a blob revision argument,
> won't it always be of the form "treeish:path"? Could we not even resolve
> the sha1 again, but instead just parse out the ":path" bit?
Do we have that, and in what form (e.g. magic expanded etc.)?
> That is sort of like what the repeated call to get_sha1_with_context
> does in your first patch. Except that we do not actually want to lookup
> the sha1, and it is harmful to do so (e.g., if the ref had moved on to a
> new tree that does not have that path, get_sha1 would fail, but we do
> not even care what is in the tree; we only want the parsing side effects
> of get_sha1).
>
> Hmm.
>
> -Peff
>
> PS By the way, while looking at the object_array code (which I have not
> really used much before), I noticed that add_pending_commit_list sets
> the "name" field to the result of sha1_to_hex. Which means that it is
> likely to be completely bogus by the time you read it. I'm not even
> sure where it gets read or if this matters. And obviously it's
> completely unrelated to what we were discussing; just something I
> noticed.
Another thing I noted is that our path mangling at least for grep has
some issues:
(cd t && git grep GET_SHA1_QUIETLY HEAD:../cache.h)
../HEAD:../cache.h:#define GET_SHA1_QUIETLY 01
Taking everything right of ":" could still work.
Michael
^ permalink raw reply
* Re: [RFC/PATCH 4/4] grep: obey --textconv for the case rev:path
From: Jeff King @ 2013-02-07 9:55 UTC (permalink / raw)
To: Michael J Gruber; +Cc: Junio C Hamano, Git Mailing List
In-Reply-To: <5113784B.10103@drmicha.warpmail.net>
On Thu, Feb 07, 2013 at 10:47:55AM +0100, Michael J Gruber wrote:
> > I'd be OK if we had an exterior object_context that could be handled
> > in the same way. But how do we tell setup_revisions that we are
> > interested in seeing the object_context from each parsed item, where
> > does the allocation come from (is it malloc'd by setup_revisions?), and
> > who is responsible for freeing it when we pop pending objects in
> > get_revisions and similar?
>
> Do we really need all of tree, path and mode in object_context (I mean
> not just here, but other users), or only the path? I'd try and resurrect
> the virtual path name objects then, they would be just like "item"
> storage-wise.
We need at least mode, since that is how the mode parameter of
object_array_entry gets set. I do not know off-hand who uses "tree". I
suspect the intent was to do .gitattributes lookups inside that tree,
but I do not think we actually do in-tree lookups currently.
> > I don't think it's as clear cut.
> >
> > I wonder, though...what we really care about here is just the pathname.
> > But if it is a pending object that comes from a blob revision argument,
> > won't it always be of the form "treeish:path"? Could we not even resolve
> > the sha1 again, but instead just parse out the ":path" bit?
>
> Do we have that, and in what form (e.g. magic expanded etc.)?
Ah, I should have mentioned that. :) We should have the original rev
name in the object_array_entry's name field, shouldn't we? It's just a
matter of re-parsing it.
> Another thing I noted is that our path mangling at least for grep has
> some issues:
>
> (cd t && git grep GET_SHA1_QUIETLY HEAD:../cache.h)
> ../HEAD:../cache.h:#define GET_SHA1_QUIETLY 01
Yuck.
-Peff
^ permalink raw reply
* Re: [PATCH v3 0/8] Hiding refs
From: Ævar Arnfjörð Bjarmason @ 2013-02-07 10:30 UTC (permalink / raw)
To: Jeff King
Cc: Junio C Hamano, Duy Nguyen, Michael Haggerty, Jonathan Nieder,
git, Shawn Pearce
In-Reply-To: <20130207001635.GA29318@sigill.intra.peff.net>
On Thu, Feb 7, 2013 at 1:16 AM, Jeff King <peff@peff.net> wrote:
> On Wed, Feb 06, 2013 at 04:12:10PM -0800, Junio C Hamano wrote:
>
>> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>>
>> > I think there's a simpler way to do this, which is that:
>> >
>> > * New clients supporting v2 of the protocol send some piece of data
>> > that would break old servers.
>> >
>> > * If that fails the new client goes "oh jeeze, I guess it's an old
>> > server", and try again with the old protocol.
>> >
>> > * The client then saves a date (or the version the server gave us)
>> > indicating that it tried the new protocol on that remote, tries
>> > again sometime later.
>>
>> For that to work, the new server needs to wait for the client to
>> speak first. How would that server handle old clients who expect to
>> be spoken first? Wait with a read timeout (no timeout is the right
>> timeout for everybody)?
>
> If the new client can handle the old-style server's response, then the
> server can start blasting out refs (optionally after a timeout) and stop
> when the client interrupts with "hey, wait, I can speak the new
> protocol". The server just has to include "you can interrupt me" in its
> capability advertisement (obviously it would have to send out at least
> the first ref with the capabilities before the timeout).
Can't this also be handled by passing an extra argument to
upload-pack? Whether you're talking http, ssh + normal shell, ssh +
git-shell or git:// you pass some argument that older clients would
reject on but would cause newer clients that know about that argument
to wait for you to speak before blasting refs at you.
It would mean that older clients (e.g. older git-shell) would reject
your initial connection, but you could just try again, and save away
info about that remote's version.
^ permalink raw reply
* Re: [RFC/PATCH 4/4] grep: obey --textconv for the case rev:path
From: Michael J Gruber @ 2013-02-07 10:31 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, Git Mailing List
In-Reply-To: <20130207095533.GA17110@sigio.peff.net>
Jeff King venit, vidit, dixit 07.02.2013 10:55:
> On Thu, Feb 07, 2013 at 10:47:55AM +0100, Michael J Gruber wrote:
>
>>> I'd be OK if we had an exterior object_context that could be handled
>>> in the same way. But how do we tell setup_revisions that we are
>>> interested in seeing the object_context from each parsed item, where
>>> does the allocation come from (is it malloc'd by setup_revisions?), and
>>> who is responsible for freeing it when we pop pending objects in
>>> get_revisions and similar?
>>
>> Do we really need all of tree, path and mode in object_context (I mean
>> not just here, but other users), or only the path? I'd try and resurrect
>> the virtual path name objects then, they would be just like "item"
>> storage-wise.
>
> We need at least mode, since that is how the mode parameter of
> object_array_entry gets set. I do not know off-hand who uses "tree". I
> suspect the intent was to do .gitattributes lookups inside that tree,
> but I do not think we actually do in-tree lookups currently.
>
>>> I don't think it's as clear cut.
>>>
>>> I wonder, though...what we really care about here is just the pathname.
>>> But if it is a pending object that comes from a blob revision argument,
>>> won't it always be of the form "treeish:path"? Could we not even resolve
>>> the sha1 again, but instead just parse out the ":path" bit?
>>
>> Do we have that, and in what form (e.g. magic expanded etc.)?
>
> Ah, I should have mentioned that. :) We should have the original rev
> name in the object_array_entry's name field, shouldn't we? It's just a
> matter of re-parsing it.
>
>> Another thing I noted is that our path mangling at least for grep has
>> some issues:
>>
>> (cd t && git grep GET_SHA1_QUIETLY HEAD:../cache.h)
>> ../HEAD:../cache.h:#define GET_SHA1_QUIETLY 01
>
> Yuck.
And even more yuck:
(cd t && git grep --full-name GET_SHA1_QUIETLY HEAD:../cache.h)
HEAD:../cache.h:#define GET_SHA1_QUIETLY 01
Someone does not expect a "rev:" to be in there, it seems ;)
Michael
^ permalink raw reply
* [PATCHv2 0/5] Make git-send-email use git-credential
From: Michal Nazarewicz @ 2013-02-07 14:01 UTC (permalink / raw)
To: Jeff King, Junio C Hamano, Matthieu Moy; +Cc: git
From: Michal Nazarewicz <mina86@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
^ permalink raw reply
* [PATCHv2 1/5] Git.pm: allow command_close_bidi_pipe to be called as method
From: Michal Nazarewicz @ 2013-02-07 14:01 UTC (permalink / raw)
To: Jeff King, Junio C Hamano, Matthieu Moy; +Cc: git
In-Reply-To: <cover.1360242782.git.mina86@mina86.com>
From: Michal Nazarewicz <mina86@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 <mina86@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
^ permalink raw reply related
* [PATCHv2 4/5] Git.pm: add interface for git credential command
From: Michal Nazarewicz @ 2013-02-07 14:01 UTC (permalink / raw)
To: Jeff King, Junio C Hamano, Matthieu Moy; +Cc: git
In-Reply-To: <cover.1360242782.git.mina86@mina86.com>
From: Michal Nazarewicz <mina86@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 <mina86@mina86.com>
---
perl/Git.pm | 110 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 109 insertions(+), 1 deletion(-)
On Thu, Feb 07 2013, Jeff King <peff@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.Moy@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 C<git credential> for a given set of credentials and
+specified operation. In both form C<CREDENTIAL_HASH> needs to be
+a reference to a hash which stores credentials. Under certain
+conditions the hash can change.
+
+In the first form, C<OPERATION> can be C<'fill'> (or omitted),
+C<'approve'> or C<'reject'>, and function will execute corresponding
+C<git credential> sub-command. In case of C<'fill'> the values stored
+in C<CREDENTIAL_HASH> will be changed to the ones returned by the
+C<git credential> 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 {
+ Git::credential \%cred, 'reject';
+ }
+
+In the second form, C<CODE> needs to be a reference to a subroutine.
+The function will execute C<git credential fill> to fill provided
+credential hash, than call C<CODE> with C<CREDENTIAL_HASH> as the sole
+argument, and finally depending on C<CODE>'s return value execute
+C<git credential approve> (if return value yields true) or C<git
+credential reject> (otherwise). The return value is the same as what
+C<CODE> returned. With this form, the usage might look as follows:
+
+ if (Git::credential {
+ 'protocol' => 'https',
+ 'host' => 'example.com',
+ 'username' => 'bob'
+ }, sub {
+ my $cred = shift;
+ return try_to_authenticate($cred->{'username'}, $cred->{'password'});
+ }) {
+ ... do more stuff ...
+ }
+
+=cut
+
+sub credential {
+ my ($self, $credential, $op_or_code) = (_maybe_self(@_), 'fill');
+
+ if ('CODE' eq ref $op_or_code) {
+ _credential_run $credential, 'fill';
+ my $ret = $op_or_code->($credential);
+ _credential_run $credential, $ret ? 'approve' : 'reject';
+ return $ret;
+ } else {
+ _credential_run $credential, $op_or_code;
+ }
+}
+
{ # %TEMP_* Lexical Context
my (%TEMP_FILEMAP, %TEMP_FILES);
--
1.8.1.2.549.g1d13f9f
^ permalink raw reply related
* [PATCHv2 2/5] Git.pm: fix example in command_close_bidi_pipe documentation
From: Michal Nazarewicz @ 2013-02-07 14:01 UTC (permalink / raw)
To: Jeff King, Junio C Hamano, Matthieu Moy; +Cc: git
In-Reply-To: <cover.1360242782.git.mina86@mina86.com>
From: Michal Nazarewicz <mina86@mina86.com>
File handle goes as the first argument when calling print on it.
Signed-off-by: Michal Nazarewicz <mina86@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 C<command_bidi_pipe()>. The call idiom
is:
my ($pid, $in, $out, $ctx) = $r->command_bidi_pipe('cat-file --batch-check');
- print "000000000\n" $out;
+ print $out "000000000\n";
while (<$in>) { ... }
$r->command_close_bidi_pipe($pid, $in, $out, $ctx);
--
1.8.1.2.549.g1d13f9f
^ permalink raw reply related
* [PATCHv2 3/5] Git.pm: allow pipes to be closed prior to calling command_close_bidi_pipe
From: Michal Nazarewicz @ 2013-02-07 14:01 UTC (permalink / raw)
To: Jeff King, Junio C Hamano, Matthieu Moy; +Cc: git
In-Reply-To: <cover.1360242782.git.mina86@mina86.com>
From: Michal Nazarewicz <mina86@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 <mina86@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 <peff@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 C<CTX>;
currently it is simply the command name but in future the context might
have more complicated structure.
+C<PIPE_IN> and C<PIPE_OUT> may be C<undef> 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 "000000000\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
^ permalink raw reply related
* [PATCHv2 5/5] git-send-email: use git credential to obtain password
From: Michal Nazarewicz @ 2013-02-07 14:01 UTC (permalink / raw)
To: Jeff King, Junio C Hamano, Matthieu Moy; +Cc: git
In-Reply-To: <cover.1360242782.git.mina86@mina86.com>
From: Michal Nazarewicz <mina86@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 <mina86@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
^ permalink raw reply related
* Re: [PATCH] git-send-email: add ~/.authinfo parsing
From: Ted Zlatanov @ 2013-02-07 14:30 UTC (permalink / raw)
To: Matthieu Moy
Cc: Jeff King, Junio C Hamano, Michal Nazarewicz, git,
Krzysztof Mazur, Michal Nazarewicz
In-Reply-To: <vpq4nhowqgk.fsf@grenoble-inp.fr>
On Thu, 07 Feb 2013 08:08:59 +0100 Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> 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
^ permalink raw reply
* Re: overriding/removing inherited credential.helper, Do not add an empty value from config credential.helper
From: Ted Zlatanov @ 2013-02-07 14:36 UTC (permalink / raw)
To: 乙酸鋰; +Cc: git
In-Reply-To: <CAHtLG6QaHOOYgVFPyOWo44-jTX__cd0dCyu+vs+Uf4_U-HxySw@mail.gmail.com>
On Sat, 10 Nov 2012 23:12:50 +0800 乙酸鋰 <ch3cooli@gmail.com> wrote:
> In credential.c, line 67:
> if (!strcmp(key, "helper"))
> string_list_append(&c->helpers, value);
> In global config, I add one credential helper.
> But I do not want to use any credential helper in a specific repository.
> Currently there is no way in local config to override and remove
> inherited credential helper.
> Of course, I do not want to change global config.
> Could you suggest a patch?
On Sat, 10 Nov 2012 23:08:04 +0800 乙酸鋰 <ch3cooli@gmail.com> wrote:
> Below is current git message when a local config credential.helper has
> an empty value. Please skip an empty value.
> $ git push --force origin master
> git: 'credential-' is not a git command. See 'git --help'.
> Did you mean this?
> credential
> Total 0 (delta 0), reused 0 (delta 0)
> To https://user@github.com/user/myrepo.git
> + d23aa6a...3405990 master -> master (forced update)
I would like that too (needed it today). Maybe the empty string (as
suggested) or "none" could be acceptable.
"none" seems friendlier and is very unlikely to collide with an actual
credential helper.
Ted
^ permalink raw reply
* Re: Credentials and the Secrets API...
From: Ted Zlatanov @ 2013-02-07 14:46 UTC (permalink / raw)
To: John Szakmeister; +Cc: git
In-Reply-To: <CAEBDL5Udooim_3Za76Q1Rt_aGXtsSv76nxRegGWRBE=WJQzfZA@mail.gmail.com>
On Thu, 27 Oct 2011 12:05:03 -0400 John Szakmeister <john@szakmeister.net> wrote:
JS> Just wanted to keep folks in the loop. It turns out that the Secrets
JS> API is still to young. I asked about the format to store credentials
JS> in (as far as attributes), and got a response from a KDE developer
JS> that says it's still to young on their front. They hope to have
JS> support in the next release of KDE. But there's still the issue of
JS> what attributes to use.
JS> With that information, I went ahead and created a
JS> gnome-credential-keyring that uses Gnome's Keyring facility. I still
JS> need to do a few more things (mainly run it against Jeff's tests), but
JS> it's generally working. Just wanted to keep folks in the loop.
JS> Hopefully, I can get patches out this weekend.
Do you think the Secrets API has matured enough? KDE has had a new
release since your post...
Thanks
Ted
^ permalink raw reply
* Re: [PATCH v3 0/8] Hiding refs
From: Jed Brown @ 2013-02-07 15:58 UTC (permalink / raw)
To: Michael Haggerty, Junio C Hamano; +Cc: git, Jeff King, Shawn Pearce
In-Reply-To: <51122D9D.9040100@alum.mit.edu>
Michael Haggerty <mhagger@alum.mit.edu> writes:
> A first weakness of your proposal is that even though the hidden refs
> are (optionally) fetchable, there is *no* way to discover them remotely
> or to bulk-download them; they would have to be retrieved one by one
> using out-of-band information. And if I understand correctly, there
> would be no way to push hidden references remotely (whether manually or
> from some automated process). Such processes would have to be local to
> the machine holding the repository.
I'm the author of git-fat [1], a smudge/clean filter system for managing
large files. I currently store files in the file system
(.git/fat/objects) and transfer them via rsync because I want to be able
to transfer exact subsets requested by the user. I would like to put
this data in a git repository so that I can take advantage of packfile
compression when applicable and so that I can use existing access
control, but I would need to store a separate reference to each blob (so
that I can transfer exact subsets). My refs would be named like
'fat-<SHA1_OF_SMUDGED_DATA>' and are known on the client side because
they are in the cleaned blob (which contains only this SHA1 and the
number of bytes [2]).
I believe that my use case would be well supported if git could push and
pull unadvertised refs, as long as basic operations were not slowed down
by the existence of a very large number of such refs.
[1] https://github.com/jedbrown/git-fat
[2] We could eliminate the performance problem of needing to buffer the
entire file if the smudge filter could be passed the object size as an
argument and if we could forward that size in a stream to 'git
hash-object --stdin'.
^ permalink raw reply
* Proposal: branch.<name>.remotepush
From: Ramkumar Ramachandra @ 2013-02-07 16:14 UTC (permalink / raw)
To: Git List
Hi,
This has been annoying me for a really long time, but I never really
got around to scratching this particular itch. I have a very common
scenario where I fork a project on GitHub. I have two configured
remotes: origin which points to "git://upstream" and mine which points
to "ssh://mine". By default, I always want to pull `master` from
origin and push to mine. Unfortunately, there's only a
branch.<name>.remote which specifies which remote to use for both
pulling and pushing. There's also a remote.<name>.pushurl, but I get
the feeling that this exists for an entirely different reason: when I
have a server with a highly-available read-only mirror of the
repository at git://anongit.*, and a less-available committer-only
mirror at ssh://*.
How about a branch.<name>.remotepush that specifies a special remote
for pushing, falling back to branch.<name>.remote?
Ram
^ permalink raw reply
* Re: Proposal: branch.<name>.remotepush
From: Michael Schubert @ 2013-02-07 17:45 UTC (permalink / raw)
To: Ramkumar Ramachandra; +Cc: Git List
In-Reply-To: <CALkWK0nA4hQ0VWivk3AVVVq8Rbb-9CpQ9xFsSOsTQtvo4w08rw@mail.gmail.com>
On 02/07/2013 05:14 PM, Ramkumar Ramachandra wrote:
> This has been annoying me for a really long time, but I never really
> got around to scratching this particular itch. I have a very common
> scenario where I fork a project on GitHub. I have two configured
> remotes: origin which points to "git://upstream" and mine which points
> to "ssh://mine". By default, I always want to pull `master` from
> origin and push to mine. Unfortunately, there's only a
> branch.<name>.remote which specifies which remote to use for both
> pulling and pushing. There's also a remote.<name>.pushurl, but I get
> the feeling that this exists for an entirely different reason: when I
> have a server with a highly-available read-only mirror of the
> repository at git://anongit.*, and a less-available committer-only
> mirror at ssh://*.
>
> How about a branch.<name>.remotepush that specifies a special remote
> for pushing, falling back to branch.<name>.remote?
Additionally, it would be nice to have branch.<name>.push or similar
to configure a default destination branch for push. Gerrit users usually
want to track refs/heads/master but push to refs/for/master for example.
^ permalink raw reply
* Re: [RFC/PATCH 4/4] grep: obey --textconv for the case rev:path
From: Junio C Hamano @ 2013-02-07 18:03 UTC (permalink / raw)
To: Michael J Gruber; +Cc: Jeff King, Git Mailing List
In-Reply-To: <5113827A.40801@drmicha.warpmail.net>
Michael J Gruber <git@drmicha.warpmail.net> writes:
>>> (cd t && git grep GET_SHA1_QUIETLY HEAD:../cache.h)
>>> ../HEAD:../cache.h:#define GET_SHA1_QUIETLY 01
>>
>> Yuck.
>
> And even more yuck:
>
> (cd t && git grep --full-name GET_SHA1_QUIETLY HEAD:../cache.h)
> HEAD:../cache.h:#define GET_SHA1_QUIETLY 01
>
> Someone does not expect a "rev:" to be in there, it seems ;)
I think stepping outside of $(cwd) is an afterthought the code does
not anticipate.
^ permalink raw reply
* Re: [PATCH 2/2] count-objects: report garbage files in .git/objects/pack directory too
From: Junio C Hamano @ 2013-02-07 18:12 UTC (permalink / raw)
To: Duy Nguyen; +Cc: git
In-Reply-To: <20130207073751.GA20672@duynguyen-vnpc.dek-tpc.internal>
Duy Nguyen <pclouds@gmail.com> writes:
> I thought about that, but we may need to do extra stat() for loose
> garbage as well. As it is now, garbage is complained loudly, which
> gives me enough motivation to clean up, even without looking at how
> much disk space it uses.
I wouldn't call a single line "garbage: 4" exactly *loud*. I also
think that this is not about *motivating* you, but about giving
more information to the users to help them assess the health of
their repository themselves.
By the way, I wonder if we also want to notice .git/objects/garbage
or .git/objects/ga/rbage if we are to do this?
> -- 8< --
> Subject: count-objects: report garbage pack directory too
>
> prepare_packed_git_one() is modified to allow count-objects to hook a
> report function to so we don't need to duplicate the pack searching
> logic in count-objects.c. When report_pack_garbage is NULL, the
> overhead is insignificant.
>
> diff --git a/Documentation/git-count-objects.txt b/Documentation/git-count-objects.txt
> index e816823..1611d7c 100644
> --- a/Documentation/git-count-objects.txt
> +++ b/Documentation/git-count-objects.txt
> @@ -33,8 +33,8 @@ size-pack: disk space consumed by the packs, in KiB
> prune-packable: the number of loose objects that are also present in
> the packs. These objects could be pruned using `git prune-packed`.
> +
> -garbage: the number of files in loose object database that are not
> -valid loose objects
> +garbage: the number of files in object database that are not valid
> +loose objects nor valid packs
>
> GIT
> ---
> diff --git a/builtin/count-objects.c b/builtin/count-objects.c
> index 9afaa88..7fdd508 100644
> --- a/builtin/count-objects.c
> +++ b/builtin/count-objects.c
> @@ -9,6 +9,8 @@
> #include "builtin.h"
> #include "parse-options.h"
>
> +static unsigned long garbage;
> +
> static void count_objects(DIR *d, char *path, int len, int verbose,
> unsigned long *loose,
> off_t *loose_size,
> @@ -65,6 +67,16 @@ static void count_objects(DIR *d, char *path, int len, int verbose,
> }
> }
>
> +extern void (*report_pack_garbage)(const char *path, int len, const char *name);
> +static void real_report_pack_garbage(const char *path, int len, const char *name)
> +{
> + if (len)
> + error("garbage found: %.*s/%s", len, path, name);
> + else
> + error("garbage found: %s", path);
> + garbage++;
> +}
> +
> static char const * const count_objects_usage[] = {
> N_("git count-objects [-v]"),
> NULL
> @@ -76,7 +88,7 @@ int cmd_count_objects(int argc, const char **argv, const char *prefix)
> const char *objdir = get_object_directory();
> int len = strlen(objdir);
> char *path = xmalloc(len + 50);
> - unsigned long loose = 0, packed = 0, packed_loose = 0, garbage = 0;
> + unsigned long loose = 0, packed = 0, packed_loose = 0;
> off_t loose_size = 0;
> struct option opts[] = {
> OPT__VERBOSE(&verbose, N_("be verbose")),
> @@ -87,6 +99,8 @@ int cmd_count_objects(int argc, const char **argv, const char *prefix)
> /* we do not take arguments other than flags for now */
> if (argc)
> usage_with_options(count_objects_usage, opts);
> + if (verbose)
> + report_pack_garbage = real_report_pack_garbage;
> memcpy(path, objdir, len);
> if (len && objdir[len-1] != '/')
> path[len++] = '/';
> diff --git a/sha1_file.c b/sha1_file.c
> index 40b2329..5b70e55 100644
> --- a/sha1_file.c
> +++ b/sha1_file.c
> @@ -21,6 +21,7 @@
> #include "sha1-lookup.h"
> #include "bulk-checkin.h"
> #include "streaming.h"
> +#include "dir.h"
>
> #ifndef O_NOATIME
> #if defined(__linux__) && (defined(__i386__) || defined(__PPC__))
> @@ -1000,15 +1001,19 @@ void install_packed_git(struct packed_git *pack)
> packed_git = pack;
> }
>
> +void (*report_pack_garbage)(const char *path, int len, const char *name);
> +
> static void prepare_packed_git_one(char *objdir, int local)
> {
> /* Ensure that this buffer is large enough so that we can
> append "/pack/" without clobbering the stack even if
> strlen(objdir) were PATH_MAX. */
> char path[PATH_MAX + 1 + 4 + 1 + 1];
> - int len;
> + int i, len;
> DIR *dir;
> struct dirent *de;
> + struct packed_git *p;
> + struct string_list garbage = STRING_LIST_INIT_DUP;
>
> sprintf(path, "%s/pack", objdir);
> len = strlen(path);
> @@ -1024,14 +1029,33 @@ static void prepare_packed_git_one(char *objdir, int local)
> int namelen = strlen(de->d_name);
> struct packed_git *p;
>
> - if (!has_extension(de->d_name, ".idx"))
> + if (len + namelen + 1 > sizeof(path)) {
> + if (report_pack_garbage)
> + report_pack_garbage(path, len - 1, de->d_name);
> continue;
> + }
>
> - if (len + namelen + 1 > sizeof(path))
> + strcpy(path + len, de->d_name);
> +
> + if (!has_extension(de->d_name, ".idx")) {
> + if (!report_pack_garbage)
> + continue;
> + if (is_dot_or_dotdot(de->d_name))
> + continue;
> + if (!has_extension(de->d_name, ".pack")) {
> + report_pack_garbage(path, 0, NULL);
> + continue;
> + }
Didn't I already say the logic should be inverted to whitelist the
known ones? Saying "Anything that is not '.pack' is bad" here is a
direct opposite, I think.
Add "A '.keep' file is OK" to this codeflow and see how it goes.
> + /*
> + * we can't decide right know if this .pack is
> + * garbage. Delay until we identify all good
> + * packs.
> + */
> + string_list_append(&garbage, path);
> continue;
> + }
>
> /* Don't reopen a pack we already have. */
> - strcpy(path + len, de->d_name);
> for (p = packed_git; p; p = p->next) {
> if (!memcmp(path, p->pack_name, len + namelen - 4))
> break;
> @@ -1047,6 +1071,25 @@ static void prepare_packed_git_one(char *objdir, int local)
> install_packed_git(p);
> }
> closedir(dir);
> +
> + if (!report_pack_garbage)
> + return;
> +
> + sort_string_list(&garbage);
> + for (p = packed_git; p; p = p->next) {
> + struct string_list_item *item;
> + if (!p->pack_local)
> + continue;
> + item = string_list_lookup(&garbage, p->pack_name);
> + if (item)
> + item->util = &garbage; /* anything but NULL */
> + }
> + for (i = 0; i < garbage.nr; i++) {
> + struct string_list_item *item = garbage.items + i;
> + if (!item->util)
> + report_pack_garbage(item->string, 0, NULL);
> + }
> + string_list_clear(&garbage, 0);
> }
>
> static int sort_pack(const void *a_, const void *b_)
> -- 8< --
^ permalink raw reply
* Re: overriding/removing inherited credential.helper, Do not add an empty value from config credential.helper
From: Junio C Hamano @ 2013-02-07 18:23 UTC (permalink / raw)
To: Ted Zlatanov; +Cc: 乙酸鋰, git
In-Reply-To: <87pq0cchsz.fsf@lifelogs.com>
Ted Zlatanov <tzz@lifelogs.com> writes:
>> Below is current git message when a local config credential.helper has
>> an empty value. Please skip an empty value.
>
>> $ git push --force origin master
>> git: 'credential-' is not a git command. See 'git --help'.
>> Did you mean this?
>> credential
Why isn't "do not add empty string, or any random string that ends
up referring to a helper that you do not have" a solution?
>> Total 0 (delta 0), reused 0 (delta 0)
>> To https://user@github.com/user/myrepo.git
>> + d23aa6a...3405990 master -> master (forced update)
>
> I would like that too (needed it today). Maybe the empty string (as
> suggested) or "none" could be acceptable.
Whatever you do, I do not think introducing a per-variable hack
[credential]
helper = none ;# or "helper = clear"
helper = mine ;# this is the only thing I use
like that is a sane way to go. "Clear everything you saw so far"
would be useful for variables other than "credential.helper";
shouldn't it be done by adding a general syntax to the configuration
file format and teach the configuration parser to clear the
cumulative definitions so far?
^ permalink raw reply
* Re: [PATCH v3 0/8] Hiding refs
From: Junio C Hamano @ 2013-02-07 18:25 UTC (permalink / raw)
To: Jeff King
Cc: Ævar Arnfjörð Bjarmason, Duy Nguyen,
Michael Haggerty, Jonathan Nieder, git, Shawn Pearce
In-Reply-To: <20130207001635.GA29318@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> If the new client can handle the old-style server's response, then the
> server can start blasting out refs (optionally after a timeout) and stop
> when the client interrupts with "hey, wait, I can speak the new
> protocol". The server just has to include "you can interrupt me" in its
> capability advertisement (obviously it would have to send out at least
> the first ref with the capabilities before the timeout).
Yeah, I would prefer people to come up with a way to share the port
and autodetect. It is *not* a requirement for the updated server to
run on a separate port at all.
^ permalink raw reply
* Re: [RFC] test-lib.sh: No POSIXPERM for cygwin
From: Ramsay Jones @ 2013-02-07 18:25 UTC (permalink / raw)
To: Torsten Bögershausen; +Cc: git, j6t
In-Reply-To: <201301271557.08994.tboegi@web.de>
Torsten Bögershausen wrote:
> t0070 and t1301 fail when running the test suite under cygwin.
> Skip the failing tests by unsetting POSIXPERM.
t1301 does not fail for me. (WIN XP (SP3) on NTFS)
[It's so long since I looked, but I'm pretty sure that the failure
in t0070 is caused by *git*, not by cygwin not supporting POSIXPERM]
> Signed-off-by: Torsten Bögershausen <tboegi@web.de>
> ---
> t/test-lib.sh | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index 1a6c4ab..94b097e 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -669,7 +669,6 @@ case $(uname -s) in
> test_set_prereq SED_STRIPS_CR
> ;;
> *CYGWIN*)
> - test_set_prereq POSIXPERM
> test_set_prereq EXECKEEPSPID
> test_set_prereq NOT_MINGW
> test_set_prereq SED_STRIPS_CR
>
So, I'm not in favour of this, FWIW.
ATB,
Ramsay Jones
^ permalink raw reply
* Re: overriding/removing inherited credential.helper, Do not add an empty value from config credential.helper
From: Ted Zlatanov @ 2013-02-07 18:38 UTC (permalink / raw)
To: Junio C Hamano; +Cc: 乙酸鋰, git
In-Reply-To: <7vvca47zl3.fsf@alter.siamese.dyndns.org>
On Thu, 07 Feb 2013 10:23:20 -0800 Junio C Hamano <gitster@pobox.com> wrote:
JCH> "Clear everything you saw so far" would be useful for variables
JCH> other than "credential.helper"; shouldn't it be done by adding a
JCH> general syntax to the configuration file format and teach the
JCH> configuration parser to clear the cumulative definitions so far?
That's a question for the configuration parser, right? I don't have an
informed opinion on how it should be implemented, and am OK with any
syntax to clear a variable.
Ted
^ permalink raw reply
* Re: [PATCHv2 5/5] git-send-email: use git credential to obtain password
From: Junio C Hamano @ 2013-02-07 18:42 UTC (permalink / raw)
To: Michal Nazarewicz; +Cc: Jeff King, Matthieu Moy, git
In-Reply-To: <0b3c9b66ccb6c8343dafd210b82c7765891d3785.1360242782.git.mina86@mina86.com>
Michal Nazarewicz <mpn@google.com> writes:
> From: Michal Nazarewicz <mina86@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 <mina86@mina86.com>
> ---
Nice ;-)
I'd expect reviews on 4/5 from Peff and Matthiew which may result in
either Reviewed-by:'s or another round, but everything else looks in
good order.
Thanks to all three of you for working on this.
> 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;
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox