repobrowse: make git diff output use qspawn

This is a potentially expensive operation, so we may want to
give it it's own limiter channel.

diff --git a/lib/PublicInbox/RepobrowseGitDiff.pm b/lib/PublicInbox/RepobrowseGitDiff.pm
index 4ed4d02e02a3ac02f4c273b9a66fad98ea3f0125..eb64d1fda0d140864c3bbfb3af510c0116df22e5 100644 (file)
--- a/lib/PublicInbox/RepobrowseGitDiff.pm
+++ b/lib/PublicInbox/RepobrowseGitDiff.pm
@@ -4,128 +4,72 @@
 # shows the /diff endpoint for git repositories for cgit compatibility
 # usage: /repo.git/diff?id=COMMIT_ID&id2=COMMIT_ID2
 #
-# FIXME: much duplicated code between this and RepobrowseGitCommit.pm
-#
 # We probably will not link to this outright because it's expensive,
-# but exists to preserve URL compatibility.
+# but exists to preserve URL compatibility with cgit.
 package PublicInbox::RepobrowseGitDiff;
 use strict;
 use warnings;
 use base qw(PublicInbox::RepobrowseBase);
-use PublicInbox::Hval qw(utf8_html to_attr);
-use PublicInbox::RepobrowseGit qw(git_unquote git_commit_title);
-use PublicInbox::RepobrowseGitDiffCommon qw/git_diffstat_emit
-       git_diff_ab_index git_diff_ab_hdr git_diff_ab_hunk/;
+use PublicInbox::Hval qw(utf8_html);
+use PublicInbox::RepobrowseGitDiffCommon;
+use PublicInbox::Qspawn;
+
+sub git_diff_sed ($$) {
+       my ($self, $req) = @_;
+       git_diff_sed_init($req);
+       $req->{dstate} = DSTATE_STAT;
+       # this filters for $fh->write or $body->getline (see Qspawn)
+       sub {
+               my $dst = delete $req->{dhtml} || '';
+               if (defined $_[0]) { # $_[0] == scalar buffer
+                       $req->{dbuf} .= $_[0];
+                       git_diff_sed_run(\$dst, $req);
+               } else { # undef means EOF from "git show", flush the last bit
+                       git_diff_sed_close(\$dst, $req);
+                       $dst .= '</pre></body></html>';
+               }
+               $dst;
+       }
+}
 
 sub call_git_diff {
        my ($self, $req) = @_;
-       my $git = $req->{repo_info}->{git};
-       my $q = PublicInbox::RepobrowseGitQuery->new($req->{env});
+       my $env = $req->{env};
+       my $q = PublicInbox::RepobrowseGitQuery->new($env);
        my $id = $q->{id};
        my $id2 = $q->{id2};
 
-       my @cmd = (qw(diff-tree -z --numstat -p --encoding=UTF-8
-                       --no-notes --no-color -M -B -D -r),
-                       $id2, $id, '--');
+       my $git = $req->{repo_info}->{git};
+       my $cmd = [ 'git', "--git-dir=$git->{git_dir}", qw(diff-tree
+                       -z --numstat -p --encoding=UTF-8
+                       --no-color -M -B -D -r),
+                       $id2, $id, '--' ];
        my $expath = $req->{expath};
-       push @cmd, $expath if $expath ne '';
-       $req->{rpipe} = $git->popen(\@cmd, undef, { 2 => $git->err_begin });
-       my $env = $req->{env};
-       my $err = $env->{'psgi.errors'};
-       my ($vin);
-       $req->{dbuf} = '';
+       push @$cmd, $expath if $expath ne '';
+       my $o = { nofollow => 1, noindex => 1 };
+       my $ex = $expath eq '' ? '' : " $expath";
+       $req->{dhtml} = $self->html_start($req, 'diff', $o). "\n\n".
+                               utf8_html("git diff-tree -r -M -B -D ".
+                               "$id2 $id --$ex"). "\n\n";
        $req->{p} = [ $id2 ];
        $req->{h} = $id;
-       my $end = sub {
-               if (my $fh = delete $req->{fh}) {
-                       # write out the last bit that was buffered
-                       my @buf = split(/\n/, delete $req->{dbuf}, -1);
-                       my $s = '';
-                       $s .= git_diff_line_i($req, $_) foreach @buf;
-                       $s .= '</pre></body></html>';
-                       $fh->write($s);
-
-                       $fh->close;
-               } elsif (my $res = delete $req->{res}) {
-                       $res->($self->r(500));
-               }
-               if (my $rpipe = delete $req->{rpipe}) {
-                       $rpipe->close; # _may_ be Danga::Socket::close
+       my $rdr = { 2 => $git->err_begin };
+       my $qsp = PublicInbox::Qspawn->new($cmd, undef, $rdr);
+       # $env->{'qspawn.quiet'} = 1;
+       $qsp->psgi_return($env, undef, sub { # parse header
+               my ($r) = @_;
+               if (!defined $r) {
+                       [ 500, [ 'Content-Type', 'text/html' ], [ $git->err ]];
+               } elsif ($r == 0) {
+                       [ 200, [ 'Content-Type', 'text/html' ], [
+                               delete($req->{dhtml}).
+                               'No differences</pre></body></html>' ]
+                       ]
+               } else {
+                       $env->{'qspawn.filter'} = git_diff_sed($self, $req);
+                       [ 200, [ 'Content-Type', 'text/html' ] ];
                }
-       };
-       my $fail = sub {
-               if ($!{EAGAIN} || $!{EINTR}) {
-                       select($vin, undef, undef, undef) if defined $vin;
-                       # $vin is undef on async, so this is a noop on EAGAIN
-                       return;
-               }
-               my $e = $!;
-               $end->();
-               $err->print("git diff ($git->{git_dir}): $e\n");
-       };
-       my $cb = sub {
-               my $off = length($req->{dbuf});
-               my $n = $req->{rpipe}->sysread($req->{dbuf}, 8192, $off);
-               return $fail->() unless defined $n;
-               return $end->() if $n == 0;
-               if (my $res = delete $req->{res}) {
-                       my $h = ['Content-Type', 'text/html; charset=UTF-8'];
-                       my $fh = $req->{fh} = $res->([200, $h]);
-                       my $o = { nofollow => 1, noindex => 1 };
-                       my $ex = $expath eq '' ? '' : " $expath";
-                       $fh->write($self->html_start($req, 'diff', $o).
-                                       "\n\n".
-                                       utf8_html("git diff-tree -r -M -B -D ".
-                                               "$id2 $id --$ex"). "\n\n");
-               }
-               git_diff_to_html($req);
-       };
-       if (my $async = $env->{'pi-httpd.async'}) {
-               $req->{rpipe} = $async->($req->{rpipe}, $cb);
-               sub { $req->{res} = $_[0] } # let Danga::Socket handle the rest.
-       } else { # synchronous loop for other PSGI servers
-               $vin = '';
-               vec($vin, fileno($req->{rpipe}), 1) = 1;
-               sub {
-                       $req->{res} = $_[0];
-                       while ($req->{rpipe}) { $cb->() }
-               }
-       }
-}
-
-sub git_diff_line_i {
-       my ($req, $l) = @_;
-       my $cmt = '[a-f0-9]+';
-
-       if ($l =~ m{^diff --git ("?a/.+) ("?b/.+)$}) { # regular
-               $l = git_diff_ab_hdr($req, $1, $2);
-       } elsif ($l =~ /^index ($cmt)\.\.($cmt)(.*)$/o) { # regular
-               $l = git_diff_ab_index($1, $2, $3);
-       } elsif ($l =~ /^@@ (\S+) (\S+) @@(.*)$/) { # regular
-               $l = git_diff_ab_hunk($req, $1, $2, $3);
-       } else {
-               $l = utf8_html($l);
-       }
-       $l .= "\n";
-}
-
-sub git_diff_to_html {
-       my ($req) = @_;
-       my $fh = $req->{fh};
-       if (!$req->{diff_state}) {
-               my ($stat, $buf) = split(/\0\0/, $req->{dbuf}, 2);
-               return unless defined $buf;
-               $req->{dbuf} = $buf;
-               git_diffstat_emit($req, $fh, $stat);
-               $req->{diff_state} = 1;
-       }
-       my @buf = split(/\n/, $req->{dbuf}, -1);
-       $req->{dbuf} = pop @buf; # last line, careful...
-       if (@buf) {
-               my $s = '';
-               $s .= git_diff_line_i($req, $_) foreach @buf;
-               $fh->write($s) if $s ne '';
-       }
+       });
 }
 
 1;

diff --git a/lib/PublicInbox/RepobrowseGitDiffCommon.pm b/lib/PublicInbox/RepobrowseGitDiffCommon.pm
index ae0e68211fceda8540dfea8220694daa6dc42215..bca50e15dbaef0048e4a16c40202c58a23fd7e0a 100644 (file)
--- a/lib/PublicInbox/RepobrowseGitDiffCommon.pm
+++ b/lib/PublicInbox/RepobrowseGitDiffCommon.pm
@@ -216,6 +216,9 @@ sub git_diff_sed_stat ($$) {
        my $ndel = \($req->{ndel});
        if (!$req->{dstat_started}) {
                $req->{dstat_started} = 1;
+
+               # merges start with an extra '\0' before the diffstat
+               # non-merge commits start with an extra '\n', instead
                if ($req->{mhelp}) {
                        if ($stat[0] eq '') {
                                shift @stat;
@@ -224,8 +227,8 @@ sub git_diff_sed_stat ($$) {
 'initial merge diffstat line was not empty';
                        }
                } else {
-                       $stat[0] =~ s/\A\n//s or warn
-'failed to remove initial newline from diffstat';
+                       # for commits, only (not diff-tree)
+                       $stat[0] =~ s/\A\n//s;
                }
        }
        while (defined(my $l = shift @stat)) {