{"thread":{"id":"34995","subject":"[PATCH] git-remote-mediawiki: bugfix for pages w/ >500 revisions","startedAt":"2013-09-22T18:44:27Z","lastAt":"2013-09-23T18:26:01Z","messageCount":3,"participants":["Benoit Person","Matthieu Moy","Jonathan Nieder"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"228035","messageId":"1379875468-24952-1-git-send-email-benoit.person@gmail.fr","threadId":"34995","inReplyTo":null,"subject":"[PATCH] git-remote-mediawiki: bugfix for pages w/ >500 revisions","fromName":"Benoit Person","fromEmail":"benoit.person@gmail.com","sentAt":"2013-09-22T18:44:27Z","receivedAt":"2013-09-22T18:44:27Z","isPatch":true,"sender":{"key":"benoit.person@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1145270?v=4"},"body":"Mediawiki introduced a new API for queries w/ more than 500 results in\nversion 1.21. That change triggered an infinite loop while cloning a\nmediawiki with such a page.\n\nFix that while still preserving the old behavior for old APIs.\n\nSigned-off-by: Benoit Person <benoit.person@gmail.fr>\nReported-by: Benjamin Cathey\n---\n\nPatch tested for all mediawiki versions from 1.19 to 1.21.\n\nFor now, if the tests suite is run without the fix, the new test\nintroduces an infinite loop. I am not sure if this should be handled ?\n(a timeout of some kind maybe ?)\n\n contrib/mw-to-git/git-remote-mediawiki.perl     | 14 ++++++++++++--\n contrib/mw-to-git/t/t9365-continuing-queries.sh | 24 ++++++++++++++++++++++++\n 2 files changed, 36 insertions(+), 2 deletions(-)\n create mode 100755 contrib/mw-to-git/t/t9365-continuing-queries.sh\n\ndiff --git a/contrib/mw-to-git/git-remote-mediawiki.perl b/contrib/mw-to-git/git-remote-mediawiki.perl\nindex c9a4805..2d7af57 100755\n--- a/contrib/mw-to-git/git-remote-mediawiki.perl\n+++ b/contrib/mw-to-git/git-remote-mediawiki.perl\n@@ -625,6 +625,9 @@ sub fetch_mw_revisions_for_page {\n \t\trvstartid => $fetch_from,\n \t\trvlimit => 500,\n \t\tpageids => $id,\n+\n+                # let the mediawiki knows that we support the latest API\n+                continue => '',\n \t};\n \n \tmy $revnum = 0;\n@@ -640,8 +643,15 @@ sub fetch_mw_revisions_for_page {\n \t\t\tpush(@page_revs, $page_rev_ids);\n \t\t\t$revnum++;\n \t\t}\n-\t\tlast if (!$result->{'query-continue'});\n-\t\t$query->{rvstartid} = $result->{'query-continue'}->{revisions}->{rvstartid};\n+\n+                if ($result->{'query-continue'}) { # For legacy APIs\n+                    $query->{rvstartid} = $result->{'query-continue'}->{revisions}->{rvstartid};\n+                } elsif ($result->{continue}) { # For newer APIs\n+                    $query->{rvstartid} = $result->{continue}->{rvcontinue};\n+                    $query->{continue} = $result->{continue}->{continue};\n+                } else {\n+                    last;\n+                }\n \t}\n \tif ($shallow_import && @page_revs) {\n \t\tprint {*STDERR} \"  Found 1 revision (shallow import).\\n\";\ndiff --git a/contrib/mw-to-git/t/t9365-continuing-queries.sh b/contrib/mw-to-git/t/t9365-continuing-queries.sh\nnew file mode 100755\nindex 0000000..6fb5df4\n--- /dev/null\n+++ b/contrib/mw-to-git/t/t9365-continuing-queries.sh\n@@ -0,0 +1,24 @@\n+#!/bin/sh\n+\n+test_description='Test the Git Mediawiki remote helper: queries w/ more than 500 results'\n+\n+. ./test-gitmw-lib.sh\n+. ./push-pull-tests.sh\n+. $TEST_DIRECTORY/test-lib.sh\n+\n+test_check_precond\n+\n+test_expect_success 'creating page w/ >500 revisions' '\n+\twiki_reset &&\n+\tfor i in $(seq 1 501)\n+\tdo\n+\t\techo \"creating revision $i\"\n+\t\twiki_editpage foo \"revision $i<br/>\" true\n+\tdone\n+'\n+\n+test_expect_success 'cloning page w/ >500 revisions' '\n+\tgit clone mediawiki::'\"$WIKI_URL\"' mw_dir\n+'\n+\n+test_done\n-- \n1.8.4.GIT\n"},{"id":"228036","messageId":"vpqsiwwejxz.fsf@anie.imag.fr","threadId":"34995","inReplyTo":"1379875468-24952-1-git-send-email-benoit.person@gmail.fr","subject":"Re: [PATCH] git-remote-mediawiki: bugfix for pages w/ >500 revisions","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2013-09-22T19:27:04Z","receivedAt":"2013-09-22T19:27:04Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Benoit Person <benoit.person@gmail.com> writes:\n\n> Mediawiki introduced a new API for queries w/ more than 500 results in\n> version 1.21. That change triggered an infinite loop while cloning a\n> mediawiki with such a page.\n>\n> Fix that while still preserving the old behavior for old APIs.\n\nThat would be nice to explain a bit more here. Where did the infinite\nloop come from? How does your patch fix it?\n\n> For now, if the tests suite is run without the fix, the new test\n> introduces an infinite loop. I am not sure if this should be handled ?\n> (a timeout of some kind maybe ?)\n\nIf the patch fix this, then it's not a really big problem. The test\nfailure is an infinite loop. That would be problematic if ran\nnon-interactively, but I think it's Ok since we only run the testsuite\nmanually.\n\n> diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl b/contrib/mw-to-git/git-remote-mediawiki.perl\n> index c9a4805..2d7af57 100755\n> --- a/contrib/mw-to-git/git-remote-mediawiki.perl\n> +++ b/contrib/mw-to-git/git-remote-mediawiki.perl\n> @@ -625,6 +625,9 @@ sub fetch_mw_revisions_for_page {\n>  \t\trvstartid => $fetch_from,\n>  \t\trvlimit => 500,\n>  \t\tpageids => $id,\n> +\n> +                # let the mediawiki knows that we support the latest API\n\ns/knows/know/\n\n> +                continue => '',\n\nIndentation with spaces. Please, use tabs.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"228076","messageId":"20130923182601.GC9464@google.com","threadId":"34995","inReplyTo":"vpqsiwwejxz.fsf@anie.imag.fr","subject":"Re: [PATCH] git-remote-mediawiki: bugfix for pages w/ >500 revisions","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-09-23T18:26:01Z","receivedAt":"2013-09-23T18:26:01Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Matthieu Moy wrote:\n> Benoit Person <benoit.person@gmail.com> writes:\n\n>> For now, if the tests suite is run without the fix, the new test\n>> introduces an infinite loop. I am not sure if this should be handled ?\n>> (a timeout of some kind maybe ?)\n>\n> If the patch fix this, then it's not a really big problem. The test\n> failure is an infinite loop.\n\nYes, I think it's fine.\n\n>                              That would be problematic if ran\n> non-interactively, but I think it's Ok since we only run the testsuite\n> manually.\n\nSome distros (e.g., Debian) occasionally do run the testsuite\nautomatically, but it is still fine since they have a timeout that\nvaries by platform to detect if the test has stalled.  I suppose\nideally git's test harness could learn to do the same thing some day,\nbut for now it's easier one level above since an appropriate timeout\ndepends on the speed on the platform, what else is creating load on\nthe test machine, and other factors that are probably not easy for us\nto guess.\n\n(other tweaks snipped)\n\nThanks, both.\n"}]}