Add a second parameter to the rcs_diff hook, and avoid bloating memory reading in enormous commits.

13 files changed, 55 insertions, 30 deletions

diff --git a/IkiWiki.pm b/IkiWiki.pm
index bbe1ad055..1102fa52a 100644
--- a/IkiWiki.pm
+++ b/IkiWiki.pm
@@ -2033,7 +2033,7 @@ sub rcs_recentchanges ($) {
 	$hooks{rcs}{rcs_recentchanges}{call}->(@_);
 }
 
-sub rcs_diff ($) {
+sub rcs_diff ($;$) {
 	$hooks{rcs}{rcs_diff}{call}->(@_);
 }
 
diff --git a/IkiWiki/Plugin/bzr.pm b/IkiWiki/Plugin/bzr.pm
index 562d5d389..3bc4ea8dd 100644
--- a/IkiWiki/Plugin/bzr.pm
+++ b/IkiWiki/Plugin/bzr.pm
@@ -271,8 +271,9 @@ sub rcs_recentchanges ($) {
 	return @ret;
 }
 
-sub rcs_diff ($) {
+sub rcs_diff ($;$) {
 	my $taintedrev=shift;
+	my $maxlines=shift;
 	my ($rev) = $taintedrev =~ /^(\d+(\.\d+)*)$/; # untaint
 
 	my $prevspec = "before:" . $rev;
@@ -281,8 +282,11 @@ sub rcs_diff ($) {
 		"--new", $config{srcdir},
 		"-r", $prevspec . ".." . $revspec);
 	open (my $out, "@cmdline |");
-
-	my @lines = <$out>;
+	my @lines;
+	while (my $line=<$out>) {
+		last if defined $maxlines && @lines == $maxlines;
+		push @lines, $line;
+	}
 	if (wantarray) {
 		return @lines;
 	}
diff --git a/IkiWiki/Plugin/cvs.pm b/IkiWiki/Plugin/cvs.pm
index 4972efb58..71566d212 100644
--- a/IkiWiki/Plugin/cvs.pm
+++ b/IkiWiki/Plugin/cvs.pm
@@ -436,8 +436,9 @@ sub rcs_recentchanges ($) {
 	return @ret;
 }
 
-sub rcs_diff ($) {
+sub rcs_diff ($;$) {
 	my $rev=IkiWiki::possibly_foolish_untaint(int(shift));
+	my $maxlines=shift;
 
 	local $CWD = $config{srcdir};
 
diff --git a/IkiWiki/Plugin/darcs.pm b/IkiWiki/Plugin/darcs.pm
index 0f63b8807..cd4fcd0ff 100644
--- a/IkiWiki/Plugin/darcs.pm
+++ b/IkiWiki/Plugin/darcs.pm
@@ -373,11 +373,13 @@ sub rcs_recentchanges ($) {
 	return @ret;
 }
 
-sub rcs_diff ($) {
+sub rcs_diff ($;$) {
 	my $rev=shift;
+	my $maxlines=shift;
 	my @lines;
 	foreach my $line (silentsystem("darcs", "diff", "--match", "hash ".$rev)) {
 		if (@lines || $line=~/^diff/) {
+			last if defined $maxlines && @lines == $maxlines;
 			push @lines, $line."\n";
 		}
 	}
diff --git a/IkiWiki/Plugin/git.pm b/IkiWiki/Plugin/git.pm
index 3db4af729..52b2bbd50 100644
--- a/IkiWiki/Plugin/git.pm
+++ b/IkiWiki/Plugin/git.pm
@@ -152,10 +152,11 @@ sub genwrapper {
 }
 
 sub safe_git (&@) {
-	# Start a child process safely without resorting /bin/sh.
-	# Return command output or success state (in scalar context).
+	# Start a child process safely without resorting to /bin/sh.
+	# Returns command output (in list content) or success state
+	# (in scalar context), or runs the specified data handler.
 
-	my ($error_handler, @cmdline) = @_;
+	my ($error_handler, $data_handler, @cmdline) = @_;
 
 	my $pid = open my $OUT, "-|";
 
@@ -187,7 +188,12 @@ sub safe_git (&@) {
 
 		chomp;
 
-		push @lines, $_;
+		if (! defined $data_handler) {
+			push @lines, $_;
+		}
+		else {
+			last unless $data_handler->($_);
+		}
 	}
 
 	close $OUT;
@@ -197,9 +203,9 @@ sub safe_git (&@) {
 	return wantarray ? @lines : ($? == 0);
 }
 # Convenient wrappers.
-sub run_or_die ($@) { safe_git(\&error, @_) }
-sub run_or_cry ($@) { safe_git(sub { warn @_ },  @_) }
-sub run_or_non ($@) { safe_git(undef,            @_) }
+sub run_or_die ($@) { safe_git(\&error, undef, @_) }
+sub run_or_cry ($@) { safe_git(sub { warn @_ }, undef, @_) }
+sub run_or_non ($@) { safe_git(undef, undef, @_) }
 
 
 sub merge_past ($$$) {
@@ -663,15 +669,18 @@ sub rcs_recentchanges ($) {
 	return @rets;
 }
 
-sub rcs_diff ($) {
+sub rcs_diff ($;$) {
 	my $rev=shift;
+	my $maxlines=shift;
 	my ($sha1) = $rev =~ /^($sha1_pattern)$/; # untaint
 	my @lines;
-	foreach my $line (run_or_non("git", "show", $sha1)) {
-		if (@lines || $line=~/^diff --git/) {
-			push @lines, $line."\n";
-		}
-	}
+	my $addlines=sub {
+		my $line=shift;
+		return if defined $maxlines && @lines == $maxlines;
+		push @lines, $line."\n"
+			if (@lines || $line=~/^diff --git/);
+	};
+	safe_git(undef, $addlines, "git", "show", $sha1);
 	if (wantarray) {
 		return @lines;
 	}
diff --git a/IkiWiki/Plugin/mercurial.pm b/IkiWiki/Plugin/mercurial.pm
index 59dc63b4e..d7399eaf0 100644
--- a/IkiWiki/Plugin/mercurial.pm
+++ b/IkiWiki/Plugin/mercurial.pm
@@ -229,7 +229,7 @@ sub rcs_recentchanges ($) {
 	return @ret;
 }
 
-sub rcs_diff ($) {
+sub rcs_diff ($;$) {
 	# TODO
 }
 
diff --git a/IkiWiki/Plugin/monotone.pm b/IkiWiki/Plugin/monotone.pm
index 02690b10e..38313a542 100644
--- a/IkiWiki/Plugin/monotone.pm
+++ b/IkiWiki/Plugin/monotone.pm
@@ -621,8 +621,9 @@ sub rcs_recentchanges ($) {
 	return @ret;
 }
 
-sub rcs_diff ($) {
+sub rcs_diff ($;$) {
 	my $rev=shift;
+	my $maxlines=shift;
 	my ($sha1) = $rev =~ /^($sha1_pattern)$/; # untaint
 	
 	chdir $config{srcdir}
@@ -633,7 +634,11 @@ sub rcs_diff ($) {
 		exec("mtn", "diff", "--root=$config{mtnrootdir}", "-r", "p:".$sha1, "-r", $sha1) || error("mtn diff $sha1 failed to run");
 	}
 
-	my (@lines) = <MTNDIFF>;
+	my @lines;
+	while (my $line=<MTNDIFF>) {
+		last if defined $maxlines && @lines == $maxlines;
+		push @lines, $line;
+	}
 
 	close MTNDIFF || debug("mtn diff $sha1 exited $?");
 
diff --git a/IkiWiki/Plugin/norcs.pm b/IkiWiki/Plugin/norcs.pm
index a3bb6240e..6fa8bfa3a 100644
--- a/IkiWiki/Plugin/norcs.pm
+++ b/IkiWiki/Plugin/norcs.pm
@@ -58,7 +58,7 @@ sub rcs_rename ($$) {
 sub rcs_recentchanges ($) {
 }
 
-sub rcs_diff ($) {
+sub rcs_diff ($;$) {
 }
 
 sub rcs_getctime ($) {
diff --git a/IkiWiki/Plugin/recentchanges.pm b/IkiWiki/Plugin/recentchanges.pm
index 6fccd16f6..3081ac131 100644
--- a/IkiWiki/Plugin/recentchanges.pm
+++ b/IkiWiki/Plugin/recentchanges.pm
@@ -121,7 +121,7 @@ sub sessioncgi ($$) {
 	}
 	elsif ($form->submitted ne 'Cancel') {
 	        $form->title(sprintf(gettext("confirm reversion of %s"), $rev));
-		$form->tmpl_param(diff => encode_entities(scalar IkiWiki::rcs_diff($rev)));
+		$form->tmpl_param(diff => encode_entities(scalar IkiWiki::rcs_diff($rev, 200)));
 		$form->field(name => "rev", type => "hidden", value => $rev, force => 1);
 		IkiWiki::showform($form, $buttons, $session, $q);
 		exit 0;
diff --git a/IkiWiki/Plugin/recentchangesdiff.pm b/IkiWiki/Plugin/recentchangesdiff.pm
index e3ba9b8d8..71297572d 100644
--- a/IkiWiki/Plugin/recentchangesdiff.pm
+++ b/IkiWiki/Plugin/recentchangesdiff.pm
@@ -28,11 +28,10 @@ sub pagetemplate (@) {
 	my $template=$params{template};
 	if ($config{rcs} && exists $params{rev} && length $params{rev} &&
 	    $template->query(name => "diff")) {
-		my @lines=IkiWiki::rcs_diff($params{rev});
+		my @lines=IkiWiki::rcs_diff($params{rev}, $maxlines+1);
 		if (@lines) {
 			my $diff;
 			if (@lines > $maxlines) {
-				# only include so many lines of diff
 				$diff=join("", @lines[0..($maxlines-1)])."\n".
 					gettext("(Diff truncated)");
 			}
diff --git a/IkiWiki/Plugin/svn.pm b/IkiWiki/Plugin/svn.pm
index 9cf82b5db..faaf567d5 100644
--- a/IkiWiki/Plugin/svn.pm
+++ b/IkiWiki/Plugin/svn.pm
@@ -345,8 +345,9 @@ sub rcs_recentchanges ($) {
 	return @ret;
 }
 
-sub rcs_diff ($) {
+sub rcs_diff ($;$) {
 	my $rev=IkiWiki::possibly_foolish_untaint(int(shift));
+	my $maxlines=shift;
 	return `svnlook diff $config{svnrepo} -r$rev --no-diff-deleted`;
 }
 
diff --git a/debian/changelog b/debian/changelog
index b06fe44e6..bf092d012 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -25,6 +25,8 @@ ikiwiki (3.20101202) UNRELEASED; urgency=low
     versions of the monotone binary. (tommyd3mdi)
   * highlight: Support highlight 3.2+svn19 (note that released version 3.2
     is not supported). Closes: #605779 (David Bremner)
+  * Add a second parameter to the rcs_diff hook, and avoid bloating memory
+    reading in enormous commits.
 
  -- Joey Hess <joeyh@debian.org>  Mon, 29 Nov 2010 14:44:13 -0400
 
diff --git a/doc/plugins/write.mdwn b/doc/plugins/write.mdwn
index adc20af72..f0f79ebc7 100644
--- a/doc/plugins/write.mdwn
+++ b/doc/plugins/write.mdwn
@@ -1151,11 +1151,13 @@ The data structure returned for each change is:
 		],
 	}
 
-#### `rcs_diff($)`
+#### `rcs_diff($;$)`
+
+The first parameter is the rev from `rcs_recentchanges`.
+The optional second parameter is how many lines to return (default: all).
 
-The parameter is the rev from `rcs_recentchanges`.
 Should return a list of lines of the diff (including \n) in list
-context, and the whole diff in scalar context.
+context, and a string containing the whole diff in scalar context.
 
 #### `rcs_getctime($)`