From: Eric Wong <e@80x24.org>
To: meta@public-inbox.org
Subject: [PATCH 2/5] import: check for git->qx errors, clearer return values
Date: Sun, 27 Dec 2020 02:53:04 +0000 [thread overview]
Message-ID: <20201227025307.77703-3-e@80x24.org> (raw)
In-Reply-To: <20201227025307.77703-1-e@80x24.org>
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.
---
lib/PublicInbox/Import.pm | 9 +++++----
t/git.t | 4 ++++
2 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/lib/PublicInbox/Import.pm b/lib/PublicInbox/Import.pm
index 2cb4896a..e0a84bfd 100644
--- 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 dcd053c5..2cfff248 100644
--- 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: {
next prev parent reply other threads:[~2020-12-27 2:53 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-12-27 2:53 [PATCH 0/5] some yak shaving Eric Wong
2020-12-27 2:53 ` [PATCH 1/5] git: qx: avoid extra "local" for scalar context case Eric Wong
2020-12-27 2:53 ` Eric Wong [this message]
2020-12-27 2:53 ` [PATCH 3/5] check defined return value for localized slurp errors Eric Wong
2020-12-27 2:53 ` [PATCH 4/5] ds: simplify EventLoop implementation Eric Wong
2020-12-27 2:53 ` [PATCH 5/5] ds: flatten + reuse @events, epoll_wait style fixes Eric Wong
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://public-inbox.org/README
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20201227025307.77703-3-e@80x24.org \
--to=e@80x24.org \
--cc=meta@public-inbox.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).