spawn: add popen_wr support

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.

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