{"thread":{"id":"30787","subject":"[PATCHv3 1/2] git-remote-mediawiki: import \"File:\" attachments","startedAt":"2012-06-12T20:14:50Z","lastAt":"2012-06-13T13:37:52Z","messageCount":4,"participants":["Pavel Volek","Junio C Hamano","volekp"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"193495","messageId":"1339532091-25232-1-git-send-email-Pavel.Volek@ensimag.imag.fr","threadId":"30787","inReplyTo":null,"subject":"[PATCHv3 1/2] git-remote-mediawiki: import \"File:\" attachments","fromName":"Pavel Volek","fromEmail":"pavel.volek@ensimag.imag.fr","sentAt":"2012-06-12T20:14:50Z","receivedAt":"2012-06-12T20:14:50Z","isPatch":false,"sender":{"key":"pavel.volek@ensimag.imag.fr","avatar":null},"body":"From: Pavel VOlek <Pavel.Volek@ensimag.imag.fr>\n\nThe current version of the git-remote-mediawiki supports only import and export\nof the pages, doesn't support import and export of file attachments which are\nalso exposed by MediaWiki API. This patch adds the functionality to import file\nattachments and description pages for these files.\n\nChages version2 -> version3:\nFixes in comments.\nVariable '$file' -> '$file_content' refactoring to be clearer.\n\nSigned-off-by: Pavel Volek <Pavel.Volek@ensimag.imag.fr>\nSigned-off-by: NGUYEN Kim Thuat <Kim-Thuat.Nguyen@ensimag.imag.fr>\nSigned-off-by: ROUCHER IGLESIAS Javier <roucherj@ensimag.imag.fr>\nSigned-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n---\n contrib/mw-to-git/git-remote-mediawiki | 223 ++++++++++++++++++++++++++++++++-\n 1 file changed, 218 insertions(+), 5 deletions(-)\n\ndiff --git a/contrib/mw-to-git/git-remote-mediawiki b/contrib/mw-to-git/git-remote-mediawiki\nindex c18bfa1..04d3959 100755\n--- a/contrib/mw-to-git/git-remote-mediawiki\n+++ b/contrib/mw-to-git/git-remote-mediawiki\n@@ -13,9 +13,6 @@\n #\n # Known limitations:\n #\n-# - Only wiki pages are managed, no support for [[File:...]]\n-#   attachments.\n-#\n # - Poor performance in the best case: it takes forever to check\n #   whether we're up-to-date (on fetch or push) or to fetch a few\n #   revisions from a large wiki, because we use exclusively a\n@@ -71,6 +68,9 @@ chomp(@tracked_pages);\n my @tracked_categories = split(/[ \\n]/, run_git(\"config --get-all remote.\". $remotename .\".categories\"));\n chomp(@tracked_categories);\n \n+# Import media files too.\n+my $import_media = run_git(\"config --get --bool remote.\". $remotename .\".mediaimport\");\n+\n my $wiki_login = run_git(\"config --get remote.\". $remotename .\".mwLogin\");\n # TODO: ideally, this should be able to read from keyboard, but we're\n # inside a remote helper, so our stdin is connect to git, not to a\n@@ -225,6 +225,10 @@ sub get_mw_pages {\n \t\t\tget_mw_first_pages(\\@slice, \\%pages);\n \t\t\t@some_pages = @some_pages[51..$#some_pages];\n \t\t}\n+\n+\t\tif ($import_media) {\n+\t\t\tget_mw_pages_for_linked_mediafiles(\\@tracked_pages, \\%pages);\n+\t\t}\n \t}\n \tif (@tracked_categories) {\n \t\t$user_defined = 1;\n@@ -244,6 +248,11 @@ sub get_mw_pages {\n \t\t\tforeach my $page (@{$mw_pages}) {\n \t\t\t\t$pages{$page->{title}} = $page;\n \t\t\t}\n+\n+\t\t\tif ($import_media) {\n+\t\t\t\tmy @titles = map $_->{title}, @{$mw_pages};\n+\t\t\t\tget_mw_pages_for_linked_mediafiles(\\@titles, \\%pages);\n+\t\t\t}\n \t\t}\n \t}\n \tif (!$user_defined) {\n@@ -263,10 +272,186 @@ sub get_mw_pages {\n \t\tforeach my $page (@{$mw_pages}) {\n \t\t\t$pages{$page->{title}} = $page;\n \t\t}\n+\n+\t\tif ($import_media) {\n+\t\t\t# Attach list of all pages for media files from the API,\n+\t\t\t# they are in a different namespace, only one namespace\n+\t\t\t# can be queried at the same moment\n+\t\t\tmy $mw_pages = $mediawiki->list({\n+\t\t\t\taction => 'query',\n+\t\t\t\tlist => 'allpages',\n+\t\t\t\tapnamespace => get_mw_namespace_id(\"File\"),\n+\t\t\t\taplimit => 500\n+\t\t\t});\n+\t\t\tif (!defined($mw_pages)) {\n+\t\t\t\tprint STDERR \"fatal: could not get the list of pages for media files.\\n\";\n+\t\t\t\tprint STDERR \"fatal: '$url' does not appear to be a mediawiki\\n\";\n+\t\t\t\tprint STDERR \"fatal: make sure '$url/api.php' is a valid page.\\n\";\n+\t\t\t\texit 1;\n+\t\t\t}\n+\t\t\tforeach my $page (@{$mw_pages}) {\n+\t\t\t\t$pages{$page->{title}} = $page;\n+\t\t\t}\n+\t\t}\n \t}\n \treturn values(%pages);\n }\n \n+sub get_mw_pages_for_linked_mediafiles {\n+\tmy $titles = shift;\n+\tmy @titles = @{$titles};\n+\tmy $pages = shift;\n+\n+\t# pattern 'page1|page2|...' required by the API\n+\tmy $mw_titles = join('|', @titles);\n+\n+\t# Media files could be included or linked from\n+\t# a page, get all related\n+\tmy $query = {\n+\t\taction => 'query',\n+\t\tprop => 'links|images',\n+\t\ttitles => $mw_titles,\n+\t\tplnamespace => get_mw_namespace_id(\"File\"),\n+\t\tpllimit => 500\n+\t};\n+\tmy $result = $mediawiki->api($query);\n+\n+\twhile (my ($id, $page) = each(%{$result->{query}->{pages}})) {\n+\t\tmy @titles;\n+\t\tif (defined($page->{links})) {\n+\t\t\tmy @link_titles = map $_->{title}, @{$page->{links}};\n+\t\t\tpush(@titles, @link_titles);\n+\t\t}\n+\t\tif (defined($page->{images})) {\n+\t\t\tmy @image_titles = map $_->{title}, @{$page->{images}};\n+\t\t\tpush(@titles, @image_titles);\n+\t\t}\n+\t\tif (@titles) {\n+\t\t\tget_mw_first_pages(\\@titles, \\%{$pages});\n+\t\t}\n+\t}\n+}\n+\n+# Return MediaWiki id for a canonical namespace name.\n+# Ex.: \"File\", \"Project\".\n+# Looks for the namespace id in the local configuration\n+# variables, if it is not found asks MW API.\n+sub get_mw_namespace_id {\n+\tmw_connect_maybe();\n+\n+\tmy $name = shift;\n+\n+\t# Look at configuration file, if the record\n+\t# for that namespace is already stored.\n+\t# Namespaces are stored in form: \"Name_of_namespace:Id_namespace\",\n+\t# Ex.: \"File:6\".\n+\tmy @tracked_namespaces = split(/[ \\n]/, run_git(\"config --get-all remote.\". $remotename .\".namespaces\"));\n+\tchomp(@tracked_namespaces);\n+\tif (@tracked_namespaces) {\n+\t\tforeach my $ns (@tracked_namespaces) {\n+\t\t\tmy @ns_split = split(/:/, $ns);\n+\t\t\tif ($ns_split[0] eq $name) {\n+\t\t\t\treturn $ns_split[1];\n+\t\t\t}\n+\t\t}\n+\t}\n+\n+\t# NS not found => get namespace id from MW and store it in\n+\t# configuration file.\n+\tmy $query = {\n+\t\taction => 'query',\n+\t\tmeta => 'siteinfo',\n+\t\tsiprop => 'namespaces'\n+\t};\n+\tmy $result = $mediawiki->api($query);\n+\n+\twhile (my ($id, $ns) = each(%{$result->{query}->{namespaces}})) {\n+\t\tif (defined($ns->{canonical}) && ($ns->{canonical} eq $name)) {\n+\t\t\trun_git(\"config --add remote.\". $remotename .\".namespaces \". $name .\":\". $ns->{id});\n+\t\t\treturn $ns->{id};\n+\t\t}\n+\t}\n+\tdie \"Namespace $name was not found on MediaWiki.\";\n+}\n+\n+sub get_mw_mediafile_for_page_revision {\n+\t# Name of the file on Wiki, with the prefix.\n+\tmy $mw_filename = shift;\n+\tmy $timestamp = shift;\n+\tmy %mediafile;\n+\n+\t# Search if on MediaWiki exists a media file with given\n+\t# timestamp. In that case download the file.\n+\tmy $query = {\n+\t\taction => 'query',\n+\t\tprop => 'imageinfo',\n+\t\ttitles => $mw_filename,\n+\t\tiistart => $timestamp,\n+\t\tiiend => $timestamp,\n+\t\tiiprop => 'timestamp|archivename|url',\n+\t\tiilimit => 1\n+\t};\n+\tmy $result = $mediawiki->api($query);\n+\n+\tmy ($fileid, $file) = each ( %{$result->{query}->{pages}} );\n+\t# If not defined it means there is no revision of the file for\n+\t# given timestamp.\n+\tif (defined($file->{imageinfo})) {\n+\t\t# Get real name of media file.\n+\t\tmy $filename;\n+\t\tif (index($mw_filename, 'File:') == 0) {\n+\t\t\t$filename = substr $mw_filename, 5;\n+\t\t} else {\n+\t\t\t$filename = substr $mw_filename, 6;\n+\t\t}\n+\t\t$mediafile{title} = $filename;\n+\n+\t\tmy $fileinfo = pop(@{$file->{imageinfo}});\n+\t\t$mediafile{timestamp} = $fileinfo->{timestamp};\n+\t\t# If this is an old version of the file, the file has to be\n+\t\t# obtained from the archive. Otherwise it can be downloaded\n+\t\t# by MediaWiki API download() function.\n+\t\tif (defined($fileinfo->{archivename})) {\n+\t\t\t$mediafile{content} = download_mw_mediafile_from_archive($fileinfo->{url});\n+\t\t} else {\n+\t\t\t$mediafile{content} = download_mw_mediafile($mw_filename);\n+\t\t}\n+\t}\n+\treturn %mediafile;\n+}\n+\n+sub download_mw_mediafile_from_archive {\n+\tmy $url = shift;\n+\tmy $file;\n+\n+\tmy $ua = LWP::UserAgent->new;\n+\tmy $response = $ua->get($url);\n+\tif ($response->code) {\n+\t\t$file = $response->decoded_content;\n+\t} else {\n+\t\tprint STDERR \"Error downloading a file from archive.\\n\";\n+\t}\n+\n+\treturn $file;\n+}\n+\n+sub download_mw_mediafile {\n+\tmy $filename = shift;\n+\n+\t$mediawiki->{config}->{files_url} = $url;\n+\n+\tmy $file_content = $mediawiki->download( { title => $filename } );\n+\tif (!defined($file_content)) {\n+\t\tprint STDERR \"\\tFile \\'$filename\\' could not be downloaded.\\n\";\n+\t\texit 1;\n+\t} elsif ($file_content eq \"\") {\n+\t\tprint STDERR \"\\tFile \\'$filename\\' does not exist on the wiki.\\n\";\n+\t\texit 1;\n+\t} else {\n+\t\treturn $file_content;\n+\t}\n+}\n+\n sub run_git {\n \topen(my $git, \"-|:encoding(UTF-8)\", \"git \" . $_[0]);\n \tmy $res = do { local $/; <$git> };\n@@ -466,6 +651,13 @@ sub import_file_revision {\n \tmy %commit = %{$commit};\n \tmy $full_import = shift;\n \tmy $n = shift;\n+\tmy $mediafile_import = shift;\n+\tmy $mediafile;\n+\tmy %mediafile;\n+\tif ($mediafile_import) {\n+\t\t$mediafile = shift;\n+\t\t%mediafile = %{$mediafile};\n+\t}\n \n \tmy $title = $commit{title};\n \tmy $comment = $commit{comment};\n@@ -485,6 +677,10 @@ sub import_file_revision {\n \tif ($content ne DELETED_CONTENT) {\n \t\tprint STDOUT \"M 644 inline $title.mw\\n\";\n \t\tliteral_data($content);\n+\t\tif ($mediafile_import) {\n+\t\t\tprint STDOUT \"M 644 inline $mediafile{title}\\n\";\n+\t\t\tliteral_data($mediafile{content});\n+\t\t}\n \t\tprint STDOUT \"\\n\\n\";\n \t} else {\n \t\tprint STDOUT \"D $title.mw\\n\";\n@@ -580,6 +776,7 @@ sub mw_import_ref {\n \n \t\t$n++;\n \n+\t\tmy $page_title = $result->{query}->{pages}->{$pagerevid->{pageid}}->{title};\n \t\tmy %commit;\n \t\t$commit{author} = $rev->{user} || 'Anonymous';\n \t\t$commit{comment} = $rev->{comment} || '*Empty MediaWiki Message*';\n@@ -596,9 +793,25 @@ sub mw_import_ref {\n \t\t}\n \t\t$commit{date} = DateTime::Format::ISO8601->parse_datetime($last_timestamp);\n \n-\t\tprint STDERR \"$n/\", scalar(@revisions), \": Revision #$pagerevid->{revid} of $commit{title}\\n\";\n+\t\t# Differentiates classic pages and media files.\n+\t\tmy @prefix = split (\":\", $page_title);\n \n-\t\timport_file_revision(\\%commit, ($fetch_from == 1), $n);\n+\t\tmy %mediafile;\n+\t\tif ($prefix[0] eq \"File\" || $prefix[0] eq \"Image\") {\n+\t\t\t# The name of the file is the same as the media page.\n+\t\t\tmy $filename = $page_title;\n+\t\t\t%mediafile = get_mw_mediafile_for_page_revision($filename, $rev->{timestamp});\n+\t\t}\n+\t\t# If this is a revision of the media page for new version\n+\t\t# of a file do one common commit for both file and media page.\n+\t\t# Else do commit only for that page.\n+\t\tprint STDERR \"$n/\", scalar(@revisions), \": Revision #$pagerevid->{revid} of $commit{title}\\n\";\n+\t\tif (%mediafile) {\n+\t\t\tprint STDERR \"\\tDownloading file $mediafile{title}, version $mediafile{timestamp}\\n\";\n+\t\t\timport_file_revision(\\%commit, ($fetch_from == 1), $n, 1, \\%mediafile);\n+\t\t} else {\n+\t\t\timport_file_revision(\\%commit, ($fetch_from == 1), $n, 0);\n+\t\t}\n \t}\n \n \tif ($fetch_from == 1 && $n == 0) {\n-- \n1.7.10.2.552.gaa3bb87\n"},{"id":"193496","messageId":"1339532091-25232-2-git-send-email-Pavel.Volek@ensimag.imag.fr","threadId":"30787","inReplyTo":"1339532091-25232-1-git-send-email-Pavel.Volek@ensimag.imag.fr","subject":"[PATCHv3 2/2] git-remote-mediawiki: refactoring get_mw_pages function","fromName":"Pavel Volek","fromEmail":"pavel.volek@ensimag.imag.fr","sentAt":"2012-06-12T20:14:51Z","receivedAt":"2012-06-12T20:14:51Z","isPatch":false,"sender":{"key":"pavel.volek@ensimag.imag.fr","avatar":null},"body":"From: Pavel VOlek <Pavel.Volek@ensimag.imag.fr>\n\nSplits the code in the get_mw_pages function into three separate functions.\nOne for getting list of all pages and all file attachments, second for pages\nin category specified in configuration file and files related to these pages\nand the last function to get from MW a list of specified pages with related\nfile attachments.\n\nChages version2 -> version3:\nFixes in comments.\n\nSigned-off-by: Pavel Volek <Pavel.Volek@ensimag.imag.fr>\nSigned-off-by: NGUYEN Kim Thuat <Kim-Thuat.Nguyen@ensimag.imag.fr>\nSigned-off-by: ROUCHER IGLESIAS Javier <roucherj@ensimag.imag.fr>\nSigned-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n---\n contrib/mw-to-git/git-remote-mediawiki | 140 ++++++++++++++++++---------------\n 1 file changed, 77 insertions(+), 63 deletions(-)\n\ndiff --git a/contrib/mw-to-git/git-remote-mediawiki b/contrib/mw-to-git/git-remote-mediawiki\nindex 04d3959..ace1868 100755\n--- a/contrib/mw-to-git/git-remote-mediawiki\n+++ b/contrib/mw-to-git/git-remote-mediawiki\n@@ -212,89 +212,103 @@ sub get_mw_pages {\n \tmy $user_defined;\n \tif (@tracked_pages) {\n \t\t$user_defined = 1;\n-\t\t# The user provided a list of pages titles, but we\n-\t\t# still need to query the API to get the page IDs.\n-\n-\t\tmy @some_pages = @tracked_pages;\n-\t\twhile (@some_pages) {\n-\t\t\tmy $last = 50;\n-\t\t\tif ($#some_pages < $last) {\n-\t\t\t\t$last = $#some_pages;\n-\t\t\t}\n-\t\t\tmy @slice = @some_pages[0..$last];\n-\t\t\tget_mw_first_pages(\\@slice, \\%pages);\n-\t\t\t@some_pages = @some_pages[51..$#some_pages];\n-\t\t}\n-\n-\t\tif ($import_media) {\n-\t\t\tget_mw_pages_for_linked_mediafiles(\\@tracked_pages, \\%pages);\n-\t\t}\n+\t\tget_mw_tracked_pages(\\%pages);\n \t}\n \tif (@tracked_categories) {\n \t\t$user_defined = 1;\n-\t\tforeach my $category (@tracked_categories) {\n-\t\t\tif (index($category, ':') < 0) {\n-\t\t\t\t# Mediawiki requires the Category\n-\t\t\t\t# prefix, but let's not force the user\n-\t\t\t\t# to specify it.\n-\t\t\t\t$category = \"Category:\" . $category;\n-\t\t\t}\n-\t\t\tmy $mw_pages = $mediawiki->list( {\n-\t\t\t\taction => 'query',\n-\t\t\t\tlist => 'categorymembers',\n-\t\t\t\tcmtitle => $category,\n-\t\t\t\tcmlimit => 'max' } )\n-\t\t\t    || die $mediawiki->{error}->{code} . ': ' . $mediawiki->{error}->{details};\n-\t\t\tforeach my $page (@{$mw_pages}) {\n-\t\t\t\t$pages{$page->{title}} = $page;\n-\t\t\t}\n-\n-\t\t\tif ($import_media) {\n-\t\t\t\tmy @titles = map $_->{title}, @{$mw_pages};\n-\t\t\t\tget_mw_pages_for_linked_mediafiles(\\@titles, \\%pages);\n-\t\t\t}\n-\t\t}\n+\t\tget_mw_tracked_categories(\\%pages);\n \t}\n \tif (!$user_defined) {\n-\t\t# No user-provided list, get the list of pages from\n-\t\t# the API.\n+\t\tget_mw_all_pages(\\%pages);\n+\t}\n+\treturn values(%pages);\n+}\n+\n+sub get_mw_all_pages {\n+\tmy $pages = shift;\n+\t# No user-provided list, get the list of pages from the API.\n+\tmy $mw_pages = $mediawiki->list({\n+\t\taction => 'query',\n+\t\tlist => 'allpages',\n+\t\taplimit => 500\n+\t});\n+\tif (!defined($mw_pages)) {\n+\t\tprint STDERR \"fatal: could not get the list of wiki pages.\\n\";\n+\t\tprint STDERR \"fatal: '$url' does not appear to be a mediawiki\\n\";\n+\t\tprint STDERR \"fatal: make sure '$url/api.php' is a valid page.\\n\";\n+\t\texit 1;\n+\t}\n+\tforeach my $page (@{$mw_pages}) {\n+\t\t$pages->{$page->{title}} = $page;\n+\t}\n+\n+\tif ($import_media) {\n+\t\t# Attach list of all pages for media files from the API,\n+\t\t# they are in a different namespace, only one namespace\n+\t\t# can be queried at the same moment\n \t\tmy $mw_pages = $mediawiki->list({\n \t\t\taction => 'query',\n \t\t\tlist => 'allpages',\n-\t\t\taplimit => 500,\n+\t\t\tapnamespace => get_mw_namespace_id(\"File\"),\n+\t\t\taplimit => 500\n \t\t});\n \t\tif (!defined($mw_pages)) {\n-\t\t\tprint STDERR \"fatal: could not get the list of wiki pages.\\n\";\n+\t\t\tprint STDERR \"fatal: could not get the list of pages for media files.\\n\";\n \t\t\tprint STDERR \"fatal: '$url' does not appear to be a mediawiki\\n\";\n \t\t\tprint STDERR \"fatal: make sure '$url/api.php' is a valid page.\\n\";\n \t\t\texit 1;\n \t\t}\n \t\tforeach my $page (@{$mw_pages}) {\n-\t\t\t$pages{$page->{title}} = $page;\n+\t\t\t$pages->{$page->{title}} = $page;\n+\t\t}\n+\t}\n+}\n+\n+sub get_mw_tracked_pages {\n+\tmy $pages = shift;\n+\t# The user provided a list of pages titles, but we\n+\t# still need to query the API to get the page IDs.\n+\tmy @some_pages = @tracked_pages;\n+\twhile (@some_pages) {\n+\t\tmy $last = 50;\n+\t\tif ($#some_pages < $last) {\n+\t\t\t$last = $#some_pages;\n+\t\t}\n+\t\tmy @slice = @some_pages[0..$last];\n+\t\tget_mw_first_pages(\\@slice, \\%{$pages});\n+\t\t@some_pages = @some_pages[51..$#some_pages];\n+\t}\n+\n+\tif ($import_media) {\n+\t\tget_mw_pages_for_linked_mediafiles(\\@tracked_pages, \\%{$pages});\n+\t}\n+}\n+\n+sub get_mw_tracked_categories {\n+\tmy $pages = shift;\n+\tforeach my $category (@tracked_categories) {\n+\t\tif (index($category, ':') < 0) {\n+\t\t\t# Mediawiki requires the Category\n+\t\t\t# prefix, but let's not force the user\n+\t\t\t# to specify it.\n+\t\t\t$category = \"Category:\" . $category;\n+\t\t}\n+\t\tmy $mw_pages = $mediawiki->list( {\n+\t\t\taction => 'query',\n+\t\t\tlist => 'categorymembers',\n+\t\t\tcmtitle => $category,\n+\t\t\tcmlimit => 'max' } )\n+\t\t\t|| die $mediawiki->{error}->{code} . ': '\n+\t\t\t\t. $mediawiki->{error}->{details};\n+\t\tforeach my $page (@{$mw_pages}) {\n+\t\t\t$pages->{$page->{title}} = $page;\n \t\t}\n \n \t\tif ($import_media) {\n-\t\t\t# Attach list of all pages for media files from the API,\n-\t\t\t# they are in a different namespace, only one namespace\n-\t\t\t# can be queried at the same moment\n-\t\t\tmy $mw_pages = $mediawiki->list({\n-\t\t\t\taction => 'query',\n-\t\t\t\tlist => 'allpages',\n-\t\t\t\tapnamespace => get_mw_namespace_id(\"File\"),\n-\t\t\t\taplimit => 500\n-\t\t\t});\n-\t\t\tif (!defined($mw_pages)) {\n-\t\t\t\tprint STDERR \"fatal: could not get the list of pages for media files.\\n\";\n-\t\t\t\tprint STDERR \"fatal: '$url' does not appear to be a mediawiki\\n\";\n-\t\t\t\tprint STDERR \"fatal: make sure '$url/api.php' is a valid page.\\n\";\n-\t\t\t\texit 1;\n-\t\t\t}\n-\t\t\tforeach my $page (@{$mw_pages}) {\n-\t\t\t\t$pages{$page->{title}} = $page;\n-\t\t\t}\n+\t\t\tmy @titles = map $_->{title}, @{$mw_pages};\n+\t\t\tget_mw_pages_for_linked_mediafiles(\\@titles, \\%{$pages});\n \t\t}\n \t}\n-\treturn values(%pages);\n }\n \n sub get_mw_pages_for_linked_mediafiles {\n-- \n1.7.10.2.552.gaa3bb87\n"},{"id":"193504","messageId":"7vy5nsi6lq.fsf@alter.siamese.dyndns.org","threadId":"30787","inReplyTo":"1339532091-25232-1-git-send-email-Pavel.Volek@ensimag.imag.fr","subject":"Re: [PATCHv3 1/2] git-remote-mediawiki: import \"File:\" attachments","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-06-12T21:05:21Z","receivedAt":"2012-06-12T21:05:21Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Pavel Volek <Pavel.Volek@ensimag.imag.fr> writes:\n\n> From: Pavel VOlek <Pavel.Volek@ensimag.imag.fr>\n\nDid you really mean this?  It does not match your S-o-b: line below.\n\n>\n> The current version of the git-remote-mediawiki supports only import and export\n> of the pages, doesn't support import and export of file attachments which are\n> also exposed by MediaWiki API. This patch adds the functionality to import file\n> attachments and description pages for these files.\n>\n> Chages version2 -> version3:\n> Fixes in comments.\n> Variable '$file' -> '$file_content' refactoring to be clearer.\n\nThese three lines do not belong here above the three-dash lines, I think.\n\n> Signed-off-by: Pavel Volek <Pavel.Volek@ensimag.imag.fr>\n> Signed-off-by: NGUYEN Kim Thuat <Kim-Thuat.Nguyen@ensimag.imag.fr>\n> Signed-off-by: ROUCHER IGLESIAS Javier <roucherj@ensimag.imag.fr>\n> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n> ---\n>  contrib/mw-to-git/git-remote-mediawiki | 223 ++++++++++++++++++++++++++++++++-\n>  1 file changed, 218 insertions(+), 5 deletions(-)\n>\n> diff --git a/contrib/mw-to-git/git-remote-mediawiki b/contrib/mw-to-git/git-remote-mediawiki\n> index c18bfa1..04d3959 100755\n> --- a/contrib/mw-to-git/git-remote-mediawiki\n> +++ b/contrib/mw-to-git/git-remote-mediawiki\n> @@ -13,9 +13,6 @@\n>  #\n>  # Known limitations:\n>  #\n> -# - Only wiki pages are managed, no support for [[File:...]]\n> -#   attachments.\n> -#\n\nNice.\n\n> @@ -71,6 +68,9 @@ chomp(@tracked_pages);\n>  my @tracked_categories = split(/[ \\n]/, run_git(\"config --get-all remote.\". $remotename .\".categories\"));\n>  chomp(@tracked_categories);\n>  \n> +# Import media files too.\n> +my $import_media = run_git(\"config --get --bool remote.\". $remotename .\".mediaimport\");\n> +\n>  my $wiki_login = run_git(\"config --get remote.\". $remotename .\".mwLogin\");\n>  # TODO: ideally, this should be able to read from keyboard, but we're\n>  # inside a remote helper, so our stdin is connect to git, not to a\n> @@ -225,6 +225,10 @@ sub get_mw_pages {\n>  \t\t\tget_mw_first_pages(\\@slice, \\%pages);\n>  \t\t\t@some_pages = @some_pages[51..$#some_pages];\n>  \t\t}\n> +\n> +\t\tif ($import_media) {\n> +\t\t\tget_mw_pages_for_linked_mediafiles(\\@tracked_pages, \\%pages);\n> +\t\t}\n\nI am guessing that the loop above is to avoid fetching and\nprocessing too many pages at once.  Doesn't the call to\nget_mw_pages_for_linked_mediafiles() need a similar consideration,\nor what the function does is significantly different from what\nget_mw_first_pages() does and there is no need to worry?\n\nBy the way, does it really have to be that overly long name?\n\n> @@ -244,6 +248,11 @@ sub get_mw_pages {\n>  \t\t\tforeach my $page (@{$mw_pages}) {\n>  \t\t\t\t$pages{$page->{title}} = $page;\n>  \t\t\t}\n> +\n> +\t\t\tif ($import_media) {\n> +\t\t\t\tmy @titles = map $_->{title}, @{$mw_pages};\n> +\t\t\t\tget_mw_pages_for_linked_mediafiles(\\@titles, \\%pages);\n> +\t\t\t}\n>  \t\t}\n>  \t}\n>  \tif (!$user_defined) {\n> @@ -263,10 +272,186 @@ sub get_mw_pages {\n>  \t\tforeach my $page (@{$mw_pages}) {\n>  \t\t\t$pages{$page->{title}} = $page;\n>  \t\t}\n> +\n> +\t\tif ($import_media) {\n> +\t\t\t# Attach list of all pages for media files from the API,\n> +\t\t\t# they are in a different namespace, only one namespace\n> +\t\t\t# can be queried at the same moment\n> +\t\t\tmy $mw_pages = $mediawiki->list({\n> +\t\t\t\taction => 'query',\n> +\t\t\t\tlist => 'allpages',\n> +\t\t\t\tapnamespace => get_mw_namespace_id(\"File\"),\n> +\t\t\t\taplimit => 500\n> +\t\t\t});\n> +\t\t\tif (!defined($mw_pages)) {\n> +\t\t\t\tprint STDERR \"fatal: could not get the list of pages for media files.\\n\";\n> +\t\t\t\tprint STDERR \"fatal: '$url' does not appear to be a mediawiki\\n\";\n> +\t\t\t\tprint STDERR \"fatal: make sure '$url/api.php' is a valid page.\\n\";\n> +\t\t\t\texit 1;\n> +\t\t\t}\n> +\t\t\tforeach my $page (@{$mw_pages}) {\n> +\t\t\t\t$pages{$page->{title}} = $page;\n> +\t\t\t}\n> +\t\t}\n\nFor categories you need to call pages-for-mediafiles with the titles\nyou learned (the hunk starting at l.224), but there is no need to\ncall pages-for-mediafiles in this hunk?\n\nNot a rhetorical question to suggest that you should; just\nwondering.\n\n> +sub get_mw_pages_for_linked_mediafiles {\n> +\tmy $titles = shift;\n> +\tmy @titles = @{$titles};\n\nDo you really need to make a copy of this array?  Wouldn't it\nsuffice to say\n\n\tmy $mw_titles = join('|', @$titles);\n\nat the only location in this function that uses this parameter?\n\n> +\tmy $pages = shift;\n> +\n> +\t# pattern 'page1|page2|...' required by the API\n> +\tmy $mw_titles = join('|', @titles);\n\nNobody seems to be making sure there won't be more than 500 (I am\nassuming that this script is considered a 'bot) pages in $mw_titles\nvariable.  Shouldn't the API call be split into such batches?\n\n> +\t# Media files could be included or linked from\n> +\t# a page, get all related\n> +\tmy $query = {\n> +\t\taction => 'query',\n> +\t\tprop => 'links|images',\n> +\t\ttitles => $mw_titles,\n> +\t\tplnamespace => get_mw_namespace_id(\"File\"),\n> +\t\tpllimit => 500\n> +\t};\n> +\tmy $result = $mediawiki->api($query);\n> +\n> +\twhile (my ($id, $page) = each(%{$result->{query}->{pages}})) {\n> +\t\tmy @titles;\n> +\t\tif (defined($page->{links})) {\n> +\t\t\tmy @link_titles = map $_->{title}, @{$page->{links}};\n> +\t\t\tpush(@titles, @link_titles);\n> +\t\t}\n> +\t\tif (defined($page->{images})) {\n> +\t\t\tmy @image_titles = map $_->{title}, @{$page->{images}};\n> +\t\t\tpush(@titles, @image_titles);\n> +\t\t}\n> +\t\tif (@titles) {\n> +\t\t\tget_mw_first_pages(\\@titles, \\%{$pages});\n> +\t\t}\n> +\t}\n> +}\n> +\n> +# Return MediaWiki id for a canonical namespace name.\n> +# Ex.: \"File\", \"Project\".\n> +# Looks for the namespace id in the local configuration\n> +# variables, if it is not found asks MW API.\n> +sub get_mw_namespace_id {\n> +\tmw_connect_maybe();\n> +\n> +\tmy $name = shift;\n> +\n> +\t# Look at configuration file, if the record\n> +\t# for that namespace is already stored.\n> +\t# Namespaces are stored in form: \"Name_of_namespace:Id_namespace\",\n> +\t# Ex.: \"File:6\".\n> +\tmy @tracked_namespaces = split(/[ \\n]/, run_git(\"config --get-all remote.\". $remotename .\".namespaces\"));\n> +\tchomp(@tracked_namespaces);\n> +\tif (@tracked_namespaces) {\n> +\t\tforeach my $ns (@tracked_namespaces) {\n> +\t\t\tmy @ns_split = split(/:/, $ns);\n> +\t\t\tif ($ns_split[0] eq $name) {\n> +\t\t\t\treturn $ns_split[1];\n> +\t\t\t}\n> +\t\t}\n> +\t}\n\nDoes it make sense to call out to run_git() every time this function\nis called?  Shoudln't this part be caching the result in a hash,\nsomething like\n\n\tif (!exists $namespace_id{$name}) {\n\t\t@temp = ... run_git() ...;\n                foreach my $ns (@temp) {\n\t\t\tmy ($n, $s) = split(/:/, $ns);\n\t\t\t$namespace_id{$n} = $s;\n\t\t}\n\t}\n\n\tif (!exists $namespace_id{$name}) {\n\t\t... similarly, ask MW API and store in %namespace_id{}\n\t}\n\n        if (exists $namespace_id{$name}) {\n        \treturn $namespace_id{$name};\n\t}\n\tdie \"No such namespace $name\";\n"},{"id":"193539","messageId":"afdeb82809f49f34073d7b57260edee9@telesun.imag.fr","threadId":"30787","inReplyTo":"7vy5nsi6lq.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCHv3 1/2] git-remote-mediawiki: import \"File:\" attachments","fromName":"volekp","fromEmail":"volekp@telesun.imag.fr","sentAt":"2012-06-13T13:37:52Z","receivedAt":"2012-06-13T13:37:52Z","isPatch":false,"sender":{"key":"volekp@telesun.imag.fr","avatar":null},"body":"On Tue, 12 Jun 2012 14:05:21 -0700, Junio C Hamano wrote:\n> Pavel Volek <Pavel.Volek@ensimag.imag.fr> writes:\n>\n>> From: Pavel VOlek <Pavel.Volek@ensimag.imag.fr>\n>\n> Did you really mean this?  It does not match your S-o-b: line below.\n\nYou're right, I will change it.\n\n>>\n>> The current version of the git-remote-mediawiki supports only import \n>> and export\n>> of the pages, doesn't support import and export of file attachments \n>> which are\n>> also exposed by MediaWiki API. This patch adds the functionality to \n>> import file\n>> attachments and description pages for these files.\n>>\n>> Chages version2 -> version3:\n>> Fixes in comments.\n>> Variable '$file' -> '$file_content' refactoring to be clearer.\n>\n> These three lines do not belong here above the three-dash lines, I \n> think.\n\nOK. But should I mention somewhere the changes I performed from the \nprecedent to\nthe actual version of the PATCH?\n\n>> @@ -71,6 +68,9 @@ chomp(@tracked_pages);\n>>  my @tracked_categories = split(/[ \\n]/, run_git(\"config --get-all \n>> remote.\". $remotename .\".categories\"));\n>>  chomp(@tracked_categories);\n>>\n>> +# Import media files too.\n>> +my $import_media = run_git(\"config --get --bool remote.\". \n>> $remotename .\".mediaimport\");\n>> +\n>>  my $wiki_login = run_git(\"config --get remote.\". $remotename \n>> .\".mwLogin\");\n>>  # TODO: ideally, this should be able to read from keyboard, but \n>> we're\n>>  # inside a remote helper, so our stdin is connect to git, not to a\n>> @@ -225,6 +225,10 @@ sub get_mw_pages {\n>>  \t\t\tget_mw_first_pages(\\@slice, \\%pages);\n>>  \t\t\t@some_pages = @some_pages[51..$#some_pages];\n>>  \t\t}\n>> +\n>> +\t\tif ($import_media) {\n>> +\t\t\tget_mw_pages_for_linked_mediafiles(\\@tracked_pages, \\%pages);\n>> +\t\t}\n>\n> I am guessing that the loop above is to avoid fetching and\n> processing too many pages at once.  Doesn't the call to\n> get_mw_pages_for_linked_mediafiles() need a similar consideration,\n> or what the function does is significantly different from what\n> get_mw_first_pages() does and there is no need to worry?\n>\n> By the way, does it really have to be that overly long name?\n\nYes, it will be better to split the processing in a similar way. Now \nthere is\na possibility that because of the MW limits, we won't get all media \nfiles.\n\nFor the name of the method I wanted to be precise and clear, I didn't \nfind\nmore fitting name. Any suggestions welcomed.\n\n>> @@ -244,6 +248,11 @@ sub get_mw_pages {\n>>  \t\t\tforeach my $page (@{$mw_pages}) {\n>>  \t\t\t\t$pages{$page->{title}} = $page;\n>>  \t\t\t}\n>> +\n>> +\t\t\tif ($import_media) {\n>> +\t\t\t\tmy @titles = map $_->{title}, @{$mw_pages};\n>> +\t\t\t\tget_mw_pages_for_linked_mediafiles(\\@titles, \\%pages);\n>> +\t\t\t}\n>>  \t\t}\n>>  \t}\n>>  \tif (!$user_defined) {\n>> @@ -263,10 +272,186 @@ sub get_mw_pages {\n>>  \t\tforeach my $page (@{$mw_pages}) {\n>>  \t\t\t$pages{$page->{title}} = $page;\n>>  \t\t}\n>> +\n>> +\t\tif ($import_media) {\n>> +\t\t\t# Attach list of all pages for media files from the API,\n>> +\t\t\t# they are in a different namespace, only one namespace\n>> +\t\t\t# can be queried at the same moment\n>> +\t\t\tmy $mw_pages = $mediawiki->list({\n>> +\t\t\t\taction => 'query',\n>> +\t\t\t\tlist => 'allpages',\n>> +\t\t\t\tapnamespace => get_mw_namespace_id(\"File\"),\n>> +\t\t\t\taplimit => 500\n>> +\t\t\t});\n>> +\t\t\tif (!defined($mw_pages)) {\n>> +\t\t\t\tprint STDERR \"fatal: could not get the list of pages for media \n>> files.\\n\";\n>> +\t\t\t\tprint STDERR \"fatal: '$url' does not appear to be a \n>> mediawiki\\n\";\n>> +\t\t\t\tprint STDERR \"fatal: make sure '$url/api.php' is a valid \n>> page.\\n\";\n>> +\t\t\t\texit 1;\n>> +\t\t\t}\n>> +\t\t\tforeach my $page (@{$mw_pages}) {\n>> +\t\t\t\t$pages{$page->{title}} = $page;\n>> +\t\t\t}\n>> +\t\t}\n>\n> For categories you need to call pages-for-mediafiles with the titles\n> you learned (the hunk starting at l.224), but there is no need to\n> call pages-for-mediafiles in this hunk?\n\nActually it is not necessary. In here the user doesn't specify the \npages or\ncategories to be imported, so the whole wiki is imported. Thats why we \ncan\nskip the searching for relations between pages and files and directly \nask\nfor all files.\n\n>> +sub get_mw_pages_for_linked_mediafiles {\n>> +\tmy $titles = shift;\n>> +\tmy @titles = @{$titles};\n>\n> Do you really need to make a copy of this array?  Wouldn't it\n> suffice to say\n>\n> \tmy $mw_titles = join('|', @$titles);\n>\n> at the only location in this function that uses this parameter?\n\nYou are right, I will change it.\n\n>> +\tmy $pages = shift;\n>> +\n>> +\t# pattern 'page1|page2|...' required by the API\n>> +\tmy $mw_titles = join('|', @titles);\n>\n> Nobody seems to be making sure there won't be more than 500 (I am\n> assuming that this script is considered a 'bot) pages in $mw_titles\n> variable.  Shouldn't the API call be split into such batches?\n\nRight, it would be better.\n\n>> +\t# Media files could be included or linked from\n>> +\t# a page, get all related\n>> +\tmy $query = {\n>> +\t\taction => 'query',\n>> +\t\tprop => 'links|images',\n>> +\t\ttitles => $mw_titles,\n>> +\t\tplnamespace => get_mw_namespace_id(\"File\"),\n>> +\t\tpllimit => 500\n>> +\t};\n>> +\tmy $result = $mediawiki->api($query);\n>> +\n>\n>\n>> +\n>> +# Return MediaWiki id for a canonical namespace name.\n>> +# Ex.: \"File\", \"Project\".\n>> +# Looks for the namespace id in the local configuration\n>> +# variables, if it is not found asks MW API.\n>> +sub get_mw_namespace_id {\n>> +\tmw_connect_maybe();\n>> +\n>> +\tmy $name = shift;\n>> +\n>> +\t# Look at configuration file, if the record\n>> +\t# for that namespace is already stored.\n>> +\t# Namespaces are stored in form: \"Name_of_namespace:Id_namespace\",\n>> +\t# Ex.: \"File:6\".\n>> +\tmy @tracked_namespaces = split(/[ \\n]/, run_git(\"config --get-all \n>> remote.\". $remotename .\".namespaces\"));\n>> +\tchomp(@tracked_namespaces);\n>> +\tif (@tracked_namespaces) {\n>> +\t\tforeach my $ns (@tracked_namespaces) {\n>> +\t\t\tmy @ns_split = split(/:/, $ns);\n>> +\t\t\tif ($ns_split[0] eq $name) {\n>> +\t\t\t\treturn $ns_split[1];\n>> +\t\t\t}\n>> +\t\t}\n>> +\t}\n>\n> Does it make sense to call out to run_git() every time this function\n> is called?  Shoudln't this part be caching the result in a hash,\n> something like\n>\n> \tif (!exists $namespace_id{$name}) {\n> \t\t@temp = ... run_git() ...;\n>                 foreach my $ns (@temp) {\n> \t\t\tmy ($n, $s) = split(/:/, $ns);\n> \t\t\t$namespace_id{$n} = $s;\n> \t\t}\n> \t}\n>\n> \tif (!exists $namespace_id{$name}) {\n> \t\t... similarly, ask MW API and store in %namespace_id{}\n> \t}\n>\n>         if (exists $namespace_id{$name}) {\n>         \treturn $namespace_id{$name};\n> \t}\n> \tdie \"No such namespace $name\";\n\nGood idea, I will look closer on it!\n\nThanks for your review!\n"}]}