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 ($;$$) {
