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,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 9F95C1F569 for ; Tue, 26 Sep 2023 07:44:41 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=80x24.org; s=selector1; t=1695714281; bh=ZM3RdjAiN/PXYHlLIxtDgbMt5y9c3r1UrJYJEShkLWo=; h=From:To:Subject:Date:In-Reply-To:References:From; b=Ve0j6FGT4W7d7a9B9lALKViaVqqFt4E2WUEYgtRx3tZ210O42yo3/bfDPHkRY1LDu xT6naiQ8ny9O3A+tHZO1YasbzJrafzy9nbIfC5H5us/P9OW/iOpANODvxbE2Yson9+ UGPjru9uKKTacZphSMWthVdR4dzI5thIr9Rlh6+c= From: Eric Wong To: meta@public-inbox.org Subject: [PATCH 6/6] spawn: add run_wait to simplify spawn+waitpid use Date: Tue, 26 Sep 2023 07:44:40 +0000 Message-ID: <20230926074440.392023-7-e@80x24.org> In-Reply-To: <20230926074440.392023-1-e@80x24.org> References: <20230926074440.392023-1-e@80x24.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit List-Id: It's basically the `system' perlop with support for env overrides, redirects, chdir, rlimits, and setpgid support. --- lib/PublicInbox/Fetch.pm | 7 +++---- lib/PublicInbox/LEI.pm | 5 ++--- lib/PublicInbox/LeiBlob.pm | 14 +++++--------- lib/PublicInbox/LeiMailDiff.pm | 6 ++---- lib/PublicInbox/LeiMirror.pm | 8 +++----- lib/PublicInbox/LeiRediff.pm | 7 +++---- lib/PublicInbox/SearchIdx.pm | 6 ++---- lib/PublicInbox/Spamcheck/Spamc.pm | 6 ++---- lib/PublicInbox/Spawn.pm | 13 +++++++++---- lib/PublicInbox/TestCommon.pm | 6 +----- 10 files changed, 32 insertions(+), 46 deletions(-) diff --git a/lib/PublicInbox/Fetch.pm b/lib/PublicInbox/Fetch.pm index 8f3a87e2..6e9b1e94 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_die spawn); +use PublicInbox::Spawn qw(popen_rd run_wait); use PublicInbox::Admin; use PublicInbox::LEI; use PublicInbox::LeiCurl; @@ -133,9 +133,8 @@ sub do_fetch { # main entry point warn "W: $edir missing remote.*.url\n"; my $o = { -C => $edir }; $o->{1} = $o->{2} = $lei->{2}; - my $pid = spawn([qw(git config -l)], undef, $o); - waitpid($pid, 0); - $lei->child_error($?) if $?; + run_wait([qw(git config -l)], undef, $o) and + $lei->child_error($?); } } @epochs = grep { !$skip->{$_} } @epochs if $skip; diff --git a/lib/PublicInbox/LEI.pm b/lib/PublicInbox/LEI.pm index 432ae61e..2f8d7a96 100644 --- a/lib/PublicInbox/LEI.pm +++ b/lib/PublicInbox/LEI.pm @@ -18,7 +18,7 @@ use IO::Handle (); use Fcntl qw(SEEK_SET); use PublicInbox::Config; use PublicInbox::Syscall qw(EPOLLIN); -use PublicInbox::Spawn qw(spawn popen_rd); +use PublicInbox::Spawn qw(run_wait popen_rd); use PublicInbox::Lock; use PublicInbox::Eml; use PublicInbox::Import; @@ -905,8 +905,7 @@ sub _config { } my $cmd = $cfg->config_cmd(\%env, \%opt); push @$cmd, @file_arg, @argv; - waitpid(spawn($cmd, \%env, \%opt), 0); - $? == 0 ? 1 : ($err_ok ? undef : fail($self, $?)); + run_wait($cmd, \%env, \%opt) ? ($err_ok ? undef : fail($self, $?)) : 1; } sub lei_daemon_pid { puts shift, $$ } diff --git a/lib/PublicInbox/LeiBlob.pm b/lib/PublicInbox/LeiBlob.pm index 1d8267c8..8df83b1d 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(spawn popen_rd which); +use PublicInbox::Spawn qw(run_wait popen_rd which); use PublicInbox::DS; sub get_git_dir ($$) { @@ -43,8 +43,7 @@ sub solver_user_cb { # called by solver when done my $cmd = [ 'git', "--git-dir=$gd", 'show', $oid ]; my $rdr = { 1 => $lei->{1}, 2 => $lei->{2} }; - waitpid(spawn($cmd, $lei->{env}, $rdr), 0); - $lei->child_error($?) if $?; + run_wait($cmd, $lei->{env}, $rdr) and $lei->child_error($?); } sub do_solve_blob { # via wq_do @@ -125,12 +124,9 @@ sub lei_blob { require PublicInbox::Eml; my $buf = do { local $/; <$fh> }; return extract_attach($lei, $blob, \$buf) if close($fh); - } else { - $rdr->{1} = $lei->{1}; - waitpid(spawn($cmd, $lei->{env}, $rdr), 0); } - my $ce = $?; - return if $ce == 0; + $rdr->{1} = $lei->{1}; + my $cerr = run_wait($cmd, $lei->{env}, $rdr) or return; my $lms = $lei->lms; if (my $bref = $lms ? $lms->local_blob($blob, 1) : undef) { defined($lei->{-attach_idx}) and @@ -139,7 +135,7 @@ sub lei_blob { } elsif ($opt->{mail}) { my $eh = $rdr->{2}; seek($eh, 0, 0); - return $lei->child_error($ce, do { local $/; <$eh> }); + return $lei->child_error($cerr, do { local $/; <$eh> }); } # else: fall through to solver below } diff --git a/lib/PublicInbox/LeiMailDiff.pm b/lib/PublicInbox/LeiMailDiff.pm index c813144f..5e2e4b0b 100644 --- a/lib/PublicInbox/LeiMailDiff.pm +++ b/lib/PublicInbox/LeiMailDiff.pm @@ -6,7 +6,7 @@ package PublicInbox::LeiMailDiff; use v5.12; use parent qw(PublicInbox::IPC PublicInbox::LeiInput PublicInbox::MailDiff); -use PublicInbox::Spawn qw(spawn which); +use PublicInbox::Spawn qw(run_wait); use File::Path (); require PublicInbox::LeiRediff; @@ -20,9 +20,7 @@ sub diff_a ($$) { push @$cmd, qw(-- a), "N$self->{nr}"; my $rdr = { -C => "$self->{tmp}" }; @$rdr{1, 2} = @$lei{1, 2}; - my $pid = spawn($cmd, $lei->{env}, $rdr); - waitpid($pid, 0); - $lei->child_error($?) if $?; # for git diff --exit-code + run_wait($cmd, $lei->{env}, $rdr) and $lei->child_error($?); File::Path::remove_tree($self->{curdir}); } diff --git a/lib/PublicInbox/LeiMirror.pm b/lib/PublicInbox/LeiMirror.pm index fed6b668..c99bafc3 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_die); +use PublicInbox::Spawn qw(popen_rd spawn run_wait run_die); use File::Path (); use File::Temp (); use File::Spec (); @@ -251,8 +251,7 @@ sub index_cloned_inbox { sub run_reap { my ($lei, $cmd, $opt) = @_; $lei->qerr("# @$cmd"); - waitpid(spawn($cmd, undef, $opt), 0) // die "waitpid: $!"; - my $ret = $?; + my $ret = run_wait($cmd, undef, $opt); $? = 0; # don't let it influence normal exit $ret; } @@ -424,8 +423,7 @@ sub fgrp_fetch_all { my $c = [ @$cmd, '--unset-all', $_ ]; $self->{lei}->qerr("# @$c"); next if $self->{dry_run}; - my $pid = spawn($c, undef, $opt); - waitpid($pid, 0) // die "waitpid: $!"; + run_wait($c, undef, $opt); die "E: @$c \$?=$?" if ($? && ($? >> 8) != 5); } diff --git a/lib/PublicInbox/LeiRediff.pm b/lib/PublicInbox/LeiRediff.pm index 9cf95c08..824289d6 100644 --- a/lib/PublicInbox/LeiRediff.pm +++ b/lib/PublicInbox/LeiRediff.pm @@ -7,7 +7,7 @@ use strict; use v5.10.1; use parent qw(PublicInbox::IPC PublicInbox::LeiInput); use File::Temp 0.19 (); # 0.19 for ->newdir -use PublicInbox::Spawn qw(spawn which); +use PublicInbox::Spawn qw(run_wait spawn which); use PublicInbox::MsgIter qw(msg_part_text); use PublicInbox::ViewDiff; use PublicInbox::LeiBlob; @@ -136,9 +136,8 @@ EOM $lei->qerr("# git @$cmd"); push @$cmd, qw(A B); unshift @$cmd, 'git', "--git-dir=$rw->{git_dir}"; - $pid = spawn($cmd, $lei->{env}, { 2 => $lei->{2}, 1 => $lei->{1} }); - waitpid($pid, 0); - $lei->child_error($?) if $?; # for git diff --exit-code + run_wait($cmd, $lei->{env}, { 2 => $lei->{2}, 1 => $lei->{1} }) and + $lei->child_error($?); # for git diff --exit-code undef; } diff --git a/lib/PublicInbox/SearchIdx.pm b/lib/PublicInbox/SearchIdx.pm index 61dc7ce3..2a0e06d1 100644 --- a/lib/PublicInbox/SearchIdx.pm +++ b/lib/PublicInbox/SearchIdx.pm @@ -22,7 +22,7 @@ use POSIX qw(strftime); use Fcntl qw(SEEK_SET); use Time::Local qw(timegm); use PublicInbox::OverIdx; -use PublicInbox::Spawn qw(spawn); +use PublicInbox::Spawn qw(run_wait); use PublicInbox::Git qw(git_unquote); use PublicInbox::MsgTime qw(msg_timestamp msg_datestamp); use PublicInbox::Address; @@ -1010,9 +1010,7 @@ sub is_ancestor ($$$) { return 0 unless $git->check($cur); my $cmd = [ 'git', "--git-dir=$git->{git_dir}", qw(merge-base --is-ancestor), $cur, $tip ]; - my $pid = spawn($cmd); - waitpid($pid, 0) == $pid or die join(' ', @$cmd) .' did not finish'; - $? == 0; + run_wait($cmd) == 0; } sub need_update ($$$$) { diff --git a/lib/PublicInbox/Spamcheck/Spamc.pm b/lib/PublicInbox/Spamcheck/Spamc.pm index 67278917..726866c8 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 spawn); +use PublicInbox::Spawn qw(popen_rd run_wait); use IO::Handle; use Fcntl qw(SEEK_SET); @@ -47,9 +47,7 @@ sub _learn { $rdr->{0} = _msg_to_fh($self, $msg); $rdr->{1} ||= $self->_devnull; $rdr->{2} ||= $self->_devnull; - my $pid = spawn($self->{$field}, undef, $rdr); - waitpid($pid, 0); - !$?; + 0 == run_wait($self->{$field}, undef, $rdr); } sub _devnull { diff --git a/lib/PublicInbox/Spawn.pm b/lib/PublicInbox/Spawn.pm index 2b84e2d5..38619fc2 100644 --- a/lib/PublicInbox/Spawn.pm +++ b/lib/PublicInbox/Spawn.pm @@ -20,8 +20,9 @@ use parent qw(Exporter); use Symbol qw(gensym); use Fcntl qw(LOCK_EX SEEK_SET); use IO::Handle (); +use Carp qw(croak); use PublicInbox::ProcessPipe; -our @EXPORT_OK = qw(which spawn popen_rd run_die); +our @EXPORT_OK = qw(which spawn popen_rd run_die run_wait); our @RLIMITS = qw(RLIMIT_CPU RLIMIT_CORE RLIMIT_DATA); BEGIN { @@ -375,11 +376,15 @@ sub popen_rd { $s; } +sub run_wait ($;$$) { + my ($cmd, $env, $opt) = @_; + waitpid(spawn($cmd, $env, $opt), 0); + $? +} + sub run_die ($;$$) { my ($cmd, $env, $rdr) = @_; - my $pid = spawn($cmd, $env, $rdr); - waitpid($pid, 0) == $pid or die "@$cmd did not finish"; - $? == 0 or die "@$cmd failed: \$?=$?\n"; + run_wait($cmd, $env, $rdr) and croak "E: @$cmd failed: \$?=$?"; } 1; diff --git a/lib/PublicInbox/TestCommon.pm b/lib/PublicInbox/TestCommon.pm index 7b628769..2c38500c 100644 --- a/lib/PublicInbox/TestCommon.pm +++ b/lib/PublicInbox/TestCommon.pm @@ -367,11 +367,7 @@ sub run_script ($;$$) { $cmd->[0] = File::Spec->rel2abs($cmd->[0]); $spawn_opt->{'-C'} = $d; } - my $pid = PublicInbox::Spawn::spawn($cmd, $env, $spawn_opt); - if (defined $pid) { - my $r = waitpid($pid, 0) // die "waitpid: $!"; - $r == $pid or die "waitpid: expected $pid, got $r"; - } + PublicInbox::Spawn::run_wait($cmd, $env, $spawn_opt); } else { # localize and run everything in the same process: # note: "local *STDIN = *STDIN;" and so forth did not work in # old versions of perl