[PATCH 0/4] a bunch of cleanups for future expansion

[PATCH 0/4] a bunch of cleanups for future expansion

[Eric Wong]

Pushing 4 changes out which are generic enough for master.
We'll be expanding the scope of the project slightly :>

      config: hoist out common functions
      hval: move PRE constant for wrapping UGC here
      git: cat-file wrapper enhancements
      rename 'GitCatFile' package to 'Git'

 MANIFEST                      |   2 +-
 lib/PublicInbox/Config.pm     |  72 +++++++++++++--------
 lib/PublicInbox/ExtMsg.pm     |  19 ++----
 lib/PublicInbox/Feed.pm       |  27 ++++----
 lib/PublicInbox/Git.pm        | 142 ++++++++++++++++++++++++++++++++++++++++++
 lib/PublicInbox/GitCatFile.pm |  91 ---------------------------
 lib/PublicInbox/Hval.pm       |   5 ++
 lib/PublicInbox/Mbox.pm       |   4 +-
 lib/PublicInbox/NNTP.pm       |   2 +-
 lib/PublicInbox/NewsGroup.pm  |   4 +-
 lib/PublicInbox/SearchIdx.pm  |  18 ++----
 lib/PublicInbox/SearchView.pm |  11 ++--
 lib/PublicInbox/View.pm       |  20 +++---
 lib/PublicInbox/WWW.pm        |  29 ++-------
 t/git.fast-import-data        | 101 ++++++++++++++++++++++++++++++
 t/git.t                       | 134 +++++++++++++++++++++++++++++++++++++++
 16 files changed, 472 insertions(+), 209 deletions(-)

[PATCH 1/4] config: hoist out common functions

[Eric Wong]

These will be reused elsewhere.
---
 lib/PublicInbox/Config.pm | 72 +++++++++++++++++++++++++++++------------------
 lib/PublicInbox/WWW.pm    | 13 +--------
 2 files changed, 46 insertions(+), 39 deletions(-)

diff --git a/lib/PublicInbox/Config.pm b/lib/PublicInbox/Config.pm
index 353a1fb..844f666 100644
--- a/lib/PublicInbox/Config.pm
+++ b/lib/PublicInbox/Config.pm
@@ -5,38 +5,16 @@
 package PublicInbox::Config;
 use strict;
 use warnings;
+use base qw/Exporter/;
+our @EXPORT_OK = qw/try_cat/;
 use File::Path::Expand qw/expand_filename/;
 
 # returns key-value pairs of config directives in a hash
 # if keys may be multi-value, the value is an array ref containing all values
 sub new {
 	my ($class, $file) = @_;
-	my ($in, $out);
-
 	$file = default_file() unless defined($file);
-	my @cmd = (qw/git config/, "--file=$file", '-l');
-	my $cmd = join(' ', @cmd);
-	my $pid = open(my $fh, '-|', @cmd);
-	defined $pid or die "$cmd failed: $!\n";
-	my %rv;
-	foreach my $line (<$fh>) {
-		chomp $line;
-		my ($k, $v) = split(/=/, $line, 2);
-		my $cur = $rv{$k};
-
-		if (defined $cur) {
-			if (ref($cur) eq "ARRAY") {
-				push @$cur, $v;
-			} else {
-				$rv{$k} = [ $cur, $v ];
-			}
-		} else {
-			$rv{$k} = $v;
-		}
-	}
-	close $fh or die "failed to close ($cmd) pipe: $!\n";
-	$? and warn "$$ $cmd exited with: ($pid) $?\n";
-	bless \%rv, $class;
+	bless git_config_dump($file), $class;
 }
 
 sub lookup {
@@ -81,11 +59,51 @@ sub get {
 	$self->{"publicinbox.$listname.$key"};
 }
 
+sub config_dir { $ENV{PI_DIR} || expand_filename('~/.public-inbox') }
+
 sub default_file {
 	my $f = $ENV{PI_CONFIG};
 	return $f if defined $f;
-	my $pi_dir = $ENV{PI_DIR} || expand_filename('~/.public-inbox');
-	"$pi_dir/config";
+	config_dir() . '/config';
+}
+
+sub git_config_dump {
+	my ($file) = @_;
+	my ($in, $out);
+	my @cmd = (qw/git config/, "--file=$file", '-l');
+	my $cmd = join(' ', @cmd);
+	my $pid = open(my $fh, '-|', @cmd);
+	defined $pid or die "$cmd failed: $!\n";
+	my %rv;
+	foreach my $line (<$fh>) {
+		chomp $line;
+		my ($k, $v) = split(/=/, $line, 2);
+		my $cur = $rv{$k};
+
+		if (defined $cur) {
+			if (ref($cur) eq "ARRAY") {
+				push @$cur, $v;
+			} else {
+				$rv{$k} = [ $cur, $v ];
+			}
+		} else {
+			$rv{$k} = $v;
+		}
+	}
+	close $fh or die "failed to close ($cmd) pipe: $!\n";
+	$? and warn "$$ $cmd exited with: ($pid) $?\n";
+	\%rv;
+}
+
+sub try_cat {
+	my ($path) = @_;
+	my $rv;
+	if (open(my $fh, '<', $path)) {
+		local $/;
+		$rv = <$fh>;
+		close $fh;
+	}
+	$rv;
 }
 
 1;
diff --git a/lib/PublicInbox/WWW.pm b/lib/PublicInbox/WWW.pm
index d00dfe7..5cd3bc6 100644
--- a/lib/PublicInbox/WWW.pm
+++ b/lib/PublicInbox/WWW.pm
@@ -13,7 +13,7 @@ package PublicInbox::WWW;
 use 5.008;
 use strict;
 use warnings;
-use PublicInbox::Config;
+use PublicInbox::Config qw(try_cat);
 use URI::Escape qw(uri_escape_utf8 uri_unescape);
 use constant SSOMA_URL => 'http://ssoma.public-inbox.org/';
 use constant PI_URL => 'http://public-inbox.org/';
@@ -218,17 +218,6 @@ sub ctx_get {
 	$val;
 }
 
-sub try_cat {
-	my ($path) = @_;
-	my $rv;
-	if (open(my $fh, '<', $path)) {
-		local $/;
-		$rv = <$fh>;
-		close $fh;
-	}
-	$rv;
-}
-
 sub footer {
 	my ($ctx) = @_;
 	return '' unless $ctx;
-- 
EW

[PATCH 2/4] hval: move PRE constant for wrapping UGC here

[Eric Wong]

User-generated content (UGC) may have excessively long lines
which screw up rendering.  This is the only bit of CSS we use.
---
 lib/PublicInbox/Feed.pm       |  4 ++--
 lib/PublicInbox/Hval.pm       |  5 +++++
 lib/PublicInbox/SearchView.pm |  2 +-
 lib/PublicInbox/View.pm       | 16 ++++++++--------
 4 files changed, 16 insertions(+), 11 deletions(-)

diff --git a/lib/PublicInbox/Feed.pm b/lib/PublicInbox/Feed.pm
index 1201dd1..68f1e67 100644
--- a/lib/PublicInbox/Feed.pm
+++ b/lib/PublicInbox/Feed.pm
@@ -134,9 +134,9 @@ sub emit_html_index {
 			  qq{ <input\nname=q\ntype=text />} .
 			  qq{<input\ntype=submit\nvalue=search />} .
 			  qq{</tt></form>} .
-			  PublicInbox::View::PRE_WRAP;
+			  PublicInbox::Hval::PRE;
 	} else {
-		$top = PublicInbox::View::PRE_WRAP . $top . "\n";
+		$top = PublicInbox::Hval::PRE . $top . "\n";
 	}
 
 	$fh->write("<html><head><title>$title</title>" .
diff --git a/lib/PublicInbox/Hval.pm b/lib/PublicInbox/Hval.pm
index ab6e044..e0b85c6 100644
--- a/lib/PublicInbox/Hval.pm
+++ b/lib/PublicInbox/Hval.pm
@@ -10,6 +10,11 @@ use Encode qw(find_encoding);
 use URI::Escape qw(uri_escape_utf8);
 use PublicInbox::MID qw/mid_clean/;
 
+# for user-generated content (UGC) which may have excessively long lines
+# and screw up rendering on some browsers.  This is the only CSS style
+# feature we use.
+use constant PRE => "<pre\nstyle=\"white-space:pre-wrap\">";
+
 my $enc_ascii = find_encoding('us-ascii');
 
 sub new {
diff --git a/lib/PublicInbox/SearchView.pm b/lib/PublicInbox/SearchView.pm
index 7f79090..ea8a45a 100644
--- a/lib/PublicInbox/SearchView.pm
+++ b/lib/PublicInbox/SearchView.pm
@@ -31,7 +31,7 @@ sub sres_top_html {
 		$total = $mset->get_matches_estimated;
 	};
 	my $err = $@;
-	my $res = html_start($q, $ctx) . PublicInbox::View::PRE_WRAP;
+	my $res = html_start($q, $ctx) . PublicInbox::Hval::PRE;
 	if ($err) {
 		$code = 400;
 		$res .= err_txt($err) . "</pre><hr /><pre>" . foot($ctx);
diff --git a/lib/PublicInbox/View.pm b/lib/PublicInbox/View.pm
index 3c4e954..68741c5 100644
--- a/lib/PublicInbox/View.pm
+++ b/lib/PublicInbox/View.pm
@@ -21,7 +21,6 @@ require POSIX;
 # TODO: make these constants tunable
 use constant MAX_INLINE_QUOTED => 12; # half an 80x24 terminal
 use constant MAX_TRUNC_LEN => 72;
-use constant PRE_WRAP => "<pre\nstyle=\"white-space:pre-wrap\">";
 use constant T_ANCHOR => '#u';
 use constant INDENT => '  ';
 
@@ -39,7 +38,7 @@ sub msg_html {
 	}
 	headers_to_html_header($mime, $full_pfx, $ctx) .
 		multipart_text_as_html($mime, $full_pfx) .
-		'</pre><hr />' . PRE_WRAP .
+		'</pre><hr />' . PublicInbox::Hval::PRE .
 		html_footer($mime, 1, $full_pfx, $ctx) .
 		$footer .
 		'</pre></body></html>';
@@ -48,7 +47,8 @@ sub msg_html {
 sub feed_entry {
 	my ($class, $mime, $full_pfx) = @_;
 
-	PRE_WRAP . multipart_text_as_html($mime, $full_pfx) . '</pre>';
+	PublicInbox::Hval::PRE .
+		multipart_text_as_html($mime, $full_pfx) . '</pre>';
 }
 
 sub in_reply_to {
@@ -107,7 +107,7 @@ sub index_entry {
 	if ($level) {
 		$rv .= '<td><pre>' . (INDENT x $level) . '</pre></td>';
 	}
-	$rv .= "<td\nid=s$midx>" . PRE_WRAP;
+	$rv .= "<td\nid=s$midx>" . PublicInbox::Hval::PRE;
 	$rv .= "<b\nid=\"$id\">$subj</b>\n";
 	$rv .= "- $from @ $ts UTC - ";
 	$rv .= "<a\nhref=\"#s$next\">next</a>";
@@ -206,8 +206,8 @@ sub emit_thread_html {
 	$next .= "\ndownload thread: ";
 	$next .= "<a\n$MBOX_TITLE\nhref=\"../t.mbox.gz\">mbox.gz</a>";
 	$next .= " / follow: <a\nhref=\"../t.atom\">Atom feed</a>";
-	$cb->write("<hr />" . PRE_WRAP . $next . "\n\n". $foot .
-		   "</pre></body></html>");
+	$cb->write("<hr />" . PublicInbox::Hval::PRE . $next . "\n\n".
+		$foot .  "</pre></body></html>");
 	$cb->close;
 }
 
@@ -457,7 +457,7 @@ sub headers_to_html_header {
 	$rv .= "\n";
 
 	("<html><head><title>".  join(' - ', @title) .
-	 "</title>$atom</head><body>" . PRE_WRAP . $rv);
+	 "</title>$atom</head><body>" . PublicInbox::Hval::PRE . $rv);
 }
 
 sub thread_inline {
@@ -634,7 +634,7 @@ sub ghost_table {
 	my ($upfx, $mid, $level) = @_;
 	"<table\nsummary=ghost><tr><td>" .
 		(INDENT x $level) . "</td><td>" .
-		PRE_WRAP . ghost_parent($upfx, $mid) .
+		PublicInbox::Hval::PRE . ghost_parent($upfx, $mid) .
 		'</pre></td></table>';
 }
 
-- 
EW

[PATCH 3/4] git: cat-file wrapper enhancements

[Eric Wong]

The "cat_file" sub now allows a block to be passed for partial
processing.  Additionally, a new "check" method is added to
retrieve only object metadata: (SHA-1 identifier, type, size)
---
 lib/PublicInbox/GitCatFile.pm | 125 ++++++++++++++++++++++++++-------------
 t/git.fast-import-data        | 101 +++++++++++++++++++++++++++++++
 t/git.t                       | 134 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 318 insertions(+), 42 deletions(-)
 create mode 100644 t/git.fast-import-data
 create mode 100644 t/git.t

diff --git a/lib/PublicInbox/GitCatFile.pm b/lib/PublicInbox/GitCatFile.pm
index dd95d5f..b3666a0 100644
--- a/lib/PublicInbox/GitCatFile.pm
+++ b/lib/PublicInbox/GitCatFile.pm
@@ -17,75 +17,116 @@ sub new {
 	bless { git_dir => $git_dir }, $class
 }
 
-sub _cat_file_begin {
-	my ($self) = @_;
-	return if $self->{pid};
+sub _bidi_pipe {
+	my ($self, $batch, $in, $out, $pid) = @_;
+	return if $self->{$pid};
 	my ($in_r, $in_w, $out_r, $out_w);
 
-	pipe($in_r, $in_w) or die "pipe failed: $!\n";
-	pipe($out_r, $out_w) or die "pipe failed: $!\n";
+	pipe($in_r, $in_w) or fail($self, "pipe failed: $!");
+	pipe($out_r, $out_w) or fail($self, "pipe failed: $!");
 
-	my @cmd = ('git', "--git-dir=$self->{git_dir}", qw(cat-file --batch));
-	my $pid = fork;
-	defined $pid or die "fork failed: $!\n";
-	if ($pid == 0) {
+	my @cmd = ('git', "--git-dir=$self->{git_dir}", qw(cat-file), $batch);
+	$self->{$pid} = fork;
+	defined $self->{$pid} or fail($self, "fork failed: $!");
+	if ($self->{$pid} == 0) {
 		dup2(fileno($out_r), 0) or die "redirect stdin failed: $!\n";
 		dup2(fileno($in_w), 1) or die "redirect stdout failed: $!\n";
 		exec(@cmd) or die 'exec `' . join(' '). "' failed: $!\n";
 	}
-	close $out_r or die "close failed: $!\n";
-	close $in_w or die "close failed: $!\n";
+	close $out_r or fail($self, "close failed: $!");
+	close $in_w or fail($self, "close failed: $!");
 	$out_w->autoflush(1);
-
-	$self->{in} = $in_r;
-	$self->{out} = $out_w;
-	$self->{pid} = $pid;
+	$self->{$out} = $out_w;
+	$self->{$in} = $in_r;
 }
 
 sub cat_file {
-	my ($self, $object, $sizeref) = @_;
+	my ($self, $obj, $ref) = @_;
 
-	$self->_cat_file_begin;
-	print { $self->{out} } $object, "\n" or die "pipe write error: $!\n";
+	$self->_bidi_pipe(qw(--batch in out pid));
+	$self->{out}->print($obj, "\n") or fail($self, "write error: $!");
 
 	my $in = $self->{in};
-	my $head = <$in>;
+	my $head = $in->getline;
 	$head =~ / missing$/ and return undef;
 	$head =~ /^[0-9a-f]{40} \S+ (\d+)$/ or
-		die "Unexpected result from git cat-file: $head\n";
+		fail($self, "Unexpected result from git cat-file: $head");
 
 	my $size = $1;
-	$$sizeref = $size if $sizeref;
-	my $bytes_left = $size;
-	my $offset = 0;
-	my $rv = '';
-
-	while ($bytes_left) {
-		my $read = read($in, $rv, $bytes_left, $offset);
-		defined($read) or die "sysread pipe failed: $!\n";
-		$bytes_left -= $read;
-		$offset += $read;
-	}
+	my $ref_type = $ref ? ref($ref) : '';
+
+	my $rv;
+	my $left = $size;
+	$$ref = $size if ($ref_type eq 'SCALAR');
+	my $cb_err;
 
-	my $read = read($in, my $buf, 1);
-	defined($read) or die "read pipe failed: $!\n";
-	if ($read != 1 || $buf ne "\n") {
-		die "newline missing after blob\n";
+	if ($ref_type eq 'CODE') {
+		$rv = eval { $ref->($in, \$left) };
+		$cb_err = $@;
+		# drain the rest
+		my $max = 8192;
+		while ($left > 0) {
+			my $r = read($in, my $x, $left > $max ? $max : $left);
+			defined($r) or fail($self, "read failed: $!");
+			$r == 0 and fail($self, 'exited unexpectedly');
+			$left -= $r;
+		}
+	} else {
+		my $offset = 0;
+		my $buf = '';
+		while ($left > 0) {
+			my $r = read($in, $buf, $left, $offset);
+			defined($r) or fail($self, "read failed: $!");
+			$r == 0 and fail($self, 'exited unexpectedly');
+			$left -= $r;
+			$offset += $r;
+		}
+		$rv = \$buf;
 	}
-	\$rv;
+
+	my $r = read($in, my $buf, 1);
+	defined($r) or fail($self, "read failed: $!");
+	fail($self, 'newline missing after blob') if ($r != 1 || $buf ne "\n");
+	die $cb_err if $cb_err;
+
+	$rv;
 }
 
-sub DESTROY {
-	my ($self) = @_;
-	my $pid = $self->{pid} or return;
-	$self->{pid} = undef;
-	foreach my $f (qw(in out)) {
+sub check {
+	my ($self, $obj) = @_;
+	$self->_bidi_pipe(qw(--batch-check in_c out_c pid_c));
+	$self->{out_c}->print($obj, "\n") or fail($self, "write error: $!");
+	chomp(my $line = $self->{in_c}->getline);
+	my ($hex, $type, $size) = split(' ', $line);
+	return if $type eq 'missing';
+	($hex, $type, $size);
+}
+
+sub _destroy {
+	my ($self, $in, $out, $pid) = @_;
+	my $p = $self->{$pid} or return;
+	$self->{$pid} = undef;
+	foreach my $f ($in, $out) {
 		my $fh = $self->{$f};
 		defined $fh or next;
 		close $fh;
 		$self->{$f} = undef;
 	}
-	waitpid $pid, 0;
+	waitpid $p, 0;
+}
+
+sub fail {
+	my ($self, $msg) = @_;
+	cleanup($self);
+	die $msg;
 }
 
+sub cleanup {
+	my ($self) = @_;
+	_destroy($self, qw(in out pid));
+	_destroy($self, qw(in_c out_c pid_c));
+}
+
+sub DESTROY { cleanup(@_) }
+
 1;
diff --git a/t/git.fast-import-data b/t/git.fast-import-data
new file mode 100644
index 0000000..4a105ee
--- /dev/null
+++ b/t/git.fast-import-data
@@ -0,0 +1,101 @@
+blob
+mark :1
+data 6
+hello
+
+reset refs/heads/header
+commit refs/heads/header
+mark :2
+author AU Thor <e@example.com> 0 +0000
+committer AU Thor <e@example.com> 0 +0000
+data 8
+initial
+M 100644 :1 foo.txt
+
+blob
+mark :3
+data 12
+hello
+world
+
+commit refs/heads/master
+mark :4
+author AU Thor <e@example.com> 0 +0000
+committer AU Thor <e@example.com> 0 +0000
+data 7
+second
+from :2
+M 100644 :3 foo.txt
+
+blob
+mark :5
+data 12
+-----
+hello
+
+commit refs/heads/header
+mark :6
+author AU Thor <e@example.com> 0 +0000
+committer AU Thor <e@example.com> 0 +0000
+data 11
+add header
+from :2
+M 100644 :5 foo.txt
+
+blob
+mark :7
+data 18
+-----
+hello
+world
+
+commit refs/heads/master
+mark :8
+author AU Thor <e@example.com> 0 +0000
+committer AU Thor <e@example.com> 0 +0000
+data 46
+Merge branch 'header'
+
+* header:
+  add header
+from :4
+merge :6
+M 100644 :7 foo.txt
+
+blob
+mark :9
+data 0
+
+blob
+mark :10
+data 16
+dir/dur/der/derp
+commit refs/heads/master
+mark :11
+author AU Thor <e@example.com> 0 +0000
+committer AU Thor <e@example.com> 0 +0000
+data 26
+add symlink and deep file
+from :8
+M 100644 :9 dir/dur/der/derp
+M 120000 :10 link
+
+blob
+mark :12
+data 78
+[submodule "git"]
+	path = git
+	url = git://git.kernel.org/pub/scm/git/git.git
+
+commit refs/heads/master
+mark :13
+author AU Thor <e@example.com> 0 +0000
+committer AU Thor <e@example.com> 0 +0000
+data 18
+add git submodule
+from :11
+M 100644 :12 .gitmodules
+M 160000 f3adf457e046f92f039353762a78dcb3afb2cb13 git
+
+reset refs/heads/master
+from :13
diff --git a/t/git.t b/t/git.t
new file mode 100644
index 0000000..4532921
--- /dev/null
+++ b/t/git.t
@@ -0,0 +1,134 @@
+# Copyright (C) 2015 all contributors <meta@public-inbox.org>
+# License: AGPL-3.0+ (https://www.gnu.org/licenses/agpl-3.0.txt)
+use strict;
+use warnings;
+use Test::More;
+use File::Temp qw/tempdir/;
+my $dir = tempdir(CLEANUP => 1);
+use Cwd qw/getcwd/;
+
+use_ok 'PublicInbox::GitCatFile';
+{
+	is(system(qw(git init -q --bare), $dir), 0, 'created git directory');
+	my @cmd = ('git', "--git-dir=$dir", 'fast-import', '--quiet');
+
+	my $fi_data = getcwd().'/t/git.fast-import-data';
+	ok(-r $fi_data, "fast-import data readable (or run test at top level)");
+	my $pid = fork;
+	defined $pid or die "fork failed: $!\n";
+	if ($pid == 0) {
+		open STDIN, '<', $fi_data or die "open $fi_data: $!\n";
+		exec @cmd;
+		die "failed exec: ",join(' ', @cmd),": $!\n";
+	}
+	waitpid $pid, 0;
+	is($?, 0, 'fast-import succeeded');
+}
+
+{
+	my $gcf = PublicInbox::GitCatFile->new($dir);
+	my $f = 'HEAD:foo.txt';
+	my @x = $gcf->check($f);
+	is(scalar @x, 3, 'returned 3 element array for existing file');
+	like($x[0], qr/\A[a-f0-9]{40}\z/, 'returns obj ID in 1st element');
+	is('blob', $x[1], 'returns obj type in 2nd element');
+	like($x[2], qr/\A\d+\z/, 'returns obj size in 3rd element');
+
+	my $raw = $gcf->cat_file($f);
+	is($x[2], length($$raw), 'length matches');
+
+	{
+		my $size;
+		my $rv = $gcf->cat_file($f, sub {
+			my ($in, $left) = @_;
+			$size = $$left;
+			'nothing'
+		});
+		is($rv, 'nothing', 'returned from callback without reading');
+		is($size, $x[2], 'set size for callback correctly');
+	}
+
+	eval { $gcf->cat_file($f, sub { die 'OMG' }) };
+	like($@, qr/\bOMG\b/, 'died in callback propagated');
+	is(${$gcf->cat_file($f)}, $$raw, 'not broken after failures');
+
+	{
+		my ($buf, $r);
+		my $rv = $gcf->cat_file($f, sub {
+			my ($in, $left) = @_;
+			$r = read($in, $buf, 2);
+			$$left -= $r;
+			'blah'
+		});
+		is($r, 2, 'only read 2 bytes');
+		is($buf, '--', 'partial read succeeded');
+		is($rv, 'blah', 'return value propagated');
+	}
+	is(${$gcf->cat_file($f)}, $$raw, 'not broken after partial read');
+}
+
+if (1) {
+	use POSIX qw(dup2);
+	my @cmd = ('git', "--git-dir=$dir", qw(hash-object -w --stdin));
+
+	# need a big file, use the AGPL-3.0 :p
+	my $big_data = getcwd().'/COPYING';
+	ok(-r $big_data, 'COPYING readable');
+	my $size = -s $big_data;
+	ok($size > 8192, 'file is big enough');
+
+	my ($r, $w);
+	ok(pipe($r, $w), 'created pipe');
+
+	my $pid = fork;
+	defined $pid or die "fork failed: $!\n";
+	if ($pid == 0) {
+		close $r;
+		open STDIN, '<', $big_data or die "open $big_data: $!\n";
+		dup2(fileno($w), 1);
+		exec @cmd;
+		die "failed exec: ",join(' ', @cmd),": $!\n";
+	}
+	close $w;
+	my $n = read $r, my $buf, 41;
+	waitpid $pid, 0;
+	is(0, $?, 'hashed object successfully');
+	chomp $buf;
+
+	my $gcf = PublicInbox::GitCatFile->new($dir);
+	my $rsize;
+	is($gcf->cat_file($buf, sub {
+		$rsize = ${$_[1]};
+		'x';
+	}), 'x', 'checked input');
+	is($rsize, $size, 'got correct size on big file');
+
+	my $x = $gcf->cat_file($buf, \$rsize);
+	is($rsize, $size, 'got correct size ref on big file');
+	is(length($$x), $size, 'read correct number of bytes');
+
+	my $rline;
+	$gcf->cat_file($buf, sub {
+		my ($in, $left) = @_;
+		$rline = <$in>;
+		$$left -= length($rline);
+	});
+	{
+		open my $fh, '<', $big_data or die "open failed: $!\n";
+		is($rline, <$fh>, 'first line matches');
+	};
+
+	my $all;
+	$gcf->cat_file($buf, sub {
+		my ($in, $left) = @_;
+		my $x = read($in, $all, $$left);
+		$$left -= $x;
+	});
+	{
+		open my $fh, '<', $big_data or die "open failed: $!\n";
+		local $/;
+		is($all, <$fh>, 'entire read matches');
+	};
+}
+
+done_testing();
-- 
EW

[PATCH 4/4] rename 'GitCatFile' package to 'Git'

[Eric Wong]

We'll be using it for more than just cat-file.
Adding a `popen' API for internal use allows us to save a bunch
of code in other places.
---
 MANIFEST                                  |  2 +-
 lib/PublicInbox/ExtMsg.pm                 | 19 ++++---------------
 lib/PublicInbox/Feed.pm                   | 23 +++++++++--------------
 lib/PublicInbox/{GitCatFile.pm => Git.pm} | 12 +++++++++++-
 lib/PublicInbox/Mbox.pm                   |  4 ++--
 lib/PublicInbox/NNTP.pm                   |  2 +-
 lib/PublicInbox/NewsGroup.pm              |  4 ++--
 lib/PublicInbox/SearchIdx.pm              | 18 ++++++------------
 lib/PublicInbox/SearchView.pm             |  9 +++------
 lib/PublicInbox/View.pm                   |  4 ++--
 lib/PublicInbox/WWW.pm                    | 16 +++-------------
 t/git.t                                   |  6 +++---
 12 files changed, 47 insertions(+), 72 deletions(-)
 rename lib/PublicInbox/{GitCatFile.pm => Git.pm} (92%)

diff --git a/MANIFEST b/MANIFEST
index 4c3b07d..5d45046 100644
--- a/MANIFEST
+++ b/MANIFEST
@@ -21,7 +21,7 @@ examples/public-inbox.psgi
 lib/PublicInbox/Config.pm
 lib/PublicInbox/Feed.pm
 lib/PublicInbox/Filter.pm
-lib/PublicInbox/GitCatFile.pm
+lib/PublicInbox/Git.pm
 lib/PublicInbox/Hval.pm
 lib/PublicInbox/MDA.pm
 lib/PublicInbox/View.pm
diff --git a/lib/PublicInbox/ExtMsg.pm b/lib/PublicInbox/ExtMsg.pm
index 82f4c63..0b66754 100644
--- a/lib/PublicInbox/ExtMsg.pm
+++ b/lib/PublicInbox/ExtMsg.pm
@@ -72,21 +72,10 @@ sub ext_msg {
 	my $path = "HEAD:" . mid2path($mid);
 
 	foreach my $n (@nox) {
-		my @cmd = ('git', "--git-dir=$n->{git_dir}", 'cat-file',
-			   '-t', $path);
-		my $pid = open my $fh, '-|';
-		defined $pid or die "fork failed: $!\n";
-
-		if ($pid == 0) {
-			open STDERR, '>', '/dev/null'; # ignore errors
-			exec @cmd or die "exec failed: $!\n";
-		} else {
-			my $type = eval { local $/; <$fh> };
-			close $fh;
-			if ($? == 0 && $type eq "blob\n") {
-				return r302($n->{url}, $mid);
-			}
-		}
+		# TODO: reuse existing PublicInbox::Git objects to save forks
+		my $git = PublicInbox::Git->new($n->{git_dir});
+		my (undef, $type, undef) = $git->check($path);
+		return r302($n->{url}, $mid) if ($type eq 'blob');
 	}
 
 	# fall back to partial MID matching
diff --git a/lib/PublicInbox/Feed.pm b/lib/PublicInbox/Feed.pm
index 68f1e67..150bea0 100644
--- a/lib/PublicInbox/Feed.pm
+++ b/lib/PublicInbox/Feed.pm
@@ -9,7 +9,7 @@ use Email::Address;
 use Email::MIME;
 use Date::Parse qw(strptime);
 use PublicInbox::Hval;
-use PublicInbox::GitCatFile;
+use PublicInbox::Git;
 use PublicInbox::View;
 use PublicInbox::MID qw/mid_clean mid2path/;
 use POSIX qw/strftime/;
@@ -66,7 +66,7 @@ sub emit_atom {
 	my $max = $ctx->{max} || MAX_PER_PAGE;
 	my $feed_opts = get_feedopts($ctx);
 	my $x = atom_header($feed_opts);
-	my $git = PublicInbox::GitCatFile->new($ctx->{git_dir});
+	my $git = $ctx->{git} ||= PublicInbox::Git->new($ctx->{git_dir});
 	each_recent_blob($ctx, sub {
 		my ($path, undef, $ts) = @_;
 		if (defined $x) {
@@ -75,7 +75,6 @@ sub emit_atom {
 		}
 		add_to_feed($feed_opts, $fh, $path, $git);
 	});
-	$git = undef; # destroy pipes
 	end_feed($fh);
 }
 
@@ -105,11 +104,10 @@ sub emit_atom_thread {
 	$feed_opts->{url} = $html_url;
 	$feed_opts->{emit_header} = 1;
 
-	my $git = PublicInbox::GitCatFile->new($ctx->{git_dir});
+	my $git = $ctx->{git} ||= PublicInbox::Git->new($ctx->{git_dir});
 	foreach my $msg (@{$res->{msgs}}) {
 		add_to_feed($feed_opts, $fh, mid2path($msg->mid), $git);
 	}
-	$git = undef; # destroy pipes
 	end_feed($fh);
 }
 
@@ -167,7 +165,7 @@ sub emit_html_index {
 
 sub emit_index_nosrch {
 	my ($ctx, $state, $fh) = @_;
-	my $git = PublicInbox::GitCatFile->new($ctx->{git_dir});
+	my $git = $ctx->{git} ||= PublicInbox::Git->new($ctx->{git_dir});
 	my (undef, $last) = each_recent_blob($ctx, sub {
 		my ($path, $commit, $ts, $u, $subj) = @_;
 		$state->{first} ||= $commit;
@@ -219,14 +217,11 @@ sub each_recent_blob {
 	# get recent messages
 	# we could use git log -z, but, we already know ssoma will not
 	# leave us with filenames with spaces in them..
-	my @cmd = ('git', "--git-dir=$ctx->{git_dir}",
-			qw/log --no-notes --no-color --raw -r
-			   --abbrev=16 --abbrev-commit/,
-			"--format=%h%x00%ct%x00%an%x00%s%x00");
-	push @cmd, $range;
-
-	my $pid = open(my $log, '-|', @cmd) or
-		die('open `'.join(' ', @cmd) . " pipe failed: $!\n");
+	my $git = $ctx->{git} ||= PublicInbox::Git->new($ctx->{git_dir});
+	my $log = $git->popen(qw/log --no-notes --no-color --raw -r
+				--abbrev=16 --abbrev-commit/,
+				"--format=%h%x00%ct%x00%an%x00%s%x00",
+				$range);
 	my %deleted; # only an optimization at this point
 	my $last;
 	my $nr = 0;
diff --git a/lib/PublicInbox/GitCatFile.pm b/lib/PublicInbox/Git.pm
similarity index 92%
rename from lib/PublicInbox/GitCatFile.pm
rename to lib/PublicInbox/Git.pm
index b3666a0..5135862 100644
--- a/lib/PublicInbox/GitCatFile.pm
+++ b/lib/PublicInbox/Git.pm
@@ -6,7 +6,7 @@
 # This is based on code in Git.pm which is GPLv2, but modified to avoid
 # dependence on environment variables for compatibility with mod_perl.
 # There are also API changes to simplify our usage and data set.
-package PublicInbox::GitCatFile;
+package PublicInbox::Git;
 use strict;
 use warnings;
 use POSIX qw(dup2);
@@ -121,6 +121,16 @@ sub fail {
 	die $msg;
 }
 
+sub popen {
+	my ($self, @cmd) = @_;
+	my $mode = '-|';
+	$mode = shift @cmd if ($cmd[0] eq '|-');
+	@cmd = ('git', "--git-dir=$self->{git_dir}", @cmd);
+	my $pid = open my $fh, $mode, @cmd or
+		die('open `'.join(' ', @cmd) . " pipe failed: $!\n");
+	$fh;
+}
+
 sub cleanup {
 	my ($self) = @_;
 	_destroy($self, qw(in out pid));
diff --git a/lib/PublicInbox/Mbox.pm b/lib/PublicInbox/Mbox.pm
index c180a0d..0d67981 100644
--- a/lib/PublicInbox/Mbox.pm
+++ b/lib/PublicInbox/Mbox.pm
@@ -86,9 +86,9 @@ sub emit_mbox {
 	my $fh = $response->([200, ['Content-Type' => "application/$type"]]);
 	$fh = PublicInbox::MboxGz->new($fh) if $sfx;
 
-	require PublicInbox::GitCatFile;
+	require PublicInbox::Git;
 	my $mid = $ctx->{mid};
-	my $git = PublicInbox::GitCatFile->new($ctx->{git_dir});
+	my $git = $ctx->{git} ||= PublicInbox::Git->new($ctx->{git_dir});
 	my %opts = (offset => 0);
 	my $nr;
 	do {
diff --git a/lib/PublicInbox/NNTP.pm b/lib/PublicInbox/NNTP.pm
index 295aee0..097c57e 100644
--- a/lib/PublicInbox/NNTP.pm
+++ b/lib/PublicInbox/NNTP.pm
@@ -9,7 +9,7 @@ use base qw(Danga::Socket);
 use fields qw(nntpd article rbuf ng long_res);
 use PublicInbox::Search;
 use PublicInbox::Msgmap;
-use PublicInbox::GitCatFile;
+use PublicInbox::Git;
 use PublicInbox::MID qw(mid2path);
 use Email::MIME;
 use Data::Dumper qw(Dumper);
diff --git a/lib/PublicInbox/NewsGroup.pm b/lib/PublicInbox/NewsGroup.pm
index 3a31895..b20180e 100644
--- a/lib/PublicInbox/NewsGroup.pm
+++ b/lib/PublicInbox/NewsGroup.pm
@@ -10,7 +10,7 @@ use Scalar::Util qw(weaken);
 require Danga::Socket;
 require PublicInbox::Msgmap;
 require PublicInbox::Search;
-require PublicInbox::GitCatFile;
+require PublicInbox::Git;
 
 sub new {
 	my ($class, $name, $git_dir, $address) = @_;
@@ -32,7 +32,7 @@ sub defer_weaken {
 sub gcf {
 	my ($self) = @_;
 	$self->{gcf} ||= eval {
-		my $gcf = PublicInbox::GitCatFile->new($self->{git_dir});
+		my $gcf = PublicInbox::Git->new($self->{git_dir});
 
 		# git repos may be repacked and old packs unlinked
 		defer_weaken($self, 'gcf');
diff --git a/lib/PublicInbox/SearchIdx.pm b/lib/PublicInbox/SearchIdx.pm
index e9af547..6727299 100644
--- a/lib/PublicInbox/SearchIdx.pm
+++ b/lib/PublicInbox/SearchIdx.pm
@@ -11,6 +11,7 @@ use strict;
 use warnings;
 use base qw(PublicInbox::Search);
 use PublicInbox::MID qw/mid_clean id_compress/;
+require PublicInbox::Git;
 *xpfx = *PublicInbox::Search::xpfx;
 
 use constant MAX_MID_SIZE => 244; # max term size - 1 in Xapian
@@ -331,16 +332,11 @@ sub rlog {
 	my $h40 = $hex .'{40}';
 	my $addmsg = qr!^:000000 100644 \S+ ($h40) A\t${hex}{2}/${hex}{38}$!;
 	my $delmsg = qr!^:100644 000000 ($h40) \S+ D\t${hex}{2}/${hex}{38}$!;
-	my $git_dir = $self->{git_dir};
-	require PublicInbox::GitCatFile;
-	my $git = PublicInbox::GitCatFile->new($git_dir);
-	my @cmd = ('git', "--git-dir=$git_dir", "log",
-		    qw/--reverse --no-notes --no-color --raw -r --no-abbrev/,
-		    $range);
+	my $git = PublicInbox::Git->new($self->{git_dir});
+	my $log = $git->popen(qw/log --reverse --no-notes --no-color
+				--raw -r --no-abbrev/, $range);
 	my $latest;
 	my $bytes;
-	my $pid = open(my $log, '-|', @cmd) or
-		die('open` '.join(' ', @cmd) . " pipe failed: $!\n");
 	while (my $line = <$log>) {
 		if ($line =~ /$addmsg/o) {
 			my $mime = do_cat_mail($git, $1, \$bytes) or next;
@@ -447,10 +443,8 @@ sub merge_threads {
 
 sub _read_git_config_perm {
 	my ($self) = @_;
-	my @cmd = ('git', "--git-dir=$self->{git_dir}",
-		   qw(config core.sharedRepository));
-	my $pid = open(my $fh, '-|', @cmd) or
-		die('open `'.join(' ', @cmd) . " pipe failed: $!\n");
+	my @cmd = qw(config core.sharedRepository);
+	my $fh = PublicInbox::Git->new($self->{git_dir})->popen(@cmd);
 	my $perm = <$fh>;
 	close $fh;
 	chomp $perm if defined $perm;
diff --git a/lib/PublicInbox/SearchView.pm b/lib/PublicInbox/SearchView.pm
index ea8a45a..fec4f39 100644
--- a/lib/PublicInbox/SearchView.pm
+++ b/lib/PublicInbox/SearchView.pm
@@ -10,6 +10,7 @@ use PublicInbox::Hval;
 use PublicInbox::View;
 use PublicInbox::MID qw(mid2path mid_clean);
 use Email::MIME;
+require PublicInbox::Git;
 our $LIM = 50;
 
 sub sres_top_html {
@@ -169,12 +170,10 @@ sub tdump {
 		$th->order(*PublicInbox::View::rsort_ts);
 	}
 
-	require PublicInbox::GitCatFile;
-	my $git = PublicInbox::GitCatFile->new($ctx->{git_dir});
+	my $git = $ctx->{git} ||= PublicInbox::Git->new($ctx->{git_dir});
 	my $state = { ctx => $ctx, anchor_idx => 0, pct => \%pct };
 	$ctx->{searchview} = 1;
 	tdump_ent($fh, $git, $state, $_, 0) for $th->rootset;
-	$git = undef;
 	Email::Address->purge_cache;
 
 	$fh->write(search_nav_bot($mset, $q). "\n\n" .
@@ -236,8 +235,7 @@ sub html_start {
 sub adump {
 	my ($cb, $mset, $q, $ctx) = @_;
 	my $fh = $cb->([ 200, ['Content-Type' => 'application/atom+xml']]);
-	require PublicInbox::GitCatFile;
-	my $git = PublicInbox::GitCatFile->new($ctx->{git_dir});
+	my $git = $ctx->{git_dir} ||= PublicInbox::Git->new($ctx->{git_dir});
 	my $feed_opts = PublicInbox::Feed::get_feedopts($ctx);
 	my $x = PublicInbox::Hval->new_oneline($q->{q})->as_html;
 	$x = qq{$x - search results};
@@ -251,7 +249,6 @@ sub adump {
 		$x = mid2path($x);
 		PublicInbox::Feed::add_to_feed($feed_opts, $fh, $x, $git);
 	}
-	$git = undef;
 	PublicInbox::Feed::end_feed($fh);
 }
 
diff --git a/lib/PublicInbox/View.pm b/lib/PublicInbox/View.pm
index 68741c5..2ca7f95 100644
--- a/lib/PublicInbox/View.pm
+++ b/lib/PublicInbox/View.pm
@@ -183,8 +183,8 @@ sub emit_thread_html {
 		anchor_idx => 0,
 	};
 
-	require PublicInbox::GitCatFile;
-	my $git = PublicInbox::GitCatFile->new($ctx->{git_dir});
+	require PublicInbox::Git;
+	my $git = $ctx->{git} ||= PublicInbox::Git->new($ctx->{git_dir});
 	if ($flat) {
 		pre_anchor_entry($seen, $_) for (@$msgs);
 		__thread_entry(\$cb, $git, $state, $_, 0) for (@$msgs);
diff --git a/lib/PublicInbox/WWW.pm b/lib/PublicInbox/WWW.pm
index 5cd3bc6..ee414e8 100644
--- a/lib/PublicInbox/WWW.pm
+++ b/lib/PublicInbox/WWW.pm
@@ -17,6 +17,7 @@ use PublicInbox::Config qw(try_cat);
 use URI::Escape qw(uri_escape_utf8 uri_unescape);
 use constant SSOMA_URL => 'http://ssoma.public-inbox.org/';
 use constant PI_URL => 'http://public-inbox.org/';
+require PublicInbox::Git;
 our $LISTNAME_RE = qr!\A/([\w\.\-]+)!;
 our $MID_RE = qr!([^/]+)!;
 our $END_RE = qr!(f/|T/|t/|t\.mbox(?:\.gz)?|t\.atom|raw|)!;
@@ -62,7 +63,6 @@ sub preload {
 	require PublicInbox::Feed;
 	require PublicInbox::View;
 	require PublicInbox::Thread;
-	require PublicInbox::GitCatFile;
 	require Email::MIME;
 	require Digest::SHA;
 	require POSIX;
@@ -96,6 +96,7 @@ sub invalid_list {
 	my $git_dir = $pi_config->get($listname, "mainrepo");
 	if (defined $git_dir) {
 		$ctx->{git_dir} = $git_dir;
+		$ctx->{git} = PublicInbox::Git->new($git_dir);
 		$ctx->{listname} = $listname;
 		return;
 	}
@@ -146,18 +147,7 @@ sub mid2blob {
 	my ($ctx) = @_;
 	require PublicInbox::MID;
 	my $path = PublicInbox::MID::mid2path($ctx->{mid});
-	my @cmd = ('git', "--git-dir=$ctx->{git_dir}",
-			qw(cat-file blob), "HEAD:$path");
-	my $pid = open my $fh, '-|';
-	defined $pid or die "fork failed: $!\n";
-	if ($pid == 0) {
-		open STDERR, '>', '/dev/null'; # ignore errors
-		exec @cmd or die "exec failed: $!\n";
-	} else {
-		my $blob = eval { local $/; <$fh> };
-		close $fh;
-		$? == 0 ? \$blob : undef;
-	}
+	$ctx->{git}->cat_file("HEAD:$path");
 }
 
 # /$LISTNAME/$MESSAGE_ID/raw                    -> raw mbox
diff --git a/t/git.t b/t/git.t
index 4532921..19fbef1 100644
--- a/t/git.t
+++ b/t/git.t
@@ -7,7 +7,7 @@ use File::Temp qw/tempdir/;
 my $dir = tempdir(CLEANUP => 1);
 use Cwd qw/getcwd/;
 
-use_ok 'PublicInbox::GitCatFile';
+use_ok 'PublicInbox::Git';
 {
 	is(system(qw(git init -q --bare), $dir), 0, 'created git directory');
 	my @cmd = ('git', "--git-dir=$dir", 'fast-import', '--quiet');
@@ -26,7 +26,7 @@ use_ok 'PublicInbox::GitCatFile';
 }
 
 {
-	my $gcf = PublicInbox::GitCatFile->new($dir);
+	my $gcf = PublicInbox::Git->new($dir);
 	my $f = 'HEAD:foo.txt';
 	my @x = $gcf->check($f);
 	is(scalar @x, 3, 'returned 3 element array for existing file');
@@ -95,7 +95,7 @@ if (1) {
 	is(0, $?, 'hashed object successfully');
 	chomp $buf;
 
-	my $gcf = PublicInbox::GitCatFile->new($dir);
+	my $gcf = PublicInbox::Git->new($dir);
 	my $rsize;
 	is($gcf->cat_file($buf, sub {
 		$rsize = ${$_[1]};
-- 
EW

[PATCH] extmsg: fixup comparison for unknown message types

[Eric Wong]

Fixes commit 4c2c2325d2948ec5340e2fcafbee798cf568f5fd
("rename 'GitCatFile' package to 'Git'")
---
 lib/PublicInbox/ExtMsg.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/PublicInbox/ExtMsg.pm b/lib/PublicInbox/ExtMsg.pm
index 0b66754..59bbae5 100644
--- a/lib/PublicInbox/ExtMsg.pm
+++ b/lib/PublicInbox/ExtMsg.pm
@@ -75,7 +75,7 @@ sub ext_msg {
 		# TODO: reuse existing PublicInbox::Git objects to save forks
 		my $git = PublicInbox::Git->new($n->{git_dir});
 		my (undef, $type, undef) = $git->check($path);
-		return r302($n->{url}, $mid) if ($type eq 'blob');
+		return r302($n->{url}, $mid) if ($type && $type eq 'blob');
 	}
 
 	# fall back to partial MID matching
-- 
EW

[PATCH] searchview: fix up Atom feed in search results

[Eric Wong]

Oops :x  We need better testing...
Fixes: commit 4c2c2325d2948ec5340e2fcafbee798cf568f5fd
("rename 'GitCatFile' package to 'Git'")
---
 lib/PublicInbox/SearchView.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/PublicInbox/SearchView.pm b/lib/PublicInbox/SearchView.pm
index 9a58156..994bf0d 100644
--- a/lib/PublicInbox/SearchView.pm
+++ b/lib/PublicInbox/SearchView.pm
@@ -235,7 +235,7 @@ sub html_start {
 sub adump {
 	my ($cb, $mset, $q, $ctx) = @_;
 	my $fh = $cb->([ 200, ['Content-Type' => 'application/atom+xml']]);
-	my $git = $ctx->{git_dir} ||= PublicInbox::Git->new($ctx->{git_dir});
+	my $git = $ctx->{git} ||= PublicInbox::Git->new($ctx->{git_dir});
 	my $feed_opts = PublicInbox::Feed::get_feedopts($ctx);
 	my $x = PublicInbox::Hval->new_oneline($q->{q})->as_html;
 	$x = qq{$x - search results};
-- 
EW