[PATCH 07/10] treewide: use run_qx where appropriate

[Eric Wong <e@80x24.org>]

This saves us some code, and is a small step towards getting
ProcessIO working with stat, fcntl and other perlops that don't
work with tied handles.
---
 lib/PublicInbox/Admin.pm           |  7 +++----
 lib/PublicInbox/Config.pm          | 14 ++++++--------
 lib/PublicInbox/Fetch.pm           |  7 +++----
 lib/PublicInbox/Import.pm          |  6 ++----
 lib/PublicInbox/LEI.pm             |  7 +++----
 lib/PublicInbox/LeiBlob.pm         | 12 +++++-------
 lib/PublicInbox/LeiMirror.pm       |  7 +++----
 lib/PublicInbox/MultiGit.pm        |  7 +++----
 lib/PublicInbox/Spamcheck/Spamc.pm | 11 +++--------
 lib/PublicInbox/XapHelperCxx.pm    | 11 +++--------
 t/solver_git.t                     | 12 ++++--------
 11 files changed, 38 insertions(+), 63 deletions(-)

diff --git a/lib/PublicInbox/Admin.pm b/lib/PublicInbox/Admin.pm
index 3140afad..893f4a1b 100644
--- a/lib/PublicInbox/Admin.pm
+++ b/lib/PublicInbox/Admin.pm
@@ -9,7 +9,7 @@ use parent qw(Exporter);
 our @EXPORT_OK = qw(setup_signals);
 use PublicInbox::Config;
 use PublicInbox::Inbox;
-use PublicInbox::Spawn qw(popen_rd);
+use PublicInbox::Spawn qw(run_qx);
 use PublicInbox::Eml;
 *rel2abs_collapsed = \&PublicInbox::Config::rel2abs_collapsed;
 
@@ -67,9 +67,8 @@ sub resolve_git_dir {
 	my ($cd) = @_;
 	# try v1 bare git dirs
 	my $cmd = [ qw(git rev-parse --git-dir) ];
-	my $fh = popen_rd($cmd, undef, {-C => $cd});
-	my $dir = do { local $/; <$fh> };
-	close $fh or die "error in @$cmd (cwd:${\($cd // '.')}): $?\n";
+	my $dir = run_qx($cmd, undef, {-C => $cd});
+	die "error in @$cmd (cwd:${\($cd // '.')}): $?\n" if $?;
 	chomp $dir;
 	# --absolute-git-dir requires git v2.13.0+
 	$dir = rel2abs_collapsed($dir, $cd) if $dir !~ m!\A/!;
diff --git a/lib/PublicInbox/Config.pm b/lib/PublicInbox/Config.pm
index d156b2d3..0a572103 100644
--- a/lib/PublicInbox/Config.pm
+++ b/lib/PublicInbox/Config.pm
@@ -13,7 +13,7 @@ use v5.10.1;
 use parent qw(Exporter);
 our @EXPORT_OK = qw(glob2re rel2abs_collapsed);
 use PublicInbox::Inbox;
-use PublicInbox::Spawn qw(popen_rd);
+use PublicInbox::Spawn qw(popen_rd run_qx);
 our $LD_PRELOAD = $ENV{LD_PRELOAD}; # only valid at startup
 our $DEDUPE; # set to {} to dedupe or clear cache
 
@@ -576,23 +576,21 @@ sub urlmatch {
 	my (%env, %opt);
 	my $cmd = $self->config_cmd(\%env, \%opt);
 	push @$cmd, @bool, qw(--includes -z --get-urlmatch), $key, $url;
-	my $fh = popen_rd($cmd, \%env, \%opt);
-	local $/ = "\0";
-	my $val = <$fh>;
-	if (!close($fh)) {
+	my $val = run_qx($cmd, \%env, \%opt);
+	if ($?) {
 		undef $val;
 		if (@bool && ($? >> 8) == 128) { # not boolean
 		} elsif (($? >> 8) != 1) {
 			$urlmatch_broken = 1;
 		} elsif ($try_git) { # n.b. this takes cwd into account
-			$fh = popen_rd([qw(git config), @bool,
+			$val = run_qx([qw(git config), @bool,
 					qw(-z --get-urlmatch), $key, $url]);
-			$val = <$fh>;
-			close($fh) or undef($val);
+			undef $val if $?;
 		}
 	}
 	$? = 0; # don't influence lei exit status
 	if (defined($val)) {
+		local $/ = "\0";
 		chomp $val;
 		$val = git_bool($val) if @bool;
 	}
diff --git a/lib/PublicInbox/Fetch.pm b/lib/PublicInbox/Fetch.pm
index e41dd448..b0f1437c 100644
--- a/lib/PublicInbox/Fetch.pm
+++ b/lib/PublicInbox/Fetch.pm
@@ -5,7 +5,7 @@ package PublicInbox::Fetch;
 use v5.12;
 use parent qw(PublicInbox::IPC);
 use URI ();
-use PublicInbox::Spawn qw(popen_rd run_wait);
+use PublicInbox::Spawn qw(popen_rd run_qx run_wait);
 use PublicInbox::Admin;
 use PublicInbox::LEI;
 use PublicInbox::LeiCurl;
@@ -20,9 +20,8 @@ sub remote_url ($$) {
 	my $rn = $lei->{opt}->{'try-remote'} // [ 'origin', '_grokmirror' ];
 	for my $r (@$rn) {
 		my $cmd = [ qw(git config), "remote.$r.url" ];
-		my $fh = popen_rd($cmd, undef, { -C => $dir, 2 => $lei->{2} });
-		my $url = <$fh>;
-		close $fh or next;
+		my $url = run_qx($cmd, undef, { -C => $dir, 2 => $lei->{2} });
+		next if $?;
 		$url =~ s!/*\n!!s;
 		return $url;
 	}
diff --git a/lib/PublicInbox/Import.pm b/lib/PublicInbox/Import.pm
index fb70b91b..6eee8774 100644
--- a/lib/PublicInbox/Import.pm
+++ b/lib/PublicInbox/Import.pm
@@ -8,7 +8,7 @@
 package PublicInbox::Import;
 use v5.12;
 use parent qw(PublicInbox::Lock);
-use PublicInbox::Spawn qw(run_die popen_rd spawn);
+use PublicInbox::Spawn qw(run_die run_qx spawn);
 use PublicInbox::MID qw(mids mid2path);
 use PublicInbox::Address;
 use PublicInbox::Smsg;
@@ -25,10 +25,8 @@ use PublicInbox::Git qw(read_all);
 
 sub default_branch () {
 	state $default_branch = do {
-		my $r = popen_rd([qw(git config --global init.defaultBranch)],
+		my $h = run_qx([qw(git config --global init.defaultBranch)],
 				 { GIT_CONFIG => undef });
-		chomp(my $h = <$r> // '');
-		CORE::close $r;
 		$h eq '' ? 'refs/heads/master' : "refs/heads/$h";
 	}
 }
diff --git a/lib/PublicInbox/LEI.pm b/lib/PublicInbox/LEI.pm
index e060bcbe..0f6f7f6f 100644
--- a/lib/PublicInbox/LEI.pm
+++ b/lib/PublicInbox/LEI.pm
@@ -19,7 +19,7 @@ use IO::Handle ();
 use Fcntl qw(SEEK_SET);
 use PublicInbox::Config;
 use PublicInbox::Syscall qw(EPOLLIN);
-use PublicInbox::Spawn qw(run_wait popen_rd);
+use PublicInbox::Spawn qw(run_wait popen_rd run_qx);
 use PublicInbox::Lock;
 use PublicInbox::Eml;
 use PublicInbox::Import;
@@ -1091,9 +1091,8 @@ sub path_to_fd {
 # caller needs to "-t $self->{1}" to check if tty
 sub start_pager {
 	my ($self, $new_env) = @_;
-	my $fh = popen_rd([qw(git var GIT_PAGER)]);
-	chomp(my $pager = <$fh> // '');
-	close($fh) or warn "`git var PAGER' error: \$?=$?";
+	chomp(my $pager = run_qx([qw(git var GIT_PAGER)]));
+	warn "`git var PAGER' error: \$?=$?" if $?;
 	return if $pager eq 'cat' || $pager eq '';
 	$new_env //= {};
 	$new_env->{LESS} //= 'FRX';
diff --git a/lib/PublicInbox/LeiBlob.pm b/lib/PublicInbox/LeiBlob.pm
index 40f64bd9..648d35b6 100644
--- a/lib/PublicInbox/LeiBlob.pm
+++ b/lib/PublicInbox/LeiBlob.pm
@@ -7,7 +7,7 @@ package PublicInbox::LeiBlob;
 use strict;
 use v5.10.1;
 use parent qw(PublicInbox::IPC);
-use PublicInbox::Spawn qw(run_wait popen_rd which);
+use PublicInbox::Spawn qw(run_wait run_qx which);
 use PublicInbox::DS;
 use PublicInbox::Eml;
 use PublicInbox::Git qw(read_all);
@@ -23,9 +23,8 @@ sub get_git_dir ($$) {
 	} else { # implicit --cwd, quiet errors
 		open $opt->{2}, '>', '/dev/null' or die "open /dev/null: $!";
 	}
-	my $r = popen_rd($cmd, {GIT_DIR => undef}, $opt);
-	chomp(my $gd = do { local $/; <$r> });
-	close($r) ? $gd : undef;
+	chomp(my $git_dir = run_qx($cmd, {GIT_DIR => undef}, $opt));
+	$? ? undef : $git_dir;
 }
 
 sub solver_user_cb { # called by solver when done
@@ -122,9 +121,8 @@ sub lei_blob {
 		my $cmd = [ 'git', '--git-dir='.$lei->ale->git->{git_dir},
 				'cat-file', 'blob', $blob ];
 		if (defined $lei->{-attach_idx}) {
-			my $fh = popen_rd($cmd, $lei->{env}, $rdr);
-			my $buf = do { local $/; <$fh> };
-			return extract_attach($lei, $blob, \$buf) if close($fh);
+			my $buf = run_qx($cmd, $lei->{env}, $rdr);
+			return extract_attach($lei, $blob, \$buf) unless $?;
 		}
 		$rdr->{1} = $lei->{1};
 		my $cerr = run_wait($cmd, $lei->{env}, $rdr) or return;
diff --git a/lib/PublicInbox/LeiMirror.pm b/lib/PublicInbox/LeiMirror.pm
index 43e59e6c..c2f7cbed 100644
--- a/lib/PublicInbox/LeiMirror.pm
+++ b/lib/PublicInbox/LeiMirror.pm
@@ -7,7 +7,7 @@ use v5.12;
 use parent qw(PublicInbox::IPC);
 use IO::Uncompress::Gunzip qw(gunzip $GunzipError);
 use IO::Compress::Gzip qw(gzip $GzipError);
-use PublicInbox::Spawn qw(popen_rd spawn run_wait run_die);
+use PublicInbox::Spawn qw(spawn run_wait run_die run_qx);
 use File::Path ();
 use File::Temp ();
 use File::Spec ();
@@ -57,9 +57,8 @@ sub try_scrape {
 	my $curl = $self->{curl} //= PublicInbox::LeiCurl->new($lei) or return;
 	my $cmd = $curl->for_uri($lei, $uri, '--compressed');
 	my $opt = { 0 => $lei->{0}, 2 => $lei->{2} };
-	my $fh = popen_rd($cmd, undef, $opt);
-	my $html = do { local $/; <$fh> } // die "read(curl $uri): $!";
-	CORE::close($fh) or return $lei->child_error($?, "@$cmd failed");
+	my $html = run_qx($cmd, undef, $opt);
+	return $lei->child_error($?, "@$cmd failed") if $?;
 
 	# we grep with URL below, we don't want Subject/From headers
 	# making us clone random URLs.  This assumes remote instances
diff --git a/lib/PublicInbox/MultiGit.pm b/lib/PublicInbox/MultiGit.pm
index 35bd0251..9074758a 100644
--- a/lib/PublicInbox/MultiGit.pm
+++ b/lib/PublicInbox/MultiGit.pm
@@ -5,7 +5,7 @@
 package PublicInbox::MultiGit;
 use strict;
 use v5.10.1;
-use PublicInbox::Spawn qw(run_die popen_rd);
+use PublicInbox::Spawn qw(run_die run_qx);
 use PublicInbox::Import;
 use File::Temp 0.19;
 use List::Util qw(max);
@@ -112,9 +112,8 @@ sub epoch_cfg_set {
 	my $f = epoch_dir($self)."/$epoch_nr.git/config";
 	my $v = "../../$self->{all}/config";
 	if (-r $f) {
-		my $rd = popen_rd([qw(git config -f), $f, 'include.path']);
-		chomp(my $have = <$rd> // '');
-		return if $have eq $v;
+		chomp(my $x = run_qx([qw(git config -f), $f, 'include.path']));
+		return if $x eq $v;
 	}
 	run_die([qw(git config -f), $f, 'include.path', $v ]);
 }
diff --git a/lib/PublicInbox/Spamcheck/Spamc.pm b/lib/PublicInbox/Spamcheck/Spamc.pm
index cba33a66..798de218 100644
--- a/lib/PublicInbox/Spamcheck/Spamc.pm
+++ b/lib/PublicInbox/Spamcheck/Spamc.pm
@@ -4,7 +4,7 @@
 # Default spam filter class for wrapping spamc(1)
 package PublicInbox::Spamcheck::Spamc;
 use v5.12;
-use PublicInbox::Spawn qw(popen_rd run_wait);
+use PublicInbox::Spawn qw(run_qx run_wait);
 use IO::Handle;
 use Fcntl qw(SEEK_SET);
 
@@ -20,14 +20,9 @@ sub new {
 sub spamcheck {
 	my ($self, $msg, $out) = @_;
 
+	$out = \(my $buf = '') unless ref($out);
 	my $rdr = { 0 => _msg_to_fh($self, $msg) };
-	my $fh = popen_rd($self->{checkcmd}, undef, $rdr);
-	unless (ref $out) {
-		my $buf = '';
-		$out = \$buf;
-	}
-	$$out = do { local $/; <$fh> };
-	close $fh; # PublicInbox::ProcessIO::CLOSE
+	$$out = run_qx($self->{checkcmd}, undef, $rdr);
 	($? || $$out eq '') ? 0 : 1;
 }
 
diff --git a/lib/PublicInbox/XapHelperCxx.pm b/lib/PublicInbox/XapHelperCxx.pm
index bd616b6f..83503035 100644
--- a/lib/PublicInbox/XapHelperCxx.pm
+++ b/lib/PublicInbox/XapHelperCxx.pm
@@ -7,7 +7,7 @@
 # The resulting executable is not linked to Perl in any way.
 package PublicInbox::XapHelperCxx;
 use v5.12;
-use PublicInbox::Spawn qw(popen_rd which);
+use PublicInbox::Spawn qw(run_qx which);
 use PublicInbox::Git qw(read_all);
 use PublicInbox::Search;
 use Fcntl qw(SEEK_SET);
@@ -29,14 +29,9 @@ my $xflags = ($ENV{CXXFLAGS} // '-Wall -ggdb3 -O0') . ' ' .
 my $xap_modversion;
 
 sub xap_cfg (@) {
-	use autodie qw(open seek);
-	open my $err, '+>', undef;
 	my $cmd = [ $ENV{PKG_CONFIG} // 'pkg-config', @_, 'xapian-core' ];
-	my $rd = popen_rd($cmd, undef, { 2 => $err });
-	chomp(my $ret = do { local $/; <$rd> });
-	return $ret if close($rd);
-	seek($err, 0, SEEK_SET);
-	$err = read_all($err);
+	chomp(my $ret = run_qx($cmd, undef, { 2 => \(my $err) }));
+	return $ret if !$?;
 	die <<EOM;
 @$cmd failed: Xapian development files missing? (\$?=$?)
 $err
diff --git a/t/solver_git.t b/t/solver_git.t
index 4f09e05b..ab8aba15 100644
--- a/t/solver_git.t
+++ b/t/solver_git.t
@@ -6,7 +6,7 @@ use PublicInbox::TestCommon;
 use Cwd qw(abs_path);
 require_git v2.6;
 use PublicInbox::ContentHash qw(git_sha);
-use PublicInbox::Spawn qw(popen_rd);
+use PublicInbox::Spawn qw(run_qx);
 require_mods(qw(DBD::SQLite Xapian URI::Escape));
 require PublicInbox::SolverGit;
 my $rdr = { 2 => \(my $null) };
@@ -234,13 +234,9 @@ SKIP: {
 		my $cmd = [ qw(git hash-object -w --stdin) ];
 		my $env = { GIT_DIR => $binfoo };
 		while (my ($label, $size) = each %bin) {
-			pipe(my ($rin, $win)) or BAIL_OUT;
-			my $rout = popen_rd($cmd , $env, { 0 => $rin });
-			$rin = undef;
-			print { $win } ("\0" x $size) or BAIL_OUT;
-			close $win or BAIL_OUT;
-			chomp(my $x = <$rout>);
-			close $rout or BAIL_OUT "$?";
+			my $rdr = { 0 => \("\0" x $size) };
+			chomp(my $x = run_qx($cmd , $env, $rdr));
+			xbail "@$cmd: \$?=$?" if $?;
 			$oid{$label} = $x;
 		}