user/dev discussion of public-inbox itself
 help / color / mirror / code / Atom feed
From: Eric Wong <e@80x24.org>
To: meta@public-inbox.org
Subject: [PATCH 09/14] solver: rewrite to use Qspawn->psgi_qx and pi-httpd.async
Date: Sun, 27 Jan 2019 04:03:36 +0000	[thread overview]
Message-ID: <20190127040341.26107-10-e@80x24.org> (raw)
In-Reply-To: <20190127040341.26107-1-e@80x24.org>

The psgi_qx routine in the now-abandoned "repobrowse" branch
allows us to break down blob-solving at each process execution
point.  It reuses the Qspawn facility for git-http-backend(1),
allowing us to limit parallel subprocesses independently of Perl
worker count.

This is actually a 2-3% slower a fully-synchronous execution;
but it is fair to other clients as it won't monopolize the server
for hundreds of milliseconds (or even seconds) at a time.
---
 lib/PublicInbox/HTTPD/Async.pm |   9 +
 lib/PublicInbox/SolverGit.pm   | 467 ++++++++++++++++++---------------
 lib/PublicInbox/ViewVCS.pm     |  51 ++--
 t/solver_git.t                 |  22 +-
 4 files changed, 313 insertions(+), 236 deletions(-)

diff --git a/lib/PublicInbox/HTTPD/Async.pm b/lib/PublicInbox/HTTPD/Async.pm
index a1f7551..a647f10 100644
--- a/lib/PublicInbox/HTTPD/Async.pm
+++ b/lib/PublicInbox/HTTPD/Async.pm
@@ -14,6 +14,15 @@ require PublicInbox::EvCleanup;
 
 sub new {
 	my ($class, $io, $cb, $cleanup) = @_;
+
+	# no $io? call $cb at the top of the next event loop to
+	# avoid recursion:
+	unless (defined($io)) {
+		PublicInbox::EvCleanup::asap($cb) if $cb;
+		PublicInbox::EvCleanup::next_tick($cleanup) if $cleanup;
+		return;
+	}
+
 	my $self = fields::new($class);
 	IO::Handle::blocking($io, 0);
 	$self->SUPER::new($io);
diff --git a/lib/PublicInbox/SolverGit.pm b/lib/PublicInbox/SolverGit.pm
index 53a6262..a7a9a0a 100644
--- a/lib/PublicInbox/SolverGit.pm
+++ b/lib/PublicInbox/SolverGit.pm
@@ -15,23 +15,41 @@ use Fcntl qw(SEEK_SET);
 use PublicInbox::Git qw(git_unquote git_quote);
 use PublicInbox::Spawn qw(spawn popen_rd);
 use PublicInbox::MsgIter qw(msg_iter msg_part_text);
+use PublicInbox::Qspawn;
 use URI::Escape qw(uri_escape_utf8);
 
+# di = diff info / a hashref with information about a diff ($di):
+# {
+#	oid_a => abbreviated pre-image oid,
+#	oid_b => abbreviated post-image oid,
+#	tmp => anonymous file handle with the diff,
+#	hdr_lines => arrayref of various header lines for mode information
+#	mode_a => original mode of oid_a (string, not integer),
+#	ibx => PublicInbox::Inbox object containing the diff
+#	smsg => PublicInbox::SearchMsg object containing diff
+#	path_a => pre-image path
+#	path_b => post-image path
+# }
+
 # don't bother if somebody sends us a patch with these path components,
 # it's junk at best, an attack attempt at worse:
 my %bad_component = map { $_ => 1 } ('', '.', '..');
 
-sub new {
-	my ($class, $gits, $inboxes) = @_;
-	bless {
-		gits => $gits,
-		inboxes => $inboxes,
-	}, $class;
+sub dbg ($$) {
+	print { $_[0]->{out} } $_[1], "\n" or ERR($_[0], "print(dbg): $!");
+}
+
+sub ERR ($$) {
+	my ($self, $err) = @_;
+	print { $self->{out} } $err, "\n";
+	my $ucb = delete($self->{user_cb});
+	eval { $ucb->($err) } if $ucb;
+	die $err;
 }
 
 # look for existing blobs already in git repos
-sub solve_existing ($$$) {
-	my ($self, $out, $want) = @_;
+sub solve_existing ($$) {
+	my ($self, $want) = @_;
 	my $oid_b = $want->{oid_b};
 	my @ambiguous; # Array of [ git, $oids]
 	foreach my $git (@{$self->{gits}}) {
@@ -50,25 +68,13 @@ sub solve_existing ($$$) {
 		# TODO: do something with the ambiguous array?
 		# push @ambiguous, [ $git, @oids ];
 
-		print $out "`$oid_b' ambiguous in ",
-				join("\n", $git->pub_urls), "\n",
-				join('', map { "$_ blob\n" } @oids), "\n";
+		dbg($self, "`$oid_b' ambiguous in " .
+				join("\n\t", $git->pub_urls) . "\n" .
+				join('', map { "$_ blob\n" } @oids));
 	}
 	scalar(@ambiguous) ? \@ambiguous : undef;
 }
 
-# returns a hashref with information about a diff ($di):
-# {
-#	oid_a => abbreviated pre-image oid,
-#	oid_b => abbreviated post-image oid,
-#	tmp => anonymous file handle with the diff,
-#	hdr_lines => arrayref of various header lines for mode information
-#	mode_a => original mode of oid_a (string, not integer),
-#	ibx => PublicInbox::Inbox object containing the diff
-#	smsg => PublicInbox::SearchMsg object containing diff
-#	path_a => pre-image path
-#	path_b => post-image path
-# }
 sub extract_diff ($$$$) {
 	my ($p, $re, $ibx, $smsg) = @_;
 	my ($part) = @$p; # ignore $depth and @idx;
@@ -182,11 +188,51 @@ sub find_extract_diff ($$$) {
 	}
 }
 
+sub prepare_index ($) {
+	my ($self) = @_;
+	my $patches = $self->{patches};
+	$self->{nr} = 0;
+	$self->{tot} = scalar @$patches;
+
+	my $di = $patches->[0] or die 'no patches';
+	my $oid_a = $di->{oid_a} or die '{oid_a} unset';
+	my $existing = $self->{found}->{$oid_a};
+
+	# no index creation for added files
+	$oid_a =~ /\A0+\z/ and return next_step($self);
+
+	die "BUG: $oid_a not not found" unless $existing;
+
+	my $oid_full = $existing->[1];
+	my $path_a = $di->{path_a} or die "BUG: path_a missing for $oid_full";
+	my $mode_a = $di->{mode_a} || extract_old_mode($di);
+
+	open my $in, '+>', undef or die "open: $!";
+	print $in "$mode_a $oid_full\t$path_a\0" or die "print: $!";
+	$in->flush or die "flush: $!";
+	sysseek($in, 0, 0) or die "seek: $!";
+
+	dbg($self, 'preparing index');
+	my $rdr = { 0 => fileno($in) };
+	my $cmd = [ qw(git -C), $self->{wt_dir},
+			qw(update-index -z --index-info) ];
+	my $qsp = PublicInbox::Qspawn->new($cmd, undef, $rdr);
+	$qsp->psgi_qx($self->{psgi_env}, undef, sub {
+		my ($bref) = @_;
+		if (my $err = $qsp->{err}) {
+			ERR($self, "git update-index error: $err");
+		}
+		dbg($self, "index prepared:\n" .
+			"$mode_a $oid_full\t" . git_quote($path_a));
+		next_step($self); # onto do_git_apply
+	});
+}
+
 # pure Perl "git init"
 sub do_git_init_wt ($) {
 	my ($self) = @_;
 	my $wt = File::Temp->newdir('solver.wt-XXXXXXXX', TMPDIR => 1);
-	my $dir = $wt->dirname;
+	my $dir = $self->{wt_dir} = $wt->dirname;
 
 	foreach ('', qw(objects refs objects/info refs/heads)) {
 		mkdir("$dir/.git/$_") or die "mkdir $_: $!";
@@ -211,7 +257,9 @@ EOF
 	print($fh (map { "$_->{git_dir}/objects\n" } @{$self->{gits}})) or
 		die "print $f: $!";
 	close $fh or die "close: $f: $!";
-	$wt;
+	my $wt_git = $self->{wt_git} = PublicInbox::Git->new("$dir/.git");
+	$wt_git->{-wt} = $wt;
+	prepare_index($self);
 }
 
 sub extract_old_mode ($) {
@@ -222,232 +270,227 @@ sub extract_old_mode ($) {
 	'100644';
 }
 
-sub reap ($$) {
-	my ($pid, $msg) = @_;
-	waitpid($pid, 0) == $pid or die "waitpid($msg): $!";
-	$? == 0 or die "$msg failed: $?";
+sub do_step ($) {
+	my ($self) = @_;
+	eval {
+		# step 1: resolve blobs to patches in the todo queue
+		if (my $want = pop @{$self->{todo}}) {
+			# this populates {patches} and {todo}
+			resolve_patch($self, $want);
+
+		# step 2: then we instantiate a working tree once
+		# the todo queue is finally empty:
+		} elsif (!defined($self->{wt_git})) {
+			do_git_init_wt($self);
+
+		# step 3: apply each patch in the stack
+		} elsif (scalar @{$self->{patches}}) {
+			do_git_apply($self);
+
+		# step 4: execute the user-supplied callback with
+		# our result: (which may be undef)
+		# Other steps may call user_cb to terminate prematurely
+		# on error
+		} elsif (my $ucb = delete($self->{user_cb})) {
+			$ucb->($self->{found}->{$self->{oid_want}});
+		} else {
+			die 'about to call user_cb twice'; # Oops :x
+		}
+	}; # eval
+	my $err = $@;
+	if ($err) {
+		$err =~ s/^\s*Exception:\s*//; # bad word to show users :P
+		dbg($self, "E: $err");
+		my $ucb = delete($self->{user_cb});
+		eval { $ucb->($err) } if $ucb;
+	}
 }
 
-sub prepare_index ($$$$) {
-	my ($out, $wt_dir, $existing, $di) = @_;
-	my $oid_full = $existing->[1];
-	my ($r, $w);
-	my $path_a = $di->{path_a} or die "BUG: path_a missing for $oid_full";
-	my $mode_a = $di->{mode_a} || extract_old_mode($di);
-
-	# unlike git-apply(1), this only gets called once in a patch
-	# series and happens too quickly to be worth making async:
-	pipe($r, $w) or die "pipe: $!";
-	my $rdr = { 0 => fileno($r) };
-	my $pid = spawn([qw(git -C), $wt_dir,
-			 qw(update-index -z --index-info)], undef, $rdr);
-	close $r or die "close pipe(r): $!";
-	print $w "$mode_a $oid_full\t$path_a\0" or die "print update-index: $!";
-
-	close $w or die "close update-index: $!";
-	reap($pid, 'update-index -z --index-info');
-
-	print $out "index prepared:\n",
-		"$mode_a $oid_full\t", git_quote($path_a), "\n";
+sub step_cb ($) {
+	my ($self) = @_;
+	sub { do_step($self) };
 }
 
-sub do_apply_begin ($$$) {
-	my ($out, $wt_dir, $di) = @_;
-
-	my $tmp = delete $di->{tmp} or die "BUG: no tmp ", di_url($di);
-	$tmp->flush or die "tmp->flush failed: $!";
-	$out->flush or die "err->flush failed: $!";
-	sysseek($tmp, 0, SEEK_SET) or die "sysseek(tmp) failed: $!";
-
-	defined(my $err_fd = fileno($out)) or die "fileno(out): $!";
-	my $rdr = { 0 => fileno($tmp), 1 => $err_fd, 2 => $err_fd };
-
-	# we need --ignore-whitespace because some patches are CRLF
-	my $cmd = [ qw(git -C), $wt_dir,
-	            qw(apply --cached --ignore-whitespace
-		       --whitespace=warn --verbose) ];
-	spawn($cmd, undef, $rdr);
+sub next_step ($) {
+	my ($self) = @_;
+	# if outside of public-inbox-httpd, caller is expected to be
+	# looping step_cb, anyways
+	my $async = $self->{psgi_env}->{'pi-httpd.async'} or return;
+	# PublicInbox::HTTPD::Async->new
+	$async->(undef, step_cb($self));
 }
 
-sub do_apply_continue ($$) {
-	my ($wt_dir, $apply_pid) = @_;
-	reap($apply_pid, 'apply');
-	popen_rd([qw(git -C), $wt_dir, qw(ls-files -s -z)]);
+sub mark_found ($$$) {
+	my ($self, $oid, $found_info) = @_;
+	$self->{found}->{$oid} = $found_info;
 }
 
-sub do_apply_end ($$$$) {
-	my ($out, $wt_git, $rd, $di) = @_;
+sub parse_ls_files ($$$$) {
+	my ($self, $qsp, $bref, $di) = @_;
+	if (my $err = $qsp->{err}) {
+		die "git ls-files error: $err";
+	}
 
-	local $/ = "\0";
-	defined(my $line = <$rd>) or die "failed to read ls-files: $!";
-	chomp $line or die "no trailing \\0 in [$line] from ls-files";
+	my ($line, @extra) = split(/\0/, $$bref);
+	scalar(@extra) and die "BUG: extra files in index: <",
+				join('> <', @extra), ">";
 
 	my ($info, $file) = split(/\t/, $line, 2);
 	my ($mode_b, $oid_b_full, $stage) = split(/ /, $info);
+	if ($file ne $di->{path_b}) {
+		die
+"BUG: index mismatch: file=$file != path_b=$di->{path_b}";
+	}
 
-	defined($line = <$rd>) and die "extra files in index: $line";
-	close $rd or die "close ls-files: $?";
-
-	$file eq $di->{path_b} or
-		die "index mismatch: file=$file != path_b=$di->{path_b}";
-
+	my $wt_git = $self->{wt_git} or die 'no git working tree';
 	my (undef, undef, $size) = $wt_git->check($oid_b_full);
+	defined($size) or die "check $oid_b_full failed";
 
-	defined($size) or die "failed to read_size from $oid_b_full";
-
-	print $out "$mode_b $oid_b_full\t$file\n";
-	[ $wt_git, $oid_b_full, 'blob', $size, $di ];
+	dbg($self, "index at:\n$mode_b $oid_b_full\t$file");
+	my $created = [ $wt_git, $oid_b_full, 'blob', $size, $di ];
+	mark_found($self, $di->{oid_b}, $created);
+	next_step($self); # onto the next patch
 }
 
-sub di_url ($) {
-	my ($di) = @_;
-	# note: we don't pass the PSGI env here, different inboxes
-	# can have different HTTP_HOST on the same instance.
-	my $url = $di->{ibx}->base_url;
-	my $mid = $di->{smsg}->{mid};
-	defined($url) ? "$url$mid/" : "<$mid>";
+sub start_ls_files ($$) {
+	my ($self, $di) = @_;
+	my $cmd = [qw(git -C), $self->{wt_dir}, qw(ls-files -s -z)];
+	my $qsp = PublicInbox::Qspawn->new($cmd);
+	$qsp->psgi_qx($self->{psgi_env}, undef, sub {
+		my ($bref) = @_;
+		eval { parse_ls_files($self, $qsp, $bref, $di) };
+		ERR($self, $@) if $@;
+	});
 }
 
-# reconstruct the oid_b blob using patches we found:
-sub apply_patches_cb ($$$$$) {
-	my ($self, $out, $found, $patches, $oid_b) = @_;
-
-	my $tot = scalar(@$patches) or return sub {
-		print $out "no patch(es) for $oid_b\n";
-		undef;
-	};
-
-	my $wt = do_git_init_wt($self);
-	my $wt_dir = $wt->dirname;
-	my $wt_git = PublicInbox::Git->new("$wt_dir/.git");
-	$wt_git->{-wt} = $wt;
-
-	my $cur = 0;
-	my ($apply_pid, $rd, $di);
-
-	# returns an empty string if in progress, undef if not found,
-	# or the final [ ::Git, oid_full, type, size, $di ] arrayref
-	# if found
-	sub {
-		if ($rd) {
-			$found->{$di->{oid_b}} =
-					do_apply_end($out, $wt_git, $rd, $di);
-			$rd = undef;
-			# continue to shift @$patches
-		} elsif ($apply_pid) {
-			$rd = do_apply_continue($wt_dir, $apply_pid);
-			$apply_pid = undef;
-			return ''; # $rd => do_apply_ned
-		}
+sub do_git_apply ($) {
+	my ($self) = @_;
 
-		# may return undef here
-		$di = shift @$patches or return $found->{$oid_b};
+	my $di = shift @{$self->{patches}} or die 'empty {patches}';
+	my $tmp = delete $di->{tmp} or die 'no tmp ', di_url($self, $di);
+	$tmp->flush or die "tmp->flush failed: $!";
+	sysseek($tmp, 0, SEEK_SET) or die "sysseek(tmp) failed: $!";
 
-		my $i = ++$cur;
-		my $oid_a = $di->{oid_a};
-		my $existing = $found->{$oid_a};
-		my $empty_oid = $oid_a =~ /\A0+\z/;
+	my $i = ++$self->{nr};
+	dbg($self, "\napplying [$i/$self->{tot}] " . di_url($self, $di) .
+		"\n" . join('', @{$di->{hdr_lines}}));
 
-		if ($empty_oid && $i != 1) {
-			die "empty oid at [$i/$tot] ", di_url($di);
-		}
-		if (!$existing && !$empty_oid) {
-			die "missing $oid_a at [$i/$tot] ", di_url($di);
+	# we need --ignore-whitespace because some patches are CRLF
+	my $cmd = [ qw(git -C), $self->{wt_dir},
+	            qw(apply --cached --ignore-whitespace
+		       --whitespace=warn --verbose) ];
+	my $rdr = { 0 => fileno($tmp), 2 => 1 };
+	my $qsp = PublicInbox::Qspawn->new($cmd, undef, $rdr);
+	$qsp->psgi_qx($self->{psgi_env}, undef, sub {
+		my ($bref) = @_;
+		close $tmp;
+		dbg($self, $$bref);
+		if (my $err = $qsp->{err}) {
+			ERR($self, "git apply error: $err");
 		}
+		eval { start_ls_files($self, $di) };
+		ERR($self, $@) if $@;
+	});
+}
 
-		# prepare the worktree for patch application:
-		if ($i == 1 && $existing) {
-			prepare_index($out, $wt_dir, $existing, $di);
-		}
+sub di_url ($$) {
+	my ($self, $di) = @_;
+	# note: we don't pass the PSGI env unconditionally, here,
+	# different inboxes can have different HTTP_HOST on the same instance.
+	my $ibx = $di->{ibx};
+	my $env = $self->{psgi_env} if $ibx eq $self->{inboxes}->[0];
+	my $url = $ibx->base_url($env);
+	my $mid = $di->{smsg}->{mid};
+	defined($url) ? "$url$mid/" : "<$mid>";
+}
 
-		print $out "\napplying [$i/$tot] ", di_url($di), "\n",
-			   join('', @{$di->{hdr_lines}}), "\n"
-			or die "print \$out failed: $!";
+sub resolve_patch ($$) {
+	my ($self, $want) = @_;
 
-		# begin the patch application patch!
-		$apply_pid = do_apply_begin($out, $wt_dir, $di);
-		# next call to this callback will call do_apply_continue
-		'';
+	if (scalar(@{$self->{patches}}) > $self->{max_patch}) {
+		die "Aborting, too many steps to $self->{oid_want}";
 	}
-}
 
-# recreate $oid_b
-# Returns an array ref: [ ::Git object, oid_full, type, size, di ]
-# or undef if nothing was found.
-#
-# TODO: complete the migration of this and ViewVCS into an evented
-# model for fairness
-sub solve ($$$$) {
-	my ($self, $out, $oid_b, $hints) = @_;
+	# see if we can find the blob in an existing git repo:
+	my $cur_want = $want->{oid_b};
+	if (my $existing = solve_existing($self, $want)) {
+		dbg($self, "found $cur_want in " .
+			join("\n", $existing->[0]->pub_urls));
 
-	# should we even get here? Probably not, but somebody
-	# could be manually typing URLs:
-	return if $oid_b =~ /\A0+\z/;
-
-	my $req = { %$hints, oid_b => $oid_b };
-	my @todo = ($req);
-	my $found = {}; # { abbrev => [ ::Git, oid_full, type, size, $di ] }
-	my $patches = []; # [ array of $di hashes ]
-	my $max = $self->{max_patches} || 200;
-	my $apply_cb;
-	my $cb = sub {
-		my $want = pop @todo;
-		unless ($want) {
-			$apply_cb ||= apply_patches_cb($self, $out, $found,
-			                               $patches, $oid_b);
-			return $apply_cb->();
-		}
-
-		if (scalar(@$patches) > $max) {
-			print $out "Aborting, too many steps to $oid_b\n";
+		if ($cur_want eq $self->{oid_want}) { # all done!
+			eval { delete($self->{user_cb})->($existing) };
+			die "E: $@" if $@;
 			return;
 		}
-		# see if we can find the blob in an existing git repo:
-		my $want_oid = $want->{oid_b};
-		if (my $existing = solve_existing($self, $out, $want)) {
-			print $out "found $want_oid in ",
-				join("\n", $existing->[0]->pub_urls), "\n";
-
-			return $existing if $want_oid eq $oid_b; # DONE!
-			$found->{$want_oid} = $existing;
-			return ''; # ok, one blob resolved, more to go?
-		}
-
-		# scan through inboxes to look for emails which results in
-		# the oid we want:
-		my $di;
-		foreach my $ibx (@{$self->{inboxes}}) {
-			$di = find_extract_diff($self, $ibx, $want) or next;
+		mark_found($self, $cur_want, $existing);
+		return next_step($self); # onto patch application
+	}
 
-			unshift @$patches, $di;
-			print $out "found $want_oid in ",di_url($di),"\n";
+	# scan through inboxes to look for emails which results in
+	# the oid we want:
+	my $di;
+	foreach my $ibx (@{$self->{inboxes}}) {
+		$di = find_extract_diff($self, $ibx, $want) or next;
 
-			# good, we can find a path to the oid we $want, now
-			# lets see if we need to apply more patches:
-			my $src = $di->{oid_a};
+		unshift @{$self->{patches}}, $di;
+		dbg($self, "found $cur_want in ".di_url($self, $di));
 
-			last if $src =~ /\A0+\z/;
+		# good, we can find a path to the oid we $want, now
+		# lets see if we need to apply more patches:
+		my $src = $di->{oid_a};
 
+		unless ($src =~ /\A0+\z/) {
 			# we have to solve it using another oid, fine:
 			my $job = { oid_b => $src, path_b => $di->{path_a} };
-			push @todo, $job;
-			last; # onto the next @todo item
+			push @{$self->{todo}}, $job;
 		}
-		unless ($di) {
-			print $out "$want_oid could not be found\n";
-			return;
-		}
-		''; # continue onto next @todo item;
-	};
+		return next_step($self); # onto the next todo item
+	}
+	dbg($self, "could not find $cur_want");
+	eval { delete($self->{user_cb})->(undef) }; # not found! :<
+	die "E: $@" if $@;
+}
 
-	while (1) {
-		my $ret = eval { $cb->() };
-		unless (defined($ret)) {
-			print $out "E: $@\n" if $@;
-			return;
-		}
-		return $ret if ref($ret);
-		# $ret == ''; so continue looping here
+# this API is designed to avoid creating self-referential structures;
+# so user_cb never references the SolverGit object
+sub new {
+	my ($class, $ibx, $user_cb) = @_;
+
+	bless {
+		gits => $ibx->{-repo_objs},
+		user_cb => $user_cb,
+		max_patch => 100,
+
+		# TODO: config option for searching related inboxes
+		inboxes => [ $ibx ],
+	}, $class;
+}
+
+# recreate $oid_want using $hints
+# Calls {user_cb} with: [ ::Git object, oid_full, type, size, di (diff_info) ]
+# with found object, or undef if nothing was found
+# Calls {user_cb} with a string error on fatal errors
+sub solve ($$$$$) {
+	my ($self, $env, $out, $oid_want, $hints) = @_;
+
+	# should we even get here? Probably not, but somebody
+	# could be manually typing URLs:
+	return (delete $self->{user_cb})->(undef) if $oid_want =~ /\A0+\z/;
+
+	$self->{oid_want} = $oid_want;
+	$self->{out} = $out;
+	$self->{psgi_env} = $env;
+	$self->{todo} = [ { %$hints, oid_b => $oid_want } ];
+	$self->{patches} = []; # [ $di, $di, ... ]
+	$self->{found} = {}; # { abbr => [ ::Git, oid, type, size, $di ] }
+
+	dbg($self, "solving $oid_want ...");
+	my $step_cb = step_cb($self);
+	if (my $async = $env->{'pi-httpd.async'}) {
+		# PublicInbox::HTTPD::Async->new
+		$async->(undef, $step_cb);
+	} else {
+		$step_cb->() while $self->{user_cb};
 	}
 }
 
diff --git a/lib/PublicInbox/ViewVCS.pm b/lib/PublicInbox/ViewVCS.pm
index 4a3896d..fa76086 100644
--- a/lib/PublicInbox/ViewVCS.pm
+++ b/lib/PublicInbox/ViewVCS.pm
@@ -27,37 +27,33 @@ my $enc_utf8 = find_encoding('UTF-8');
 
 sub html_page ($$$) {
 	my ($ctx, $code, $strref) = @_;
+	my $wcb = delete $ctx->{-wcb};
 	$ctx->{-upfx} = '../../'; # from "/$INBOX/$OID/s/"
-	PublicInbox::WwwStream->response($ctx, $code, sub {
+	my $res = PublicInbox::WwwStream->response($ctx, $code, sub {
 		my ($nr, undef) =  @_;
 		$nr == 1 ? $$strref : undef;
 	});
+	$wcb->($res);
 }
 
-sub show ($$;$) {
-	my ($ctx, $oid_b, $fn) = @_;
-	my $ibx = $ctx->{-inbox};
-	my $inboxes = [ $ibx ];
-	my $solver = PublicInbox::SolverGit->new($ibx->{-repo_objs}, $inboxes);
-	my $qp = $ctx->{qp};
-	my $hints = {};
-	while (my ($from, $to) = each %QP_MAP) {
-		defined(my $v = $qp->{$from}) or next;
-		$hints->{$to} = $v;
-	}
-
-	open my $log, '+>', undef or die "open: $!";
-	my $res = $solver->solve($log, $oid_b, $hints);
+sub solve_result {
+	my ($ctx, $res, $log, $hints, $fn) = @_;
 
-	seek($log, 0, 0) or die "seek: $!";
+	unless (seek($log, 0, 0)) {
+		$ctx->{env}->{'psgi.errors'}->print("seek(log): $!\n");
+		return html_page($ctx, 500, \'seek error');
+	}
 	$log = do { local $/; <$log> };
 
+	my $ref = ref($res);
+	$log .= $res unless $ref;
 	my $l = PublicInbox::Linkify->new;
 	$l->linkify_1($log);
 	$log = '<pre>debug log:</pre><hr /><pre>' .
 		$l->linkify_2(ascii_html($log)) . '</pre>';
 
 	$res or return html_page($ctx, 404, \$log);
+	$ref eq 'ARRAY' or return html_page($ctx, 500, \$log);
 
 	my ($git, $oid, $type, $size, $di) = @$res;
 	if ($size > $max_size) {
@@ -78,7 +74,7 @@ sub show ($$;$) {
 	if ($fn) {
 		my $h = [ 'Content-Length', $size, 'Content-Type' ];
 		push(@$h, ($binary ? 'application/octet-stream' : 'text/plain'));
-		return [ 200, $h, [ $$blob ]];
+		return delete($ctx->{-wcb})->([200, $h, [ $$blob ]]);
 	}
 
 	my $path = to_filename($di->{path_b} || $hints->{path_b} || 'blob');
@@ -107,4 +103,25 @@ sub show ($$;$) {
 	html_page($ctx, 200, \$log);
 }
 
+sub show ($$;$) {
+	my ($ctx, $oid_b, $fn) = @_;
+	my $qp = $ctx->{qp};
+	my $hints = {};
+	while (my ($from, $to) = each %QP_MAP) {
+		defined(my $v = $qp->{$from}) or next;
+		$hints->{$to} = $v;
+	}
+
+	open my $log, '+>', undef or die "open: $!";
+	my $solver = PublicInbox::SolverGit->new($ctx->{-inbox}, sub {
+		solve_result($ctx, $_[0], $log, $hints, $fn);
+	});
+
+	# PSGI server will call this and give us a callback
+	sub {
+		$ctx->{-wcb} = $_[0]; # HTTP write callback
+		$solver->solve($ctx->{env}, $log, $oid_b, $hints);
+	};
+}
+
 1;
diff --git a/t/solver_git.t b/t/solver_git.t
index fe322ea..197a003 100644
--- a/t/solver_git.t
+++ b/t/solver_git.t
@@ -40,10 +40,12 @@ sub deliver_patch ($) {
 
 deliver_patch('t/solve/0001-simple-mod.patch');
 
-my $gits = [ PublicInbox::Git->new($git_dir) ];
-my $solver = PublicInbox::SolverGit->new($gits, [ $ibx ]);
+$ibx->{-repo_objs} = [ PublicInbox::Git->new($git_dir) ];
+my $res;
+my $solver = PublicInbox::SolverGit->new($ibx, sub { $res = $_[0] });
 open my $log, '+>>', "$mainrepo/solve.log" or die "open: $!";
-my $res = $solver->solve($log, '69df7d5', {});
+my $psgi_env = { 'psgi.url_scheme' => 'http', HTTP_HOST => 'example.com' };
+$solver->solve($psgi_env, $log, '69df7d5', {});
 ok($res, 'solved a blob!');
 my $wt_git = $res->[0];
 is(ref($wt_git), 'PublicInbox::Git', 'got a git object for the blob');
@@ -62,20 +64,24 @@ if (0) { # TODO: check this?
 	diag $z;
 }
 
+$solver = undef;
 $res = undef;
 my $wt_git_dir = $wt_git->{git_dir};
 $wt_git = undef;
 ok(!-d $wt_git_dir, 'no references to WT held');
 
-$res = $solver->solve($log, '0'x40, {});
+$solver = PublicInbox::SolverGit->new($ibx, sub { $res = $_[0] });
+$solver->solve($psgi_env, $log, '0'x40, {});
 is($res, undef, 'no error on z40');
 
 my $git_v2_20_1_tag = '7a95a1cd084cb665c5c2586a415e42df0213af74';
-$res = $solver->solve($log, $git_v2_20_1_tag, {});
+$solver = PublicInbox::SolverGit->new($ibx, sub { $res = $_[0] });
+$solver->solve($psgi_env, $log, $git_v2_20_1_tag, {});
 is($res, undef, 'no error on a tag not in our repo');
 
 deliver_patch('t/solve/0002-rename-with-modifications.patch');
-$res = $solver->solve($log, '0a92431', {});
+$solver = PublicInbox::SolverGit->new($ibx, sub { $res = $_[0] });
+$solver->solve($psgi_env, $log, '0a92431', {});
 ok($res, 'resolved without hints');
 
 my $hints = {
@@ -83,7 +89,9 @@ my $hints = {
 	path_a => 'HACKING',
 	path_b => 'CONTRIBUTING'
 };
-my $hinted = $solver->solve($log, '0a92431', $hints);
+$solver = PublicInbox::SolverGit->new($ibx, sub { $res = $_[0] });
+$solver->solve($psgi_env, $log, '0a92431', $hints);
+my $hinted = $res;
 # don't compare ::Git objects:
 shift @$res; shift @$hinted;
 is_deeply($res, $hinted, 'hints work (or did not hurt :P');
-- 
EW


  parent reply	other threads:[~2019-01-27  4:03 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-27  4:03 [PATCH 00/14] convert solver to use pi-httpd.async Eric Wong
2019-01-27  4:03 ` [PATCH 01/14] httpd/async: remove needless sysread wrapper Eric Wong
2019-01-27  4:03 ` [PATCH 02/14] qspawn: implement psgi_return and use it for githttpbackend Eric Wong
2019-01-27  4:03 ` [PATCH 03/14] qspawn|getlinebody: support streaming filters Eric Wong
2019-01-27  4:03 ` [PATCH 04/14] qspawn|httpd/async: improve and fix out-of-date comments Eric Wong
2019-01-27  4:03 ` [PATCH 05/14] httpd/async: stop running command if client disconnects Eric Wong
2019-01-27  4:03 ` [PATCH 06/14] qspawn: implement psgi_qx Eric Wong
2019-01-27  4:03 ` [PATCH 07/14] t/qspawn.t: psgi_qx stderr test Eric Wong
2019-01-27  4:03 ` [PATCH 08/14] view: swap CRLF for LF in HTML output Eric Wong
2019-01-27  4:03 ` Eric Wong [this message]
2019-01-27  4:03 ` [PATCH 10/14] solver: hold patches in temporary directory Eric Wong
2019-01-27  4:03 ` [PATCH 11/14] solver: reduce "git apply" invocations Eric Wong
2019-01-27  4:03 ` [PATCH 12/14] qspawn: decode $? for user-friendliness Eric Wong
2019-01-27  4:03 ` [PATCH 13/14] viewvcs: do not show final error message twice Eric Wong
2019-01-27  4:03 ` [PATCH 14/14] solver: crank up max patches to 9999 Eric Wong

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://public-inbox.org/README

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190127040341.26107-10-e@80x24.org \
    --to=e@80x24.org \
    --cc=meta@public-inbox.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/public-inbox.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).