On Sun, 10 Jun 2012 15:37:42 +0200, Matthieu Moy wrote:
Jeff King <peff@xxxxxxxx> writes:
On Sat, Jun 09, 2012 at 08:53:48PM +0200,
Javier.Roucher-Iglesias@xxxxxxxxxxxxxxx wrote:
diff --git a/contrib/mw-to-git/git-remote-mediawiki
b/contrib/mw-to-git/git-remote-mediawiki
index c18bfa1..4b14d78 100755
--- a/contrib/mw-to-git/git-remote-mediawiki
+++ b/contrib/mw-to-git/git-remote-mediawiki
@@ -152,28 +152,111 @@ while (<STDIN>) {
########################## Functions
##############################
# MediaWiki API instance, created lazily.
+sub run_credential {
Is there any reason not to add this to perl/Git.pm? I suspect that
other
scripts will want to use it, too (for example, send-email could
probably
use it for SMTP credentials).
Currently, git-remote-mediawiki is a standalone script (doesn't use
Git.pm). This is good because it makes it trivial to install, but bad
in
the sense that it may force us (or others) to reinvent the wheel.
Until now, the wheels we reinvented were very simple (run_git
essentially), but we may be reaching the point where it makes sense
to
use and contribute to Git.pm.
Unfortunately, from a non-technical point of view, Javier is
contributing this as part of a student project, which ends this week,
and it's probably not reasonable to introduce such change so late.
So,
I'd keep it here at least for now, and a move to Git.pm could be a
separate future topic.
Thank's for explain my situation.
+ if (scalar(@_) == 2) {
+ if ($_[1] eq ("store" || "cache")) {
+ run_git("config credential.helper \'$_[1]\'");
+ } else {
+ print STDERR "ERROR: run_credential (fill|approve|reject)
[store|cache]\n";
+ exit 1;
+ }
+ }
This hunk looks wrong. You should never be setting the
credential.helper
config; that is the responsibility of the user to set, as they want
to
select whatever helper is appropriate. Nor do you need to care about
which helpers are in use; the point of git-credential is that it
will do
that for you.
Absolutely.
I have add this with no advance warning, but i will remove it in the
next patch.
sub fill_credential {
my $quoted_url = quotemeta(shift);
my $verbatim = `git credential fill $quoted_url`;
$? and die "git-credential failed";
$verbatim =~ /^username=(.*)$/m
or die "git-credential did not give us a
username";
my $username = $1;
$verbatim =~ /^password=(.*)$/m
or die "git-credential did not give us a
password";
return ($username, $password, $verbatim);
}
sub report_credential {
my ($type, $verbatim) = @_;
open(my $fh, '|-', "git credential $type");
print $fh $verbatim;
}
That sounds sensible too. We should be careful not to give a password
as
argument (or users of the same machine will be able to find it with
e.g.
"ps u"), but your proposal is OK with that.
+ # error if key undef
+ if (not defined $key) {
+ print STDERR "ERROR reciving reponse git credential fill\n";
+ exit 1;
+ }
[...]
to be change, thanks for the corrections
+ } else {
+ while (<Reader>) {
+ print STDERR "\nERROR while running git credential $op:\n$_";
+ }
+ }
+}
This isn't a good way to check for errors. The non-fill actions will
never produce output on stdout, and you are not intercepting their
stderr. Besides which, checking for errors by reading stderr is not
a
good practice; you should check the return value of the command in
$?
after it finishes.
I think it should do both. In case "git credential fill" returns
something that doesn't match the regexp, we don't want perl to error
with "use of undefined value", but that's just being defensive
because
it shouldn't happen.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html