solvergit: allow passing arg to user-supplied callback

This allows us to get rid of the requirement to capture
on-stack variables with an anonymous sub, as illustrated
with the update to viewvcs to take advantage of this.

v2: fix error handling for missing OIDs

diff --git a/lib/PublicInbox/SolverGit.pm b/lib/PublicInbox/SolverGit.pm
index 9266fb087b8ca429d6e7b56a98b6a7aded339426..17a430606dfc64678583fd85a16b217a399213d8 100644 (file)
--- a/lib/PublicInbox/SolverGit.pm
+++ b/lib/PublicInbox/SolverGit.pm
@@ -55,11 +55,16 @@ sub dbg ($$) {
        print { $_[0]->{out} } $_[1], "\n" or ERR($_[0], "print(dbg): $!");
 }
 
+sub done ($$) {
+       my ($self, $res) = @_;
+       my $ucb = delete($self->{user_cb}) or return;
+       $ucb->($res, $self->{uarg});
+}
+
 sub ERR ($$) {
        my ($self, $err) = @_;
        print { $self->{out} } $err, "\n";
-       my $ucb = delete($self->{user_cb});
-       eval { $ucb->($err) } if $ucb;
+       eval { done($self, $err) };
        die $err;
 }
 
@@ -311,24 +316,23 @@ sub extract_old_mode ($) {
        '100644';
 }
 
-sub do_finish ($$) {
-       my ($self, $user_cb) = @_;
-       my $found = $self->{found};
-       my $oid_want = $self->{oid_want};
+sub do_finish ($) {
+       my ($self) = @_;
+       my ($found, $oid_want) = @$self{qw(found oid_want)};
        if (my $exists = $found->{$oid_want}) {
-               return $user_cb->($exists);
+               return done($self, $exists);
        }
 
        # let git disambiguate if oid_want was too short,
        # but long enough to be unambiguous:
        my $tmp_git = $self->{tmp_git};
        if (my @res = $tmp_git->check($oid_want)) {
-               return $user_cb->($found->{$res[0]});
+               return done($self, $found->{$res[0]});
        }
        if (my $err = $tmp_git->last_check_err) {
                dbg($self, $err);
        }
-       $user_cb->(undef);
+       done($self, undef);
 }
 
 sub event_step ($) {
@@ -352,8 +356,8 @@ sub event_step ($) {
                # our result: (which may be undef)
                # Other steps may call user_cb to terminate prematurely
                # on error
-               } elsif (my $user_cb = delete($self->{user_cb})) {
-                       do_finish($self, $user_cb);
+               } elsif (exists $self->{user_cb}) {
+                       do_finish($self);
                } else {
                        die 'about to call user_cb twice'; # Oops :x
                }
@@ -362,8 +366,7 @@ sub event_step ($) {
        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;
+               eval { done($self, $err) };
        }
 }
 
@@ -524,7 +527,7 @@ sub resolve_patch ($$) {
                        join("\n", $found_git->pub_urls($self->{psgi_env})));
 
                if ($cur_want eq $self->{oid_want} || $type ne 'blob') {
-                       eval { delete($self->{user_cb})->($existing) };
+                       eval { done($self, $existing) };
                        die "E: $@" if $@;
                        return;
                }
@@ -562,18 +565,19 @@ sub resolve_patch ($$) {
        }
 
        dbg($self, "could not find $cur_want");
-       eval { delete($self->{user_cb})->(undef) }; # not found! :<
+       eval { done($self, undef) };
        die "E: $@" if $@;
 }
 
 # 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) = @_;
+       my ($class, $ibx, $user_cb, $uarg) = @_;
 
        bless {
                gits => $ibx->{-repo_objs},
                user_cb => $user_cb,
+               uarg => $uarg,
                # -cur_di, -qsp, -msg => temporary fields for Qspawn callbacks
 
                # TODO: config option for searching related inboxes
@@ -591,7 +595,7 @@ sub solve ($$$$$) {
 
        # 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/;
+       return done($self, undef) if $oid_want =~ /\A0+\z/;
 
        $self->{oid_want} = $oid_want;
        $self->{out} = $out;

diff --git a/lib/PublicInbox/ViewVCS.pm b/lib/PublicInbox/ViewVCS.pm
index a6dbb9a9080eba3fd197d10599c74287f302241a..ead8c2b45fe1fd8e1791227b13872f480e2560aa 100644 (file)
--- a/lib/PublicInbox/ViewVCS.pm
+++ b/lib/PublicInbox/ViewVCS.pm
@@ -112,8 +112,10 @@ sub show_other ($$$$) {
        $qsp->psgi_qx($env, undef, \&show_other_result, $ctx);
 }
 
+# user_cb for SolverGit, called as: user_cb->($result_or_error, $uarg)
 sub solve_result {
-       my ($ctx, $res, $log, $hints, $fn) = @_;
+       my ($res, $ctx) = @_;
+       my ($log, $hints, $fn) = delete @$ctx{qw(log hints fn)};
 
        unless (seek($log, 0, 0)) {
                $ctx->{env}->{'psgi.errors'}->print("seek(log): $!\n");
@@ -192,21 +194,20 @@ sub solve_result {
 sub show ($$;$) {
        my ($ctx, $oid_b, $fn) = @_;
        my $qp = $ctx->{qp};
-       my $hints = {};
+       my $hints = $ctx->{hints} = {};
        while (my ($from, $to) = each %QP_MAP) {
                defined(my $v = $qp->{$from}) or next;
                $hints->{$to} = $v;
        }
 
-       my $log = tmpfile("solve.$oid_b");
-       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
+       $ctx->{'log'} = tmpfile("solve.$oid_b");
+       $ctx->{fn} = $fn;
+       my $solver = PublicInbox::SolverGit->new($ctx->{-inbox},
+                                               \&solve_result, $ctx);
+       # PSGI server will call this immediately and give us a callback (-wcb)
        sub {
                $ctx->{-wcb} = $_[0]; # HTTP write callback
-               $solver->solve($ctx->{env}, $log, $oid_b, $hints);
+               $solver->solve($ctx->{env}, $ctx->{log}, $oid_b, $hints);
        };
 }
 

diff --git a/t/solver_git.t b/t/solver_git.t
index af5bf7bc33f86b296066e0df8a9d55c2a4dc91d4..0c272d777c752ce9a10b3a003f009d1c0dbc5f4a 100644 (file)
--- a/t/solver_git.t
+++ b/t/solver_git.t
@@ -157,6 +157,7 @@ EOF
        close $cfgfh or die;
        my $cfg = PublicInbox::Config->new($cfgpath);
        my $www = PublicInbox::WWW->new($cfg);
+       my $non_existent = 'ee5e32211bf62ab6531bdf39b84b6920d0b6775a';
        my $client = sub {
                my ($cb) = @_;
                my $res = $cb->(GET("/$name/3435775/s/"));
@@ -165,6 +166,9 @@ EOF
                $res = $cb->(GET("/$name/".('0'x40).'/s/'));
                is($res->code, 404, 'failure with null OID');
 
+               $res = $cb->(GET("/$name/$non_existent/s/"));
+               is($res->code, 404, 'failure with null OID');
+
                $res = $cb->(GET("/$name/$v1_0_0_tag/s/"));
                is($res->code, 200, 'shows commit');
                while (my ($label, $size) = each %bin) {