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

Mediawiki introduces 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.

The latest API renamed and moved the "continuing" information in the
response, necessary to build the next query. The code failed to retrieve
that information but still detected that it was in a "continuing
query". As a result, it launched the same query over and over again.

If a "continuing" information is detected in the response (old or new),
the next query is updated accordingly. If not, we quit assuming it's not
a continuing query.

Signed-off-by: Benoit Person <redacted>
Reported-by: Benjamin Cathey
---
 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

diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl b/contrib/mw-to-git/git-remote-mediawiki.perl
index c9a4805..f1db5d2 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 know 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

On Mon, Sep 23, 2013 at 1:26 PM, Benoit Person [off-list ref] wrote:

s/seq/test_seq/

d17cf5f3a32f07bf (tests: Introduce test_seq;  2012-08-03)

+       do
+               echo "creating revision $i"

Do you want to end this line with '&&'?

+               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
--

On 23 September 2013 19:58, Matthieu Moy [off-list ref] wrote:

I'd rather have the comments say "# API version < X" and "# API version

quoted

= X". Next time the API change, "new" Vs "old" will become meaningless.

done, thanks


On 23 September 2013 20:26, Jonathan Nieder [off-list ref] wrote:

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.

great explanation, thanks


On 23 September 2013 22:17, Eric Sunshine [off-list ref] wrote:

s/seq/test_seq/

done, thanks

d17cf5f3a32f07bf (tests: Introduce test_seq;  2012-08-03)

quoted

+       do
+               echo "creating revision $i"

Do you want to end this line with '&&'?

The way it's intended is that it's more a debug information to see how
it's going on (creating >500 revs is *quite* long). If I understand it
correctly, using '&&' would mean that the return value of the echo
statement will be tested for success ? Anyway, I am not sure it makes
sense to fail on a "debug echo" ?

-- 
Benoit Person