user/dev discussion of public-inbox itself
 help / color / mirror / code / Atom feed
From: Eric Wong <e@80x24.org>
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	[thread overview]
Message-ID: <20230926074440.392023-7-e@80x24.org> (raw)
In-Reply-To: <20230926074440.392023-1-e@80x24.org>

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

      parent reply	other threads:[~2023-09-26  7:44 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-09-26  7:44 [PATCH 0/6] waitpid-related cleanups Eric Wong
2023-09-26  7:44 ` [PATCH 1/6] ds: awaitpid: Perl waitpid retries on EINTR automatically Eric Wong
2023-09-26  7:44 ` [PATCH 2/6] auto_reap: waitpid never returns undef Eric Wong
2023-09-26  7:44 ` [PATCH 3/6] lei_blob: use ProcessPipe to eliminate a waitpid call Eric Wong
2023-09-26  7:44 ` [PATCH 4/6] fetch: fix missing chdir arg for error reporting Eric Wong
2023-09-26  7:44 ` [PATCH 5/6] spamcheck/spamc: rely on ProcessPipe instead of waitpid Eric Wong
2023-09-26  7:44 ` Eric Wong [this message]

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=20230926074440.392023-7-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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/public-inbox.git

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).