threads / patch / 34995

patchgit-remote-mediawiki: bugfix for pages w/ >500 revisions

Subject: [PATCH] git-remote-mediawiki: bugfix for pages w/ >500 revisions

## tl;dr

3 messages between Sep 22, 2013 and Sep 23, 2013. Diffs are folded; open one to read it.

replies: 2people: 3as markdown or json

Benoit Person· Sep 22, 2013, 18:44 UTC · lore

Mediawiki introduced a new API for queries w/ more than 500 results in version 1.21. That change triggered an infinite loop while cloning a mediawiki with such a page.

Fix that while still preserving the old behavior for old APIs.

Signed-off-by: Benoit Person <benoit.person@gmail.fr>
Reported-by: Benjamin Cathey
---

Patch tested for all mediawiki versions from 1.19 to 1.21.

For now, if the tests suite is run without the fix, the new test introduces an infinite loop. I am not sure if this should be handled ? (a timeout of some kind maybe ?)

 contrib/mw-to-git/git-remote-mediawiki.perl     | 14 ++++++++++++--
 contrib/mw-to-git/t/t9365-continuing-queries.sh | 24 ++++++++++++++++++++++++
 2 files changed, 36 insertions(+), 2 deletions(-)
 create mode 100755 contrib/mw-to-git/t/t9365-continuing-queries.sh

Show changes to 2 files +36 −2

contrib/mw-to-git/git-remote-mediawiki.perl, contrib/mw-to-git/t/t9365-continuing-queries.sh

diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl b/contrib/mw-to-git/git-remote-mediawiki.perl
index c9a4805..2d7af57 100755
--- a/contrib/mw-to-git/git-remote-mediawiki.perl
+++ b/contrib/mw-to-git/git-remote-mediawiki.perl
@@ -625,6 +625,9 @@ sub fetch_mw_revisions_for_page {
 		rvstartid => $fetch_from,
 		rvlimit => 500,
 		pageids => $id,
+
+                # let the mediawiki knows that we support the latest API
+                continue => '',
 	};
 
 	my $revnum = 0;
@@ -640,8 +643,15 @@ sub fetch_mw_revisions_for_page {
 			push(@page_revs, $page_rev_ids);
 			$revnum++;
 		}
-		last if (!$result->{'query-continue'});
-		$query->{rvstartid} = $result->{'query-continue'}->{revisions}->{rvstartid};
+
+                if ($result->{'query-continue'}) { # For legacy APIs
+                    $query->{rvstartid} = $result->{'query-continue'}->{revisions}->{rvstartid};
+                } elsif ($result->{continue}) { # For newer APIs
+                    $query->{rvstartid} = $result->{continue}->{rvcontinue};
+                    $query->{continue} = $result->{continue}->{continue};
+                } else {
+                    last;
+                }
 	}
 	if ($shallow_import && @page_revs) {
 		print {*STDERR} "  Found 1 revision (shallow import).\n";
diff --git a/contrib/mw-to-git/t/t9365-continuing-queries.sh b/contrib/mw-to-git/t/t9365-continuing-queries.sh
new file mode 100755
index 0000000..6fb5df4
--- /dev/null
+++ b/contrib/mw-to-git/t/t9365-continuing-queries.sh
@@ -0,0 +1,24 @@
+#!/bin/sh
+
+test_description='Test the Git Mediawiki remote helper: queries w/ more than 500 results'
+
+. ./test-gitmw-lib.sh
+. ./push-pull-tests.sh
+. $TEST_DIRECTORY/test-lib.sh
+
+test_check_precond
+
+test_expect_success 'creating page w/ >500 revisions' '
+	wiki_reset &&
+	for i in $(seq 1 501)
+	do
+		echo "creating revision $i"
+		wiki_editpage foo "revision $i<br/>" true
+	done
+'
+
+test_expect_success 'cloning page w/ >500 revisions' '
+	git clone mediawiki::'"$WIKI_URL"' mw_dir
+'
+
+test_done

-- 
1.8.4.GIT

Matthieu Moy· Sep 22, 2013, 19:27 UTC · re: Benoit Person · lore

Re: [PATCH] git-remote-mediawiki: bugfix for pages w/ >500 revisions

Benoit Person <benoit.person@gmail.com> writes:

Show 5 quoted lines

> Mediawiki introduced a new API for queries w/ more than 500 results in
> version 1.21. That change triggered an infinite loop while cloning a
> mediawiki with such a page.
>
> Fix that while still preserving the old behavior for old APIs.

That would be nice to explain a bit more here. Where did the infinite loop come from? How does your patch fix it?

> For now, if the tests suite is run without the fix, the new test
> introduces an infinite loop. I am not sure if this should be handled ?
> (a timeout of some kind maybe ?)

If the patch fix this, then it's not a really big problem. The test failure is an infinite loop. That would be problematic if ran non-interactively, but I think it's Ok since we only run the testsuite manually.

Show 10 quoted lines

> diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl b/contrib/mw-to-git/git-remote-mediawiki.perl
> index c9a4805..2d7af57 100755
> --- a/contrib/mw-to-git/git-remote-mediawiki.perl
> +++ b/contrib/mw-to-git/git-remote-mediawiki.perl
> @@ -625,6 +625,9 @@ sub fetch_mw_revisions_for_page {
>  		rvstartid => $fetch_from,
>  		rvlimit => 500,
>  		pageids => $id,
> +
> +                # let the mediawiki knows that we support the latest API

s/knows/know/

> +                continue => '',

Indentation with spaces. Please, use tabs.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Jonathan Nieder· Sep 23, 2013, 18:26 UTC · re: Matthieu Moy · lore

Re: [PATCH] git-remote-mediawiki: bugfix for pages w/ >500 revisions

Matthieu Moy wrote:

> Benoit Person <benoit.person@gmail.com> writes:

Show 6 quoted lines

>> For now, if the tests suite is run without the fix, the new test
>> introduces an infinite loop. I am not sure if this should be handled ?
>> (a timeout of some kind maybe ?)
>
> If the patch fix this, then it's not a really big problem. The test
> failure is an infinite loop.

Yes, I think it's fine.

>                              That would be problematic if ran
> non-interactively, but I think it's Ok since we only run the testsuite
> manually.

Some distros (e.g., Debian) occasionally do run the testsuite automatically, but it is still fine since they have a timeout that varies by platform to detect if the test has stalled. I suppose ideally git's test harness could learn to do the same thing some day, but for now it's easier one level above since an appropriate timeout depends on the speed on the platform, what else is creating load on the test machine, and other factors that are probably not easy for us to guess.

(other tweaks snipped)

Thanks, both.