import: check for git->qx errors, clearer return values

Those git commands can fail and git->qx will set $? when it
fails.  There's no need for the extra indirection of the @ret
array, either.

Improve git->qx coverage to check for $? while we're at it.

diff --git a/lib/PublicInbox/Import.pm b/lib/PublicInbox/Import.pm
index 2cb4896af5a83a08ca45cb7340f5c1e548ed1e42..e0a84bfd58ed82bc14126d707212d30a64dd11f5 100644 (file)
--- a/lib/PublicInbox/Import.pm
+++ b/lib/PublicInbox/Import.pm
@@ -48,7 +48,7 @@ sub gfi_start {
 
        return ($self->{in}, $self->{out}) if $self->{pid};
 
-       my (@ret, $out_r, $out_w);
+       my ($in_r, $pid, $out_r, $out_w);
        pipe($out_r, $out_w) or die "pipe failed: $!";
 
        $self->lock_acquire;
@@ -56,27 +56,28 @@ sub gfi_start {
                my ($git, $ref) = @$self{qw(git ref)};
                local $/ = "\n";
                chomp($self->{tip} = $git->qx(qw(rev-parse --revs-only), $ref));
+               die "fatal: rev-parse --revs-only $ref: \$?=$?" if $?;
                if ($self->{path_type} ne '2/38' && $self->{tip}) {
                        local $/ = "\0";
                        my @t = $git->qx(qw(ls-tree -r -z --name-only), $ref);
+                       die "fatal: ls-tree -r -z --name-only $ref: \$?=$?" if $?;
                        chomp @t;
                        $self->{-tree} = { map { $_ => 1 } @t };
                }
                my @cmd = ('git', "--git-dir=$git->{git_dir}",
                        qw(fast-import --quiet --done --date-format=raw));
-               my ($in_r, $pid) = popen_rd(\@cmd, undef, { 0 => $out_r });
+               ($in_r, $pid) = popen_rd(\@cmd, undef, { 0 => $out_r });
                $out_w->autoflush(1);
                $self->{in} = $in_r;
                $self->{out} = $out_w;
                $self->{pid} = $pid;
                $self->{nchg} = 0;
-               @ret = ($in_r, $out_w);
        };
        if ($@) {
                $self->lock_release;
                die $@;
        }
-       @ret;
+       ($in_r, $out_w);
 }
 
 sub wfail () { die "write to fast-import failed: $!" }

diff --git a/t/git.t b/t/git.t
index dcd053c569429efc5a91a86857ee932ee938da82..2cfff248a01e6f51a3d838c01e9d17c3cff793a1 100644 (file)
--- a/t/git.t
+++ b/t/git.t
@@ -76,13 +76,17 @@ if (1) {
        is(length($$x), $size, 'read correct number of bytes');
 
        my $ref = $gcf->qx(qw(cat-file blob), $buf);
+       is($?, 0, 'no error on scalar success');
        my @ref = $gcf->qx(qw(cat-file blob), $buf);
+       is($?, 0, 'no error on wantarray success');
        my $nl = scalar @ref;
        ok($nl > 1, "qx returned array length of $nl");
        is(join('', @ref), $ref, 'qx array and scalar context both work');
 
        $gcf->qx(qw(repack -adq));
        ok($gcf->packed_bytes > 0, 'packed size is positive');
+       $gcf->qx(qw(rev-parse --verify bogus));
+       isnt($?, 0, '$? set on failure'.$?);
 }
 
 SKIP: {