* [PATCH 9/9] use popen_rd for bidirectional pipes
2020-01-11 22:34 7% [PATCH 0/9] more small fixes and cleanups Eric Wong
@ 2020-01-11 22:35 5% ` Eric Wong
0 siblings, 0 replies; 2+ results
From: Eric Wong @ 2020-01-11 22:35 UTC (permalink / raw)
To: meta
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:
^ permalink raw reply related [relevance 5%]
* [PATCH 0/9] more small fixes and cleanups
@ 2020-01-11 22:34 7% Eric Wong
2020-01-11 22:35 5% ` [PATCH 9/9] use popen_rd for bidirectional pipes Eric Wong
0 siblings, 1 reply; 2+ results
From: Eric Wong @ 2020-01-11 22:34 UTC (permalink / raw)
To: meta
One bugfix for solver, a couple of optimizations which will
make sense when dealing with many and larger code repos.
And some cleanups to reduce redundant code.
Eric Wong (9):
config: do not slurp entire cgitrc at once
git: modified: don't slurp `rev-parse --branches'
git: packed_bytes: use GLOB_NOSORT
solver: path_a may be undef from /dev/null
cgit: drop cgit_parse_hdr wrapper
xapcmd: use popen_rd for running xapian-compact
xt/git_async_cmp: do not slurp large OID list into memory
t/solver_git: avoid uninitialized warnings in hostname generation
use popen_rd for bidirectional pipes
lib/PublicInbox/Cgit.pm | 9 ++-------
lib/PublicInbox/Config.pm | 4 ++--
lib/PublicInbox/Git.pm | 24 +++++++++++-------------
lib/PublicInbox/Import.pm | 8 +++-----
lib/PublicInbox/SolverGit.pm | 4 ++--
lib/PublicInbox/V2Writable.pm | 11 +++--------
lib/PublicInbox/Xapcmd.pm | 16 +++++-----------
t/solver_git.t | 12 ++++++------
xt/git_async_cmp.t | 4 ++--
9 files changed, 36 insertions(+), 56 deletions(-)
DELETE! DELETE! DELETE!
^ permalink raw reply [relevance 7%]
Results 1-2 of 2 | reverse | options above
-- pct% links below jump to the message on this page, permalinks otherwise --
2020-01-11 22:34 7% [PATCH 0/9] more small fixes and cleanups Eric Wong
2020-01-11 22:35 5% ` [PATCH 9/9] use popen_rd for bidirectional pipes 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).