From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: X-Spam-Status: No, score=-4.0 required=3.0 tests=ALL_TRUSTED,AWL,BAYES_00 shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from localhost (dcvr.yhbt.net [127.0.0.1]) by dcvr.yhbt.net (Postfix) with ESMTP id 8A4AA1FD5A for ; Sat, 11 Jan 2020 22:35:05 +0000 (UTC) From: Eric Wong To: meta@public-inbox.org Subject: [PATCH 9/9] use popen_rd for bidirectional pipes Date: Sat, 11 Jan 2020 22:35:03 +0000 Message-Id: <20200111223503.24473-10-e@yhbt.net> In-Reply-To: <20200111223503.24473-1-e@yhbt.net> References: <20200111223503.24473-1-e@yhbt.net> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit List-Id: popen_rd accepts arbitrary redirects, so we can reuse its code to setup the pipe end we want to read, saving each caller a few lines of code compared to calling pipe+spawn. --- lib/PublicInbox/Git.pm | 19 ++++++++----------- lib/PublicInbox/Import.pm | 8 +++----- lib/PublicInbox/V2Writable.pm | 11 +++-------- t/solver_git.t | 9 ++++----- 4 files changed, 18 insertions(+), 29 deletions(-) diff --git a/lib/PublicInbox/Git.pm b/lib/PublicInbox/Git.pm index 15f53495..4f230197 100644 --- a/lib/PublicInbox/Git.pm +++ b/lib/PublicInbox/Git.pm @@ -12,7 +12,7 @@ use warnings; use POSIX qw(dup2); use IO::Handle; # ->autoflush use File::Glob qw(bsd_glob GLOB_NOSORT); -use PublicInbox::Spawn qw(spawn popen_rd); +use PublicInbox::Spawn qw(popen_rd); use PublicInbox::Tmpfile; use base qw(Exporter); our @EXPORT_OK = qw(git_unquote git_quote); @@ -104,27 +104,24 @@ sub _bidi_pipe { } return; } - my ($in_r, $in_w, $out_r, $out_w); - - pipe($in_r, $in_w) or fail($self, "pipe failed: $!"); + my ($out_r, $out_w); pipe($out_r, $out_w) or fail($self, "pipe failed: $!"); - if ($^O eq 'linux') { # 1031: F_SETPIPE_SZ - fcntl($out_w, 1031, 4096); - fcntl($in_w, 1031, 4096) if $batch eq '--batch-check'; - } - my @cmd = (qw(git), "--git-dir=$self->{git_dir}", qw(-c core.abbrev=40 cat-file), $batch); - my $redir = { 0 => $out_r, 1 => $in_w }; + my $redir = { 0 => $out_r }; if ($err) { my $id = "git.$self->{git_dir}$batch.err"; my $fh = tmpfile($id) or fail($self, "tmpfile($id): $!"); $self->{$err} = $fh; $redir->{2} = $fh; } - my $p = spawn(\@cmd, undef, $redir); + my ($in_r, $p) = popen_rd(\@cmd, undef, $redir); $self->{$pid} = $p; $out_w->autoflush(1); + if ($^O eq 'linux') { # 1031: F_SETPIPE_SZ + fcntl($out_w, 1031, 4096); + fcntl($in_r, 1031, 4096) if $batch eq '--batch-check'; + } $self->{$out} = $out_w; $self->{$in} = $in_r; } diff --git a/lib/PublicInbox/Import.pm b/lib/PublicInbox/Import.pm index 6ac43d37..d279fea1 100644 --- a/lib/PublicInbox/Import.pm +++ b/lib/PublicInbox/Import.pm @@ -9,7 +9,7 @@ package PublicInbox::Import; use strict; use warnings; use base qw(PublicInbox::Lock); -use PublicInbox::Spawn qw(spawn); +use PublicInbox::Spawn qw(spawn popen_rd); use PublicInbox::MID qw(mids mid2path); use PublicInbox::Address; use PublicInbox::MsgTime qw(msg_timestamp msg_datestamp); @@ -46,8 +46,7 @@ sub gfi_start { return ($self->{in}, $self->{out}) if $self->{pid}; - my ($in_r, $in_w, $out_r, $out_w); - pipe($in_r, $in_w) or die "pipe failed: $!"; + my ($out_r, $out_w); pipe($out_r, $out_w) or die "pipe failed: $!"; my $git = $self->{git}; @@ -66,8 +65,7 @@ sub gfi_start { my $git_dir = $git->{git_dir}; my @cmd = ('git', "--git-dir=$git_dir", qw(fast-import --quiet --done --date-format=raw)); - my $rdr = { 0 => $out_r, 1 => $in_w }; - my $pid = spawn(\@cmd, undef, $rdr); + my ($in_r, $pid) = popen_rd(\@cmd, undef, { 0 => $out_r }); $out_w->autoflush(1); $self->{in} = $in_r; $self->{out} = $out_w; diff --git a/lib/PublicInbox/V2Writable.pm b/lib/PublicInbox/V2Writable.pm index 51794326..9073d9ef 100644 --- a/lib/PublicInbox/V2Writable.pm +++ b/lib/PublicInbox/V2Writable.pm @@ -16,7 +16,7 @@ use PublicInbox::ContentId qw(content_id content_digest); use PublicInbox::Inbox; use PublicInbox::OverIdx; use PublicInbox::Msgmap; -use PublicInbox::Spawn qw(spawn); +use PublicInbox::Spawn qw(spawn popen_rd); use PublicInbox::SearchIdx; use IO::Handle; # ->autoflush use File::Temp qw(tempfile); @@ -471,17 +471,12 @@ sub git_hash_raw ($$) { print $tmp_fh $$raw or die "print \$tmp_fh: $!"; sysseek($tmp_fh, 0, 0) or die "seek failed: $!"; - my ($r, $w); - pipe($r, $w) or die "failed to create pipe: $!"; - my $rdr = { 0 => $tmp_fh, 1 => $w }; my $git_dir = $self->{-inbox}->git->{git_dir}; my $cmd = ['git', "--git-dir=$git_dir", qw(hash-object --stdin)]; - my $pid = spawn($cmd, undef, $rdr); - close $w; + my $r = popen_rd($cmd, undef, { 0 => $tmp_fh }); local $/ = "\n"; chomp(my $oid = <$r>); - waitpid($pid, 0) == $pid or die "git hash-object did not finish"; - die "git hash-object failed: $?" if $?; + close $r or die "git hash-object failed: $?"; $oid =~ /\A[a-f0-9]{40}\z/ or die "OID not expected: $oid"; $oid; } diff --git a/t/solver_git.t b/t/solver_git.t index 98317fae..92402c3a 100644 --- a/t/solver_git.t +++ b/t/solver_git.t @@ -6,7 +6,7 @@ use Test::More; use Cwd qw(abs_path); use PublicInbox::TestCommon; require_git(2.6); -use PublicInbox::Spawn qw(spawn); +use PublicInbox::Spawn qw(popen_rd); require_mods(qw(DBD::SQLite Search::Xapian Plack::Util)); chomp(my $git_dir = `git rev-parse --git-dir 2>/dev/null`); plan skip_all => "$0 must be run from a git working tree" if $?; @@ -120,14 +120,13 @@ SKIP: { my $cmd = [ qw(git hash-object -w --stdin) ]; my $env = { GIT_DIR => $binfoo }; while (my ($label, $size) = each %bin) { - pipe(my ($rout, $wout)) or die; pipe(my ($rin, $win)) or die; - my $rdr = { 0 => $rin, 1 => $wout }; - my $pid = spawn($cmd , $env, $rdr); - $wout = $rin = undef; + my $rout = popen_rd($cmd , $env, { 0 => $rin }); + $rin = undef; print { $win } ("\0" x $size) or die; close $win or die; chomp($oid{$label} = <$rout>); + close $rout or die "$?"; } # ensure the PSGI frontend (ViewVCS) works: