From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: X-Spam-Status: No, score=-4.2 required=3.0 tests=ALL_TRUSTED,AWL,BAYES_00, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF shortcircuit=no autolearn=ham autolearn_force=no version=3.4.6 Received: from localhost (dcvr.yhbt.net [127.0.0.1]) by dcvr.yhbt.net (Postfix) with ESMTP id 635F81F69D for ; Fri, 27 Oct 2023 22:21:17 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=80x24.org; s=selector1; t=1698445277; bh=lKsPppNJQvaqfcV3NnHHbsOj7NJV5j9URYvVZezkwuM=; h=From:To:Subject:Date:In-Reply-To:References:From; b=BA+z5U+pms9XhkJvS7VWLCM3LwwFIFjJvwRjDYzHJzoE0rTdbaE1sgmdgR95r8ZrM 16D8J12hl5Lp/Goh8WP6+n0eijpuWjlQLbgt9BksYVb1UEx1TQnnOgGD6JjTjxLTOe P4qg2HpUdUVy8uPFGWDZ6gSiQZthrZUQMCrM0XEU= From: Eric Wong To: meta@public-inbox.org Subject: [PATCH 07/10] treewide: use run_qx where appropriate Date: Fri, 27 Oct 2023 22:21:13 +0000 Message-ID: <20231027222116.4034363-8-e@80x24.org> In-Reply-To: <20231027222116.4034363-1-e@80x24.org> References: <20231027222116.4034363-1-e@80x24.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit List-Id: 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 < \(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; }