[PATCH 0/7] an even bigger git show than before...

[PATCH 0/7] an even bigger git show than before...

[Eric Wong]

We can do things with patch emails, we might as well do some
things with git commits for folks with coderepos configured.

Eric Wong (7):
  config: fix confusing space in ->repo_objs
  xt/solver: improve diagnostics
  www: /s/: 404 for unconfigured coderepos
  qspawn: add type comments in a few places
  qspawn: improve error reporting and handling
  viewvcs: drop pointless variable assignment
  viewvcs: start improving display of git commits

 lib/PublicInbox/Config.pm    |   2 +-
 lib/PublicInbox/LeiBlob.pm   |   4 +-
 lib/PublicInbox/Qspawn.pm    |  33 +++++-----
 lib/PublicInbox/SolverGit.pm |  28 ++++-----
 lib/PublicInbox/ViewVCS.pm   | 116 +++++++++++++++++++++++++++++++----
 lib/PublicInbox/WWW.pm       |   3 +-
 lib/PublicInbox/WwwStream.pm |   9 +++
 t/qspawn.t                   |  14 ++++-
 xt/solver.t                  |   7 +--
 9 files changed, 162 insertions(+), 54 deletions(-)

[PATCH 1/7] config: fix confusing space in ->repo_objs

[Eric Wong]

It's actually valid Perl to have "$foo ->{field} = ..."
but it's confusing and I noticed it while tracking down
a configuration error.
---
 lib/PublicInbox/Config.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/PublicInbox/Config.pm b/lib/PublicInbox/Config.pm
index ad8b8e9d..1b5d87e2 100644
--- a/lib/PublicInbox/Config.pm
+++ b/lib/PublicInbox/Config.pm
@@ -402,7 +402,7 @@ sub repo_objs {
 			push @repo_objs, $repo if $repo;
 		}
 		if (scalar @repo_objs) {
-			$ibxish ->{-repo_objs} = \@repo_objs;
+			$ibxish->{-repo_objs} = \@repo_objs;
 		} else {
 			delete $ibxish->{coderepo};
 		}

[PATCH 2/7] xt/solver: improve diagnostics

[Eric Wong]

I'm making some tweaks to solver and will probably find extra
output useful, and also update to v5.12 while we're at it.
---
 xt/solver.t | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/xt/solver.t b/xt/solver.t
index 880458fb..32cd43cf 100644
--- a/xt/solver.t
+++ b/xt/solver.t
@@ -1,8 +1,7 @@
 #!perl -w
-# Copyright (C) 2020-2021 all contributors <meta@public-inbox.org>
+# Copyright (C) all contributors <meta@public-inbox.org>
 # License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
-use strict;
-use Test::More;
+use v5.12;
 use PublicInbox::TestCommon;
 use PublicInbox::Config; # this relies on PI_CONFIG // ~/.public-inbox/config
 my @psgi = qw(HTTP::Request::Common Plack::Test URI::Escape Plack::Builder);
@@ -41,8 +40,8 @@ my $client = sub {
 		my $res = $cb->(GET($url));
 		is($res->code, 200, $url);
 		next if $res->code == 200;
-		# diag $res->content;
 		diag "$url failed";
+		diag $res->content;
 	}
 };
 

[PATCH 3/7] www: /s/: 404 for unconfigured coderepos

[Eric Wong]

The $r404 variable is unset if we have a valid inbox, but no
coderepos configured for that inbox, thus we must `r(404)'
explicitly.
---
 lib/PublicInbox/WWW.pm | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/PublicInbox/WWW.pm b/lib/PublicInbox/WWW.pm
index a33709e9..77f463d3 100644
--- a/lib/PublicInbox/WWW.pm
+++ b/lib/PublicInbox/WWW.pm
@@ -303,7 +303,8 @@ sub get_text {
 sub get_vcs_object ($$$;$) {
 	my ($ctx, $inbox, $oid, $filename) = @_;
 	my $r404 = invalid_inbox($ctx, $inbox);
-	return $r404 if $r404 || !$ctx->{www}->{pi_cfg}->repo_objs($ctx->{ibx});
+	return $r404 if $r404;
+	return r(404) if !$ctx->{www}->{pi_cfg}->repo_objs($ctx->{ibx});
 	require PublicInbox::ViewVCS;
 	PublicInbox::ViewVCS::show($ctx, $oid, $filename);
 }

[PATCH 4/7] qspawn: add type comments in a few places

[Eric Wong]

This makes things easier-to-follow in a minimally-typed language.
---
 lib/PublicInbox/LeiBlob.pm |  4 ++--
 lib/PublicInbox/Qspawn.pm  | 13 ++++++-------
 2 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/lib/PublicInbox/LeiBlob.pm b/lib/PublicInbox/LeiBlob.pm
index 004b156c..1692289c 100644
--- a/lib/PublicInbox/LeiBlob.pm
+++ b/lib/PublicInbox/LeiBlob.pm
@@ -1,4 +1,4 @@
-# Copyright (C) 2021 all contributors <meta@public-inbox.org>
+# Copyright (C) all contributors <meta@public-inbox.org>
 # License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
 
 # "lei blob $OID" command
@@ -70,7 +70,7 @@ sub do_solve_blob { # via wq_do
 			} @$git_dirs ],
 		user_cb => \&solver_user_cb,
 		uarg => $self,
-		# -cur_di, -qsp, -msg => temporary fields for Qspawn callbacks
+		# -cur_di, -msg => temporary fields for Qspawn callbacks
 		inboxes => [ $self->{lxs}->locals, @rmt ],
 	}, 'PublicInbox::SolverGit';
 	local $PublicInbox::DS::in_loop = 0; # waitpid synchronously
diff --git a/lib/PublicInbox/Qspawn.pm b/lib/PublicInbox/Qspawn.pm
index 53d0ad55..79db9e76 100644
--- a/lib/PublicInbox/Qspawn.pm
+++ b/lib/PublicInbox/Qspawn.pm
@@ -132,7 +132,7 @@ sub start ($$$) {
 
 sub psgi_qx_init_cb {
 	my ($self) = @_;
-	my $async = delete $self->{async};
+	my $async = delete $self->{async}; # PublicInbox::HTTPD::Async
 	my ($r, $buf);
 	my $qx_fh = $self->{qx_fh};
 reread:
@@ -227,11 +227,10 @@ sub psgi_return_init_cb {
 		PublicInbox::GzipFilter::qsp_maybe($r->[1], $env);
 
 	my $wcb = delete $env->{'qspawn.wcb'};
-	my $async = delete $self->{async};
+	my $async = delete $self->{async}; # PublicInbox::HTTPD::Async
 	if (scalar(@$r) == 3) { # error
-		if ($async) {
-			# calls rpipe->close && ->event_step
-			$async->close;
+		if ($async) { # calls rpipe->close && ->event_step
+			$async->close; # PublicInbox::HTTPD::Async::close
 		} else {
 			$self->{rpipe}->close;
 			event_step($self);
@@ -255,9 +254,9 @@ sub psgi_return_init_cb {
 
 sub psgi_return_start { # may run later, much later...
 	my ($self) = @_;
-	if (my $async = $self->{psgi_env}->{'pi-httpd.async'}) {
+	if (my $cb = $self->{psgi_env}->{'pi-httpd.async'}) {
 		# PublicInbox::HTTPD::Async->new(rpipe, $cb, $cb_arg, $end_obj)
-		$self->{async} = $async->($self->{rpipe},
+		$self->{async} = $cb->($self->{rpipe},
 					\&psgi_return_init_cb, $self, $self);
 	} else { # generic PSGI
 		psgi_return_init_cb($self) while $self->{parse_hdr};

[PATCH 5/7] qspawn: improve error reporting and handling

[Eric Wong]

First off, avoid potential circular references (via {qx_arg}) by
dropping the {-qsp} field from $ctx and SolverGit objects.
Instead, we only share a reference to an optional error buffer
string {qsp_err}.

We'll also attempt to call qspawn.wcb if qx_cb fails, and warn
in more places w/o checking for $env since we now rely on warn()
instead of $env->{'psgi.errors'}.

This makes error handling simpler and safer in future callers.
---
 lib/PublicInbox/Qspawn.pm    | 20 +++++++++++---------
 lib/PublicInbox/SolverGit.pm | 28 ++++++++++++----------------
 lib/PublicInbox/ViewVCS.pm   | 14 ++++++--------
 t/qspawn.t                   | 14 +++++++++++---
 4 files changed, 40 insertions(+), 36 deletions(-)

diff --git a/lib/PublicInbox/Qspawn.pm b/lib/PublicInbox/Qspawn.pm
index 79db9e76..cea34fc3 100644
--- a/lib/PublicInbox/Qspawn.pm
+++ b/lib/PublicInbox/Qspawn.pm
@@ -1,4 +1,4 @@
-# Copyright (C) 2016-2021 all contributors <meta@public-inbox.org>
+# Copyright (C) all contributors <meta@public-inbox.org>
 # License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
 
 # Like most Perl modules in public-inbox, this is internal and
@@ -26,6 +26,7 @@
 
 package PublicInbox::Qspawn;
 use strict;
+use v5.10.1;
 use PublicInbox::Spawn qw(popen_rd);
 use PublicInbox::GzipFilter;
 
@@ -38,6 +39,7 @@ my $def_limiter;
 # $cmd is the command to spawn
 # $cmd_env is the environ for the child process (not PSGI env)
 # $opt can include redirects and perhaps other process spawning options
+# {qsp_err} is an optional error buffer callers may access themselves
 sub new ($$$;) {
 	my ($class, $cmd, $cmd_env, $opt) = @_;
 	bless { args => [ $cmd, $cmd_env, $opt ] }, $class;
@@ -93,18 +95,18 @@ sub finalize ($$) {
 	}
 
 	if ($err) {
-		if (defined $self->{err}) {
-			$self->{err} .= "; $err";
-		} else {
-			$self->{err} = $err;
-		}
-		if ($env && $self->{cmd}) {
-			warn join(' ', @{$self->{cmd}}) . ": $err";
+		utf8::decode($err);
+		if (my $dst = $self->{qsp_err}) {
+			$$dst .= $$dst ? " $err" : "; $err";
 		}
+		warn "@{$self->{cmd}}: $err" if $self->{cmd};
 	}
 	if ($qx_cb) {
 		eval { $qx_cb->($qx_buf, $qx_arg) };
-	} elsif (my $wcb = delete $env->{'qspawn.wcb'}) {
+		return unless $@;
+		warn "E: $@"; # hope qspawn.wcb can handle it
+	}
+	if (my $wcb = delete $env->{'qspawn.wcb'}) {
 		# have we started writing, yet?
 		require PublicInbox::WwwStatic;
 		$wcb->(PublicInbox::WwwStatic::r(500));
diff --git a/lib/PublicInbox/SolverGit.pm b/lib/PublicInbox/SolverGit.pm
index d3567aa2..dbb9b739 100644
--- a/lib/PublicInbox/SolverGit.pm
+++ b/lib/PublicInbox/SolverGit.pm
@@ -242,10 +242,8 @@ sub find_smsgs ($$$) {
 
 sub update_index_result ($$) {
 	my ($bref, $self) = @_;
-	my ($qsp, $msg) = delete @$self{qw(-qsp -msg)};
-	if (my $err = $qsp->{err}) {
-		ERR($self, "git update-index error: $err");
-	}
+	my ($qsp_err, $msg) = delete @$self{qw(-qsp_err -msg)};
+	ERR($self, "git update-index error:$qsp_err") if $qsp_err;
 	dbg($self, $msg);
 	next_step($self); # onto do_git_apply
 }
@@ -278,7 +276,7 @@ sub prepare_index ($) {
 	my $cmd = [ qw(git update-index -z --index-info) ];
 	my $qsp = PublicInbox::Qspawn->new($cmd, $self->{git_env}, $rdr);
 	$path_a = git_quote($path_a);
-	$self->{-qsp} = $qsp;
+	$qsp->{qsp_err} = \($self->{-qsp_err} = '');
 	$self->{-msg} = "index prepared:\n$mode_a $oid_full\t$path_a";
 	$qsp->psgi_qx($self->{psgi_env}, undef, \&update_index_result, $self);
 }
@@ -405,10 +403,8 @@ sub mark_found ($$$) {
 
 sub parse_ls_files ($$) {
 	my ($self, $bref) = @_;
-	my ($qsp, $di) = delete @$self{qw(-qsp -cur_di)};
-	if (my $err = $qsp->{err}) {
-		die "git ls-files error: $err";
-	}
+	my ($qsp_err, $di) = delete @$self{qw(-qsp_err -cur_di)};
+	die "git ls-files error:$qsp_err" if $qsp_err;
 
 	my ($line, @extra) = split(/\0/, $$bref);
 	scalar(@extra) and die "BUG: extra files in index: <",
@@ -456,11 +452,11 @@ sub skip_identical ($$$) {
 
 sub apply_result ($$) {
 	my ($bref, $self) = @_;
-	my ($qsp, $di) = delete @$self{qw(-qsp -cur_di)};
+	my ($qsp_err, $di) = delete @$self{qw(-qsp_err -cur_di)};
 	dbg($self, $$bref);
 	my $patches = $self->{patches};
-	if (my $err = $qsp->{err}) {
-		my $msg = "git apply error: $err";
+	if ($qsp_err) {
+		my $msg = "git apply error:$qsp_err";
 		my $nxt = $patches->[0];
 		if ($nxt && oids_same_ish($nxt->{oid_b}, $di->{oid_b})) {
 			dbg($self, $msg);
@@ -474,9 +470,9 @@ sub apply_result ($$) {
 	}
 
 	my @cmd = qw(git ls-files -s -z);
-	$qsp = PublicInbox::Qspawn->new(\@cmd, $self->{git_env});
+	my $qsp = PublicInbox::Qspawn->new(\@cmd, $self->{git_env});
 	$self->{-cur_di} = $di;
-	$self->{-qsp} = $qsp;
+	$qsp->{qsp_err} = \($self->{-qsp_err} = '');
 	$qsp->psgi_qx($self->{psgi_env}, undef, \&ls_files_result, $self);
 }
 
@@ -508,7 +504,7 @@ sub do_git_apply ($) {
 	my $opt = { 2 => 1, -C => $dn, quiet => 1 };
 	my $qsp = PublicInbox::Qspawn->new(\@cmd, $self->{git_env}, $opt);
 	$self->{-cur_di} = $di;
-	$self->{-qsp} = $qsp;
+	$qsp->{qsp_err} = \($self->{-qsp_err} = '');
 	$qsp->psgi_qx($self->{psgi_env}, undef, \&apply_result, $self);
 }
 
@@ -660,7 +656,7 @@ sub new {
 		gits => $ibx->{-repo_objs},
 		user_cb => $user_cb,
 		uarg => $uarg,
-		# -cur_di, -qsp, -msg => temporary fields for Qspawn callbacks
+		# -cur_di, -qsp_err, -msg => temp fields for Qspawn callbacks
 
 		# TODO: config option for searching related inboxes
 		inboxes => [ $ibx ],
diff --git a/lib/PublicInbox/ViewVCS.pm b/lib/PublicInbox/ViewVCS.pm
index 3cbc363b..d3fd152d 100644
--- a/lib/PublicInbox/ViewVCS.pm
+++ b/lib/PublicInbox/ViewVCS.pm
@@ -1,4 +1,4 @@
-# Copyright (C) 2019-2021 all contributors <meta@public-inbox.org>
+# Copyright (C) all contributors <meta@public-inbox.org>
 # License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
 
 # show any VCS object, similar to "git show"
@@ -76,10 +76,9 @@ sub stream_large_blob ($$$$) {
 
 sub show_other_result ($$) {
 	my ($bref, $ctx) = @_;
-	my ($qsp, $logref) = delete @$ctx{qw(-qsp -logref)};
-	if (my $err = $qsp->{err}) {
-		utf8::decode($$err);
-		$$logref .= "git show error: $err";
+	my ($qsp_err, $logref) = delete @$ctx{qw(-qsp_err -logref)};
+	if ($qsp_err) {
+		$$logref .= "git show error:$qsp_err";
 		return html_page($ctx, 500, $logref);
 	}
 	my $l = PublicInbox::Linkify->new;
@@ -99,10 +98,9 @@ sub show_other ($$$$) {
 	my $cmd = ['git', "--git-dir=$git->{git_dir}",
 		qw(show --encoding=UTF-8 --no-color --no-abbrev), $oid ];
 	my $qsp = PublicInbox::Qspawn->new($cmd);
-	my $env = $ctx->{env};
-	$ctx->{-qsp} = $qsp;
+	$qsp->{qsp_err} = \($ctx->{-qsp_err} = '');
 	$ctx->{-logref} = $logref;
-	$qsp->psgi_qx($env, undef, \&show_other_result, $ctx);
+	$qsp->psgi_qx($ctx->{env}, undef, \&show_other_result, $ctx);
 }
 
 # user_cb for SolverGit, called as: user_cb->($result_or_error, $uarg)
diff --git a/t/qspawn.t b/t/qspawn.t
index 4b9dc8a5..224e20db 100644
--- a/t/qspawn.t
+++ b/t/qspawn.t
@@ -1,6 +1,6 @@
-# Copyright (C) 2016-2021 all contributors <meta@public-inbox.org>
+# Copyright (C) all contributors <meta@public-inbox.org>
 # License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
-use strict;
+use v5.12;
 use Test::More;
 use_ok 'PublicInbox::Qspawn';
 
@@ -20,12 +20,13 @@ use_ok 'PublicInbox::Qspawn';
 sub finish_err ($) {
 	my ($qsp) = @_;
 	$qsp->finish;
-	$qsp->{err};
+	$qsp->{qsp_err} && ${$qsp->{qsp_err}};
 }
 
 my $limiter = PublicInbox::Qspawn::Limiter->new(1);
 {
 	my $x = PublicInbox::Qspawn->new([qw(true)]);
+	$x->{qsp_err} = \(my $err = '');
 	my $run = 0;
 	$x->start($limiter, sub {
 		my ($self) = @_;
@@ -37,7 +38,9 @@ my $limiter = PublicInbox::Qspawn::Limiter->new(1);
 }
 
 {
+	my @err; local $SIG{__WARN__} = sub { push @err, @_ };
 	my $x = PublicInbox::Qspawn->new([qw(false)]);
+	$x->{qsp_err} = \(my $err = '');
 	my $run = 0;
 	$x->start($limiter, sub {
 		my ($self) = @_;
@@ -47,10 +50,13 @@ my $limiter = PublicInbox::Qspawn::Limiter->new(1);
 		$run = 1;
 	});
 	is($run, 1, 'callback ran alright');
+	ok(scalar @err, 'got warning');
 }
 
 foreach my $cmd ([qw(sleep 1)], [qw(sh -c), 'sleep 1; false']) {
+	my @err; local $SIG{__WARN__} = sub { push @err, @_ };
 	my $s = PublicInbox::Qspawn->new($cmd);
+	$s->{qsp_err} = \(my $err = '');
 	my @run;
 	$s->start($limiter, sub {
 		my ($self) = @_;
@@ -70,8 +76,10 @@ foreach my $cmd ([qw(sleep 1)], [qw(sh -c), 'sleep 1; false']) {
 
 	if ($cmd->[-1] =~ /false\z/) {
 		ok(finish_err($s), 'got error on false after sleep');
+		ok(scalar @err, 'got warning');
 	} else {
 		ok(!finish_err($s), 'no error on sleep');
+		is_deeply([], \@err, 'no warnings');
 	}
 	ok(!finish_err($_->[0]), "true $_->[1] succeeded") foreach @t;
 	is_deeply([qw(sleep 0 1 2)], \@run, 'ran in order');

[PATCH 6/7] viewvcs: drop pointless variable assignment

[Eric Wong]

Not needed since commit:
41f03a3bd4b13dfa (viewvcs: do not show final error message twice, 2019-01-27)
---
 lib/PublicInbox/ViewVCS.pm | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/lib/PublicInbox/ViewVCS.pm b/lib/PublicInbox/ViewVCS.pm
index d3fd152d..94656ad3 100644
--- a/lib/PublicInbox/ViewVCS.pm
+++ b/lib/PublicInbox/ViewVCS.pm
@@ -114,13 +114,12 @@ sub solve_result {
 	}
 	$log = do { local $/; <$log> };
 
-	my $ref = ref($res);
 	my $l = PublicInbox::Linkify->new;
 	$log = '<pre>debug log:</pre><hr /><pre>' .
 		$l->to_html($log) . '</pre>';
 
 	$res or return html_page($ctx, 404, \$log);
-	$ref eq 'ARRAY' or return html_page($ctx, 500, \$log);
+	ref($res) eq 'ARRAY' or return html_page($ctx, 500, \$log);
 
 	my ($git, $oid, $type, $size, $di) = @$res;
 	return show_other($ctx, $res, \$log, $fn) if $type ne 'blob';

[PATCH 7/7] viewvcs: start improving display of git commits

[Eric Wong]

For non-merge git commits, we already have ViewDiff for
displaying patch emails, we can reuse it to display non-merge
git commits.

AFAIK, this is the first web-based git repository viewer
to display the output of "git-patch-id --stable".
It currently fills in the search form box with "patchid:",
but maybe it'll do more than that.

More work will be done to support bidirectional mapping
of commits to emails in the future.
---
 lib/PublicInbox/ViewVCS.pm   | 99 +++++++++++++++++++++++++++++++++++-
 lib/PublicInbox/WwwStream.pm |  9 ++++
 2 files changed, 107 insertions(+), 1 deletion(-)

diff --git a/lib/PublicInbox/ViewVCS.pm b/lib/PublicInbox/ViewVCS.pm
index 94656ad3..96883f6c 100644
--- a/lib/PublicInbox/ViewVCS.pm
+++ b/lib/PublicInbox/ViewVCS.pm
@@ -2,7 +2,6 @@
 # License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
 
 # show any VCS object, similar to "git show"
-# FIXME: we only show blobs for now
 #
 # This can use a "solver" to reconstruct blobs based on git
 # patches (with abbreviated OIDs in the header).  However, the
@@ -16,10 +15,12 @@
 package PublicInbox::ViewVCS;
 use strict;
 use v5.10.1;
+use File::Temp 0.19 (); # newdir
 use PublicInbox::SolverGit;
 use PublicInbox::WwwStream qw(html_oneshot);
 use PublicInbox::Linkify;
 use PublicInbox::Tmpfile;
+use PublicInbox::ViewDiff qw(flush_diff);
 use PublicInbox::Hval qw(ascii_html to_filename);
 my $hl = eval {
 	require PublicInbox::HlMod;
@@ -29,6 +30,8 @@ my $hl = eval {
 my %QP_MAP = ( A => 'oid_a', a => 'path_a', b => 'path_b' );
 our $MAX_SIZE = 1024 * 1024; # TODO: configurable
 my $BIN_DETECT = 8000; # same as git
+my $SHOW_FMT = '--pretty=format:'.join('%n', '%H', '%T', '%P', '%s',
+	'%an <%ae>%x09%ai', '%cn <%ce>%x09%ci', '%b%x00');
 
 sub html_page ($$$) {
 	my ($ctx, $code, $strref) = @_;
@@ -88,6 +91,99 @@ sub show_other_result ($$) {
 	html_page($ctx, 200, $bref);
 }
 
+sub show_commit_result ($$) {
+	my ($bref, $ctx) = @_;
+	my ($qsp_err, $logref, $tmp) = @$ctx{qw(-qsp_err -logref -tmp)};
+	if ($qsp_err) {
+		$$logref .= "git show/patch-id error:$qsp_err";
+		return html_page($ctx, 500, $logref);
+	}
+	my $upfx = $ctx->{-upfx} = '../../'; # from "/$INBOX/$OID/s/"
+	my $patchid = (split(/ /, $$bref))[0]; # ignore commit
+	if (defined $patchid) {
+		$ctx->{-q_value_html} = "patchid:$patchid";
+		$patchid = "\n  patchid $patchid";
+	} else {
+		$patchid = '';
+	}
+	my $l = $ctx->{-linkify} = PublicInbox::Linkify->new;
+	open my $fh, '<:utf8', "$tmp/h" or die "open $tmp/h: $!";
+	chop(my $buf = do { local $/ = "\0"; <$fh> });
+	my ($H, $T, $P, $s, $au, $co, $bdy) = split(/\n/, $buf, 7);
+	chomp $bdy;
+	# try to keep author and committer dates lined up
+	my $x = length($au) - length($co);
+	if ($x > 0) {
+		$x = ' ' x $x;
+		$co =~ s/\t/$x\t/;
+	} elsif ($x < 0) {
+		$x = ' ' x (-$x);
+		$au =~ s/\t/$x\t/;
+	}
+	$_ = ascii_html($_) for ($au, $co);
+	$_ = $l->to_html($_) for ($s, $bdy);
+	$ctx->{-title_html} = $s;
+	my @p = split(/ /, $P);
+	if (@p == 1) {
+		$P = qq(\n   parent <a href="$upfx$P/s/">$P</a>);
+	} elsif (@p > 1) {
+		$P = qq(\n  parents <a href="$upfx$p[0]/s/">$p[0]</a>\n);
+		shift @p;
+		$P .= qq(          <a href="$upfx$_/s/">$_</a>\n) for @p;
+		chop $P;
+	} else { # root commit
+		$P = ' (root commit)';
+	}
+	PublicInbox::WwwStream::html_init($ctx);
+	$ctx->zmore(<<EOM);
+<pre>   commit $H$P
+     tree <a href="$upfx$T/s/">$T</a>
+   author $au
+committer $co$patchid
+
+<b>$s</b>\n
+EOM
+	$ctx->zmore($bdy);
+	open $fh, '<', "$tmp/p" or die "open $tmp/p: $!";
+	if (-s $fh > $MAX_SIZE) {
+		$ctx->zmore("---\n patch is too large to show\n");
+	} else { # prepare flush_diff:
+		$buf = '';
+		$ctx->{obuf} = \$buf;
+		$ctx->{-apfx} = $ctx->{-spfx} = $upfx;
+		$ctx->{-anchors} = {};
+		$bdy = '';
+		read($fh, $bdy, -s _);
+		$bdy =~ s/\r?\n/\n/gs;
+		flush_diff($ctx, \$bdy);
+		$ctx->zmore($buf);
+	}
+	$x = $ctx->zflush($ctx->_html_end);
+	my $res_hdr = delete $ctx->{-res_hdr};
+	push @$res_hdr, 'Content-Length', length($x);
+	delete($ctx->{env}->{'qspawn.wcb'})->([200, $res_hdr, [$x]]);
+}
+
+sub show_commit ($$$$) {
+	my ($ctx, $res, $logref, $fn) = @_;
+	my ($git, $oid) = @$res;
+	# patch-id needs two passes, and we use the initial show to ensure
+	# a patch embedded inside the commit message body doesn't get fed
+	# to patch-id:
+	my $cmd = [ '/bin/sh', '-c',
+		"git show '$SHOW_FMT' -z --no-notes --no-patch $oid >h && ".
+		"git show --pretty=format:%n -M --stat -p $oid >p && ".
+		"git patch-id --stable <p" ];
+	my $xenv = { GIT_DIR => $git->{git_dir} };
+	my $tmp = File::Temp->newdir("show-$oid-XXXX", TMPDIR => 1);
+	my $qsp = PublicInbox::Qspawn->new($cmd, $xenv, { -C => "$tmp" });
+	$qsp->{qsp_err} = \($ctx->{-qsp_err} = '');
+	$ctx->{-logref} = $logref;
+	$ctx->{-tmp} = $tmp;
+	$ctx->{env}->{'qspawn.wcb'} = delete $ctx->{-wcb};
+	$qsp->psgi_qx($ctx->{env}, undef, \&show_commit_result, $ctx);
+}
+
 sub show_other ($$$$) {
 	my ($ctx, $res, $logref, $fn) = @_;
 	my ($git, $oid, $type, $size) = @$res;
@@ -122,6 +218,7 @@ sub solve_result {
 	ref($res) eq 'ARRAY' or return html_page($ctx, 500, \$log);
 
 	my ($git, $oid, $type, $size, $di) = @$res;
+	return show_commit($ctx, $res, \$log, $fn) if $type eq 'commit';
 	return show_other($ctx, $res, \$log, $fn) if $type ne 'blob';
 	my $path = to_filename($di->{path_b} // $hints->{path_b} // 'blob');
 	my $raw_link = "(<a\nhref=$path>raw</a>)";
diff --git a/lib/PublicInbox/WwwStream.pm b/lib/PublicInbox/WwwStream.pm
index 0416db0b..ab006c40 100644
--- a/lib/PublicInbox/WwwStream.pm
+++ b/lib/PublicInbox/WwwStream.pm
@@ -201,4 +201,13 @@ sub aresponse {
 	$ctx->psgi_response($code, $res_hdr);
 }
 
+sub html_init {
+	my ($ctx) = @_;
+	$ctx->{base_url} = base_url($ctx);
+	my $h = $ctx->{-res_hdr} = ['Content-Type', 'text/html; charset=UTF-8'];
+	$ctx->{gz} = PublicInbox::GzipFilter::gz_or_noop($h, $ctx->{env});
+	bless $ctx, __PACKAGE__;
+	$ctx->zmore(html_top($ctx));
+}
+
 1;

[PATCH 8/7] viewvcs: use :utf8 for opening patch, too

[Eric Wong]

We'll also force --encoding=UTF-8 for "git show" even
if that's the default in git, since the rest of our code
already assumes UTF-8.
---
 lib/PublicInbox/ViewVCS.pm | 8 +++++---
 t/solver_git.t             | 7 ++++++-
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/lib/PublicInbox/ViewVCS.pm b/lib/PublicInbox/ViewVCS.pm
index 96883f6c..19d34092 100644
--- a/lib/PublicInbox/ViewVCS.pm
+++ b/lib/PublicInbox/ViewVCS.pm
@@ -144,7 +144,7 @@ committer $co$patchid
 <b>$s</b>\n
 EOM
 	$ctx->zmore($bdy);
-	open $fh, '<', "$tmp/p" or die "open $tmp/p: $!";
+	open $fh, '<:utf8', "$tmp/p" or die "open $tmp/p: $!";
 	if (-s $fh > $MAX_SIZE) {
 		$ctx->zmore("---\n patch is too large to show\n");
 	} else { # prepare flush_diff:
@@ -171,8 +171,10 @@ sub show_commit ($$$$) {
 	# a patch embedded inside the commit message body doesn't get fed
 	# to patch-id:
 	my $cmd = [ '/bin/sh', '-c',
-		"git show '$SHOW_FMT' -z --no-notes --no-patch $oid >h && ".
-		"git show --pretty=format:%n -M --stat -p $oid >p && ".
+		"git show --encoding=UTF-8 '$SHOW_FMT'".
+		" -z --no-notes --no-patch $oid >h && ".
+		'git show --encoding=UTF-8 --pretty=format:%n -M'.
+		" --stat -p $oid >p && ".
 		"git patch-id --stable <p" ];
 	my $xenv = { GIT_DIR => $git->{git_dir} };
 	my $tmp = File::Temp->newdir("show-$oid-XXXX", TMPDIR => 1);
diff --git a/t/solver_git.t b/t/solver_git.t
index 1baa012b..5c7bfa28 100644
--- a/t/solver_git.t
+++ b/t/solver_git.t
@@ -1,5 +1,5 @@
 #!perl -w
-# Copyright (C) 2019-2021 all contributors <meta@public-inbox.org>
+# Copyright (C)  all contributors <meta@public-inbox.org>
 # License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
 use strict;
 use v5.10.1;
@@ -294,6 +294,11 @@ EOF
 			is($res->content, "\0" x $size,
 				"$label content matches");
 		}
+		my $utf8 = 'e022d3377fd2c50fd9931bf96394728958a90bf3';
+		$res = $cb->(GET("/$name/$utf8/s/"));
+		is($res->code, 200, 'shows commit w/ utf8.eml');
+		like($res->content, qr/El&#233;anor/,
+				'UTF-8 commit shown properly');
 	};
 	test_psgi(sub { $www->call(@_) }, $client);
 	SKIP: {

extindex for git? [was: an even bigger git show than before...]

[Eric Wong]

Eric Wong <e@80x24.org> wrote:
> We can do things with patch emails, we might as well do some
> things with git commits for folks with coderepos configured.

I wanted to add search to git repos ages ago, but it was silly
expensive in terms of space.  That was before extindex...

extindex ought to be able to offer space savings across forks
and similar documents (commits vs patch mails).

At least dfpre/dfpost/dfn/subject may be enough, even...

Re: extindex for git? [was: an even bigger git show than before...]

[Eric Wong]

Eric Wong <e@80x24.org> wrote:
> Eric Wong <e@80x24.org> wrote:
> > We can do things with patch emails, we might as well do some
> > things with git commits for folks with coderepos configured.
> 
> I wanted to add search to git repos ages ago, but it was silly
> expensive in terms of space.  That was before extindex...
> 
> extindex ought to be able to offer space savings across forks
> and similar documents (commits vs patch mails).
> 
> At least dfpre/dfpost/dfn/subject may be enough, even...

And I'm also thinking extindexing coderepos can make
auto-assocation with inboxes possible.

Right now, configuring coderepos on a large scale is a huge PITA
given the M:N associations between inboxes and coderepos.

Being able to do fuzzy JOIN-ish operations based on
blobs/filenames/subjects would allow extindex to automatically
associate coderepos with inboxes and vice-versa.

Re: extindex for git? [was: an even bigger git show than before...]

[Konstantin Ryabitsev]

On Thu, Aug 25, 2022 at 09:34:42PM +0000, Eric Wong wrote:
> > I wanted to add search to git repos ages ago, but it was silly
> > expensive in terms of space.  That was before extindex...
> > 
> > extindex ought to be able to offer space savings across forks
> > and similar documents (commits vs patch mails).
> > 
> > At least dfpre/dfpost/dfn/subject may be enough, even...
> 
> And I'm also thinking extindexing coderepos can make
> auto-assocation with inboxes possible.
> 
> Right now, configuring coderepos on a large scale is a huge PITA
> given the M:N associations between inboxes and coderepos.
> 
> Being able to do fuzzy JOIN-ish operations based on
> blobs/filenames/subjects would allow extindex to automatically
> associate coderepos with inboxes and vice-versa.

I wonder how well this would work in the presence of many forks? E.g. most of
the content on git.kernel.org are thin forks of linux.git, so matching by
blobs/filenames/subjects across all of them would return too many hits and
some kind of priority ordering would be required, I think.

Overall, though, I do agree that this would be really handy.

-K

Re: extindex for git? [was: an even bigger git show than before...]

[Eric Wong]

Konstantin Ryabitsev <konstantin@linuxfoundation.org> wrote:
> On Thu, Aug 25, 2022 at 09:34:42PM +0000, Eric Wong wrote:
> > > I wanted to add search to git repos ages ago, but it was silly
> > > expensive in terms of space.  That was before extindex...
> > > 
> > > extindex ought to be able to offer space savings across forks
> > > and similar documents (commits vs patch mails).
> > > 
> > > At least dfpre/dfpost/dfn/subject may be enough, even...
> > 
> > And I'm also thinking extindexing coderepos can make
> > auto-assocation with inboxes possible.
> > 
> > Right now, configuring coderepos on a large scale is a huge PITA
> > given the M:N associations between inboxes and coderepos.
> > 
> > Being able to do fuzzy JOIN-ish operations based on
> > blobs/filenames/subjects would allow extindex to automatically
> > associate coderepos with inboxes and vice-versa.
> 
> I wonder how well this would work in the presence of many forks? E.g. most of
> the content on git.kernel.org are thin forks of linux.git, so matching by
> blobs/filenames/subjects across all of them would return too many hits and
> some kind of priority ordering would be required, I think.

Auto-grouping of coderepos should be possible by common root commit(s).
Config file ordering will be taken into account, of course;
and that's at the discretion of whoever controls $PI_CONFIG.

> Overall, though, I do agree that this would be really handy.

Yes, it's something I've wanted for years; but couldn't figure
out how to do it efficiently until extindex.