* [PATCH 0/3] more process management cleanups + bugfix
@ 2023-09-27 6:02 7% Eric Wong
2023-09-27 6:02 5% ` [PATCH 2/3] spawn: add popen_wr support Eric Wong
0 siblings, 1 reply; 2+ results
From: Eric Wong @ 2023-09-27 6:02 UTC (permalink / raw)
To: meta
3/3 is a bugfix found while working on this series, but probably
nobody noticed it... The code reductions in lib/ are nice.
Eric Wong (3):
convert: use ProcessPipe with popen_rd
spawn: add popen_wr support
lei: don't gzip --rsyncable by default for mbox*
lib/PublicInbox/LEI.pm | 3 ++-
lib/PublicInbox/LeiRediff.pm | 25 ++++++++++-------------
lib/PublicInbox/LeiToMail.pm | 9 +++------
lib/PublicInbox/MboxReader.pm | 36 ++++++++--------------------------
lib/PublicInbox/ProcessPipe.pm | 9 +++++++++
lib/PublicInbox/Spawn.pm | 20 +++++++++----------
script/public-inbox-convert | 6 ++----
t/lei-convert.t | 27 +++++++++++++++++++++++--
8 files changed, 68 insertions(+), 67 deletions(-)
^ permalink raw reply [relevance 7%]
* [PATCH 2/3] spawn: add popen_wr support
2023-09-27 6:02 7% [PATCH 0/3] more process management cleanups + bugfix Eric Wong
@ 2023-09-27 6:02 5% ` Eric Wong
0 siblings, 0 replies; 2+ results
From: Eric Wong @ 2023-09-27 6:02 UTC (permalink / raw)
To: meta
This makes interesting parts of our code easier to read IMHO.
We can take advantage of `local' while avoiding `fileno' calls
since it's called in spawn() anyways to reduce LoC even further.
---
lib/PublicInbox/LeiRediff.pm | 25 ++++++++++---------------
lib/PublicInbox/LeiToMail.pm | 7 ++-----
lib/PublicInbox/ProcessPipe.pm | 9 +++++++++
lib/PublicInbox/Spawn.pm | 20 +++++++++-----------
4 files changed, 30 insertions(+), 31 deletions(-)
diff --git a/lib/PublicInbox/LeiRediff.pm b/lib/PublicInbox/LeiRediff.pm
index 824289d6..efd24d17 100644
--- a/lib/PublicInbox/LeiRediff.pm
+++ b/lib/PublicInbox/LeiRediff.pm
@@ -1,4 +1,4 @@
-# Copyright (C) 2021 all contributors <meta@public-inbox.org>
+# Copyright (C) all contributors <meta@public-inbox.org>
# License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
# The "lei rediff" sub-command, regenerates diffs with new options
@@ -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(run_wait spawn which);
+use PublicInbox::Spawn qw(run_wait popen_wr which);
use PublicInbox::MsgIter qw(msg_part_text);
use PublicInbox::ViewDiff;
use PublicInbox::LeiBlob;
@@ -121,15 +121,11 @@ EOM
close $fh or die "close $f: $!";
$rw = PublicInbox::Git->new($d);
}
- pipe(my ($r, $w)) or die "pipe: $!";
- my $pid = spawn(['git', "--git-dir=$rw->{git_dir}",
+ my $w = popen_wr(['git', "--git-dir=$rw->{git_dir}",
qw(fast-import --quiet --done --date-format=raw)],
- $lei->{env}, { 2 => $lei->{2}, 0 => $r });
- close $r or die "close r fast-import: $!";
+ $lei->{env}, { 2 => $lei->{2} });
print $w $ta, "\n", $tb, "\ndone\n" or die "print fast-import: $!";
- close $w or die "close w fast-import: $!";
- waitpid($pid, 0);
- die "fast-import failed: \$?=$?" if $?;
+ close $w or die "close w fast-import: \$?=$? \$!=$!";
my $cmd = [ 'diff' ];
_lei_diff_prepare($lei, $cmd);
@@ -141,7 +137,7 @@ EOM
undef;
}
-sub wait_requote ($$$) { # OnDestroy callback
+sub wait_requote { # OnDestroy callback
my ($lei, $pid, $old_1) = @_;
$lei->{1} = $old_1; # closes stdin of `perl -pE 's/^/> /'`
waitpid($pid, 0) == $pid or die "BUG(?) waitpid: \$!=$! \$?=$?";
@@ -150,13 +146,12 @@ sub wait_requote ($$$) { # OnDestroy callback
sub requote ($$) {
my ($lei, $pfx) = @_;
- pipe(my($r, $w)) or die "pipe: $!";
- my $rdr = { 0 => $r, 1 => $lei->{1}, 2 => $lei->{2} };
- # $^X (perl) is overkill, but maybe there's a weird system w/o sed
- my $pid = spawn([$^X, '-pE', "s/^/$pfx/"], $lei->{env}, $rdr);
my $old_1 = $lei->{1};
+ my $opt = { 1 => $old_1, 2 => $lei->{2} };
+ # $^X (perl) is overkill, but maybe there's a weird system w/o sed
+ my ($w, $pid) = popen_wr([$^X, '-pE', "s/^/$pfx/"], $lei->{env}, $opt);
$w->autoflush(1);
- binmode $w, ':utf8';
+ binmode $w, ':utf8'; # incompatible with ProcessPipe due to syswrite
$lei->{1} = $w;
PublicInbox::OnDestroy->new(\&wait_requote, $lei, $pid, $old_1);
}
diff --git a/lib/PublicInbox/LeiToMail.pm b/lib/PublicInbox/LeiToMail.pm
index 4adcc33e..a2cd8650 100644
--- a/lib/PublicInbox/LeiToMail.pm
+++ b/lib/PublicInbox/LeiToMail.pm
@@ -9,7 +9,6 @@ use parent qw(PublicInbox::IPC);
use PublicInbox::Eml;
use PublicInbox::ProcessPipe;
use PublicInbox::Spawn qw(spawn);
-use Symbol qw(gensym);
use IO::Handle; # ->autoflush
use Fcntl qw(SEEK_SET SEEK_END O_CREAT O_EXCL O_WRONLY);
use PublicInbox::Syscall qw(rename_noreplace);
@@ -163,10 +162,8 @@ sub _post_augment_mbox { # open a compressor process from top-level process
my ($r, $w) = @{delete $lei->{zpipe}};
my $rdr = { 0 => $r, 1 => $lei->{1}, 2 => $lei->{2}, pgid => 0 };
my $pid = spawn($cmd, undef, $rdr);
- my $pp = gensym;
- tie *$pp, 'PublicInbox::ProcessPipe', $pid, $w,
- \&reap_compress, $lei, $cmd, $lei->{1};
- $lei->{1} = $pp;
+ $lei->{1} = PublicInbox::ProcessPipe->maybe_new($pid, $w, {
+ cb_arg => [\&reap_compress, $lei, $cmd, $lei->{1} ] });
}
# --augment existing output destination, with deduplication
diff --git a/lib/PublicInbox/ProcessPipe.pm b/lib/PublicInbox/ProcessPipe.pm
index bbba75a2..16971801 100644
--- a/lib/PublicInbox/ProcessPipe.pm
+++ b/lib/PublicInbox/ProcessPipe.pm
@@ -8,6 +8,15 @@
package PublicInbox::ProcessPipe;
use v5.12;
use PublicInbox::DS qw(awaitpid);
+use Symbol qw(gensym);
+
+sub maybe_new {
+ my ($cls, $pid, $fh, $opt) = @_;
+ return ($fh, $pid) if wantarray;
+ my $s = gensym;
+ tie *$s, $cls, $pid, $fh, @{$opt->{cb_arg} // []};
+ $s;
+}
sub waitcb { # awaitpid callback
my ($pid, $err_ref, $cb, @args) = @_;
diff --git a/lib/PublicInbox/Spawn.pm b/lib/PublicInbox/Spawn.pm
index 38619fc2..75ef0137 100644
--- a/lib/PublicInbox/Spawn.pm
+++ b/lib/PublicInbox/Spawn.pm
@@ -17,12 +17,11 @@
package PublicInbox::Spawn;
use v5.12;
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 run_wait);
+our @EXPORT_OK = qw(which spawn popen_rd popen_wr run_die run_wait);
our @RLIMITS = qw(RLIMIT_CPU RLIMIT_CORE RLIMIT_DATA);
BEGIN {
@@ -332,7 +331,6 @@ sub spawn ($;$$) {
my ($cmd, $env, $opts) = @_;
my $f = which($cmd->[0]) // die "$cmd->[0]: command not found\n";
my @env;
- $opts ||= {};
my %env = (%ENV, $env ? %$env : ());
while (my ($k, $v) = each %env) {
push @env, "$k=$v" if defined($v);
@@ -366,14 +364,14 @@ sub spawn ($;$$) {
sub popen_rd {
my ($cmd, $env, $opt) = @_;
- pipe(my ($r, $w)) or die "pipe: $!\n";
- $opt ||= {};
- $opt->{1} = fileno($w);
- my $pid = spawn($cmd, $env, $opt);
- return ($r, $pid) if wantarray;
- my $s = gensym;
- tie *$s, 'PublicInbox::ProcessPipe', $pid, $r, @{$opt->{cb_arg} // []};
- $s;
+ pipe(my $r, local $opt->{1}) or die "pipe: $!\n";
+ PublicInbox::ProcessPipe->maybe_new(spawn($cmd, $env, $opt), $r, $opt)
+}
+
+sub popen_wr {
+ my ($cmd, $env, $opt) = @_;
+ pipe(local $opt->{0}, my $w) or die "pipe: $!\n";
+ PublicInbox::ProcessPipe->maybe_new(spawn($cmd, $env, $opt), $w, $opt)
}
sub run_wait ($;$$) {
^ permalink raw reply related [relevance 5%]
Results 1-2 of 2 | reverse | options above
-- pct% links below jump to the message on this page, permalinks otherwise --
2023-09-27 6:02 7% [PATCH 0/3] more process management cleanups + bugfix Eric Wong
2023-09-27 6:02 5% ` [PATCH 2/3] spawn: add popen_wr support Eric Wong
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).