From 4fb26f4e60f2df282fc972e4b8506ccd306de789 Mon Sep 17 00:00:00 2001 From: Joey Hess Date: Wed, 29 Dec 2010 19:58:49 -0400 Subject: [PATCH] Add a second parameter to the rcs_diff hook, and avoid bloating memory reading in enormous commits. --- IkiWiki.pm | 2 +- IkiWiki/Plugin/bzr.pm | 10 ++++++--- IkiWiki/Plugin/cvs.pm | 3 ++- IkiWiki/Plugin/darcs.pm | 4 +++- IkiWiki/Plugin/git.pm | 35 ++++++++++++++++++----------- IkiWiki/Plugin/mercurial.pm | 2 +- IkiWiki/Plugin/monotone.pm | 9 ++++++-- IkiWiki/Plugin/norcs.pm | 2 +- IkiWiki/Plugin/recentchanges.pm | 2 +- IkiWiki/Plugin/recentchangesdiff.pm | 3 +-- IkiWiki/Plugin/svn.pm | 3 ++- debian/changelog | 2 ++ doc/plugins/write.mdwn | 8 ++++--- 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) = ; + my @lines; + while (my $line=) { + 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 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($)`