{"thread":{"id":"27555","subject":"[PATCH] Add a remote helper to interact with mediawiki, pull & clone handled","startedAt":"2011-06-06T10:20:35Z","lastAt":"2011-06-06T22:11:12Z","messageCount":5,"participants":["Jeremie Nikaes","Junio C Hamano","Thomas Adam","Jérémie NIKAES"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"169357","messageId":"1307355635-5580-1-git-send-email-jeremie.nikaes@ensimag.imag.fr","threadId":"27555","inReplyTo":null,"subject":"[PATCH] Add a remote helper to interact with mediawiki, pull & clone handled","fromName":"Jeremie Nikaes","fromEmail":"jeremie.nikaes@ensimag.imag.fr","sentAt":"2011-06-06T10:20:35Z","receivedAt":"2011-06-06T10:20:35Z","isPatch":true,"sender":{"key":"jeremie.nikaes@ensimag.imag.fr","avatar":null},"body":"Implement a gate between git and mediawiki, allowing git users to\npush and pull objects from mediawiki just as one would do with a\nclassic git repository thanks to remote-helpers.\n\nCurrently supported commands are :\n      git clone mediawiki::http://onewiki.com\n      git pull\n\nYou need the following packages installed (available on common\nrepositories):\n      libmediawiki-api-perl\n      libdatetime-format-iso8601-perl\n\nUse remote helpers in order to be as transparent as possible\nto the git user.\n\nDownload Mediawiki revisions through the Mediawiki API and then\nfast-import into git.\n\nMediawiki revisions and git commits are linked thanks to notes bound to\ncommits.\n\nThe import part is done on a refs/mediawiki/<remote> branch before\ncoming to refs/remote/origin/master (Huge thanks to Jonathan Nieder\nfor his help)\n\nFor now, the whole wiki is cloned, but it will be possible to clone only\nsome pages: the clone is based on a list of pages which is now all\npages.\n\nCode clarified & improved with the help of Jeff King and Junio C Hamano.\n\nWe were not able to reproduce the empty timestamp bug noticed by Jeff\nKing, thus needing some further testing. A placeholder is still\nimplemented in case.\n\nSigned-off-by: Jérémie Nikaes <jeremie.nikaes@ensimag.imag.fr>\nSigned-off-by: Arnaud Lacurie <arnaud.lacurie@ensimag.imag.fr>\nSigned-off-by: Claire Fousse <claire.fousse@ensimag.imag.fr>\nSigned-off-by: David Amouyal <david.amouyal@ensimag.imag.fr>\nSigned-off-by: Matthieu Moy <matthieu.moy@grenoble-inp.fr>\nSigned-off-by: Sylvain Boulmé <sylvain.boulme@imag.fr>\n---\n contrib/mw-to-git/git-remote-mediawiki     |  306 ++++++++++++++++++++++++++++\n contrib/mw-to-git/git-remote-mediawiki.txt |    7 +\n 2 files changed, 313 insertions(+), 0 deletions(-)\n create mode 100755 contrib/mw-to-git/git-remote-mediawiki\n create mode 100644 contrib/mw-to-git/git-remote-mediawiki.txt\n\ndiff --git a/contrib/mw-to-git/git-remote-mediawiki b/contrib/mw-to-git/git-remote-mediawiki\nnew file mode 100755\nindex 0000000..9db64c2\n--- /dev/null\n+++ b/contrib/mw-to-git/git-remote-mediawiki\n@@ -0,0 +1,306 @@\n+#! /usr/bin/perl\n+\n+use strict;\n+use Switch;\n+use MediaWiki::API;\n+use Storable qw(freeze thaw);\n+use DateTime::Format::ISO8601;\n+use Encode qw(encode_utf8);\n+\n+# Mediawiki filenames can contain forward slashes. This variable decides by which pattern they should be replaced\n+my $slash_replacement = \"<slash>\";\n+\n+my $remotename = $ARGV[0];\n+my $url = $ARGV[1];\n+\n+print STDERR \"$url\\n\";\n+\n+# commands parser\n+my $loop = 1;\n+my $entry;\n+my @cmd;\n+while ($loop) {\n+\t$| = 1; #flush STDOUT\n+\t$entry = <STDIN>;\n+\tprint STDERR $entry;\n+\tchomp($entry);\n+\t@cmd = undef;\n+\t@cmd = split(/ /,$entry);\n+\tswitch ($cmd[0]) {\n+\t\tcase \"capabilities\" {\n+\t\t\tif ($cmd[1] eq \"\") {\n+\t\t\t\tmw_capabilities();\n+\t\t\t} else {\n+\t\t\t       $loop = 0;\n+\t\t\t}\n+\t\t}\n+\t\tcase \"list\" {\n+\t\t\tif ($cmd[2] eq \"\") {\n+\t\t\t\tmw_list($cmd[1]);\n+\t\t\t} else {\n+\t\t\t       $loop = 0;\n+\t\t\t}\n+\t\t}\n+\t\tcase \"import\" {\n+\t\t\tif ($cmd[1] ne \"\" && $cmd[2] eq \"\") {\n+\t\t\t\tmw_import($cmd[1]);\n+\t\t\t} \n+\t\t}\n+\t\tcase \"option\" {\n+\t\t\tif ($cmd[1] ne \"\" && $cmd[2] ne \"\" && $cmd[3] eq \"\") {\n+\t\t\t\tmw_option($cmd[1],$cmd[2]);\n+\t\t\t} \n+\t\t}\n+\t\tcase \"push\" {\n+\t\t\t#check the pattern <src>:<dst>\n+\t\t\tmy @pushargs = split(/:/,$cmd[1]);\n+\t\t\tif ($pushargs[1] ne \"\" && $pushargs[2] eq \"\") {\n+\t\t\t\tmw_push($pushargs[0],$pushargs[1]);\n+\t\t\t} else {\n+\t\t\t       $loop = 0;\n+\t\t\t}\n+\t\t} else {\n+\t\t\t$loop = 0;\n+\t\t}\n+\t}\n+\tclose(FILE);\n+}\n+\n+########################## Functions ##############################\n+\n+sub get_last_local_revision {\n+\t# Get last commit sha1\n+\tmy $commit_sha1 = `git rev-parse refs/mediawiki/$remotename/master 2>/dev/null`;\n+\n+\t# Get note regarding that commit\n+\tchomp($commit_sha1);\n+\tmy $note = `git log --notes=mediawiki 2>/dev/null | grep \"mediawiki_revision: \" | sed -n 1p`;\n+\t$note =~ s/^\\s+//; #left trim of note \n+\tmy @note_info = split(/ /, $note);\n+\n+\tmy $lastrevision_number;\n+\tif (!($note_info[0] eq \"mediawiki_revision:\")) {\n+\t\tprint STDERR \"No previous mediawiki revision found\";\n+\t\t$lastrevision_number = 0;\n+\t} else {\n+\t\t# Notes are formatted : mediawiki_revision: #number\n+\t\t$lastrevision_number = $note_info[1];\n+\t\tchomp($lastrevision_number);\n+\t\tprint STDERR \"Last local mediawiki revision found is $lastrevision_number \";\n+\t}\n+\treturn $lastrevision_number;\n+}\n+\n+sub get_last_remote_revision {\n+\tmy $mediawiki = MediaWiki::API->new;\n+\t$mediawiki->{config}->{api_url} = \"$url/api.php\";\n+\n+\tmy $pages = $mediawiki->list({\n+\t\taction => 'query',\n+\t\tlist => 'allpages',\n+\t\taplimit => 500,\n+\t});\n+\n+\tmy $max_rev_num = 0;\n+\n+\tforeach my $page (@$pages) {\n+\t\tmy $id = $page->{pageid};\n+\n+\n+\t\tmy $query = {\n+\t\t\taction => 'query',\n+\t\t\tprop => 'revisions',\n+\t\t\trvprop => 'ids',\n+\t\t\tpageids => $id,\n+\t\t};\n+\n+\t\tmy $result = $mediawiki->api($query);\n+\t\t\n+\t\tmy $lastrev = pop(@{$result->{query}->{pages}->{$id}->{revisions}});\n+\t\t\n+\t\t$max_rev_num = ($lastrev->{revid} > $max_rev_num ? $lastrev->{revid} : $max_rev_num);\n+\t}\n+\n+\tprint STDERR \"Last remote revision found is $max_rev_num\\n\";\n+\treturn $max_rev_num;\n+}\n+\n+sub literal_data {\n+\tmy ($content) = @_;\n+\tprint \"data \", bytes::length($content), \"\\n\", $content;\n+}\n+\n+sub mw_capabilities {\n+\t# Revisions are imported to the private namespace\n+\t# refs/mediawiki/$remotename/ by the helper and fetched into\n+\t# refs/remotes/$remotename later by fetch.\n+\tprint STDOUT \"refspec refs/heads/*:refs/mediawiki/$remotename/*\\n\";\n+\tprint STDOUT \"import\\n\";\n+\tprint STDOUT \"list\\n\";\n+\tprint STDOUT \"option\\n\";\n+\tprint STDOUT \"push\\n\";\n+\tprint STDOUT \"\\n\";\n+}\n+\n+sub mw_list {\n+\t# MediaWiki do not have branches, we consider one branch arbitrarily\n+\t# called master\n+\tprint STDOUT \"? refs/heads/master\\n\";\n+\tprint STDOUT '@'.\"refs/heads/master HEAD\\n\";\n+\tprint STDOUT \"\\n\";\n+\n+}\n+\n+sub mw_option {\n+\tprint STDERR \"not yet implemented \\n\";\n+\tprint STDOUT \"unsupported\\n\";\n+}\n+\n+sub mw_import {\n+\tmy @wiki_name = split(/:\\/\\//,$url);\n+\tmy $wiki_name = $wiki_name[1];\n+\n+\tmy $mediawiki = MediaWiki::API->new;\n+\t$mediawiki->{config}->{api_url} = \"$url/api.php\";\n+\n+\tmy $pages = $mediawiki->list({\n+\t\taction => 'query',\n+\t\tlist => 'allpages',\n+\t\taplimit => 500,\n+\t});\n+\tif ($pages == undef) {\n+\t\tprint STDERR \"fatal: '$url' does not appear to be a mediawiki\\n\";\n+\t\texit;\n+\t}\n+\n+\tmy @revisions;\n+\tprint STDERR \"Searching revisions...\\n\";\n+\tmy $fetch_from = get_last_local_revision() + 1;\n+\tif ($fetch_from == 1) {\n+\t\tprint STDERR \", fetching from beginning\\n\";\n+\t} else {\n+\t\tprint STDERR \", fetching from here\\n\";\n+\t}\n+\tmy $n = 1;\n+\tmy $last_timestamp = 0;\n+\tforeach my $page (@$pages) {\n+\t\tmy $id = $page->{pageid};\n+\n+\t\tprint STDERR \"$n/\", scalar(@$pages), \": \". encode_utf8($page->{title}).\"\\n\";\n+\t\t$n++;\n+\n+\t\tmy $query = {\n+\t\t\taction => 'query',\n+\t\t\tprop => 'revisions',\n+\t\t\trvprop => 'ids',\n+\t\t\trvdir => 'newer',\n+\t\t\trvstartid => $fetch_from,\n+\t\t\trvlimit => 500,\n+\t\t\tpageids => $page->{pageid},\n+\t\t};\n+\n+\t\tmy $revnum = 0;\n+\t\t# Get 500 revisions at a time due to the mediawiki api limit\n+\t\twhile (1) {\n+\t\t\tmy $result = $mediawiki->api($query);\n+\n+\t\t\t# Parse each of those 500 revisions\n+\t\t\tforeach my $revision (@{$result->{query}->{pages}->{$id}->{revisions}}) {\n+\t\t\t\tmy $page_rev_ids;\n+\t\t\t\t$page_rev_ids->{pageid} = $page->{pageid};\n+\t\t\t\t$page_rev_ids->{revid} = $revision->{revid};\n+\t\t\t\tpush (@revisions, $page_rev_ids);\n+\t\t\t\t$revnum++;\n+\t\t\t}\n+\t\t\tlast unless $result->{'query-continue'};\n+\t\t\t$query->{rvstartid} = $result->{'query-continue'}->{revisions}->{rvstartid};\n+\t\t}\n+\t\tprint STDERR \"  Found \", $revnum, \" revision(s).\\n\";\n+\t}\n+\n+\t# Creation of the fast-import stream\n+\tprint STDERR \"Fetching & writing export data...\\n\";\n+\tbinmode STDOUT, ':binary';\n+\t$n = 0;\n+\n+\tforeach my $pagerevids (sort {$a->{revid} <=> $b->{revid}} @revisions) {\n+\t\t#fetch the content of the pages\n+\t\tmy $query = {\n+\t\t\taction => 'query',\n+\t\t\tprop => 'revisions',\n+\t\t\trvprop => 'content|timestamp|comment|user|ids',\n+\t\t\trevids => $pagerevids->{revid},\n+\t\t};\n+\n+\t\tmy $result = $mediawiki->api($query);\n+\n+\t\tmy $rev = pop(@{$result->{query}->{pages}->{$pagerevids->{pageid}}->{revisions}});\n+\n+\t\t$n++;\n+\t\tmy $user = $rev->{user} || 'Anonymous';\n+\t\tmy $dt;\n+\n+\t\t# This has to be verified, haven't been able to reproduce the scenario\n+\t\tif ($rev->{timestamp} != undef) {\n+\t\t\t$dt = DateTime::Format::ISO8601->parse_datetime($rev->{timestamp});\n+\t\t} else {\n+\t\t\t$dt = $last_timestamp + 1;\n+\t\t}\n+\t\t$last_timestamp = $dt;\n+\n+\n+\t\tmy $comment = defined $rev->{comment} ? $rev->{comment} : '*Empty MediaWiki Message*';\n+\t\tmy $title = encode_utf8($result->{query}->{pages}->{$pagerevids->{pageid}}->{title});\n+\t\tmy $content = $rev->{'*'};\n+\t\t\n+\t\t$title =~ y/ /_/;\n+\t\t$title =~ s/\\//$slash_replacement/g;\n+\n+\t\tprint STDERR \"$n/\", scalar(@revisions), \": Revision n°$pagerevids->{revid} of $title\\n\";\n+\n+\t\tprint \"commit refs/mediawiki/$remotename/master\\n\";\n+\t\tprint \"mark :$n\\n\";\n+\t\tprint \"committer $user <$user\\@$wiki_name> \", $dt->epoch, \" +0000\\n\";\n+\t\tliteral_data(encode_utf8($comment));\n+\t\t# If it's not a clone, needs to know where to start from\n+\t\tif ($fetch_from != 1 && $n == 1) {\n+\t\t\tprint \"from refs/mediawiki/$remotename/master^0\\n\";\n+\t\t}\n+\t\tprint \"M 644 inline $title.mw\\n\";\n+\t\tliteral_data(encode_utf8($content));\n+\t\tprint \"\\n\\n\";\n+\n+\n+\t\t# mediawiki revision number in the git note\n+\t\tif ($fetch_from == 1 && $n == 1) {\n+\t\t\tprint \"reset refs/notes/mediawiki\\n\";\n+\t\t}\n+\t\tprint \"commit refs/notes/mediawiki\\n\";\n+\t\tprint \"committer $user <$user\\@$wiki_name> \", $dt->epoch, \" +0000\\n\";\n+\t\tliteral_data(encode_utf8(\"note added by git-mediawiki\"));\n+\t\tif ($fetch_from != 1 && $n == 1) {\n+\t\t\tprint \"from refs/notes/mediawiki^0\\n\";\n+\t\t}\n+\t\tprint \"N inline :$n\\n\";\n+\t\tliteral_data(encode_utf8(\"mediawiki_revision: \" . $pagerevids->{revid}));\n+\t\tprint \"\\n\\n\";\n+\t}\n+\n+\tif ($fetch_from == 1) {\n+\t\tif ($n != 0) {\n+\t\t\tprint \"reset $_[0]\\n\"; #$_[0] contains refs/heads/master\n+\t\t\tprint \"from :$n\\n\";\n+\t\t} else {\n+\t\t\tprint STDERR \"You appear to have cloned an empty mediawiki\\n\";\n+\t\t\t#What do we have to do here ? If nothing is done, an error is thrown saying that\n+\t\t\t#HEAD is refering to unknown object 0000000000000000000\n+\t\t}\n+\t}\n+\n+}\n+\n+sub mw_push {\n+\n+\tprint STDERR \"Not yet implemented\";\n+\tprint \"\\n\";\n+}\ndiff --git a/contrib/mw-to-git/git-remote-mediawiki.txt b/contrib/mw-to-git/git-remote-mediawiki.txt\nnew file mode 100644\nindex 0000000..4d211f5\n--- /dev/null\n+++ b/contrib/mw-to-git/git-remote-mediawiki.txt\n@@ -0,0 +1,7 @@\n+Git-Mediawiki is a project which aims the creation of a gate\n+between git and mediawiki, allowing git users to push and pull\n+objects from mediawiki just as one would do with a classic git\n+repository thanks to remote-helpers.\n+\n+For more information, visit the wiki at\n+https://github.com/Bibzball/Git-Mediawiki/wiki\n-- \n1.7.4.1\n"},{"id":"169392","messageId":"7vmxhu92u5.fsf@alter.siamese.dyndns.org","threadId":"27555","inReplyTo":"1307355635-5580-1-git-send-email-jeremie.nikaes@ensimag.imag.fr","subject":"Re: [PATCH] Add a remote helper to interact with mediawiki, pull & clone handled","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-06-06T19:28:34Z","receivedAt":"2011-06-06T19:28:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeremie Nikaes <jeremie.nikaes@ensimag.imag.fr> writes:\n\n> Implement a gate between git and mediawiki, allowing git users to\n> ...\n> For now, the whole wiki is cloned, but it will be possible to clone only\n> some pages: the clone is based on a list of pages which is now all\n> pages.\n>\n> Code clarified & improved with the help of Jeff King and Junio C Hamano.\n>\n> We were not able to reproduce the empty timestamp bug noticed by Jeff\n> King, thus needing some further testing. A placeholder is still\n> implemented in case.\n>\n> Signed-off-by: Jérémie Nikaes <jeremie.nikaes@ensimag.imag.fr>\n> Signed-off-by: Arnaud Lacurie <arnaud.lacurie@ensimag.imag.fr>\n> Signed-off-by: Claire Fousse <claire.fousse@ensimag.imag.fr>\n> Signed-off-by: David Amouyal <david.amouyal@ensimag.imag.fr>\n> Signed-off-by: Matthieu Moy <matthieu.moy@grenoble-inp.fr>\n> Signed-off-by: Sylvain Boulmé <sylvain.boulme@imag.fr>\n> ---\n>  contrib/mw-to-git/git-remote-mediawiki     |  306 ++++++++++++++++++++++++++++\n>  contrib/mw-to-git/git-remote-mediawiki.txt |    7 +\n>  2 files changed, 313 insertions(+), 0 deletions(-)\n\nThis seems to have grown a bit.\n\nI won't repeat issues I pointed out in the earlier round but not updated\nin this patch to save time.\n\n> +########################## Functions ##############################\n> +\n> +sub get_last_local_revision {\n> +\t# Get last commit sha1\n> +\tmy $commit_sha1 = `git rev-parse refs/mediawiki/$remotename/master 2>/dev/null`;\n> +\n> +\t# Get note regarding that commit\n> +\tchomp($commit_sha1);\n> +\tmy $note = `git log --notes=mediawiki 2>/dev/null | grep \"mediawiki_revision: \" | sed -n 1p`;\n\nTwo and half issues:\n\n - You are writing Perl no?  Don't call grep/sed from it.  Your\n   environment is much richer and more flexible ;-).\n\n - You grab $commit_sha1 but never use it. Did you mean to throw it at the\n   \"git log\" above?\n\n - Is there a reason you use \"git log\" to traverse the history all the way\n   down to the root commit?  Wouldn't\n\n\tgit notes --ref=mediawiki show $commit_sha1\n\n   or even better yet, just doing\n\n\tgit notes --ref=mediawiki show refs/mediawiki/$remotename/master\n\n   without the first rev-parse sufficient?\n\n   Are you protecting against the case where some commits in the history\n   leading to mediawiki/$remotename/master may not have the \"mediawiki\"\n   note, and falling back to the latest commit that has note? You may find\n   such a commit, but that may be different from the commit at the tip of\n   mediawiki/$remotename/master branch. Is that a correct thing to do?\n   IOW, does _any_ previous version do for the purpose of this function?\n   (This paragraph is not a rhetorical question).\n\n> +sub mw_capabilities {\n> +\t# Revisions are imported to the private namespace\n> +\t# refs/mediawiki/$remotename/ by the helper and fetched into\n> +\t# refs/remotes/$remotename later by fetch.\n> +\tprint STDOUT \"refspec refs/heads/*:refs/mediawiki/$remotename/*\\n\";\n> +\tprint STDOUT \"import\\n\";\n> +\tprint STDOUT \"list\\n\";\n> +\tprint STDOUT \"option\\n\";\n> +\tprint STDOUT \"push\\n\";\n> +\tprint STDOUT \"\\n\";\n\nThere are many explicit references to STDOUT like this, and also many\nunqualified \"print\" that spits out to the default which is STDOUT in the\ncodepath to feed fast-import. Is that intentional, or is it just coming\nfrom difference in style of people who worked on different parts of the\ncode?\n\nIf there is no reason to use two styles, please pick one and stick to\nit. If there _is_ reason, please document what are the criteria to choose\nwhich one in each codepath.  Otherwise you would waste time of your\nreviewers who have to wonder which one is correct in which codepath.\n"},{"id":"169394","messageId":"BANLkTi=nOa2B_KNfv-XkYFVr8Ngdd2msFA@mail.gmail.com","threadId":"27555","inReplyTo":"1307355635-5580-1-git-send-email-jeremie.nikaes@ensimag.imag.fr","subject":"Re: [PATCH] Add a remote helper to interact with mediawiki, pull & clone handled","fromName":"Thomas Adam","fromEmail":"thomas@xteddy.org","sentAt":"2011-06-06T19:48:17Z","receivedAt":"2011-06-06T19:48:17Z","isPatch":true,"sender":{"key":"thomas@xteddy.org","avatar":"https://gravatar.com/avatar/e7256db4738e501e5d2e84f00bb0bd99503165729573848a03330301fc2adc4a?d=mp&s=160"},"body":"Hi,\n\nOn 6 June 2011 10:20, Jeremie Nikaes <jeremie.nikaes@ensimag.imag.fr> wrote:\n\nI've got some points of my own for consideration.\n\n> @@ -0,0 +1,306 @@\n> +#! /usr/bin/perl\n> +\n> +use strict;\n\nuse warnings; ?\n\n> +use Switch;\n\nUgh -- no.  This is terrible.  Look at this:\n\n[~]% corelist Switch\nSwitch was first released with perl v5.7.3 and removed from v5.13.1\n\nSince you do not specify a minimum perl version you might be alright,\nbut for those people on 5.14 -- they won't have this module, for good\nreason.  You can, if you wanted use \"given/when\" as alternate\nconstructs to this.\n\n> +use MediaWiki::API;\n> +use Storable qw(freeze thaw);\n\nThis might have problems transcending storable formats made on a 32bit\nmachine, and then trying to unpack them again on 64bit.  Do you really\nneed the need for these storable items to be encoded as binary?\n\n[...]\n\n-- Thomas Adam\n"},{"id":"169410","messageId":"BANLkTi=eGz-NydXngtAprNedmfG_UgvDfw@mail.gmail.com","threadId":"27555","inReplyTo":"7vmxhu92u5.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] Add a remote helper to interact with mediawiki, pull & clone handled","fromName":"Jérémie NIKAES","fromEmail":"jeremie.nikaes@ensimag.imag.fr","sentAt":"2011-06-06T21:17:34Z","receivedAt":"2011-06-06T21:17:34Z","isPatch":true,"sender":{"key":"jeremie.nikaes@ensimag.imag.fr","avatar":null},"body":"> This seems to have grown a bit.\n>\n> I won't repeat issues I pointed out in the earlier round but not updated\n> in this patch to save time.\n\nYes sorry, I actually commited a patch where the weird loop control\nwas not fixed yet. My mistake, will fix this asap.\n\nAbout the print STDERR lines, what should we do about them ? They do\ngrow the patch up but if we remove them completely the script seems to\nbe frozen because of no text output for the user. Should we put them\nin a verbose mode ?\n\n> Two and half issues:\n>\n>  - You are writing Perl no?  Don't call grep/sed from it.  Your\n>   environment is much richer and more flexible ;-).\n\nOkay it's true that we discovered Perl as we started this project so\nwe were more comfortable with this kind of commands. Will work on\nthat.\n>\n>  - You grab $commit_sha1 but never use it. Did you mean to throw it at the\n>   \"git log\" above?\n>\n>  - Is there a reason you use \"git log\" to traverse the history all the way\n>   down to the root commit?  Wouldn't\n>\n>        git notes --ref=mediawiki show $commit_sha1\n>\n>   or even better yet, just doing\n>\n>        git notes --ref=mediawiki show refs/mediawiki/$remotename/master\n>\n>   without the first rev-parse sufficient?\n>\n>   Are you protecting against the case where some commits in the history\n>   leading to mediawiki/$remotename/master may not have the \"mediawiki\"\n>   note, and falling back to the latest commit that has note? You may find\n>   such a commit, but that may be different from the commit at the tip of\n>   mediawiki/$remotename/master branch. Is that a correct thing to do?\n>   IOW, does _any_ previous version do for the purpose of this function?\n>   (This paragraph is not a rhetorical question).\n\nYou are totally right. We actually were trying to find a command that\ndid this without having to parse the git log, git notes\n--ref=mediawiki show refs/mediawiki/$remotename/master does this\nperfectly.\nIn practice, a mediawiki/$remotename/master commit should ALWAYS have\na note attached to it, so we did not try to protect against this case\nhere.\nShould we be concerned and do something about it ?\n\n> There are many explicit references to STDOUT like this, and also many\n> unqualified \"print\" that spits out to the default which is STDOUT in the\n> codepath to feed fast-import. Is that intentional, or is it just coming\n> from difference in style of people who worked on different parts of the\n> code?\n>\n> If there is no reason to use two styles, please pick one and stick to\n> it. If there _is_ reason, please document what are the criteria to choose\n> which one in each codepath.  Otherwise you would waste time of your\n> reviewers who have to wonder which one is correct in which codepath.\n\nYes you are right, that slipped too. Thanks for noticing this, fixing it.\n\nAs always, thanks for the feedback, working on improving this.\n\n--\nJérémie Nikaes\n"},{"id":"169416","messageId":"BANLkTi=ABYYY7KwVa6=bY_DXXopKnzZOtg@mail.gmail.com","threadId":"27555","inReplyTo":"BANLkTi=nOa2B_KNfv-XkYFVr8Ngdd2msFA@mail.gmail.com","subject":"Re: [PATCH] Add a remote helper to interact with mediawiki, pull & clone handled","fromName":"Jérémie NIKAES","fromEmail":"jeremie.nikaes@ensimag.imag.fr","sentAt":"2011-06-06T22:11:12Z","receivedAt":"2011-06-06T22:11:12Z","isPatch":true,"sender":{"key":"jeremie.nikaes@ensimag.imag.fr","avatar":null},"body":"> use warnings; ?\n\nThanks, added this, showed some uses of undefined variables now fixed.\nWill figure in the next patch.\n>\n>> +use Switch;\n>\n> Ugh -- no.  This is terrible.  Look at this:\n>\n> [~]% corelist Switch\n> Switch was first released with perl v5.7.3 and removed from v5.13.1\n>\n> Since you do not specify a minimum perl version you might be alright,\n> but for those people on 5.14 -- they won't have this module, for good\n> reason.  You can, if you wanted use \"given/when\" as alternate\n> constructs to this.\n\nYes, as I said to Junio Hamano in my previous mail, that's a mistake\nfrom my part, we don't use switch anymore. Sorry about that\n\n>\n>> +use MediaWiki::API;\n>> +use Storable qw(freeze thaw);\n>\n> This might have problems transcending storable formats made on a 32bit\n> machine, and then trying to unpack them again on 64bit.  Do you really\n> need the need for these storable items to be encoded as binary?\n\nHmmm that is a leftover use from the script of Jeff King that we used.\nWe don't use this anymore, so that should not be a problem.\n\nThanks for your advices !\n\n--\nJérémie Nikaes\n"}]}