{"thread":{"id":"31049","subject":"[PATCH 0/4] mediawiki/credential cleanups","startedAt":"2012-07-18T12:03:07Z","lastAt":"2012-07-18T13:06:42Z","messageCount":14,"participants":["Jeff King","Matthieu Moy"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"195272","messageId":"20120718120307.GA6399@sigill.intra.peff.net","threadId":"31049","inReplyTo":null,"subject":"[PATCH 0/4] mediawiki/credential cleanups","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-07-18T12:03:07Z","receivedAt":"2012-07-18T12:03:07Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Hi Matthieu,\n\nYour initial credential plumbing series went by while I was on vacation,\nand I finally got a chance to look at it in more detail. Overall it\nlooks very good; thank you (and your students) for working on it. It was\nespecially nice to have a test harness with complete instructions. :)\n\nI came up with a few commits on top. Patches 1 and 2 are cleanups that should\nhopefully be non-controversial. Patch 3 is the \"url=\" attribute we\ndiscussed earlier, and then patch 4 makes use of it in mw-to-git.\n\n [1/4]: docs/credential: minor clarity fixups\n [2/4]: mw-to-git: check blank credential attributes via length\n [3/4]: credential: convert \"url\" attribute into its parsed subparts\n [4/4]: mw-to-git: use git-credential's URL parser\n\n Documentation/git-credential.txt | 22 ++++++++++++++++------\n contrib/mw-to-git/git-remote-mediawiki | 34 +++++-----------------------------\n credential.c | 2 ++\n 3 files changed, 23 insertions(+), 35 deletions(-)\n\n-Peff\n"},{"id":"195273","messageId":"20120718120401.GA6726@sigill.intra.peff.net","threadId":"31049","inReplyTo":"20120718120307.GA6399@sigill.intra.peff.net","subject":"[PATCH 1/4] docs/credential: minor clarity fixups","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-07-18T12:04:02Z","receivedAt":"2012-07-18T12:04:02Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The text in git-credential(1) was copied from\ntechnical/api-credentials, so it still talks about the\ninput/output format as coming from git to the helper. Since\nthe surrounding text already indicates that this format is\nused for reading and writing with git credential, we can\njust remove the extraneous confusing bits.\n\nSigned-off-by: Jeff King \n---\n Documentation/git-credential.txt | 10 ++++------\n 1 file changed, 4 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/git-credential.txt b/Documentation/git-credential.txt\nindex a81684e..afd5365 100644\n--- a/Documentation/git-credential.txt\n+++ b/Documentation/git-credential.txt\n@@ -102,22 +102,20 @@ INPUT/OUTPUT FORMAT\n -------------------\n \n `git credential` reads and/or writes (depending on the action used)\n-credential information in its standard input/output. These information\n+credential information in its standard input/output. This information\n can correspond either to keys for which `git credential` will obtain\n the login/password information (e.g. host, protocol, path), or to the\n actual credential data to be obtained (login/password).\n \n-The credential is split into a set of named attributes.\n-Attributes are provided to the helper, one per line. Each attribute is\n+The credential is split into a set of named attributes, with one\n+attribute per line. Each attribute is\n specified by a key-value pair, separated by an `=` (equals) sign,\n followed by a newline. The key may contain any bytes except `=`,\n newline, or NUL. The value may contain any bytes except newline or NUL.\n In both cases, all bytes are treated as-is (i.e., there is no quoting,\n and one cannot transmit a value with newline or NUL in it). The list of\n attributes is terminated by a blank line or end-of-file.\n-Git will send the following attributes (but may not send all of\n-them for a given credential; for example, a `host` attribute makes no\n-sense when dealing with a non-network protocol):\n+Git understands the following attributes:\n \n `protocol`::\n \n-- \n1.7.10.5.40.g059818d\n"},{"id":"195274","messageId":"20120718120430.GB6726@sigill.intra.peff.net","threadId":"31049","inReplyTo":"20120718120307.GA6399@sigill.intra.peff.net","subject":"[PATCH 2/4] mw-to-git: check blank credential attributes via length","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-07-18T12:04:30Z","receivedAt":"2012-07-18T12:04:30Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"When writing a credential to git-credential, we omit fields\nthat do not have a true value. This will skip empty or\nundefined fields (which we want), but will also accidentally\nskip usernames or passwords which happen to have a non-true\nvalue (e.g., \"0\"). Be more careful by checking for non-zero\nlength.\n\nSigned-off-by: Jeff King \n---\n contrib/mw-to-git/git-remote-mediawiki | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/contrib/mw-to-git/git-remote-mediawiki b/contrib/mw-to-git/git-remote-mediawiki\nindex accd70a..b06f27b 100755\n--- a/contrib/mw-to-git/git-remote-mediawiki\n+++ b/contrib/mw-to-git/git-remote-mediawiki\n@@ -207,7 +207,7 @@ sub credential_write {\n \tmy $credential = shift;\n \tmy $writer = shift;\n \twhile (my ($key, $value) = each(%$credential) ) {\n-\t\tif ($value) {\n+\t\tif (length $value) {\n \t\t\tprint $writer \"$key=$value\\n\";\n \t\t}\n \t}\n-- \n1.7.10.5.40.g059818d\n"},{"id":"195275","messageId":"20120718120626.GC6726@sigill.intra.peff.net","threadId":"31049","inReplyTo":"20120718120307.GA6399@sigill.intra.peff.net","subject":"[PATCH 3/4] credential: convert \"url\" attribute into its parsed subparts","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-07-18T12:06:26Z","receivedAt":"2012-07-18T12:06:26Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The git-credential command requires that you feed it a\nbroken-down credential, which means that the client needs to\nparse a URL itself. Since we have our own URL-parsing\nroutines, we can easily allow the caller to just give us the\nURL as-is, saving them some code.\n\nSigned-off-by: Jeff King \n---\nThe implementation turned out to be delightfully simple. I stopped short\nof adding an \"ident\" command to git-credential where you could do\nsomething like:\n\n $ echo https://user@example.com | git credential ident\n protocol=https\n host=example.com\n username=user\n\nsince I had no use for it, but it would obviously be an easy one-liner\nto write (it's just \"fill\" without the actual fill call).\n\n Documentation/git-credential.txt | 12 ++++++++++++\n credential.c | 2 ++\n 2 files changed, 14 insertions(+)\n\ndiff --git a/Documentation/git-credential.txt b/Documentation/git-credential.txt\nindex afd5365..53adee3 100644\n--- a/Documentation/git-credential.txt\n+++ b/Documentation/git-credential.txt\n@@ -140,3 +140,15 @@ Git understands the following attributes:\n `password`::\n \n \tThe credential's password, if we are asking it to be stored.\n+\n+`url`::\n+\n+\tWhen this special attribute is read by `git credential`, the\n+\tvalue is parsed as a URL and treated as if its constituent parts\n+\twere read (e.g., `url=https://example.com` would behave as if\n+\t`protocol=https` and `host=example.com` had been provided). This\n+\tcan help callers avoid parsing URLs themselves. Note that any\n+\tcomponents which are missing from the URL (e.g., there is no\n+\tusername in the example above) will be set to empty; if you want\n+\tto provide a URL and override some attributes, provide the URL\n+\tattribute first, followed by any overrides.\ndiff --git a/credential.c b/credential.c\nindex 2c40007..e54753c 100644\n--- a/credential.c\n+++ b/credential.c\n@@ -172,6 +172,8 @@ int credential_read(struct credential *c, FILE *fp)\n \t\t} else if (!strcmp(key, \"path\")) {\n \t\t\tfree(c->path);\n \t\t\tc->path = xstrdup(value);\n+\t\t} else if (!strcmp(key, \"url\")) {\n+\t\t\tcredential_from_url(c, value);\n \t\t}\n \t\t/*\n \t\t * Ignore other lines; we don't know what they mean, but\n-- \n1.7.10.5.40.g059818d\n"},{"id":"195276","messageId":"20120718120656.GD6726@sigill.intra.peff.net","threadId":"31049","inReplyTo":"20120718120307.GA6399@sigill.intra.peff.net","subject":"[PATCH 4/4] mw-to-git: use git-credential's URL parser","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-07-18T12:06:56Z","receivedAt":"2012-07-18T12:06:56Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"We can just feed our URL straight to git-credential and it\nwill parse it for us, saving us some code.\n\nSigned-off-by: Jeff King \n---\n contrib/mw-to-git/git-remote-mediawiki | 32 ++++----------------------------\n 1 file changed, 4 insertions(+), 28 deletions(-)\n\ndiff --git a/contrib/mw-to-git/git-remote-mediawiki b/contrib/mw-to-git/git-remote-mediawiki\nindex b06f27b..c9ac416 100755\n--- a/contrib/mw-to-git/git-remote-mediawiki\n+++ b/contrib/mw-to-git/git-remote-mediawiki\n@@ -163,32 +163,6 @@ while () {\n \n ## credential API management (generic functions)\n \n-sub credential_from_url {\n-\tmy $url = shift;\n-\tmy $parsed = URI->new($url);\n-\tmy %credential;\n-\n-\tif ($parsed->scheme) {\n-\t\t$credential{protocol} = $parsed->scheme;\n-\t}\n-\tif ($parsed->host) {\n-\t\t$credential{host} = $parsed->host;\n-\t}\n-\tif ($parsed->path) {\n-\t\t$credential{path} = $parsed->path;\n-\t}\n-\tif ($parsed->userinfo) {\n-\t\tif ($parsed->userinfo =~ /([^:]*):(.*)/) {\n-\t\t\t$credential{username} = $1;\n-\t\t\t$credential{password} = $2;\n-\t\t} else {\n-\t\t\t$credential{username} = $parsed->userinfo;\n-\t\t}\n-\t}\n-\n-\treturn %credential;\n-}\n-\n sub credential_read {\n \tmy %credential;\n \tmy $reader = shift;\n@@ -216,7 +190,9 @@ sub credential_write {\n sub credential_run {\n \tmy $op = shift;\n \tmy $credential = shift;\n+\tmy $url = shift;\n \tmy $pid = open2(my $reader, my $writer, \"git credential $op\");\n+\tprint $writer \"url=$url\\n\" if defined $url;\n \tcredential_write($credential, $writer);\n \tprint $writer \"\\n\";\n \tclose($writer);\n@@ -246,10 +222,10 @@ sub mw_connect_maybe {\n \t$mediawiki = MediaWiki::API->new;\n \t$mediawiki->{config}->{api_url} = \"$url/api.php\";\n \tif ($wiki_login) {\n-\t\tmy %credential = credential_from_url($url);\n+\t\tmy %credential;\n \t\t$credential{username} = $wiki_login;\n \t\t$credential{password} = $wiki_passwd;\n-\t\tcredential_run(\"fill\", \\%credential);\n+\t\tcredential_run(\"fill\", \\%credential, $url);\n \t\tmy $request = {lgname => $credential{username},\n \t\t\t lgpassword => $credential{password},\n \t\t\t lgdomain => $wiki_domain};\n-- \n1.7.10.5.40.g059818d\n"},{"id":"195278","messageId":"vpqeho9jlzy.fsf@bauges.imag.fr","threadId":"31049","inReplyTo":"20120718120626.GC6726@sigill.intra.peff.net","subject":"Re: [PATCH 3/4] credential: convert \"url\" attribute into its parsed subparts","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2012-07-18T12:24:01Z","receivedAt":"2012-07-18T12:24:01Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Jeff King writes:\n\n> $ echo https://user@example.com | git credential ident\n> protocol=https\n> host=example.com\n> username=user\n>\n> since I had no use for it, but it would obviously be an easy one-liner\n> to write (it's just \"fill\" without the actual fill call).\n\nI was thinking the same, except I would have spelled it \"git credential\nparse\" (but ident is fine too). On the perl side, that would allow\ngetting a credential hash very simply (but it was already simple in\nperl, and made useless by your code).\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"195279","messageId":"vpqd33tjlzm.fsf@bauges.imag.fr","threadId":"31049","inReplyTo":"20120718120656.GD6726@sigill.intra.peff.net","subject":"Re: [PATCH 4/4] mw-to-git: use git-credential's URL parser","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2012-07-18T12:24:13Z","receivedAt":"2012-07-18T12:24:13Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Jeff King writes:\n\n> @@ -216,7 +190,9 @@ sub credential_write {\n> sub credential_run {\n> \tmy $op = shift;\n> \tmy $credential = shift;\n> +\tmy $url = shift;\n> \tmy $pid = open2(my $reader, my $writer, \"git credential $op\");\n> +\tprint $writer \"url=$url\\n\" if defined $url;\n> \tcredential_write($credential, $writer);\n> \tprint $writer \"\\n\";\n> \tclose($writer);\n> @@ -246,10 +222,10 @@ sub mw_connect_maybe {\n> \t$mediawiki = MediaWiki::API->new;\n> \t$mediawiki->{config}->{api_url} = \"$url/api.php\";\n> \tif ($wiki_login) {\n> -\t\tmy %credential = credential_from_url($url);\n> +\t\tmy %credential;\n> \t\t$credential{username} = $wiki_login;\n> \t\t$credential{password} = $wiki_passwd;\n> -\t\tcredential_run(\"fill\", \\%credential);\n> +\t\tcredential_run(\"fill\", \\%credential, $url);\n> \t\tmy $request = {lgname => $credential{username},\n> \t\t\t lgpassword => $credential{password},\n> \t\t\t lgdomain => $wiki_domain};\n\nI would find it more elegant not to special-case the URL field, and just\n\n my %credential = ('url' => $url);\n\nbut I'm fine with your version too.\n\nOther than that, and for the 4 patches of the serie:\n\nReviewed-by: Matthieu Moy \n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"195280","messageId":"20120718122555.GA11482@sigill.intra.peff.net","threadId":"31049","inReplyTo":"vpqeho9jlzy.fsf@bauges.imag.fr","subject":"Re: [PATCH 3/4] credential: convert \"url\" attribute into its parsed subparts","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-07-18T12:25:56Z","receivedAt":"2012-07-18T12:25:56Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jul 18, 2012 at 02:24:01PM +0200, Matthieu Moy wrote:\n\n> Jeff King writes:\n> \n> > $ echo https://user@example.com | git credential ident\n> > protocol=https\n> > host=example.com\n> > username=user\n> >\n> > since I had no use for it, but it would obviously be an easy one-liner\n> > to write (it's just \"fill\" without the actual fill call).\n> \n> I was thinking the same, except I would have spelled it \"git credential\n> parse\" (but ident is fine too). On the perl side, that would allow\n> getting a credential hash very simply (but it was already simple in\n> perl, and made useless by your code).\n\nI wanted to give it some name that meant \"pass-through\" rather than just\nparse, in case we add more magic attributes later. I meant \"ident\" to be\nlike \"mathematical identity\", but in the context of a credential tool,\nit is probably somewhat ambiguous. :)\n\nAnyway, the fact that we can just do the parsing as part of the \"fill\"\nmeans we don't need it for now, so I'll leave it until somebody really\ncares.\n\n-Peff\n"},{"id":"195281","messageId":"20120718122848.GB11482@sigill.intra.peff.net","threadId":"31049","inReplyTo":"vpqd33tjlzm.fsf@bauges.imag.fr","subject":"Re: [PATCH 4/4] mw-to-git: use git-credential's URL parser","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-07-18T12:28:48Z","receivedAt":"2012-07-18T12:28:48Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jul 18, 2012 at 02:24:13PM +0200, Matthieu Moy wrote:\n\n> Jeff King writes:\n> \n> > @@ -216,7 +190,9 @@ sub credential_write {\n> > sub credential_run {\n> > \tmy $op = shift;\n> > \tmy $credential = shift;\n> > +\tmy $url = shift;\n> > \tmy $pid = open2(my $reader, my $writer, \"git credential $op\");\n> > +\tprint $writer \"url=$url\\n\" if defined $url;\n> > \tcredential_write($credential, $writer);\n> > \tprint $writer \"\\n\";\n> > \tclose($writer);\n> > @@ -246,10 +222,10 @@ sub mw_connect_maybe {\n> > \t$mediawiki = MediaWiki::API->new;\n> > \t$mediawiki->{config}->{api_url} = \"$url/api.php\";\n> > \tif ($wiki_login) {\n> > -\t\tmy %credential = credential_from_url($url);\n> > +\t\tmy %credential;\n> > \t\t$credential{username} = $wiki_login;\n> > \t\t$credential{password} = $wiki_passwd;\n> > -\t\tcredential_run(\"fill\", \\%credential);\n> > +\t\tcredential_run(\"fill\", \\%credential, $url);\n> > \t\tmy $request = {lgname => $credential{username},\n> > \t\t\t lgpassword => $credential{password},\n> > \t\t\t lgdomain => $wiki_domain};\n> \n> I would find it more elegant not to special-case the URL field, and just\n> \n> my %credential = ('url' => $url);\n> \n> but I'm fine with your version too.\n\nI started with a version that did that, but there are two complications:\n\n 1. credential_write needs to know that the 'url' field must come\n first, as it overwrites the other fields. So we end up\n special-casing it either way.\n\n 2. Git hands us back the broken-down version, which we add to the\n credential. So it is superfluous to keep sending 'url' for the\n other, non-fill interactions. I don't think it's technically wrong,\n but it also seemed inelegant. Having \"credential_write\" delete the\n 'url' field seemed even more inelegant.\n\n> Other than that, and for the 4 patches of the serie:\n> \n> Reviewed-by: Matthieu Moy \n\nThanks.\n\n-Peff\n"},{"id":"195282","messageId":"vpq1uk9jldk.fsf@bauges.imag.fr","threadId":"31049","inReplyTo":"20120718122848.GB11482@sigill.intra.peff.net","subject":"Re: [PATCH 4/4] mw-to-git: use git-credential's URL parser","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2012-07-18T12:37:27Z","receivedAt":"2012-07-18T12:37:27Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Jeff King writes:\n\n> I started with a version that did that, but there are two complications:\n>\n> 1. credential_write needs to know that the 'url' field must come\n> first, as it overwrites the other fields. So we end up\n> special-casing it either way.\n\nRight, I didn't think of that. But you'd have to special-case it only\nwithin credential_run, and not for the caller.\n\n> 2. Git hands us back the broken-down version, which we add to the\n> credential.\n\nWe don't add it, but already replace the whole structure. This is\nsomehow needed because \"git credential fill\" can remove fields from the\nstructure (the path attribute is removed with\ncredential.useHttpPath=false). So, this point doesn't seem problematic.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"195285","messageId":"20120718125741.GA11605@sigill.intra.peff.net","threadId":"31049","inReplyTo":"vpq1uk9jldk.fsf@bauges.imag.fr","subject":"Re: [PATCH 4/4] mw-to-git: use git-credential's URL parser","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-07-18T12:57:41Z","receivedAt":"2012-07-18T12:57:41Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jul 18, 2012 at 02:37:27PM +0200, Matthieu Moy wrote:\n\n> Jeff King writes:\n> \n> > I started with a version that did that, but there are two complications:\n> >\n> > 1. credential_write needs to know that the 'url' field must come\n> > first, as it overwrites the other fields. So we end up\n> > special-casing it either way.\n> \n> Right, I didn't think of that. But you'd have to special-case it only\n> within credential_run, and not for the caller.\n\nYeah. It would look like this:\n\ndiff --git a/contrib/mw-to-git/git-remote-mediawiki b/contrib/mw-to-git/git-remote-mediawiki\nindex c9ac416..e1392b0 100755\n--- a/contrib/mw-to-git/git-remote-mediawiki\n+++ b/contrib/mw-to-git/git-remote-mediawiki\n@@ -190,9 +190,11 @@ sub credential_write {\n sub credential_run {\n \tmy $op = shift;\n \tmy $credential = shift;\n-\tmy $url = shift;\n \tmy $pid = open2(my $reader, my $writer, \"git credential $op\");\n-\tprint $writer \"url=$url\\n\" if defined $url;\n+\tif (exists $credential->{url}) {\n+\t\tprint $writer \"url=$credential->{url}\\n\";\n+\t\tdelete $credential->{url};\n+\t}\n \tcredential_write($credential, $writer);\n \tprint $writer \"\\n\";\n \tclose($writer);\n\nwhich is still kind of ugly. We could also push it down into\ncredential_write, like:\n\ndiff --git a/contrib/mw-to-git/git-remote-mediawiki b/contrib/mw-to-git/git-remote-mediawiki\nindex c9ac416..0a821fd 100755\n--- a/contrib/mw-to-git/git-remote-mediawiki\n+++ b/contrib/mw-to-git/git-remote-mediawiki\n@@ -180,8 +180,9 @@ sub credential_read {\n sub credential_write {\n \tmy $credential = shift;\n \tmy $writer = shift;\n+\tprint $writer \"url=$credential->{url}\\n\" if exists $credential->{url};\n \twhile (my ($key, $value) = each(%$credential) ) {\n-\t\tif (length $value) {\n+\t\tif (length $value && $key ne 'url') {\n \t\t\tprint $writer \"$key=$value\\n\";\n \t\t}\n \t}\n@@ -190,9 +191,7 @@ sub credential_write {\n sub credential_run {\n \tmy $op = shift;\n \tmy $credential = shift;\n-\tmy $url = shift;\n \tmy $pid = open2(my $reader, my $writer, \"git credential $op\");\n-\tprint $writer \"url=$url\\n\" if defined $url;\n \tcredential_write($credential, $writer);\n \tprint $writer \"\\n\";\n \tclose($writer);\n\nwhich is probably the least gross. I was originally hesitant because the\nissue (2) I brought up, but...\n\n> > 2. Git hands us back the broken-down version, which we add to the\n> > credential.\n> \n> We don't add it, but already replace the whole structure. This is\n> somehow needed because \"git credential fill\" can remove fields from the\n> structure (the path attribute is removed with\n> credential.useHttpPath=false). So, this point doesn't seem problematic.\n\nHmph. I considered that we might do it and even checked, but I somehow\nread the code wrong (I think I was thrown off by the pass-by-reference\nto credential_run, but of course it overwrites it inside that function).\n\nSo since that is a non-issue, I think the second diff I provided above\nis a bit nicer.\n\n-Peff\n"},{"id":"195286","messageId":"20120718130308.GA11769@sigill.intra.peff.net","threadId":"31049","inReplyTo":"20120718125741.GA11605@sigill.intra.peff.net","subject":"[PATCHv2 4/4] mw-to-git: use git-credential's URL parser","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-07-18T13:03:08Z","receivedAt":"2012-07-18T13:03:08Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jul 18, 2012 at 08:57:41AM -0400, Jeff King wrote:\n\n> So since that is a non-issue, I think the second diff I provided above\n> is a bit nicer.\n\nAnd here is a replacement patch 4/4.\n\n-- >8 --\nSubject: mw-to-git: use git-credential's URL parser\n\nWe can just feed our URL straight to git-credential and it\nwill parse it for us, saving us some code.\n\nSigned-off-by: Jeff King \n---\n contrib/mw-to-git/git-remote-mediawiki | 32 ++++----------------------------\n 1 file changed, 4 insertions(+), 28 deletions(-)\n\ndiff --git a/contrib/mw-to-git/git-remote-mediawiki b/contrib/mw-to-git/git-remote-mediawiki\nindex b06f27b..38afa76 100755\n--- a/contrib/mw-to-git/git-remote-mediawiki\n+++ b/contrib/mw-to-git/git-remote-mediawiki\n@@ -163,32 +163,6 @@ while () {\n \n ## credential API management (generic functions)\n \n-sub credential_from_url {\n-\tmy $url = shift;\n-\tmy $parsed = URI->new($url);\n-\tmy %credential;\n-\n-\tif ($parsed->scheme) {\n-\t\t$credential{protocol} = $parsed->scheme;\n-\t}\n-\tif ($parsed->host) {\n-\t\t$credential{host} = $parsed->host;\n-\t}\n-\tif ($parsed->path) {\n-\t\t$credential{path} = $parsed->path;\n-\t}\n-\tif ($parsed->userinfo) {\n-\t\tif ($parsed->userinfo =~ /([^:]*):(.*)/) {\n-\t\t\t$credential{username} = $1;\n-\t\t\t$credential{password} = $2;\n-\t\t} else {\n-\t\t\t$credential{username} = $parsed->userinfo;\n-\t\t}\n-\t}\n-\n-\treturn %credential;\n-}\n-\n sub credential_read {\n \tmy %credential;\n \tmy $reader = shift;\n@@ -206,8 +180,10 @@ sub credential_read {\n sub credential_write {\n \tmy $credential = shift;\n \tmy $writer = shift;\n+\t# url overwrites other fields, so it must come first\n+\tprint $writer \"url=$credential->{url}\\n\" if exists $credential->{url};\n \twhile (my ($key, $value) = each(%$credential) ) {\n-\t\tif (length $value) {\n+\t\tif (length $value && $key ne 'url') {\n \t\t\tprint $writer \"$key=$value\\n\";\n \t\t}\n \t}\n@@ -246,7 +222,7 @@ sub mw_connect_maybe {\n \t$mediawiki = MediaWiki::API->new;\n \t$mediawiki->{config}->{api_url} = \"$url/api.php\";\n \tif ($wiki_login) {\n-\t\tmy %credential = credential_from_url($url);\n+\t\tmy %credential = (url => $url);\n \t\t$credential{username} = $wiki_login;\n \t\t$credential{password} = $wiki_passwd;\n \t\tcredential_run(\"fill\", \\%credential);\n-- \n1.7.10.5.40.g059818d\n"},{"id":"195287","messageId":"vpqtxx5i5ie.fsf@bauges.imag.fr","threadId":"31049","inReplyTo":"20120718125741.GA11605@sigill.intra.peff.net","subject":"Re: [PATCH 4/4] mw-to-git: use git-credential's URL parser","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2012-07-18T13:05:29Z","receivedAt":"2012-07-18T13:05:29Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Jeff King writes:\n\n> So since that is a non-issue, I think the second diff I provided above\n> is a bit nicer.\n\nI like this one best too. It just lacks a comment saying why it should\nbe displayed first ;-).\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"195288","messageId":"vpqmx2xi5gd.fsf@bauges.imag.fr","threadId":"31049","inReplyTo":"20120718130308.GA11769@sigill.intra.peff.net","subject":"Re: [PATCHv2 4/4] mw-to-git: use git-credential's URL parser","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2012-07-18T13:06:42Z","receivedAt":"2012-07-18T13:06:42Z","isPatch":false,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Jeff King writes:\n\n> On Wed, Jul 18, 2012 at 08:57:41AM -0400, Jeff King wrote:\n>\n>> So since that is a non-issue, I think the second diff I provided above\n>> is a bit nicer.\n>\n> And here is a replacement patch 4/4.\n>\n> -- >8 --\n> Subject: mw-to-git: use git-credential's URL parser\n\nPerfect, thanks.\n\n(and you've been faster than me about the comment ;-) )\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"}]}