From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: X-Spam-Status: No, score=-4.0 required=3.0 tests=ALL_TRUSTED,AWL,BAYES_00 shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from localhost (dcvr.yhbt.net [127.0.0.1]) by dcvr.yhbt.net (Postfix) with ESMTP id 18F0A1F462 for ; Thu, 13 Jun 2019 08:10:03 +0000 (UTC) From: Eric Wong To: meta@public-inbox.org Subject: [PATCH] git: remove cat_file sub callback interface Date: Thu, 13 Jun 2019 08:10:02 +0000 Message-Id: <20190613081002.19309-1-e@80x24.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit List-Id: We weren't using it, and in retrospect, it makes no sense to use this API cat_file for giant responses which can't read quickly with minimal context-switching (or sanely fit into memory for Email::Simple/Email::MIME). For giant blobs which we don't want slurped in memory, we'll spawn a short-lived git-cat-file process like we do in ViewVCS. Otherwise, monopolizing a git-cat-file process for a giant blob is harmful to other PSGI/NNTP users. A better interface is coming which will be more suitable for for batch processing of "small" objects such as commits and email blobs. --- lib/PublicInbox/Git.pm | 43 ++++++++++--------------------- t/git.t | 58 ------------------------------------------ 2 files changed, 13 insertions(+), 88 deletions(-) diff --git a/lib/PublicInbox/Git.pm b/lib/PublicInbox/Git.pm index 68445b3..6a87661 100644 --- a/lib/PublicInbox/Git.pm +++ b/lib/PublicInbox/Git.pm @@ -145,41 +145,24 @@ again: fail($self, "Unexpected result from git cat-file: $head"); my $size = $1; - my $ref_type = $ref ? ref($ref) : ''; - my $rv; my $left = $size; - $$ref = $size if ($ref_type eq 'SCALAR'); - my $cb_err; - - 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; + $$ref = $size if $ref; + + 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; - my $r = read($in, my $buf, 1); + my $r = read($in, my $lf, 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; + fail($self, 'newline missing after blob') if ($r != 1 || $lf ne "\n"); $rv; } diff --git a/t/git.t b/t/git.t index 913f6e5..9bc8900 100644 --- a/t/git.t +++ b/t/git.t @@ -33,33 +33,7 @@ use_ok 'PublicInbox::Git'; 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'); } @@ -79,44 +53,12 @@ if (1) { my $gcf = PublicInbox::Git->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'); - }; - my $ref = $gcf->qx(qw(cat-file blob), $buf); - is($all, $ref, 'qx read giant single string'); - my @ref = $gcf->qx(qw(cat-file blob), $buf); - is($all, join('', @ref), 'qx returned array when wanted'); my $nl = scalar @ref; ok($nl > 1, "qx returned array length of $nl"); -- EW