* [PATCH 00/30] autodie-ification and code simplifications
@ 2023-10-17 23:37 Eric Wong
2023-10-17 23:37 ` [PATCH 01/30] lei_mirror: start converting to autodie Eric Wong
` (30 more replies)
0 siblings, 31 replies; 32+ messages in thread
From: Eric Wong @ 2023-10-17 23:37 UTC (permalink / raw)
To: meta
Noisy code is less pleasant to work on, so use autodie more and
a few more simplifications. There's a couple of small bugfixes
discovered along the way, too.
Eric Wong (30):
lei_mirror: start converting to autodie
lei_mirror: autodie most `close' calls
lei_mirror: use autodie for most `open' calls
git: introduce read_all function
import: use read_all to detect short reads
lei_mirror: use read_all
use read_all in more places to improve safety
xap_helper*: use autodie in more places
xap_helper: die more easily in both implementations
xap_helper: simplify SIGTERM exit checks
xap_helper: autodie for getsockopt
xap_client: autodie for pipe and socketpair
xt/git-http-backend: remove Net::HTTP usage
ds: introduce and use do_fork helper
ds: get rid of SetLoopTimeout
cindex: drop some unused functions
syscall: common $F_SETPIPE_SZ definition
t/lei-up: additional diagnostics for match failures
test_common: use autodie and read_all where possible
test_common: only hide TCP port in messages
test_common: use $cwdfh for every run_script command
init: drop extraneous `+'
init: use autodie to reduce distractions
xt/mem-imapd-tls: remove unused/broken epoll imports
xt/mem-imapd-tls: reduce FDs for lsof use
lei: use autodie where appropriate
lei_auth: update comments and use v5.12
lei_config: drop redundant open check
convert: use read_all to simplify error checks
idx_stack: use autodie + read_all
lib/PublicInbox/CidxLogP.pm | 4 +-
lib/PublicInbox/CodeSearchIdx.pm | 5 --
lib/PublicInbox/DS.pm | 36 ++++----
lib/PublicInbox/Daemon.pm | 16 ++--
lib/PublicInbox/EOFpipe.pm | 6 +-
lib/PublicInbox/Gcf2.pm | 7 +-
lib/PublicInbox/Git.pm | 19 +++--
lib/PublicInbox/IPC.pm | 12 +--
lib/PublicInbox/IdxStack.pm | 20 ++---
lib/PublicInbox/Import.pm | 8 +-
lib/PublicInbox/InboxWritable.pm | 6 +-
lib/PublicInbox/LEI.pm | 48 +++++------
lib/PublicInbox/LeiALE.pm | 11 +--
lib/PublicInbox/LeiAuth.pm | 7 +-
lib/PublicInbox/LeiBlob.pm | 6 +-
lib/PublicInbox/LeiConfig.pm | 4 +-
lib/PublicInbox/LeiMailSync.pm | 5 +-
lib/PublicInbox/LeiMirror.pm | 131 ++++++++++++++----------------
lib/PublicInbox/LeiSucks.pm | 5 +-
lib/PublicInbox/LeiXSearch.pm | 2 +-
lib/PublicInbox/MultiGit.pm | 3 +-
lib/PublicInbox/SearchIdxShard.pm | 14 ++--
lib/PublicInbox/Syscall.pm | 16 ++--
lib/PublicInbox/TestCommon.pm | 85 +++++++++----------
lib/PublicInbox/ViewVCS.pm | 12 ++-
lib/PublicInbox/WWW.pm | 4 +-
lib/PublicInbox/Watch.pm | 11 +--
lib/PublicInbox/XapClient.pm | 11 +--
lib/PublicInbox/XapHelper.pm | 24 ++----
lib/PublicInbox/XapHelperCxx.pm | 11 +--
lib/PublicInbox/Xapcmd.pm | 5 +-
lib/PublicInbox/xap_helper.h | 60 ++++++--------
script/public-inbox-convert | 8 +-
script/public-inbox-edit | 4 +-
script/public-inbox-init | 30 +++----
t/dir_idle.t | 2 +-
t/ds-leak.t | 4 +-
t/gcf2.t | 5 +-
t/init.t | 7 ++
t/lei-sigpipe.t | 7 +-
t/lei-up.t | 4 +-
xt/git-http-backend.t | 30 +++----
xt/mem-imapd-tls.t | 21 ++---
xt/mem-nntpd-tls.t | 8 +-
44 files changed, 335 insertions(+), 409 deletions(-)
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 01/30] lei_mirror: start converting to autodie
2023-10-17 23:37 [PATCH 00/30] autodie-ification and code simplifications Eric Wong
@ 2023-10-17 23:37 ` Eric Wong
2023-10-17 23:37 ` [PATCH 02/30] lei_mirror: autodie most `close' calls Eric Wong
` (29 subsequent siblings)
30 siblings, 0 replies; 32+ messages in thread
From: Eric Wong @ 2023-10-17 23:37 UTC (permalink / raw)
To: meta
This code is too noisy and not critical for startup performance;
so autodie provides a nice noise reduction while improving error
reporting in most cases.
For places where failures are expected, the `CORE::' prefix
gives us an easy escape hatch to fall back to normal error
checking.
---
lib/PublicInbox/LeiMirror.pm | 38 ++++++++++++++++++------------------
1 file changed, 19 insertions(+), 19 deletions(-)
diff --git a/lib/PublicInbox/LeiMirror.pm b/lib/PublicInbox/LeiMirror.pm
index 9d8a8963..f4c16a9d 100644
--- a/lib/PublicInbox/LeiMirror.pm
+++ b/lib/PublicInbox/LeiMirror.pm
@@ -21,6 +21,7 @@ use PublicInbox::LeiCurl;
use PublicInbox::OnDestroy;
use PublicInbox::SHA qw(sha256_hex sha1_hex);
use POSIX qw(strftime);
+use autodie qw(chmod pipe readlink seek sysopen truncate unlink);
our $LIVE; # pid => callback
our $FGRP_TODO; # objstore -> [[ to resume ], [ to clone ]]
@@ -122,7 +123,7 @@ sub ft_rename ($$$;$) {
my ($ft, $dst, $open_mode, $fh) = @_;
my @st = stat($fh // $dst);
my $mode = @st ? ($st[2] & 07777) : ($open_mode & ~umask);
- chmod($mode, $ft) or croak "E: chmod($ft): $!";
+ chmod($mode, $ft);
require File::Copy;
File::Copy::mv($ft->filename, $dst) or croak "E: mv($ft => $dst): $!";
$ft->unlink_on_destroy(0);
@@ -170,7 +171,7 @@ sub _get_txt_done { # returns true on error (non-fatal), undef on success
$? = 0; # don't influence normal lei exit
return warn("$uri missing\n") if ($cerr >> 8) == 22;
return warn("# @$cmd failed (non-fatal)\n") if $cerr;
- seek($fh, SEEK_SET, 0) or die "seek: $!";
+ seek($fh, SEEK_SET, 0);
$self->{"mtime.$endpoint"} = (stat($fh))[9];
local $/;
$self->{"txt.$endpoint"} = <$fh>;
@@ -183,9 +184,9 @@ sub _write_inbox_config {
my $dst = $self->{cur_dst} // $self->{dst};
my $f = "$dst/inbox.config.example";
my $mtime = delete $self->{'mtime._/text/config/raw'};
- if (sysopen(my $fh, $f, O_CREAT|O_EXCL|O_WRONLY)) {
+ if (CORE::sysopen(my $fh, $f, O_CREAT|O_EXCL|O_WRONLY)) {
print $fh $buf or die "print: $!";
- chmod(0444 & ~umask, $fh) or die "chmod($f): $!";
+ chmod(0444 & ~umask, $fh);
$fh->flush or die "flush($f): $!";
if (defined $mtime) {
utime($mtime, $mtime, $fh) or die "utime($f): $!";
@@ -289,7 +290,7 @@ sub upr { # feed `git update-ref --stdin -z' verbosely
sub start_update_ref {
my ($fgrp) = @_;
- pipe(my ($r, $w)) or die "pipe: $!";
+ pipe(my $r, my $w);
my $cmd = [ 'git', "--git-dir=$fgrp->{cur_dst}",
qw(update-ref --stdin -z) ];
my $pack = PublicInbox::OnDestroy->new($$, \&satellite_done, $fgrp);
@@ -305,8 +306,8 @@ sub fgrp_update {
return if !keep_going($fgrp);
my $srcfh = delete $fgrp->{srcfh} or return;
my $dstfh = delete $fgrp->{dstfh} or return;
- seek($srcfh, SEEK_SET, 0) or die "seek(src): $!";
- seek($dstfh, SEEK_SET, 0) or die "seek(dst): $!";
+ seek($srcfh, SEEK_SET, 0);
+ seek($dstfh, SEEK_SET, 0);
my %src = map { chomp; split(/\0/) } (<$srcfh>);
close $srcfh;
my %dst = map { chomp; split(/\0/) } (<$dstfh>);
@@ -359,7 +360,7 @@ sub pack_refs {
sub unlink_fetch_head ($) {
my ($git_dir) = @_;
- return if unlink("$git_dir/FETCH_HEAD") || $!{ENOENT};
+ return if CORE::unlink("$git_dir/FETCH_HEAD") || $!{ENOENT};
warn "W: unlink($git_dir/FETCH_HEAD): $!";
}
@@ -447,8 +448,7 @@ sub fgrp_fetch_all {
if (!$self->{dry_run}) {
# update the config atomically via O_APPEND while
# respecting git-config locking
- sysopen(my $lk, "$f.lock", O_CREAT|O_EXCL|O_WRONLY)
- or die "open($f.lock): $!";
+ sysopen(my $lk, "$f.lock", O_CREAT|O_EXCL|O_WRONLY);
open my $fh, '>>', $f or die "open(>>$f): $!";
$fh->autoflush(1);
my $buf = '';
@@ -464,7 +464,7 @@ sub fgrp_fetch_all {
(map { "\t$grp = $_->{-remote}\n" } @$new));
print $fh $buf or die "print($f): $!";
close $fh or die "close($f): $!";
- unlink("$f.lock") or die "unlink($f.lock): $!";
+ unlink("$f.lock");
}
$cmd = [ @git, "--git-dir=$osdir", @fetch, $grp ];
push @$old, @$new;
@@ -515,7 +515,7 @@ EOM
my $f = "$o/info/alternates";
my $l = File::Spec->abs2rel($alt, File::Spec->rel2abs($o));
open my $fh, '+>>', $f or die "open($f): $!";
- seek($fh, SEEK_SET, 0) or die "seek($f): $!";
+ seek($fh, SEEK_SET, 0);
chomp(my @cur = <$fh>);
if (!grep(/\A\Q$l\E\z/, @cur)) {
say $fh $l or die "say($f): $!";
@@ -535,7 +535,7 @@ sub fp_done {
}
return if !keep_going($self);
my $fh = delete $self->{-show_ref} // die 'BUG: no show-ref output';
- seek($fh, SEEK_SET, 0) or die "seek(show_ref): $!";
+ seek($fh, SEEK_SET, 0);
$self->{-ent} // die 'BUG: no -ent';
my $A = $self->{-ent}->{fingerprint} // die 'BUG: no fingerprint';
my $B = sha1_hex(do { local $/; <$fh> } // die("read(show_ref): $!"));
@@ -735,7 +735,7 @@ sub up_fp_done {
my ($self) = @_;
return if !keep_going($self);
my $fh = delete $self->{-show_ref_up} // die 'BUG: no show-ref output';
- seek($fh, SEEK_SET, 0) or die "seek(show_ref): $!";
+ seek($fh, SEEK_SET, 0);
$self->{-ent} // die 'BUG: no -ent';
my $A = $self->{-ent}->{fingerprint} // die 'BUG: no fingerprint';
my $B = sha1_hex(do { local $/; <$fh> } // die("read(show_ref): $!"));
@@ -811,7 +811,7 @@ sub update_ent {
if (lstat($ln)) {
if (-l _) {
next if readlink($ln) eq $tgt;
- unlink($ln) or die "unlink($ln): $!";
+ unlink($ln);
} else {
push @{$self->{chg}->{badlink}}, $p;
}
@@ -882,7 +882,7 @@ sub v2_done { # called via OnDestroy
for my $i ($mg->git_epochs) { $mg->epoch_cfg_set($i) }
for my $edst (@{delete($self->{-read_only}) // []}) {
my @st = stat($edst) or die "stat($edst): $!";
- chmod($st[2] & 0555, $edst) or die "chmod(a-w, $edst): $!";
+ chmod($st[2] & 0555, $edst);
}
write_makefile($dst, 2);
undef $lck; # unlock
@@ -1089,8 +1089,8 @@ sub dump_manifest ($$) {
# epoch they no longer want to skip
my $json = PublicInbox::Config->json->encode($m);
my $mtime = (stat($ft))[9];
- seek($ft, SEEK_SET, 0) or die "seek($ft): $!";
- truncate($ft, 0) or die "truncate($ft): $!";
+ seek($ft, SEEK_SET, 0);
+ truncate($ft, 0);
gzip(\$json => $ft) or die "gzip($ft): $GzipError";
$ft->flush or die "flush($ft): $!";
utime($mtime, $mtime, "$ft") or die "utime(..., $ft): $!";
@@ -1344,7 +1344,7 @@ sub ipc_atfork_child {
sub write_makefile {
my ($dir, $ibx_ver) = @_;
my $f = "$dir/Makefile";
- if (sysopen my $fh, $f, O_CREAT|O_EXCL|O_WRONLY) {
+ if (CORE::sysopen my $fh, $f, O_CREAT|O_EXCL|O_WRONLY) {
print $fh <<EOM or die "print($f) $!";
# This is a v$ibx_ver public-inbox, see the public-inbox-v$ibx_ver-format(5)
# manpage for more information on the format. This Makefile is
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 02/30] lei_mirror: autodie most `close' calls
2023-10-17 23:37 [PATCH 00/30] autodie-ification and code simplifications Eric Wong
2023-10-17 23:37 ` [PATCH 01/30] lei_mirror: start converting to autodie Eric Wong
@ 2023-10-17 23:37 ` Eric Wong
2023-10-17 23:37 ` [PATCH 03/30] lei_mirror: use autodie for most `open' calls Eric Wong
` (28 subsequent siblings)
30 siblings, 0 replies; 32+ messages in thread
From: Eric Wong @ 2023-10-17 23:37 UTC (permalink / raw)
To: meta
Another step towards reducing our code burden and having
more consistent error messages.
---
lib/PublicInbox/LeiMirror.pm | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/lib/PublicInbox/LeiMirror.pm b/lib/PublicInbox/LeiMirror.pm
index f4c16a9d..84344f37 100644
--- a/lib/PublicInbox/LeiMirror.pm
+++ b/lib/PublicInbox/LeiMirror.pm
@@ -21,7 +21,7 @@ use PublicInbox::LeiCurl;
use PublicInbox::OnDestroy;
use PublicInbox::SHA qw(sha256_hex sha1_hex);
use POSIX qw(strftime);
-use autodie qw(chmod pipe readlink seek sysopen truncate unlink);
+use autodie qw(chmod close pipe readlink seek sysopen truncate unlink);
our $LIVE; # pid => callback
our $FGRP_TODO; # objstore -> [[ to resume ], [ to clone ]]
@@ -58,7 +58,7 @@ sub try_scrape {
my $opt = { 0 => $lei->{0}, 2 => $lei->{2} };
my $fh = popen_rd($cmd, undef, $opt);
my $html = do { local $/; <$fh> } // die "read(curl $uri): $!";
- close($fh) or return $lei->child_error($?, "@$cmd failed");
+ CORE::close($fh) or return $lei->child_error($?, "@$cmd failed");
# we grep with URL below, we don't want Subject/From headers
# making us clone random URLs. This assumes remote instances
@@ -295,7 +295,7 @@ sub start_update_ref {
qw(update-ref --stdin -z) ];
my $pack = PublicInbox::OnDestroy->new($$, \&satellite_done, $fgrp);
start_cmd($fgrp, $cmd, { 0 => $r, 2 => $fgrp->{lei}->{2} }, $pack);
- close $r or die "close(r): $!";
+ close $r;
$fgrp->{dry_run} ? undef : $w;
}
@@ -335,7 +335,7 @@ sub fgrp_update {
upr($lei, $w, 'create', $ref, $oid);
}
}
- close($w) or upref_warn();
+ CORE::close($w) or upref_warn();
}
sub satellite_done {
@@ -345,7 +345,7 @@ sub satellite_done {
while (my ($ref, $oid) = each %$create) {
upr($fgrp->{lei}, $w, 'create', $ref, $oid);
}
- close($w) or upref_warn();
+ CORE::close($w) or upref_warn();
} else {
pack_refs($fgrp, $fgrp->{cur_dst});
run_puh($fgrp);
@@ -463,7 +463,7 @@ sub fgrp_fetch_all {
(map { "\t$grp = $_->{-remote}\n" } @$old),
(map { "\t$grp = $_->{-remote}\n" } @$new));
print $fh $buf or die "print($f): $!";
- close $fh or die "close($f): $!";
+ close $fh;
unlink("$f.lock");
}
$cmd = [ @git, "--git-dir=$osdir", @fetch, $grp ];
@@ -490,7 +490,7 @@ sub forkgroup_prep {
[pack]
island = refs/remotes/([^/]+)/
EOM
- close $fh or die "close($f): $!";
+ close $fh;
}
my $key = $self->{-key} // die 'BUG: no -key';
my $rn = substr(sha256_hex($key), 0, 16);
@@ -507,7 +507,7 @@ EOM
; fetch = +refs/*:refs/*
; mirror = true
EOM
- close $fh or die "close($f): $!";
+ close $fh;
}
if (!$self->{dry_run}) {
my $alt = File::Spec->rel2abs("$dir/objects");
@@ -520,7 +520,7 @@ EOM
if (!grep(/\A\Q$l\E\z/, @cur)) {
say $fh $l or die "say($f): $!";
}
- close $fh or die "close($f): $!";
+ close $fh;
}
bless {
%$self, -osdir => $dir, -remote => $rn, -uri => $uri
@@ -712,7 +712,7 @@ EOM
owner = $ent->{owner}
EOM
}
- close $fh or die "close($f): $!";
+ close $fh;
my %map = (head => 'HEAD', description => undef);
while (my ($key, $fn) = each %map) {
my $val = $ent->{$key} // next;
@@ -720,7 +720,7 @@ EOM
$fn = "$edst/$fn";
open $fh, '>', $fn or die "open($fn): $!";
print $fh $val, "\n" or die "print($fn): $!";
- close $fh or die "close($fn): $!";
+ close $fh;
}
}
@@ -1395,7 +1395,7 @@ compact :
.PHONY : help fetch update index reindex compact
EOM
- close $fh or die "close($f): $!";
+ close $fh;
} else {
die "open($f): $!" unless $!{EEXIST};
}
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 03/30] lei_mirror: use autodie for most `open' calls
2023-10-17 23:37 [PATCH 00/30] autodie-ification and code simplifications Eric Wong
2023-10-17 23:37 ` [PATCH 01/30] lei_mirror: start converting to autodie Eric Wong
2023-10-17 23:37 ` [PATCH 02/30] lei_mirror: autodie most `close' calls Eric Wong
@ 2023-10-17 23:37 ` Eric Wong
2023-10-17 23:37 ` [PATCH 04/30] git: introduce read_all function Eric Wong
` (27 subsequent siblings)
30 siblings, 0 replies; 32+ messages in thread
From: Eric Wong @ 2023-10-17 23:37 UTC (permalink / raw)
To: meta
We'll also drop most `print' checks since we need to rely on
`close' or IO::Handle->flush for error checking, anyways.
---
lib/PublicInbox/LeiMirror.pm | 62 +++++++++++++++++-------------------
1 file changed, 29 insertions(+), 33 deletions(-)
diff --git a/lib/PublicInbox/LeiMirror.pm b/lib/PublicInbox/LeiMirror.pm
index 84344f37..b402eb5f 100644
--- a/lib/PublicInbox/LeiMirror.pm
+++ b/lib/PublicInbox/LeiMirror.pm
@@ -21,7 +21,8 @@ use PublicInbox::LeiCurl;
use PublicInbox::OnDestroy;
use PublicInbox::SHA qw(sha256_hex sha1_hex);
use POSIX qw(strftime);
-use autodie qw(chmod close pipe readlink seek sysopen truncate unlink);
+use autodie qw(chdir chmod close open pipe readlink seek symlink sysopen
+ truncate unlink);
our $LIVE; # pid => callback
our $FGRP_TODO; # objstore -> [[ to resume ], [ to clone ]]
@@ -185,7 +186,7 @@ sub _write_inbox_config {
my $f = "$dst/inbox.config.example";
my $mtime = delete $self->{'mtime._/text/config/raw'};
if (CORE::sysopen(my $fh, $f, O_CREAT|O_EXCL|O_WRONLY)) {
- print $fh $buf or die "print: $!";
+ print $fh $buf;
chmod(0444 & ~umask, $fh);
$fh->flush or die "flush($f): $!";
if (defined $mtime) {
@@ -381,12 +382,12 @@ sub fgrpv_done {
my $src = [ 'git', "--git-dir=$fgrp->{-osdir}", 'for-each-ref',
"--format=refs/%(refname:lstrip=3)%00%(objectname)",
"refs/remotes/$rn/" ];
- open(my $sfh, '+>', undef) or die "open(src): $!";
+ open(my $sfh, '+>', undef);
$fgrp->{srcfh} = $sfh;
start_cmd($fgrp, $src, { %opt, 1 => $sfh }, $update_ref);
my $dst = [ 'git', "--git-dir=$fgrp->{cur_dst}", 'for-each-ref',
'--format=%(refname)%00%(objectname)' ];
- open(my $dfh, '+>', undef) or die "open(dst): $!";
+ open(my $dfh, '+>', undef);
$fgrp->{dstfh} = $dfh;
start_cmd($fgrp, $dst, { %opt, 1 => $dfh }, $update_ref);
}
@@ -449,7 +450,7 @@ sub fgrp_fetch_all {
# update the config atomically via O_APPEND while
# respecting git-config locking
sysopen(my $lk, "$f.lock", O_CREAT|O_EXCL|O_WRONLY);
- open my $fh, '>>', $f or die "open(>>$f): $!";
+ open my $fh, '>>', $f;
$fh->autoflush(1);
my $buf = '';
if (@$old) {
@@ -482,9 +483,8 @@ sub forkgroup_prep {
my $dir = "$os/$fg.git";
if (!-d $dir && !$self->{dry_run}) {
PublicInbox::Import::init_bare($dir);
- my $f = "$dir/config";
- open my $fh, '+>>', $f or die "open:($f): $!";
- print $fh <<EOM or die "print($f): $!";
+ open my $fh, '+>>', "$dir/config";
+ print $fh <<EOM;
[repack]
useDeltaIslands = true
[pack]
@@ -496,9 +496,8 @@ EOM
my $rn = substr(sha256_hex($key), 0, 16);
if (!-d $self->{cur_dst} && !$self->{dry_run}) {
PublicInbox::Import::init_bare($self->{cur_dst});
- my $f = "$self->{cur_dst}/config";
- open my $fh, '+>>', $f or die "open:($f): $!";
- print $fh <<EOM or die "print($f): $!";
+ open my $fh, '+>>', "$self->{cur_dst}/config";
+ print $fh <<EOM;
; rely on the "$rn" remote in the
; $fg fork group for fetches
; only uncomment the following iff you detach from fork groups
@@ -514,11 +513,11 @@ EOM
my $o = "$self->{cur_dst}/objects";
my $f = "$o/info/alternates";
my $l = File::Spec->abs2rel($alt, File::Spec->rel2abs($o));
- open my $fh, '+>>', $f or die "open($f): $!";
+ open my $fh, '+>>', $f;
seek($fh, SEEK_SET, 0);
chomp(my @cur = <$fh>);
if (!grep(/\A\Q$l\E\z/, @cur)) {
- say $fh $l or die "say($f): $!";
+ say $fh $l;
}
close $fh;
}
@@ -556,7 +555,7 @@ sub cmp_fp_do {
my $dst = $self->{cur_dst} // $self->{dst};
my $cmd = ['git', "--git-dir=$dst", 'show-ref'];
my $opt = { 2 => $self->{lei}->{2} };
- open($opt->{1}, '+>', undef) or die "open(tmp): $!";
+ open($opt->{1}, '+>', undef);
$self->{-show_ref} = $opt->{1};
do_reap($self);
$self->{lei}->qerr("# @$cmd");
@@ -695,8 +694,8 @@ sub init_placeholder ($$$) {
my ($src, $edst, $ent) = @_;
PublicInbox::Import::init_bare($edst);
my $f = "$edst/config";
- open my $fh, '>>', $f or die "open($f): $!";
- print $fh <<EOM or die "print($f): $!";
+ open my $fh, '>>', $f;
+ print $fh <<EOM;
[remote "origin"]
url = $src
fetch = +refs/*:refs/*
@@ -706,20 +705,17 @@ sub init_placeholder ($$$) {
; will not fetch updates for it unless write permission is added.
; Hint: chmod +w $edst
EOM
- if (defined($ent->{owner})) {
- print $fh <<EOM or die "print($f): $!";
+ print $fh <<EOM if defined($ent->{owner});
[gitweb]
owner = $ent->{owner}
EOM
- }
close $fh;
my %map = (head => 'HEAD', description => undef);
while (my ($key, $fn) = each %map) {
my $val = $ent->{$key} // next;
$fn //= $key;
- $fn = "$edst/$fn";
- open $fh, '>', $fn or die "open($fn): $!";
- print $fh $val, "\n" or die "print($fn): $!";
+ open $fh, '>', "$edst/$fn";
+ say $fh $val;
close $fh;
}
}
@@ -747,7 +743,7 @@ sub up_fp_done {
sub atomic_write ($$$) {
my ($dn, $bn, $raw) = @_;
my $ft = File::Temp->new(DIR => $dn, TEMPLATE => "$bn-XXXX");
- print $ft $raw or die "print($ft): $!";
+ print $ft $raw;
$ft->flush or die "flush($ft): $!";
ft_rename($ft, "$dn/$bn", 0666);
}
@@ -778,7 +774,7 @@ sub update_ent {
if (defined($new) && $new ne $cur) {
my $cmd = ['git', "--git-dir=$dst", 'show-ref'];
my $opt = { 2 => $self->{lei}->{2} };
- open($opt->{1}, '+>', undef) or die "open(tmp): $!";
+ open($opt->{1}, '+>', undef);
$self->{-show_ref_up} = $opt->{1};
my $done = PublicInbox::OnDestroy->new($$, \&up_fp_done, $self);
start_cmd($self, $cmd, $opt, $done);
@@ -816,7 +812,7 @@ sub update_ent {
push @{$self->{chg}->{badlink}}, $p;
}
}
- symlink($tgt, $ln) or die "symlink($tgt, $ln): $!";
+ symlink($tgt, $ln);
++$self->{chg}->{nr_chg};
}
}
@@ -844,7 +840,7 @@ sub v1_done { # called via OnDestroy
unlink_fetch_head($dst);
update_ent($self) if $self->{-ent};
my $o = "$dst/objects";
- if (open(my $fh, '<', my $fn = "$o/info/alternates")) {;
+ if (CORE::open(my $fh, '<', my $fn = "$o/info/alternates")) {;
my $base = File::Spec->rel2abs($o);
my @l = <$fh>;
my $ft;
@@ -855,7 +851,7 @@ sub v1_done { # called via OnDestroy
DIR => "$o/info");
}
if ($ft) {
- print $ft @l or die "print($ft): $!";
+ print $ft @l;
$ft->flush or die "flush($ft): $!";
ft_rename($ft, $fn, 0666, $fh);
}
@@ -964,7 +960,7 @@ sub decode_manifest ($$$) {
sub load_current_manifest ($) {
my ($self) = @_;
my $fn = $self->{-manifest} // return;
- if (open(my $fh, '<', $fn)) {
+ if (CORE::open(my $fh, '<', $fn)) {
decode_manifest($fh, $fn, $fn);
} elsif ($!{ENOENT}) { # non-fatal, we can just do it slowly
warn "open($fn): $!\n" if !$self->{-initial_clone};
@@ -1102,12 +1098,12 @@ sub dump_project_list ($$) {
my $old = defined($f) ? PublicInbox::Git::try_cat($f) : '';
my %new;
- open my $dh, '<', '.' or die "open(.): $!";
+ open my $dh, '<', '.';
if (!$self->{dry_run} || -d $self->{dst}) {
- chdir($self->{dst}) or die "chdir($self->{dst}): $!";
+ chdir($self->{dst});
}
my @local = grep { -e $_ ? ($new{$_} = undef) : 1 } split(/\n/s, $old);
- chdir($dh) or die "chdir(restore): $!";
+ chdir($dh);
$new{substr($_, 1)} = 1 for keys %$m; # drop leading '/'
my @list = sort keys %new;
@@ -1345,7 +1341,7 @@ sub write_makefile {
my ($dir, $ibx_ver) = @_;
my $f = "$dir/Makefile";
if (CORE::sysopen my $fh, $f, O_CREAT|O_EXCL|O_WRONLY) {
- print $fh <<EOM or die "print($f) $!";
+ print $fh <<EOM;
# This is a v$ibx_ver public-inbox, see the public-inbox-v$ibx_ver-format(5)
# manpage for more information on the format. This Makefile is
# intended as a familiar wrapper for users unfamiliar with
@@ -1359,7 +1355,7 @@ sub write_makefile {
# so you may edit it freely with your own convenience targets
# and notes. public-inbox-fetch will recreate it if removed.
EOM
- print $fh <<'EOM' or die "print($f): $!";
+ print $fh <<'EOM';
# the default target:
help :
@echo Common targets:
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 04/30] git: introduce read_all function
2023-10-17 23:37 [PATCH 00/30] autodie-ification and code simplifications Eric Wong
` (2 preceding siblings ...)
2023-10-17 23:37 ` [PATCH 03/30] lei_mirror: use autodie for most `open' calls Eric Wong
@ 2023-10-17 23:37 ` Eric Wong
2023-10-17 23:37 ` [PATCH 05/30] import: use read_all to detect short reads Eric Wong
` (26 subsequent siblings)
30 siblings, 0 replies; 32+ messages in thread
From: Eric Wong @ 2023-10-17 23:37 UTC (permalink / raw)
To: meta
This makes it easier to improve error checking, since the
`do { local $/; readline(FH) }' construct does not detect
errors (autodie does not cover `readline' or `<FH>').
I'm not sure exactly where this should be, but PublicInbox::Git
is used nearly everywhere in our code base and it's probably
not worth creating a new package for it.
---
lib/PublicInbox/Git.pm | 18 +++++++++++++-----
1 file changed, 13 insertions(+), 5 deletions(-)
diff --git a/lib/PublicInbox/Git.pm b/lib/PublicInbox/Git.pm
index 448cfaf7..35bd10ef 100644
--- a/lib/PublicInbox/Git.pm
+++ b/lib/PublicInbox/Git.pm
@@ -10,7 +10,7 @@ package PublicInbox::Git;
use strict;
use v5.10.1;
use parent qw(Exporter PublicInbox::DS);
-use autodie qw(socketpair);
+use autodie qw(socketpair read);
use POSIX ();
use Socket qw(AF_UNIX SOCK_STREAM);
use PublicInbox::Syscall qw(EPOLLIN EPOLLET);
@@ -26,7 +26,7 @@ use Carp qw(croak carp);
use PublicInbox::SHA ();
our %HEXLEN2SHA = (40 => 1, 64 => 256);
our %OFMT2HEXLEN = (sha1 => 40, sha256 => 64);
-our @EXPORT_OK = qw(git_unquote git_quote %HEXLEN2SHA %OFMT2HEXLEN);
+our @EXPORT_OK = qw(git_unquote git_quote %HEXLEN2SHA %OFMT2HEXLEN read_all);
our $in_cleanup;
our $RDTIMEO = 60_000; # milliseconds
our $async_warn; # true in read-only daemons
@@ -548,11 +548,19 @@ sub modified ($;$) {
(split(/ /, <$fh> // time))[0] + 0; # integerize for JSON
}
+# read_all/try_cat can probably be moved somewhere else...
+
+sub read_all ($;$) {
+ my ($fh, $len) = @_;
+ my $r = read($fh, my $buf, $len //= -s $fh);
+ croak("$fh read ($r != $len)") if $len != $r;
+ $buf;
+}
+
sub try_cat {
my ($path) = @_;
open(my $fh, '<', $path) or return '';
- local $/;
- <$fh> // '';
+ read_all($fh);
}
sub cat_desc ($) {
@@ -606,7 +614,7 @@ sub manifest_entry {
}
}
my $dig = PublicInbox::SHA->new(1);
- while (read($sr, $buf, 65536)) { $dig->add($buf) }
+ while (CORE::read($sr, $buf, 65536)) { $dig->add($buf) }
CORE::close $sr or return; # empty, uninitialized git repo
$ent->{fingerprint} = $dig->hexdigest;
$ent->{modified} = modified(undef, $mod);
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 05/30] import: use read_all to detect short reads
2023-10-17 23:37 [PATCH 00/30] autodie-ification and code simplifications Eric Wong
` (3 preceding siblings ...)
2023-10-17 23:37 ` [PATCH 04/30] git: introduce read_all function Eric Wong
@ 2023-10-17 23:37 ` Eric Wong
2023-10-17 23:37 ` [PATCH 06/30] lei_mirror: use read_all Eric Wong
` (25 subsequent siblings)
30 siblings, 0 replies; 32+ messages in thread
From: Eric Wong @ 2023-10-17 23:37 UTC (permalink / raw)
To: meta
Our Import package was depending on Git, anyways.
---
lib/PublicInbox/Import.pm | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/lib/PublicInbox/Import.pm b/lib/PublicInbox/Import.pm
index 6bb2c66d..fb70b91b 100644
--- a/lib/PublicInbox/Import.pm
+++ b/lib/PublicInbox/Import.pm
@@ -21,6 +21,7 @@ use POSIX qw(strftime);
use autodie qw(read close socketpair);
use Carp qw(croak);
use Socket qw(AF_UNIX SOCK_STREAM);
+use PublicInbox::Git qw(read_all);
sub default_branch () {
state $default_branch = do {
@@ -114,8 +115,7 @@ sub _cat_blob ($$) {
local $/ = "\n";
my $info = <$io> // die "EOF from fast-import / cat-blob: $!";
$info =~ /\A[a-f0-9]{40,} blob ([0-9]+)\n\z/ or return;
- my $n = read($io, my $buf, my $len = $1 + 1);
- $n == $len or croak "cat-blob: short read: $n < $len";
+ my $buf = read_all($io, my $len = $1 + 1);
my $lf = chop $buf;
croak "bad read on final byte: <$lf>" if $lf ne "\n";
\$buf;
@@ -562,9 +562,7 @@ sub replace_oids {
} elsif (/^data ([0-9]+)/) {
# only commit message, so $len is small:
push @buf, $_;
- my $n = read($rd, my $buf, my $len = $1);
- $len == $n or croak "short read ($n < $len)";
- push @buf, $buf;
+ push @buf, read_all($rd, my $len = $1);
} elsif (/^M 100644 ([a-f0-9]+) (\w+)/) {
my ($oid, $path) = ($1, $2);
$tree->{$path} = 1;
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 06/30] lei_mirror: use read_all
2023-10-17 23:37 [PATCH 00/30] autodie-ification and code simplifications Eric Wong
` (4 preceding siblings ...)
2023-10-17 23:37 ` [PATCH 05/30] import: use read_all to detect short reads Eric Wong
@ 2023-10-17 23:37 ` Eric Wong
2023-10-17 23:37 ` [PATCH 07/30] use read_all in more places to improve safety Eric Wong
` (24 subsequent siblings)
30 siblings, 0 replies; 32+ messages in thread
From: Eric Wong @ 2023-10-17 23:37 UTC (permalink / raw)
To: meta
This gives us better error checking for regular files.
---
lib/PublicInbox/LeiMirror.pm | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/lib/PublicInbox/LeiMirror.pm b/lib/PublicInbox/LeiMirror.pm
index b402eb5f..47fb767b 100644
--- a/lib/PublicInbox/LeiMirror.pm
+++ b/lib/PublicInbox/LeiMirror.pm
@@ -16,7 +16,7 @@ use Carp qw(croak);
use URI;
use PublicInbox::Config qw(glob2re);
use PublicInbox::Inbox;
-use PublicInbox::Git;
+use PublicInbox::Git qw(read_all);
use PublicInbox::LeiCurl;
use PublicInbox::OnDestroy;
use PublicInbox::SHA qw(sha256_hex sha1_hex);
@@ -174,8 +174,7 @@ sub _get_txt_done { # returns true on error (non-fatal), undef on success
return warn("# @$cmd failed (non-fatal)\n") if $cerr;
seek($fh, SEEK_SET, 0);
$self->{"mtime.$endpoint"} = (stat($fh))[9];
- local $/;
- $self->{"txt.$endpoint"} = <$fh>;
+ $self->{"txt.$endpoint"} = read_all($fh, -s _);
undef; # success
}
@@ -537,7 +536,7 @@ sub fp_done {
seek($fh, SEEK_SET, 0);
$self->{-ent} // die 'BUG: no -ent';
my $A = $self->{-ent}->{fingerprint} // die 'BUG: no fingerprint';
- my $B = sha1_hex(do { local $/; <$fh> } // die("read(show_ref): $!"));
+ my $B = sha1_hex(read_all($fh));
return $cb->($self, @arg) if $A ne $B;
$self->{lei}->qerr("# $self->{-key} up-to-date");
}
@@ -734,7 +733,7 @@ sub up_fp_done {
seek($fh, SEEK_SET, 0);
$self->{-ent} // die 'BUG: no -ent';
my $A = $self->{-ent}->{fingerprint} // die 'BUG: no fingerprint';
- my $B = sha1_hex(do { local $/; <$fh> } // die("read(show_ref): $!"));
+ my $B = sha1_hex(read_all($fh));
return if $A eq $B;
$self->{-ent}->{fingerprint} = $B;
push @{$self->{chg}->{fp_mismatch}}, $self->{-key};
@@ -948,7 +947,7 @@ failed to extract epoch number from $src
sub decode_manifest ($$$) {
my ($fh, $fn, $uri) = @_;
my $js;
- my $gz = do { local $/; <$fh> } // die "slurp($fn): $!";
+ my $gz = read_all($fh);
gunzip(\$gz => \$js, MultiStream => 1) or
die "gunzip($uri): $GunzipError\n";
my $m = eval { PublicInbox::Config->json->decode($js) };
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 07/30] use read_all in more places to improve safety
2023-10-17 23:37 [PATCH 00/30] autodie-ification and code simplifications Eric Wong
` (5 preceding siblings ...)
2023-10-17 23:37 ` [PATCH 06/30] lei_mirror: use read_all Eric Wong
@ 2023-10-17 23:37 ` Eric Wong
2023-10-17 23:37 ` [PATCH 08/30] xap_helper*: use autodie in more places Eric Wong
` (23 subsequent siblings)
30 siblings, 0 replies; 32+ messages in thread
From: Eric Wong @ 2023-10-17 23:37 UTC (permalink / raw)
To: meta
`readline' ops may not detect errors on partial reads.
This saves us some code to reduce cognitive overhead for
readers. We'll also support reusing a destination buffers so it
can work more nicely with existing code.
---
lib/PublicInbox/Gcf2.pm | 7 +++----
lib/PublicInbox/Git.pm | 9 +++++----
lib/PublicInbox/InboxWritable.pm | 6 +++---
lib/PublicInbox/LeiALE.pm | 11 +++--------
lib/PublicInbox/LeiBlob.pm | 6 +++---
lib/PublicInbox/LeiConfig.pm | 2 +-
lib/PublicInbox/LeiMailSync.pm | 5 ++---
lib/PublicInbox/LeiSucks.pm | 5 +++--
lib/PublicInbox/MultiGit.pm | 3 ++-
lib/PublicInbox/ViewVCS.pm | 12 +++++-------
lib/PublicInbox/WWW.pm | 4 ++--
lib/PublicInbox/XapHelper.pm | 3 ++-
lib/PublicInbox/XapHelperCxx.pm | 6 +++---
script/public-inbox-edit | 4 ++--
script/public-inbox-init | 3 ++-
15 files changed, 41 insertions(+), 45 deletions(-)
diff --git a/lib/PublicInbox/Gcf2.pm b/lib/PublicInbox/Gcf2.pm
index 37262e28..4f163cde 100644
--- a/lib/PublicInbox/Gcf2.pm
+++ b/lib/PublicInbox/Gcf2.pm
@@ -9,7 +9,7 @@ use PublicInbox::Spawn qw(which popen_rd); # may set PERL_INLINE_DIRECTORY
use Fcntl qw(SEEK_SET);
use Time::HiRes qw(clock_gettime CLOCK_MONOTONIC);
use IO::Handle; # autoflush
-use PublicInbox::Git;
+use PublicInbox::Git qw(read_all);
use PublicInbox::Lock;
BEGIN {
@@ -43,12 +43,11 @@ BEGIN {
# build them.
my $f = "$dir/gcf2_libgit2.h";
open my $src, '<', $f;
- local $/;
- $c_src = <$src> // die "read $f: $!";
+ $c_src = read_all($src);
}
unless ($c_src) {
seek($err, 0, SEEK_SET);
- $err = do { local $/; <$err> };
+ $err = read_all($err);
die "E: libgit2 not installed: $err\n";
}
# append pkg-config results to the source to ensure Inline::C
diff --git a/lib/PublicInbox/Git.pm b/lib/PublicInbox/Git.pm
index 35bd10ef..a460d155 100644
--- a/lib/PublicInbox/Git.pm
+++ b/lib/PublicInbox/Git.pm
@@ -550,11 +550,12 @@ sub modified ($;$) {
# read_all/try_cat can probably be moved somewhere else...
-sub read_all ($;$) {
- my ($fh, $len) = @_;
- my $r = read($fh, my $buf, $len //= -s $fh);
+sub read_all ($;$$) {
+ my ($fh, $len, $bref) = @_;
+ $bref //= \(my $buf);
+ my $r = read($fh, $$bref, $len //= -s $fh);
croak("$fh read ($r != $len)") if $len != $r;
- $buf;
+ $$bref;
}
sub try_cat {
diff --git a/lib/PublicInbox/InboxWritable.pm b/lib/PublicInbox/InboxWritable.pm
index 65952aa2..6af72e71 100644
--- a/lib/PublicInbox/InboxWritable.pm
+++ b/lib/PublicInbox/InboxWritable.pm
@@ -7,6 +7,7 @@ use strict;
use v5.10.1;
use parent qw(PublicInbox::Inbox PublicInbox::Umask Exporter);
use PublicInbox::Import;
+use PublicInbox::Git qw(read_all);
use PublicInbox::Filter::Base qw(REJECT);
use Errno qw(ENOENT);
our @EXPORT_OK = qw(eml_from_path);
@@ -114,9 +115,8 @@ sub filter {
sub eml_from_path ($) {
my ($path) = @_;
if (sysopen(my $fh, $path, O_RDONLY|O_NONBLOCK)) {
- return unless -f $fh; # no FIFOs or directories
- my $str = do { local $/; <$fh> } or return;
- PublicInbox::Eml->new(\$str);
+ return unless -f $fh && -s _; # no FIFOs or directories
+ PublicInbox::Eml->new(\(my $str = read_all($fh, -s _)));
} else { # ENOENT is common with Maildir
warn "failed to open $path: $!\n" if $! != ENOENT;
undef;
diff --git a/lib/PublicInbox/LeiALE.pm b/lib/PublicInbox/LeiALE.pm
index cc9a2095..b198af1c 100644
--- a/lib/PublicInbox/LeiALE.pm
+++ b/lib/PublicInbox/LeiALE.pm
@@ -9,7 +9,7 @@ package PublicInbox::LeiALE;
use strict;
use v5.10.1;
use parent qw(PublicInbox::LeiSearch PublicInbox::Lock);
-use PublicInbox::Git;
+use PublicInbox::Git qw(read_all);
use PublicInbox::Import;
use PublicInbox::LeiXSearch;
use Fcntl qw(SEEK_SET);
@@ -54,11 +54,7 @@ sub refresh_externals {
$self->git->cleanup;
my $lk = $self->lock_for_scope;
my $cur_lxs = ref($lxs)->new;
- my $orig = do {
- local $/;
- readline($self->{lockfh}) //
- die "readline($self->{lock_path}): $!";
- };
+ my $orig = read_all($self->{lockfh});
my $new = '';
my $old = '';
my $gone = 0;
@@ -91,8 +87,7 @@ sub refresh_externals {
$new = $old = '';
my $f = $self->git->{git_dir}.'/objects/info/alternates';
if (open my $fh, '<', $f) {
- local $/;
- $old = <$fh> // die "readline($f): $!";
+ read_all($fh, -s $fh, \$old);
}
for my $x (@ibxish) {
$new .= $lei->canonpath_harder($x->git->{git_dir})."/objects\n";
diff --git a/lib/PublicInbox/LeiBlob.pm b/lib/PublicInbox/LeiBlob.pm
index d069d4a8..40f64bd9 100644
--- a/lib/PublicInbox/LeiBlob.pm
+++ b/lib/PublicInbox/LeiBlob.pm
@@ -10,6 +10,7 @@ use parent qw(PublicInbox::IPC);
use PublicInbox::Spawn qw(run_wait popen_rd which);
use PublicInbox::DS;
use PublicInbox::Eml;
+use PublicInbox::Git qw(read_all);
sub get_git_dir ($$) {
my ($lei, $d) = @_;
@@ -137,9 +138,8 @@ sub lei_blob {
extract_attach($lei, $blob, $bref) :
$lei->out($$bref);
if ($opt->{mail}) {
- my $eh = $rdr->{2};
- seek($eh, 0, 0);
- return $lei->child_error($cerr, do { local $/; <$eh> });
+ seek($rdr->{2}, 0, 0);
+ return $lei->child_error($cerr, read_all($rdr->{2}));
} # else: fall through to solver below
}
diff --git a/lib/PublicInbox/LeiConfig.pm b/lib/PublicInbox/LeiConfig.pm
index b3495487..c47708d8 100644
--- a/lib/PublicInbox/LeiConfig.pm
+++ b/lib/PublicInbox/LeiConfig.pm
@@ -28,7 +28,7 @@ sub cfg_edit_done { # PktOp lei->do_env cb
$lei->cfg_dump($self->{-f});
} or do {
seek($fh, 0, SEEK_SET);
- return cfg_do_edit($self, do { local $/; <$fh> });
+ return cfg_do_edit($self, read_all($fh));
};
$self->cfg_verify($cfg) if $self->can('cfg_verify');
}
diff --git a/lib/PublicInbox/LeiMailSync.pm b/lib/PublicInbox/LeiMailSync.pm
index 415459d5..74ef1362 100644
--- a/lib/PublicInbox/LeiMailSync.pm
+++ b/lib/PublicInbox/LeiMailSync.pm
@@ -10,7 +10,7 @@ use PublicInbox::Compat qw(uniqstr);
use DBI qw(:sql_types); # SQL_BLOB
use PublicInbox::ContentHash qw(git_sha);
use Carp ();
-use PublicInbox::Git qw(%HEXLEN2SHA);
+use PublicInbox::Git qw(%HEXLEN2SHA read_all);
sub dbh_new {
my ($self) = @_;
@@ -456,8 +456,7 @@ WHERE b.oidbin = ?
open my $fh, '<', $f or next;
# some (buggy) Maildir writers are non-atomic:
next unless -s $fh;
- local $/;
- my $raw = <$fh>;
+ my $raw = read_all($fh, -s _);
if ($vrfy) {
my $sha = $HEXLEN2SHA{length($oidhex)};
my $got = git_sha($sha, \$raw)->hexdigest;
diff --git a/lib/PublicInbox/LeiSucks.pm b/lib/PublicInbox/LeiSucks.pm
index 35d0a8de..82aea8d4 100644
--- a/lib/PublicInbox/LeiSucks.pm
+++ b/lib/PublicInbox/LeiSucks.pm
@@ -12,6 +12,7 @@ use Config;
use POSIX ();
use PublicInbox::Config;
use PublicInbox::IPC;
+use PublicInbox::Git qw(read_all);
sub lei_sucks {
my ($lei, @argv) = @_;
@@ -58,8 +59,8 @@ sub lei_sucks {
for my $m (grep(m{^PublicInbox/}, sort keys %INC)) {
my $f = $INC{$m} // next; # lazy require failed (missing dep)
open my $fh, '<', $f or do { warn "open($f): $!"; next };
- my $hex = sha1_hex('blob '.(-s $fh)."\0".
- (do { local $/; <$fh> } // die("read: $!")));
+ my $size = -s $fh;
+ my $hex = sha1_hex("blob $size\0".read_all($fh, $size));
push @out, ' '.$hex.' '.$m."\n";
}
push @out, <<'EOM';
diff --git a/lib/PublicInbox/MultiGit.pm b/lib/PublicInbox/MultiGit.pm
index 74a9e1df..35bd0251 100644
--- a/lib/PublicInbox/MultiGit.pm
+++ b/lib/PublicInbox/MultiGit.pm
@@ -9,6 +9,7 @@ use PublicInbox::Spawn qw(run_die popen_rd);
use PublicInbox::Import;
use File::Temp 0.19;
use List::Util qw(max);
+use PublicInbox::Git qw(read_all);
sub new {
my ($cls, $topdir, $all, $epfx) = @_;
@@ -31,7 +32,7 @@ sub read_alternates {
qr!\A\Q../../$self->{epfx}\E/([0-9]+)\.git/objects\z! :
undef;
$$moderef = (stat($fh))[2] & 07777;
- for my $rel (split(/^/m, do { local $/; <$fh> })) {
+ for my $rel (split(/^/m, read_all($fh, -s _))) {
chomp(my $dir = $rel);
my $score;
if (defined($is_edir) && $dir =~ $is_edir) {
diff --git a/lib/PublicInbox/ViewVCS.pm b/lib/PublicInbox/ViewVCS.pm
index 8a90c2d6..86c46e69 100644
--- a/lib/PublicInbox/ViewVCS.pm
+++ b/lib/PublicInbox/ViewVCS.pm
@@ -17,6 +17,7 @@ use strict;
use v5.10.1;
use File::Temp 0.19 (); # newdir
use PublicInbox::SolverGit;
+use PublicInbox::Git qw(read_all);
use PublicInbox::GitAsyncCat;
use PublicInbox::WwwStream qw(html_oneshot);
use PublicInbox::Linkify;
@@ -61,12 +62,9 @@ sub dbg_log ($) {
warn "seek(log): $!";
return '<pre>debug log seek error</pre>';
}
- $log = do { local $/; <$log> } // do {
- if (!eof($log)) {
- warn "readline(log): $!";
- return '<pre>debug log read error</pre>';
- }
- '';
+ $log = eval { read_all($log) } // do {
+ warn "read(log): $@";
+ return '<pre>debug log read error</pre>';
};
return '' if $log eq '';
$ctx->{-linkify} //= PublicInbox::Linkify->new;
@@ -251,7 +249,7 @@ EOM
if (-s $fh > $MAX_SIZE) {
print $zfh "---\n patch is too large to show\n";
} else { # prepare flush_diff:
- read($fh, $x, -s _);
+ read_all($fh, -s _, \$x);
utf8_maybe($x);
$ctx->{-apfx} = $ctx->{-spfx} = $upfx;
$x =~ s/\r?\n/\n/gs;
diff --git a/lib/PublicInbox/WWW.pm b/lib/PublicInbox/WWW.pm
index 9919f975..86fd7e22 100644
--- a/lib/PublicInbox/WWW.pm
+++ b/lib/PublicInbox/WWW.pm
@@ -587,9 +587,9 @@ sub stylesheets_prepare ($$) {
next;
};
my $ctime = 0;
- my $local = do { local $/; <$fh> };
+ my $local = read_all($fh, -s $fh);
if ($local =~ /\S/) {
- $ctime = sprintf('%x',(stat($fh))[10]);
+ $ctime = sprintf('%x',(stat(_))[10]);
$local = $mini->($local);
}
diff --git a/lib/PublicInbox/XapHelper.pm b/lib/PublicInbox/XapHelper.pm
index ae907766..ca993ca8 100644
--- a/lib/PublicInbox/XapHelper.pm
+++ b/lib/PublicInbox/XapHelper.pm
@@ -10,6 +10,7 @@ $GLP->configure(qw(require_order bundling no_ignore_case no_auto_abbrev));
use PublicInbox::Search qw(xap_terms);
use PublicInbox::CodeSearch;
use PublicInbox::IPC;
+use PublicInbox::Git qw(read_all);
use Socket qw(SOL_SOCKET SO_TYPE SOCK_SEQPACKET AF_UNIX);
use PublicInbox::DS qw(awaitpid);
use POSIX qw(:signal_h);
@@ -123,7 +124,7 @@ sub cmd_dump_roots {
$req->{A} or return warn('dump_roots requires -A PREFIX');
open my $fh, '<', $root2id_file or die "open($root2id_file): $!";
my $root2id; # record format: $OIDHEX "\0" uint32_t
- my @x = split(/\0/, do { local $/; <$fh> } // die "readline: $!");
+ my @x = split(/\0/, read_all($fh));
while (@x) {
my $oidhex = shift @x;
$root2id->{$oidhex} = shift @x;
diff --git a/lib/PublicInbox/XapHelperCxx.pm b/lib/PublicInbox/XapHelperCxx.pm
index dbb0a915..83015379 100644
--- a/lib/PublicInbox/XapHelperCxx.pm
+++ b/lib/PublicInbox/XapHelperCxx.pm
@@ -8,6 +8,7 @@
package PublicInbox::XapHelperCxx;
use v5.12;
use PublicInbox::Spawn qw(popen_rd which);
+use PublicInbox::Git qw(read_all);
use PublicInbox::Search;
use Fcntl qw(SEEK_SET);
use Config;
@@ -34,7 +35,7 @@ sub xap_cfg (@) {
chomp(my $ret = do { local $/; <$rd> });
return $ret if close($rd);
seek($err, 0, SEEK_SET) or die "seek: $!";
- $err = do { local $/; <$err> };
+ $err = read_all($err);
die <<EOM;
@$cmd failed: Xapian development files missing? (\$?=$?)
$err
@@ -70,8 +71,7 @@ sub build () {
for (@srcs) {
say $fh qq(# line 1 "$_");
open my $rfh, '<', $_;
- local $/;
- print $fh readline($rfh);
+ print $fh read_all($rfh);
}
print $fh PublicInbox::Search::generate_cxx();
print $fh PublicInbox::CodeSearch::generate_cxx();
diff --git a/script/public-inbox-edit b/script/public-inbox-edit
index 1fb6f32b..77028817 100755
--- a/script/public-inbox-edit
+++ b/script/public-inbox-edit
@@ -15,6 +15,7 @@ PublicInbox::Admin::check_require('-index');
use PublicInbox::Eml;
use PublicInbox::InboxWritable qw(eml_from_path);
use PublicInbox::Import;
+use PublicInbox::Git qw(read_all);
my $help = <<'EOF';
usage: public-inbox-edit -m MESSAGE-ID [--all] [INBOX_DIRS]
@@ -184,8 +185,7 @@ retry_edit:
# rename/relink $edit_fn
open my $new_fh, '<', $edit_fn or
die "can't read edited file ($edit_fn): $!\n";
- defined(my $new_raw = do { local $/; <$new_fh> }) or die
- "read $edit_fn: $!\n";
+ my $new_raw = read_all($new_fh);
if (!$opt->{raw}) {
PublicInbox::Eml::strip_from($new_raw);
diff --git a/script/public-inbox-init b/script/public-inbox-init
index b3a16cfb..33bee310 100755
--- a/script/public-inbox-init
+++ b/script/public-inbox-init
@@ -125,13 +125,14 @@ my $auto_unlink = PublicInbox::OnDestroy->new($$, sub { unlink $lockfile });
my $perm = 0644 & ~umask;
my %seen;
if (-e $pi_config) {
+ require PublicInbox::Git;
open(my $oh, '<', $pi_config) or die "unable to read $pi_config: $!\n";
my @st = stat($oh);
$perm = $st[2];
defined $perm or die "(f)stat failed on $pi_config: $!\n";
chmod($perm & 07777, $fh) or
die "(f)chmod failed on future $pi_config: $!\n";
- defined(my $old = do { local $/; <$oh> }) or die "read $pi_config: $!\n";
+ my $old = PublicInbox::Git::read_all($oh);
print $fh $old or die "failed to write: $!\n";
close $oh or die "failed to close $pi_config: $!\n";
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 08/30] xap_helper*: use autodie in more places
2023-10-17 23:37 [PATCH 00/30] autodie-ification and code simplifications Eric Wong
` (6 preceding siblings ...)
2023-10-17 23:37 ` [PATCH 07/30] use read_all in more places to improve safety Eric Wong
@ 2023-10-17 23:37 ` Eric Wong
2023-10-17 23:37 ` [PATCH 09/30] xap_helper: die more easily in both implementations Eric Wong
` (22 subsequent siblings)
30 siblings, 0 replies; 32+ messages in thread
From: Eric Wong @ 2023-10-17 23:37 UTC (permalink / raw)
To: meta
This ought to lower the cognitive overhead of reading our code.
---
lib/PublicInbox/XapHelperCxx.pm | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/lib/PublicInbox/XapHelperCxx.pm b/lib/PublicInbox/XapHelperCxx.pm
index 83015379..bd616b6f 100644
--- a/lib/PublicInbox/XapHelperCxx.pm
+++ b/lib/PublicInbox/XapHelperCxx.pm
@@ -29,12 +29,13 @@ my $xflags = ($ENV{CXXFLAGS} // '-Wall -ggdb3 -O0') . ' ' .
my $xap_modversion;
sub xap_cfg (@) {
- open my $err, '+>', undef or die "open(undef): $!";
+ use autodie qw(open seek);
+ open my $err, '+>', undef;
my $cmd = [ $ENV{PKG_CONFIG} // 'pkg-config', @_, 'xapian-core' ];
my $rd = popen_rd($cmd, undef, { 2 => $err });
chomp(my $ret = do { local $/; <$rd> });
return $ret if close($rd);
- seek($err, 0, SEEK_SET) or die "seek: $!";
+ seek($err, 0, SEEK_SET);
$err = read_all($err);
die <<EOM;
@$cmd failed: Xapian development files missing? (\$?=$?)
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 09/30] xap_helper: die more easily in both implementations
2023-10-17 23:37 [PATCH 00/30] autodie-ification and code simplifications Eric Wong
` (7 preceding siblings ...)
2023-10-17 23:37 ` [PATCH 08/30] xap_helper*: use autodie in more places Eric Wong
@ 2023-10-17 23:37 ` Eric Wong
2023-10-17 23:37 ` [PATCH 10/30] xap_helper: simplify SIGTERM exit checks Eric Wong
` (21 subsequent siblings)
30 siblings, 0 replies; 32+ messages in thread
From: Eric Wong @ 2023-10-17 23:37 UTC (permalink / raw)
To: meta
We don't need to tolerate bad requests since it's only handling
requests from the parent process. So simplify error management
and just die||exit if we get a bad request.
---
lib/PublicInbox/XapHelper.pm | 11 +++--------
lib/PublicInbox/xap_helper.h | 32 ++++++++++----------------------
2 files changed, 13 insertions(+), 30 deletions(-)
diff --git a/lib/PublicInbox/XapHelper.pm b/lib/PublicInbox/XapHelper.pm
index ca993ca8..fe8d20f4 100644
--- a/lib/PublicInbox/XapHelper.pm
+++ b/lib/PublicInbox/XapHelper.pm
@@ -13,6 +13,7 @@ use PublicInbox::IPC;
use PublicInbox::Git qw(read_all);
use Socket qw(SOL_SOCKET SO_TYPE SOCK_SEQPACKET AF_UNIX);
use PublicInbox::DS qw(awaitpid);
+use autodie qw(open);
use POSIX qw(:signal_h);
use Fcntl qw(LOCK_UN LOCK_EX);
my $X = \%PublicInbox::Search::X;
@@ -122,7 +123,7 @@ sub cmd_dump_roots {
$qry_str // return
warn('usage: dump_roots [OPTIONS] ROOT2ID_FILE QRY_STR');
$req->{A} or return warn('dump_roots requires -A PREFIX');
- open my $fh, '<', $root2id_file or die "open($root2id_file): $!";
+ open my $fh, '<', $root2id_file;
my $root2id; # record format: $OIDHEX "\0" uint32_t
my @x = split(/\0/, read_all($fh));
while (@x) {
@@ -184,13 +185,7 @@ sub recv_loop {
PublicInbox::DS::block_signals();
my $req = bless {}, __PACKAGE__;
my $i = 0;
- for my $fd (@fds) {
- open($req->{$i++}, '+<&=', $fd) and next;
- warn("open(+<&=$fd) (FD=$i): $!");
- undef $req;
- last;
- }
- $req or next;
+ open($req->{$i++}, '+<&=', $_) for @fds;
local $stderr = $req->{1} // \*STDERR;
if (chop($rbuf) ne "\0") {
warn "not NUL-terminated";
diff --git a/lib/PublicInbox/xap_helper.h b/lib/PublicInbox/xap_helper.h
index 3fa615a5..c68202c3 100644
--- a/lib/PublicInbox/xap_helper.h
+++ b/lib/PublicInbox/xap_helper.h
@@ -668,40 +668,28 @@ static bool recv_req(struct req *req, char *rbuf, size_t *len)
size_t len = cmsg.hdr.cmsg_len;
int *fdp = (int *)CMSG_DATA(&cmsg.hdr);
size_t i;
- bool fd_ok = true;
for (i = 0; CMSG_LEN((i + 1) * sizeof(int)) <= len; i++) {
int fd = *fdp++;
const char *mode = NULL;
- int fl = fd_ok ? fcntl(fd, F_GETFL) : 0;
- if (fl == 0) {
- continue; // hit previous error
- } else if (fl == -1) {
- warnx("invalid fd=%d", fd);
- fd_ok = false;
+ int fl = fcntl(fd, F_GETFL);
+ if (fl == -1) {
+ errx(EXIT_FAILURE, "invalid fd=%d", fd);
} else if (fl & O_WRONLY) {
mode = "w";
} else if (fl & O_RDWR) {
mode = "r+";
if (i == 0) req->has_input = true;
} else {
- warnx("invalid mode from F_GETFL: 0x%x", fl);
- fd_ok = false;
- }
- if (!fd_ok) {
- xclose(fd);
- } else {
- req->fp[i] = fdopen(fd, mode);
- if (!req->fp[i]) {
- warn("fdopen(fd=%d)", fd);
- fd_ok = false;
- }
+ errx(EXIT_FAILURE,
+ "invalid mode from F_GETFL: 0x%x", fl);
}
+ req->fp[i] = fdopen(fd, mode);
+ if (!req->fp[i])
+ err(EXIT_FAILURE, "fdopen(fd=%d)", fd);
}
- for (i = 0; !fd_ok && i < MY_ARRAY_SIZE(req->fp); i++)
- if (req->fp[i]) fclose(req->fp[i]);
- return fd_ok;
+ return true;
}
- warnx("no FD received in %zd-byte request", r);
+ errx(EXIT_FAILURE, "no FD received in %zd-byte request", r);
return false;
}
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 10/30] xap_helper: simplify SIGTERM exit checks
2023-10-17 23:37 [PATCH 00/30] autodie-ification and code simplifications Eric Wong
` (8 preceding siblings ...)
2023-10-17 23:37 ` [PATCH 09/30] xap_helper: die more easily in both implementations Eric Wong
@ 2023-10-17 23:37 ` Eric Wong
2023-10-17 23:37 ` [PATCH 11/30] xap_helper: autodie for getsockopt Eric Wong
` (20 subsequent siblings)
30 siblings, 0 replies; 32+ messages in thread
From: Eric Wong @ 2023-10-17 23:37 UTC (permalink / raw)
To: meta
We can just close the socket FD to ensure things fail ASAP
when a SIGTERM hits instead of wasting time making getppid(2)
syscalls.
---
lib/PublicInbox/XapHelper.pm | 7 +++----
lib/PublicInbox/xap_helper.h | 28 ++++++++++++++++------------
2 files changed, 19 insertions(+), 16 deletions(-)
diff --git a/lib/PublicInbox/XapHelper.pm b/lib/PublicInbox/XapHelper.pm
index fe8d20f4..c31fe9a2 100644
--- a/lib/PublicInbox/XapHelper.pm
+++ b/lib/PublicInbox/XapHelper.pm
@@ -17,7 +17,7 @@ use autodie qw(open);
use POSIX qw(:signal_h);
use Fcntl qw(LOCK_UN LOCK_EX);
my $X = \%PublicInbox::Search::X;
-our (%SRCH, %WORKERS, $parent_pid, $alive, $nworker, $workerset);
+our (%SRCH, %WORKERS, $alive, $nworker, $workerset);
our $stderr = \*STDERR;
# only short options for portability in C++ implementation
@@ -177,7 +177,8 @@ sub recv_loop {
local $SIG{__WARN__} = sub { print $stderr @_ };
my $rbuf;
my $in = \*STDIN;
- while (!defined($parent_pid) || getppid == $parent_pid) {
+ local $SIG{TERM} = sub { undef $in };
+ while (defined($in)) {
PublicInbox::DS::sig_setmask($workerset);
my @fds = $PublicInbox::IPC::recv_cmd->($in, $rbuf, 4096*33);
scalar(@fds) or exit(66); # EX_NOINPUT
@@ -218,7 +219,6 @@ sub start_worker ($) {
if ($pid == 0) {
undef %WORKERS;
PublicInbox::DS::Reset();
- $SIG{TERM} = sub { $parent_pid = -1 };
$SIG{TTIN} = $SIG{TTOU} = 'IGNORE';
$SIG{CHLD} = 'DEFAULT'; # Xapian may use this
recv_loop();
@@ -267,7 +267,6 @@ sub start (@) {
for (POSIX::SIGTERM, POSIX::SIGCHLD) {
$workerset->delset($_) or die "delset($_): $!";
}
- local $parent_pid = $$;
my $sig = {
TTIN => sub {
if ($alive) {
diff --git a/lib/PublicInbox/xap_helper.h b/lib/PublicInbox/xap_helper.h
index c68202c3..fafb392d 100644
--- a/lib/PublicInbox/xap_helper.h
+++ b/lib/PublicInbox/xap_helper.h
@@ -72,8 +72,8 @@
assert(ckvar______ == (expect) && "BUG" && __FILE__ && __LINE__); \
} while (0)
-static const int sock_fd = STDIN_FILENO; // SOCK_SEQPACKET as stdin :P
-static volatile pid_t parent_pid; // may be set in worker sighandler (sigw)
+// sock_fd is modified in signal handler, yes, it's SOCK_SEQPACKET
+static volatile int sock_fd = STDIN_FILENO;
static sigset_t fullset, workerset;
static bool alive = true;
#if STDERR_ASSIGNABLE
@@ -640,6 +640,7 @@ static bool recv_req(struct req *req, char *rbuf, size_t *len)
union my_cmsg cmsg = {};
struct msghdr msg = {};
struct iovec iov;
+ ssize_t r;
iov.iov_base = rbuf;
iov.iov_len = *len;
msg.msg_iov = &iov;
@@ -650,25 +651,29 @@ static bool recv_req(struct req *req, char *rbuf, size_t *len)
// allow SIGTERM to hit
CHECK(int, 0, sigprocmask(SIG_SETMASK, &workerset, NULL));
- ssize_t r = recvmsg(sock_fd, &msg, 0);
+again:
+ r = recvmsg(sock_fd, &msg, 0);
if (r == 0) {
exit(EX_NOINPUT); /* grandparent went away */
} else if (r < 0) {
- if (errno == EINTR)
- return false; // retry recv_loop
- err(EXIT_FAILURE, "recvmsg");
+ switch (errno) {
+ case EINTR: goto again;
+ case EBADF: if (sock_fd < 0) exit(0);
+ // fall-through
+ default: err(EXIT_FAILURE, "recvmsg");
+ }
}
// success! no signals for the rest of the request/response cycle
CHECK(int, 0, sigprocmask(SIG_SETMASK, &fullset, NULL));
*len = r;
- if (r > 0 && cmsg.hdr.cmsg_level == SOL_SOCKET &&
+ if (cmsg.hdr.cmsg_level == SOL_SOCKET &&
cmsg.hdr.cmsg_type == SCM_RIGHTS) {
- size_t len = cmsg.hdr.cmsg_len;
+ size_t clen = cmsg.hdr.cmsg_len;
int *fdp = (int *)CMSG_DATA(&cmsg.hdr);
size_t i;
- for (i = 0; CMSG_LEN((i + 1) * sizeof(int)) <= len; i++) {
+ for (i = 0; CMSG_LEN((i + 1) * sizeof(int)) <= clen; i++) {
int fd = *fdp++;
const char *mode = NULL;
int fl = fcntl(fd, F_GETFL);
@@ -923,7 +928,7 @@ static void stderr_restore(FILE *tmp_err)
static void sigw(int sig) // SIGTERM handler for worker
{
- parent_pid = -1; // break out of recv_loop
+ sock_fd = -1; // break out of recv_loop
}
static void recv_loop(void) // worker process loop
@@ -934,7 +939,7 @@ static void recv_loop(void) // worker process loop
CHECK(int, 0, sigaction(SIGTERM, &sa, NULL));
- while (!parent_pid || getppid() == parent_pid) {
+ while (sock_fd == 0) {
size_t len = sizeof(rbuf);
struct req req = {};
if (!recv_req(&req, rbuf, &len))
@@ -1197,7 +1202,6 @@ int main(int argc, char *argv[])
}
CHECK(int, 0, sigdelset(&workerset, SIGTERM));
CHECK(int, 0, sigdelset(&workerset, SIGCHLD));
- parent_pid = getpid();
nworker_hwm = nworker;
worker_pids = (pid_t *)calloc(nworker, sizeof(pid_t));
if (!worker_pids) err(EXIT_FAILURE, "calloc");
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 11/30] xap_helper: autodie for getsockopt
2023-10-17 23:37 [PATCH 00/30] autodie-ification and code simplifications Eric Wong
` (9 preceding siblings ...)
2023-10-17 23:37 ` [PATCH 10/30] xap_helper: simplify SIGTERM exit checks Eric Wong
@ 2023-10-17 23:37 ` Eric Wong
2023-10-17 23:37 ` [PATCH 12/30] xap_client: autodie for pipe and socketpair Eric Wong
` (19 subsequent siblings)
30 siblings, 0 replies; 32+ messages in thread
From: Eric Wong @ 2023-10-17 23:37 UTC (permalink / raw)
To: meta
Only caveat is we can't use bareword filehandles, but that's
a minor inconvenience.
---
lib/PublicInbox/XapHelper.pm | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/lib/PublicInbox/XapHelper.pm b/lib/PublicInbox/XapHelper.pm
index c31fe9a2..eea10a44 100644
--- a/lib/PublicInbox/XapHelper.pm
+++ b/lib/PublicInbox/XapHelper.pm
@@ -13,11 +13,11 @@ use PublicInbox::IPC;
use PublicInbox::Git qw(read_all);
use Socket qw(SOL_SOCKET SO_TYPE SOCK_SEQPACKET AF_UNIX);
use PublicInbox::DS qw(awaitpid);
-use autodie qw(open);
+use autodie qw(open getsockopt);
use POSIX qw(:signal_h);
use Fcntl qw(LOCK_UN LOCK_EX);
my $X = \%PublicInbox::Search::X;
-our (%SRCH, %WORKERS, $alive, $nworker, $workerset);
+our (%SRCH, %WORKERS, $alive, $nworker, $workerset, $in);
our $stderr = \*STDERR;
# only short options for portability in C++ implementation
@@ -176,7 +176,6 @@ sub dispatch {
sub recv_loop {
local $SIG{__WARN__} = sub { print $stderr @_ };
my $rbuf;
- my $in = \*STDIN;
local $SIG{TERM} = sub { undef $in };
while (defined($in)) {
PublicInbox::DS::sig_setmask($workerset);
@@ -247,7 +246,7 @@ sub xh_alive { $alive || scalar(keys %WORKERS) }
sub start (@) {
my (@argv) = @_;
- my $c = getsockopt(STDIN, SOL_SOCKET, SO_TYPE) or die "getsockopt: $!";
+ my $c = getsockopt($in = \*STDIN, SOL_SOCKET, SO_TYPE);
unpack('i', $c) == SOCK_SEQPACKET or die 'stdin is not SOCK_SEQPACKET';
local (%SRCH, %WORKERS);
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 12/30] xap_client: autodie for pipe and socketpair
2023-10-17 23:37 [PATCH 00/30] autodie-ification and code simplifications Eric Wong
` (10 preceding siblings ...)
2023-10-17 23:37 ` [PATCH 11/30] xap_helper: autodie for getsockopt Eric Wong
@ 2023-10-17 23:37 ` Eric Wong
2023-10-17 23:37 ` [PATCH 13/30] xt/git-http-backend: remove Net::HTTP usage Eric Wong
` (18 subsequent siblings)
30 siblings, 0 replies; 32+ messages in thread
From: Eric Wong @ 2023-10-17 23:37 UTC (permalink / raw)
To: meta
This saves us a few lines of code.
---
lib/PublicInbox/XapClient.pm | 11 ++++-------
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/lib/PublicInbox/XapClient.pm b/lib/PublicInbox/XapClient.pm
index 9e2d71a0..21c89265 100644
--- a/lib/PublicInbox/XapClient.pm
+++ b/lib/PublicInbox/XapClient.pm
@@ -11,14 +11,12 @@ use v5.12;
use PublicInbox::Spawn qw(spawn);
use Socket qw(AF_UNIX SOCK_SEQPACKET);
use PublicInbox::IPC;
+use autodie qw(pipe socketpair);
sub mkreq {
my ($self, $ios, @arg) = @_;
- my ($r, $w, $n);
- if (!defined($ios->[0])) {
- pipe($r, $w) or die "pipe: $!";
- $ios->[0] = $w;
- }
+ my ($r, $n);
+ pipe($r, $ios->[0]) if !defined($ios->[0]);
my @fds = map fileno($_), @$ios;
my $buf = join("\0", @arg, '');
$n = $PublicInbox::IPC::send_cmd->($self->{io}, \@fds, $buf, 0) //
@@ -29,8 +27,7 @@ sub mkreq {
sub start_helper {
my @argv = @_;
- socketpair(my $sock, my $in, AF_UNIX, SOCK_SEQPACKET, 0) or
- die "socketpair: $!";
+ socketpair(my $sock, my $in, AF_UNIX, SOCK_SEQPACKET, 0);
my $cls = ($ENV{PI_NO_CXX} ? undef : eval {
require PublicInbox::XapHelperCxx;
PublicInbox::XapHelperCxx::check_build();
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 13/30] xt/git-http-backend: remove Net::HTTP usage
2023-10-17 23:37 [PATCH 00/30] autodie-ification and code simplifications Eric Wong
` (11 preceding siblings ...)
2023-10-17 23:37 ` [PATCH 12/30] xap_client: autodie for pipe and socketpair Eric Wong
@ 2023-10-17 23:37 ` Eric Wong
2023-10-17 23:37 ` [PATCH 14/30] ds: introduce and use do_fork helper Eric Wong
` (17 subsequent siblings)
30 siblings, 0 replies; 32+ messages in thread
From: Eric Wong @ 2023-10-17 23:37 UTC (permalink / raw)
To: meta
HTTP::Tiny is part of the Perl standard library since Perl 5.14
while Net::HTTP has never been (unlike Net::NNTP or Net::POP3).
For the test which forces server-side buffering, we'll just use
regular socket handle.
---
xt/git-http-backend.t | 30 +++++++++++++-----------------
1 file changed, 13 insertions(+), 17 deletions(-)
diff --git a/xt/git-http-backend.t b/xt/git-http-backend.t
index d78fe79f..6c384faf 100644
--- a/xt/git-http-backend.t
+++ b/xt/git-http-backend.t
@@ -12,7 +12,7 @@ use PublicInbox::TestCommon;
my $git_dir = $ENV{GIANT_GIT_DIR};
plan 'skip_all' => 'GIANT_GIT_DIR not defined' unless $git_dir;
require_mods(qw(BSD::Resource Plack::Util Plack::Builder
- HTTP::Date HTTP::Status Net::HTTP));
+ HTTP::Date HTTP::Status HTTP::Tiny));
my $psgi = "./t/git-http-backend.psgi";
my ($tmpdir, $for_destroy) = tmpdir();
my $err = "$tmpdir/stderr.log";
@@ -20,15 +20,12 @@ my $out = "$tmpdir/stdout.log";
my $sock = tcp_server();
my ($host, $port) = tcp_host_port($sock);
my $td;
+my $http = HTTP::Tiny->new;
my $get_maxrss = sub {
- my $http = Net::HTTP->new(Host => "$host:$port");
- ok($http, 'Net::HTTP object created for maxrss');
- $http->write_request(GET => '/');
- my ($code, $mess, %h) = $http->read_response_headers;
- is($code, 200, 'success reading maxrss');
- my $n = $http->read_entity_body(my $buf, 256);
- ok(defined $n, 'read response body');
+ my $res = $http->get("http://$host:$port/");
+ is($res->{status}, 200, 'success reading maxrss');
+ my $buf = $res->{content};
like($buf, qr/\A\d+\n\z/, 'got memory response');
ok(int($buf) > 0, 'got non-zero memory response');
int($buf);
@@ -55,16 +52,15 @@ SKIP: {
if ($pack !~ m!(/objects/pack/pack-[a-f0-9]{40,64}.pack)\z!) {
skip "bad pack name: $pack";
}
- my $url = $1;
- my $http = Net::HTTP->new(Host => "$host:$port");
- ok($http, 'Net::HTTP object created');
- $http->write_request(GET => $url);
- my ($code, $mess, %h) = $http->read_response_headers;
- is(200, $code, 'got 200 success for pack');
- is($max, $h{'Content-Length'}, 'got expected Content-Length for pack');
+ my $s = tcp_connect($sock);
+ print $s "GET $1 HTTP/1.1\r\nHost: $host:$port\r\n\r\n" or xbail $!;
+ my $hdr = do { local $/ = "\r\n\r\n"; readline($s) };
+ like $hdr, qr!\AHTTP/1\.1\s+200\b!, 'got 200 success for pack';
+ like $hdr, qr/^content-length:\s*$max\r\n/ims,
+ 'got expected Content-Length for pack';
- # no $http->read_entity_body, here, since we want to force buffering
- foreach my $i (1..3) {
+ # don't read the body
+ for my $i (1..3) {
sleep 1;
my $diff = $get_maxrss->() - $mem_a;
note "${diff}K memory increase after $i seconds";
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 14/30] ds: introduce and use do_fork helper
2023-10-17 23:37 [PATCH 00/30] autodie-ification and code simplifications Eric Wong
` (12 preceding siblings ...)
2023-10-17 23:37 ` [PATCH 13/30] xt/git-http-backend: remove Net::HTTP usage Eric Wong
@ 2023-10-17 23:37 ` Eric Wong
2023-10-17 23:38 ` [PATCH 15/30] ds: get rid of SetLoopTimeout Eric Wong
` (16 subsequent siblings)
30 siblings, 0 replies; 32+ messages in thread
From: Eric Wong @ 2023-10-17 23:37 UTC (permalink / raw)
To: meta
This ensures we handle RNG reseeding and resetting the event
loop properly in child processes after forking.
---
lib/PublicInbox/DS.pm | 12 ++++++++++++
lib/PublicInbox/Daemon.pm | 16 +++++-----------
lib/PublicInbox/IPC.pm | 12 ++----------
lib/PublicInbox/TestCommon.pm | 3 +--
lib/PublicInbox/Watch.pm | 11 ++---------
lib/PublicInbox/Xapcmd.pm | 5 ++---
6 files changed, 24 insertions(+), 35 deletions(-)
diff --git a/lib/PublicInbox/DS.pm b/lib/PublicInbox/DS.pm
index eefbdcc3..9960937d 100644
--- a/lib/PublicInbox/DS.pm
+++ b/lib/PublicInbox/DS.pm
@@ -34,6 +34,7 @@ use PublicInbox::Tmpfile;
use PublicInbox::Select;
use Errno qw(EAGAIN EINVAL ECHILD);
use Carp qw(carp croak);
+use autodie qw(fork);
our @EXPORT_OK = qw(now msg_more awaitpid add_timer add_uniq_timer);
my $nextq; # queue for next_tick
@@ -737,6 +738,17 @@ sub awaitpid {
}
}
+sub do_fork () {
+ my $seed = rand(0xffffffff);
+ my $pid = fork;
+ if ($pid == 0) {
+ srand($seed);
+ eval { Net::SSLeay::randomize() };
+ Reset();
+ }
+ $pid;
+}
+
package PublicInbox::DummyPoller; # only used during Reset
use v5.12;
diff --git a/lib/PublicInbox/Daemon.pm b/lib/PublicInbox/Daemon.pm
index 520cef72..f33f6f17 100644
--- a/lib/PublicInbox/Daemon.pm
+++ b/lib/PublicInbox/Daemon.pm
@@ -541,17 +541,11 @@ sub reap_worker { # awaitpid CB
sub start_worker ($) {
my ($nr) = @_;
- my $seed = rand(0xffffffff);
return unless @listeners;
- my $pid = fork;
- if (!defined($pid)) {
- warn "fork: $!";
- } elsif ($pid == 0) {
+ my $pid = PublicInbox::DS::do_fork;
+ if ($pid == 0) {
undef %WORKERS;
- PublicInbox::DS::Reset();
local $PublicInbox::DS::Poller; # allow epoll/kqueue
- srand($seed);
- eval { Net::SSLeay::randomize() };
$set_user->() if $set_user;
PublicInbox::EOFpipe->new($parent_pipe, \&worker_quit);
worker_loop();
@@ -563,9 +557,9 @@ sub start_worker ($) {
}
sub start_workers {
- for my $nr (grep { !defined($WORKERS{$_}) } (0..($nworker - 1))) {
- start_worker($nr);
- }
+ my @idx = grep { !defined($WORKERS{$_}) } (0..($nworker - 1)) or return;
+ eval { start_worker($_) for @idx };
+ warn "E: $@\n" if $@;
}
sub trim_workers {
diff --git a/lib/PublicInbox/IPC.pm b/lib/PublicInbox/IPC.pm
index 5964645e..3292d960 100644
--- a/lib/PublicInbox/IPC.pm
+++ b/lib/PublicInbox/IPC.pm
@@ -102,12 +102,8 @@ sub ipc_worker_spawn {
pipe(my $r_res, my $w_res);
my $sigset = $oldset // PublicInbox::DS::block_signals();
$self->ipc_atfork_prepare;
- my $seed = rand(0xffffffff);
- my $pid = fork;
+ my $pid = PublicInbox::DS::do_fork;
if ($pid == 0) {
- srand($seed);
- eval { Net::SSLeay::randomize() };
- eval { PublicInbox::DS->Reset };
delete @$self{qw(-wq_s1 -wq_s2 -wq_workers -wq_ppid)};
$w_req = $r_res = undef;
$w_res->autoflush(1);
@@ -341,13 +337,9 @@ sub _wq_worker_start {
my ($self, $oldset, $fields, $one, @cb_args) = @_;
my ($bcast1, $bcast2);
$one or socketpair($bcast1, $bcast2, AF_UNIX, SOCK_SEQPACKET, 0);
- my $seed = rand(0xffffffff);
- my $pid = fork;
+ my $pid = PublicInbox::DS::do_fork;
if ($pid == 0) {
- srand($seed);
- eval { Net::SSLeay::randomize() };
undef $bcast1;
- eval { PublicInbox::DS->Reset };
delete @$self{qw(-wq_s1 -wq_ppid)};
$self->{-wq_worker_nr} =
keys %{delete($self->{-wq_workers}) // {}};
diff --git a/lib/PublicInbox/TestCommon.pm b/lib/PublicInbox/TestCommon.pm
index 323152b4..77da822b 100644
--- a/lib/PublicInbox/TestCommon.pm
+++ b/lib/PublicInbox/TestCommon.pm
@@ -549,9 +549,8 @@ sub start_script {
require PublicInbox::OnDestroy;
my $tmp_mask = PublicInbox::OnDestroy->new(
\&PublicInbox::DS::sig_setmask, $oset);
- my $pid = fork // die "fork: $!";
+ my $pid = PublicInbox::DS::do_fork();
if ($pid == 0) {
- eval { PublicInbox::DS->Reset };
for (@{delete($opt->{-CLOFORK}) // []}) {
close($_) or die "close $!";
}
diff --git a/lib/PublicInbox/Watch.pm b/lib/PublicInbox/Watch.pm
index 3426d4a7..41b77dc1 100644
--- a/lib/PublicInbox/Watch.pm
+++ b/lib/PublicInbox/Watch.pm
@@ -385,7 +385,6 @@ sub watch_atfork_child ($) {
my ($self) = @_;
delete $self->{pids};
delete $self->{opendirs};
- PublicInbox::DS->Reset;
my $sig = delete $self->{sig};
$sig->{CHLD} = $sig->{HUP} = $sig->{USR1} = 'DEFAULT';
# TERM/QUIT/INT call ->quit, which works in both parent+child
@@ -413,11 +412,8 @@ sub imap_idle_reap { # awaitpid callback
sub imap_idle_fork {
my ($self, $uri, $intvl) = @_;
return if $self->{quit};
- my $seed = rand(0xffffffff);
- my $pid = fork // die "fork: $!";
+ my $pid = PublicInbox::DS::do_fork;
if ($pid == 0) {
- srand($seed);
- eval { Net::SSLeay::randomize() };
watch_atfork_child($self);
watch_imap_idle_1($self, $uri, $intvl);
_exit(0);
@@ -477,11 +473,8 @@ sub poll_fetch_fork { # DS::add_timer callback
my @imap = grep { # push() always returns > 0
$_->scheme =~ m!\Aimaps?!i ? 1 : (push(@nntp, $_) < 0)
} @$uris;
- my $seed = rand(0xffffffff);
- my $pid = fork // die "fork: $!";
+ my $pid = PublicInbox::DS::do_fork;
if ($pid == 0) {
- srand($seed);
- eval { Net::SSLeay::randomize() };
watch_atfork_child($self);
watch_imap_fetch_all($self, \@imap) if @imap;
watch_nntp_fetch_all($self, \@nntp) if @nntp;
diff --git a/lib/PublicInbox/Xapcmd.pm b/lib/PublicInbox/Xapcmd.pm
index 4e055acf..c2b66e69 100644
--- a/lib/PublicInbox/Xapcmd.pm
+++ b/lib/PublicInbox/Xapcmd.pm
@@ -11,6 +11,7 @@ use PublicInbox::SearchIdx;
use File::Temp 0.19 (); # ->newdir
use File::Path qw(remove_tree);
use POSIX qw(WNOHANG _exit);
+use PublicInbox::DS;
# support testing with dev versions of Xapian which installs
# commands with a version number suffix (e.g. "xapian-compact-1.5")
@@ -102,10 +103,8 @@ sub commit_changes ($$$$) {
sub cb_spawn {
my ($cb, $args, $opt) = @_; # $cb = cpdb() or compact()
- my $seed = rand(0xffffffff);
- my $pid = fork // die "fork: $!";
+ my $pid = PublicInbox::DS::do_fork;
return $pid if $pid > 0;
- srand($seed);
$SIG{__DIE__} = sub { warn @_; _exit(1) }; # don't jump up stack
$cb->($args, $opt);
_exit(0);
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 15/30] ds: get rid of SetLoopTimeout
2023-10-17 23:37 [PATCH 00/30] autodie-ification and code simplifications Eric Wong
` (13 preceding siblings ...)
2023-10-17 23:37 ` [PATCH 14/30] ds: introduce and use do_fork helper Eric Wong
@ 2023-10-17 23:38 ` Eric Wong
2023-10-17 23:38 ` [PATCH 16/30] cindex: drop some unused functions Eric Wong
` (15 subsequent siblings)
30 siblings, 0 replies; 32+ messages in thread
From: Eric Wong @ 2023-10-17 23:38 UTC (permalink / raw)
To: meta
It's not worth the code and memory to have a setter method we
never use outside of tests.
---
lib/PublicInbox/DS.pm | 24 +++++++-----------------
t/dir_idle.t | 2 +-
t/ds-leak.t | 4 ++--
xt/mem-imapd-tls.t | 8 ++++----
xt/mem-nntpd-tls.t | 8 ++++----
5 files changed, 18 insertions(+), 28 deletions(-)
diff --git a/lib/PublicInbox/DS.pm b/lib/PublicInbox/DS.pm
index 9960937d..6041c6b5 100644
--- a/lib/PublicInbox/DS.pm
+++ b/lib/PublicInbox/DS.pm
@@ -47,7 +47,7 @@ our (%AWAIT_PIDS, # pid => [ $callback, @args ]
@post_loop_do, # subref + args to call at the end of each loop
- $LoopTimeout, # timeout of event loop in milliseconds
+ $loop_timeout, # timeout of event loop in milliseconds
@Timers, # timers
%UniqTimer,
$in_loop,
@@ -89,20 +89,10 @@ sub Reset {
scalar(@{$cur_runq // []})); # do not vivify cur_runq
$reap_armed = undef;
- $LoopTimeout = -1; # no timeout by default
+ $loop_timeout = -1; # no timeout by default
$Poller = PublicInbox::Select->new;
}
-=head2 C<< CLASS->SetLoopTimeout( $timeout ) >>
-
-Set the loop timeout for the event loop to some value in milliseconds.
-
-A timeout of 0 (zero) means poll forever. A timeout of -1 means poll and return
-immediately.
-
-=cut
-sub SetLoopTimeout { $LoopTimeout = $_[1] + 0 }
-
sub _add_named_timer {
my ($name, $secs, $coderef, @args) = @_;
my $fire_time = now() + $secs;
@@ -163,7 +153,7 @@ sub next_tick () {
sub RunTimers {
next_tick();
- return (($nextq || $ToClose) ? 0 : $LoopTimeout) unless @Timers;
+ return (($nextq || $ToClose) ? 0 : $loop_timeout) unless @Timers;
my $now = now();
@@ -177,16 +167,16 @@ sub RunTimers {
# timers may enqueue into nextq:
return 0 if ($nextq || $ToClose);
- return $LoopTimeout unless @Timers;
+ return $loop_timeout unless @Timers;
# convert time to an even number of milliseconds, adding 1
# extra, otherwise floating point fun can occur and we'll
# call RunTimers like 20-30 times, each returning a timeout
# of 0.0000212 seconds
- my $timeout = int(($Timers[0][0] - $now) * 1000) + 1;
+ my $t = int(($Timers[0][0] - $now) * 1000) + 1;
# -1 is an infinite timeout, so prefer a real timeout
- ($LoopTimeout < 0 || $LoopTimeout >= $timeout) ? $timeout : $LoopTimeout
+ ($loop_timeout < 0 || $loop_timeout >= $t) ? $t : $loop_timeout
}
sub sig_setmask { sigprocmask(SIG_SETMASK, @_) or die "sigprocmask: $!" }
@@ -305,7 +295,7 @@ sub event_loop (;$$) {
sig_setmask($oldset) if $oldset;
sigprocmask(SIG_UNBLOCK, unblockset($sig)) or
die "SIG_UNBLOCK: $!";
- PublicInbox::DS->SetLoopTimeout(1000);
+ $loop_timeout = 1000;
}
$_[0] = $sigfd = $sig = undef; # $_[0] == sig
local $in_loop = 1;
diff --git a/t/dir_idle.t b/t/dir_idle.t
index 02759b54..35c800f9 100644
--- a/t/dir_idle.t
+++ b/t/dir_idle.t
@@ -11,7 +11,7 @@ my @x;
my $cb = sub { push @x, \@_ };
my $di = PublicInbox::DirIdle->new($cb);
$di->add_watches(["$tmpdir/a", "$tmpdir/c"], 1);
-PublicInbox::DS->SetLoopTimeout(1000);
+$PublicInbox::DS::loop_timeout = 1000;
my $end = 3 + now;
local @PublicInbox::DS::post_loop_do = (sub { scalar(@x) == 0 && now < $end });
rmdir("$tmpdir/a/b") or xbail "rmdir $!";
diff --git a/t/ds-leak.t b/t/ds-leak.t
index eaca05b8..179997eb 100644
--- a/t/ds-leak.t
+++ b/t/ds-leak.t
@@ -11,7 +11,7 @@ if ('close-on-exec for epoll and kqueue') {
my $pid;
my $evfd_re = qr/(?:kqueue|eventpoll)/i;
- PublicInbox::DS->SetLoopTimeout(0);
+ $PublicInbox::DS::loop_timeout = 0;
local @PublicInbox::DS::post_loop_do = (sub { 0 });
# make sure execve closes if we're using fork()
@@ -54,7 +54,7 @@ SKIP: {
}
my $cb = sub {};
for my $i (0..$n) {
- PublicInbox::DS->SetLoopTimeout(0);
+ $PublicInbox::DS::loop_timeout = 0;
local @PublicInbox::DS::post_loop_do = ($cb);
PublicInbox::DS::event_loop();
PublicInbox::DS->Reset;
diff --git a/xt/mem-imapd-tls.t b/xt/mem-imapd-tls.t
index 00199a9b..58581017 100644
--- a/xt/mem-imapd-tls.t
+++ b/xt/mem-imapd-tls.t
@@ -81,7 +81,7 @@ sub once { 0 }; # stops event loop
# setup the event loop so that it exits at every step
# while we're still doing connect(2)
-PublicInbox::DS->SetLoopTimeout(0);
+$PublicInbox::DS::loop_timeout = 0;
local @PublicInbox::DS::post_loop_do = (\&once);
my $pid = $td->{pid};
if ($^O eq 'linux' && open(my $f, '<', "/proc/$pid/status")) {
@@ -100,21 +100,21 @@ foreach my $n (1..$nfd) {
# try not to overflow the listen() backlog:
if (!($n % 128) && $DONE != $n) {
diag("nr: ($n) $DONE/$nfd");
- PublicInbox::DS->SetLoopTimeout(-1);
+ $PublicInbox::DS::loop_timeout = -1;
local @PublicInbox::DS::post_loop_do = (sub { $DONE != $n });
# clear the backlog:
PublicInbox::DS::event_loop();
# resume looping
- PublicInbox::DS->SetLoopTimeout(0);
+ $PublicInbox::DS::loop_timeout = 0;
}
}
# run the event loop normally, now:
diag "done?: @".time." $DONE/$nfd";
if ($DONE != $nfd) {
- PublicInbox::DS->SetLoopTimeout(-1);
+ $PublicInbox::DS::loop_timeout = -1;
local @PublicInbox::DS::post_loop_do = (sub { $DONE != $nfd });
PublicInbox::DS::event_loop();
}
diff --git a/xt/mem-nntpd-tls.t b/xt/mem-nntpd-tls.t
index f9b98a6b..ec639a8b 100644
--- a/xt/mem-nntpd-tls.t
+++ b/xt/mem-nntpd-tls.t
@@ -104,7 +104,7 @@ sub once { 0 }; # stops event loop
# setup the event loop so that it exits at every step
# while we're still doing connect(2)
-PublicInbox::DS->SetLoopTimeout(0);
+$PublicInbox::DS::loop_timeout = 0;
local @PublicInbox::DS::post_loop_do = (\&once);
foreach my $n (1..$nfd) {
@@ -119,14 +119,14 @@ foreach my $n (1..$nfd) {
# try not to overflow the listen() backlog:
if (!($n % 128) && $n != $DONE) {
diag("nr: ($n) $DONE/$nfd");
- PublicInbox::DS->SetLoopTimeout(-1);
+ $PublicInbox::DS::loop_timeout = -1;
@PublicInbox::DS::post_loop_do = (sub { $DONE != $n });
# clear the backlog:
PublicInbox::DS::event_loop();
# resume looping
- PublicInbox::DS->SetLoopTimeout(0);
+ $PublicInbox::DS::loop_timeout = 0;
@PublicInbox::DS::post_loop_do = (\&once);
}
}
@@ -140,7 +140,7 @@ $dump_rss->();
# run the event loop normally, now:
if ($DONE != $nfd) {
- PublicInbox::DS->SetLoopTimeout(-1);
+ $PublicInbox::DS::loop_timeout = -1;
@PublicInbox::DS::post_loop_do = (sub {
diag "done: ".time." $DONE";
$DONE != $nfd;
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 16/30] cindex: drop some unused functions
2023-10-17 23:37 [PATCH 00/30] autodie-ification and code simplifications Eric Wong
` (14 preceding siblings ...)
2023-10-17 23:38 ` [PATCH 15/30] ds: get rid of SetLoopTimeout Eric Wong
@ 2023-10-17 23:38 ` Eric Wong
2023-10-17 23:38 ` [PATCH 17/30] syscall: common $F_SETPIPE_SZ definition Eric Wong
` (14 subsequent siblings)
30 siblings, 0 replies; 32+ messages in thread
From: Eric Wong @ 2023-10-17 23:38 UTC (permalink / raw)
To: meta
They're no longer needed with the way PublicInbox::CidxLogP is
currently implemented.
---
lib/PublicInbox/CodeSearchIdx.pm | 5 -----
1 file changed, 5 deletions(-)
diff --git a/lib/PublicInbox/CodeSearchIdx.pm b/lib/PublicInbox/CodeSearchIdx.pm
index feb37be8..36d00aea 100644
--- a/lib/PublicInbox/CodeSearchIdx.pm
+++ b/lib/PublicInbox/CodeSearchIdx.pm
@@ -1222,9 +1222,4 @@ sub shard_done_wait { # awaitpid cb via ipc_worker_reap
PublicInbox::DS::enqueue_reap() if !shards_active(); # once more for PLC
}
-sub do_quit { $DO_QUIT }
-
-sub tmpdir { $TMPDIR }
-
-
1;
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 17/30] syscall: common $F_SETPIPE_SZ definition
2023-10-17 23:37 [PATCH 00/30] autodie-ification and code simplifications Eric Wong
` (15 preceding siblings ...)
2023-10-17 23:38 ` [PATCH 16/30] cindex: drop some unused functions Eric Wong
@ 2023-10-17 23:38 ` Eric Wong
2023-10-17 23:38 ` [PATCH 18/30] t/lei-up: additional diagnostics for match failures Eric Wong
` (13 subsequent siblings)
30 siblings, 0 replies; 32+ messages in thread
From: Eric Wong @ 2023-10-17 23:38 UTC (permalink / raw)
To: meta
We use this in various places to minimize or maximize pipe
size on Linux. So keep it all in one place.
---
lib/PublicInbox/CidxLogP.pm | 4 ++--
lib/PublicInbox/EOFpipe.pm | 6 +++---
lib/PublicInbox/LeiXSearch.pm | 2 +-
lib/PublicInbox/SearchIdxShard.pm | 14 +++++++-------
lib/PublicInbox/Syscall.pm | 16 ++++++++--------
t/gcf2.t | 5 +++--
t/lei-sigpipe.t | 7 +++----
7 files changed, 27 insertions(+), 27 deletions(-)
diff --git a/lib/PublicInbox/CidxLogP.pm b/lib/PublicInbox/CidxLogP.pm
index 7877d5ac..34f7201d 100644
--- a/lib/PublicInbox/CidxLogP.pm
+++ b/lib/PublicInbox/CidxLogP.pm
@@ -10,12 +10,12 @@
package PublicInbox::CidxLogP;
use v5.12;
use parent qw(PublicInbox::DS);
-use PublicInbox::Syscall qw(EPOLLIN EPOLLONESHOT);
+use PublicInbox::Syscall qw(EPOLLIN EPOLLONESHOT $F_SETPIPE_SZ);
sub new {
my ($cls, $rd, $cidx, $git, $roots) = @_;
my $self = bless { cidx => $cidx, git => $git, roots => $roots }, $cls;
- fcntl($rd, 1031, 1048576) if $^O eq 'linux'; # fatter pipes
+ fcntl($rd, $F_SETPIPE_SZ, 1048576) if $F_SETPIPE_SZ;
$self->SUPER::new($rd, EPOLLIN|EPOLLONESHOT);
}
diff --git a/lib/PublicInbox/EOFpipe.pm b/lib/PublicInbox/EOFpipe.pm
index 628e9366..3474874f 100644
--- a/lib/PublicInbox/EOFpipe.pm
+++ b/lib/PublicInbox/EOFpipe.pm
@@ -4,13 +4,13 @@
package PublicInbox::EOFpipe;
use v5.12;
use parent qw(PublicInbox::DS);
-use PublicInbox::Syscall qw(EPOLLIN EPOLLONESHOT);
+use PublicInbox::Syscall qw(EPOLLIN EPOLLONESHOT $F_SETPIPE_SZ);
sub new {
my (undef, $rd, $cb) = @_;
my $self = bless { cb => $cb }, __PACKAGE__;
- # 1031: F_SETPIPE_SZ, 4096: page size
- fcntl($rd, 1031, 4096) if $^O eq 'linux';
+ # 4096: page size
+ fcntl($rd, $F_SETPIPE_SZ, 4096) if $F_SETPIPE_SZ;
$self->SUPER::new($rd, EPOLLIN|EPOLLONESHOT);
}
diff --git a/lib/PublicInbox/LeiXSearch.pm b/lib/PublicInbox/LeiXSearch.pm
index d83a403c..25b66b3b 100644
--- a/lib/PublicInbox/LeiXSearch.pm
+++ b/lib/PublicInbox/LeiXSearch.pm
@@ -21,6 +21,7 @@ use Fcntl qw(SEEK_SET F_SETFL O_APPEND O_RDWR);
use PublicInbox::ContentHash qw(git_sha);
use POSIX qw(strftime);
use autodie qw(open read seek truncate);
+use PublicInbox::Syscall qw($F_SETPIPE_SZ);
sub new {
my ($class) = @_;
@@ -536,7 +537,6 @@ sub do_query {
if ($lei->{opt}->{augment} && delete $lei->{early_mua}) {
$lei->start_mua;
}
- my $F_SETPIPE_SZ = $^O eq 'linux' ? 1031 : undef;
if ($l2m->{-wq_nr_workers} > 1 &&
$l2m->{base_type} =~ /\A(?:maildir|mbox)\z/) {
# setup two barriers to coordinate ->has_entries
diff --git a/lib/PublicInbox/SearchIdxShard.pm b/lib/PublicInbox/SearchIdxShard.pm
index 21bd56c2..1630eb4a 100644
--- a/lib/PublicInbox/SearchIdxShard.pm
+++ b/lib/PublicInbox/SearchIdxShard.pm
@@ -7,6 +7,7 @@ package PublicInbox::SearchIdxShard;
use v5.12;
use parent qw(PublicInbox::SearchIdx PublicInbox::IPC);
use PublicInbox::OnDestroy;
+use PublicInbox::Syscall qw($F_SETPIPE_SZ);
sub new {
my ($class, $v2w, $shard) = @_; # v2w may be ExtSearchIdx
@@ -20,13 +21,12 @@ sub new {
if ($v2w->{parallel}) {
local $self->{-v2w_afc} = $v2w;
$self->ipc_worker_spawn("shard[$shard]");
- # F_SETPIPE_SZ = 1031 on Linux; increasing the pipe size for
- # inputs speeds V2Writable batch imports across 8 cores by
- # nearly 20%. Since any of our responses are small, make
- # the response pipe as small as possible
- if ($^O eq 'linux') {
- fcntl($self->{-ipc_req}, 1031, 1048576);
- fcntl($self->{-ipc_res}, 1031, 4096);
+ # Increasing the pipe size for requests speeds V2 batch imports
+ # across 8 cores by nearly 20%. Since many of our responses
+ # are small, make the response pipe as small as possible
+ if ($F_SETPIPE_SZ) {
+ fcntl($self->{-ipc_req}, $F_SETPIPE_SZ, 1048576);
+ fcntl($self->{-ipc_res}, $F_SETPIPE_SZ, 4096);
}
}
$self;
diff --git a/lib/PublicInbox/Syscall.pm b/lib/PublicInbox/Syscall.pm
index e83beb6a..78181bb6 100644
--- a/lib/PublicInbox/Syscall.pm
+++ b/lib/PublicInbox/Syscall.pm
@@ -28,7 +28,7 @@ our @EXPORT_OK = qw(epoll_ctl epoll_create epoll_wait
EPOLLIN EPOLLOUT EPOLLET
EPOLL_CTL_ADD EPOLL_CTL_DEL EPOLL_CTL_MOD
EPOLLONESHOT EPOLLEXCLUSIVE
- signalfd rename_noreplace %SIGNUM);
+ signalfd rename_noreplace %SIGNUM $F_SETPIPE_SZ);
use constant {
EPOLLIN => 1,
EPOLLOUT => 4,
@@ -55,13 +55,12 @@ use constant {
my @BYTES_4_hole = BYTES_4_hole ? (0) : ();
-our (
- $SYS_epoll_create,
- $SYS_epoll_ctl,
- $SYS_epoll_wait,
- $SYS_signalfd4,
- $SYS_renameat2,
- );
+our ($SYS_epoll_create,
+ $SYS_epoll_ctl,
+ $SYS_epoll_wait,
+ $SYS_signalfd4,
+ $SYS_renameat2,
+ $F_SETPIPE_SZ);
my ($SYS_sendmsg, $SYS_recvmsg);
my $SYS_fstatfs; # don't need fstatfs64, just statfs.f_type
@@ -70,6 +69,7 @@ my $SFD_CLOEXEC = 02000000; # Perl does not expose O_CLOEXEC
our $no_deprecated = 0;
if ($^O eq "linux") {
+ $F_SETPIPE_SZ = 1031;
my (undef, undef, $release, undef, $machine) = POSIX::uname();
my ($maj, $min) = ($release =~ /\A([0-9]+)\.([0-9]+)/);
$SYS_renameat2 = 0 if "$maj.$min" < 3.15;
diff --git a/t/gcf2.t b/t/gcf2.t
index d12a4420..33f3bbca 100644
--- a/t/gcf2.t
+++ b/t/gcf2.t
@@ -10,6 +10,7 @@ use POSIX qw(_exit);
use Cwd qw(abs_path);
require_mods('PublicInbox::Gcf2');
use_ok 'PublicInbox::Gcf2';
+use PublicInbox::Syscall qw($F_SETPIPE_SZ);
use PublicInbox::Import;
my ($tmpdir, $for_destroy) = tmpdir();
@@ -109,7 +110,7 @@ SKIP: {
for my $blk (1, 0) {
my ($r, $w);
pipe($r, $w) or BAIL_OUT $!;
- fcntl($w, 1031, 4096) or
+ fcntl($w, $F_SETPIPE_SZ, 4096) or
skip('Linux too old for F_SETPIPE_SZ', 14);
$w->blocking($blk);
seek($fh, 0, SEEK_SET) or BAIL_OUT "seek: $!";
@@ -129,7 +130,7 @@ SKIP: {
$ck_copying->("pipe blocking($blk)");
pipe($r, $w) or BAIL_OUT $!;
- fcntl($w, 1031, 4096) or BAIL_OUT $!;
+ fcntl($w, $F_SETPIPE_SZ, 4096) or BAIL_OUT $!;
$w->blocking($blk);
close $r;
local $SIG{PIPE} = 'IGNORE';
diff --git a/t/lei-sigpipe.t b/t/lei-sigpipe.t
index 55c208e2..622598a4 100644
--- a/t/lei-sigpipe.t
+++ b/t/lei-sigpipe.t
@@ -6,6 +6,7 @@ use v5.10.1;
use PublicInbox::TestCommon;
use POSIX qw(WTERMSIG WIFSIGNALED SIGPIPE);
use PublicInbox::OnDestroy;
+use PublicInbox::Syscall qw($F_SETPIPE_SZ);
# undo systemd (and similar) ignoring SIGPIPE, since lei expects to be run
# from an interactive terminal:
@@ -21,10 +22,8 @@ test_lei(sub {
my $imported;
for my $out ([], [qw(-f mboxcl2)], [qw(-f text)]) {
pipe(my ($r, $w)) or BAIL_OUT $!;
- my $size = 65536;
- if ($^O eq 'linux' && fcntl($w, 1031, 4096)) {
- $size = 4096;
- }
+ my $size = $F_SETPIPE_SZ && fcntl($w, $F_SETPIPE_SZ, 4096) ?
+ 4096 : 65536;
unless (-f $f) {
open my $fh, '>', $f or xbail "open $f: $!";
print $fh <<'EOM' or xbail;
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 18/30] t/lei-up: additional diagnostics for match failures
2023-10-17 23:37 [PATCH 00/30] autodie-ification and code simplifications Eric Wong
` (16 preceding siblings ...)
2023-10-17 23:38 ` [PATCH 17/30] syscall: common $F_SETPIPE_SZ definition Eric Wong
@ 2023-10-17 23:38 ` Eric Wong
2023-10-17 23:38 ` [PATCH 19/30] test_common: use autodie and read_all where possible Eric Wong
` (12 subsequent siblings)
30 siblings, 0 replies; 32+ messages in thread
From: Eric Wong @ 2023-10-17 23:38 UTC (permalink / raw)
To: meta
I'm not sure why, but this test just failed for some odd reason
from `make check-run' on my Debian bullseye workstatation.
---
t/lei-up.t | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/t/lei-up.t b/t/lei-up.t
index baed6507..2d3afd82 100644
--- a/t/lei-up.t
+++ b/t/lei-up.t
@@ -18,11 +18,11 @@ test_lei(sub {
gunzip("$home/$x.mbox.gz" => \$uc, MultiStream => 1) or
xbail "gunzip $GunzipError";
ok(index($uc, $qp->body_raw) >= 0,
- "original mail in $x.mbox.gz");
+ "original mail in $x.mbox.gz") or diag $uc;
open my $fh, '<', "$home/$x" or xbail $!;
$uc = do { local $/; <$fh> } // xbail $!;
ok(index($uc, $qp->body_raw) >= 0,
- "original mail in uncompressed $x");
+ "original mail in uncompressed $x") or diag $uc;
}
lei_ok qw(ls-search);
$s = eml_load('t/utf8.eml')->as_string;
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 19/30] test_common: use autodie and read_all where possible
2023-10-17 23:37 [PATCH 00/30] autodie-ification and code simplifications Eric Wong
` (17 preceding siblings ...)
2023-10-17 23:38 ` [PATCH 18/30] t/lei-up: additional diagnostics for match failures Eric Wong
@ 2023-10-17 23:38 ` Eric Wong
2023-10-17 23:38 ` [PATCH 20/30] test_common: only hide TCP port in messages Eric Wong
` (11 subsequent siblings)
30 siblings, 0 replies; 32+ messages in thread
From: Eric Wong @ 2023-10-17 23:38 UTC (permalink / raw)
To: meta
Noise reduction to help my aging eyes.
---
lib/PublicInbox/TestCommon.pm | 73 ++++++++++++++++-------------------
1 file changed, 34 insertions(+), 39 deletions(-)
diff --git a/lib/PublicInbox/TestCommon.pm b/lib/PublicInbox/TestCommon.pm
index 77da822b..3839ab45 100644
--- a/lib/PublicInbox/TestCommon.pm
+++ b/lib/PublicInbox/TestCommon.pm
@@ -16,6 +16,7 @@ our @EXPORT;
my $lei_loud = $ENV{TEST_LEI_ERR_LOUD};
my $tail_cmd = $ENV{TAIL};
our ($lei_opt, $lei_out, $lei_err, $lei_cwdfh);
+use autodie qw(chdir close fcntl open opendir seek unlink);
$_ = File::Spec->rel2abs($_) for (grep(!m!^/!, @INC));
@@ -46,11 +47,16 @@ sub require_bsd (;$) {
sub xbail (@) { BAIL_OUT join(' ', map { ref() ? (explain($_)) : ($_) } @_) }
+sub read_all ($;$$) {
+ require PublicInbox::Git;
+ PublicInbox::Git::read_all($_[0], $_[1], $_[2])
+}
+
sub eml_load ($) {
my ($path, $cb) = @_;
- open(my $fh, '<', $path) or die "open $path: $!";
+ open(my $fh, '<', $path);
require PublicInbox::Eml;
- PublicInbox::Eml->new(\(do { local $/; <$fh> }));
+ PublicInbox::Eml->new(\(read_all($fh)));
}
sub tmpdir (;$) {
@@ -243,9 +249,9 @@ sub _prepare_redirects ($) {
for (my $fd = 0; $fd <= $#io_mode; $fd++) {
my $fh = $fhref->[$fd] or next;
my ($oldfh, $mode) = @{$io_mode[$fd]};
- open my $orig, $mode, $oldfh or die "$oldfh $mode stash: $!";
+ open(my $orig, $mode, $oldfh);
$orig_io->[$fd] = $orig;
- open $oldfh, $mode, $fh or die "$oldfh $mode redirect: $!";
+ open $oldfh, $mode, $fh;
}
$orig_io;
}
@@ -255,7 +261,7 @@ sub _undo_redirects ($) {
for (my $fd = 0; $fd <= $#io_mode; $fd++) {
my $fh = $orig_io->[$fd] or next;
my ($oldfh, $mode) = @{$io_mode[$fd]};
- open $oldfh, $mode, $fh or die "$$oldfh $mode redirect: $!";
+ open $oldfh, $mode, $fh;
}
}
@@ -281,8 +287,8 @@ sub key2sub ($) {
my ($key) = @_;
$cached_scripts{$key} //= do {
my $f = key2script($key);
- open my $fh, '<', $f or die "open $f: $!";
- my $str = do { local $/; <$fh> };
+ open my $fh, '<', $f;
+ my $str = read_all($fh);
my $pkg = (split(m!/!, $f))[-1];
$pkg =~ s/([a-z])([a-z0-9]+)(\.t)?\z/\U$1\E$2/;
$pkg .= "_T" if $3;
@@ -329,7 +335,7 @@ sub _run_sub ($$$) {
sub no_coredump (@) {
my @dirs = @_;
my $cwdfh;
- if (@dirs) { opendir($cwdfh, '.') or die "opendir(.): $!" }
+ opendir($cwdfh, '.') if @dirs;
my @found;
for (@dirs, '.') {
chdir $_;
@@ -375,7 +381,7 @@ sub run_script ($;$$) {
next if $fd > 0;
$fh->autoflush(1);
print $fh $$redir or die "print: $!";
- seek($fh, 0, SEEK_SET) or die "seek: $!";
+ seek($fh, 0, SEEK_SET);
} elsif ($ref eq 'GLOB') {
$spawn_opt->{$fd} = $fhref->[$fd] = $redir;
} elsif ($ref) {
@@ -406,15 +412,13 @@ sub run_script ($;$$) {
my $orig_io = _prepare_redirects($fhref);
my $cwdfh = $lei_cwdfh;
if (my $d = $opt->{'-C'}) {
- unless ($cwdfh) {
- opendir $cwdfh, '.' or die "opendir .: $!";
- }
- chdir $d or die "chdir $d: $!";
+ $cwdfh or opendir $cwdfh, '.';
+ chdir $d;
}
_run_sub($sub, $key, \@argv);
# n.b. all our uses of PublicInbox::DS should be fine
# with this and we can't Reset here.
- die "fchdir(restore): $!" if $cwdfh && !chdir($cwdfh);
+ chdir($cwdfh) if $cwdfh;
_undo_redirects($orig_io);
select STDOUT;
umask($umask);
@@ -425,10 +429,8 @@ sub run_script ($;$$) {
for my $fd (1..2) {
my $fh = $fhref->[$fd] or next;
next unless -f $fh;
- seek($fh, 0, SEEK_SET) or die "seek: $!";
- my $redir = $opt->{$fd};
- local $/;
- $$redir = <$fh>;
+ seek($fh, 0, SEEK_SET);
+ ${$opt->{$fd}} = read_all($fh);
}
no_coredump($opt->{-C} ? ($opt->{-C}) : ());
$? == 0;
@@ -458,7 +460,7 @@ sub wait_for_tail {
$ino[0] =~ s!/fd/!/fdinfo/!;
my @info;
do {
- if (open my $fh, '<', $ino[0]) {
+ if (CORE::open(my $fh, '<', $ino[0])) {
local $/ = "\n";
@info = grep(/^inotify wd:/, <$fh>);
}
@@ -500,7 +502,6 @@ sub tail_f (@) {
$tail_cmd or return; # "tail -F" or "tail -f"
my $opt = (ref($f[-1]) eq 'HASH') ? pop(@f) : {};
my $clofork = $opt->{-CLOFORK} // [];
- use autodie qw(fcntl open);
my @cfmap = map {
my $fl = fcntl($_, F_GETFD, 0);
fcntl($_, F_SETFD, $fl | FD_CLOEXEC) unless $fl & FD_CLOEXEC;
@@ -551,9 +552,7 @@ sub start_script {
\&PublicInbox::DS::sig_setmask, $oset);
my $pid = PublicInbox::DS::do_fork();
if ($pid == 0) {
- for (@{delete($opt->{-CLOFORK}) // []}) {
- close($_) or die "close $!";
- }
+ close($_) for (@{delete($opt->{-CLOFORK}) // []});
# pretend to be systemd (cf. sd_listen_fds(3))
# 3 == SD_LISTEN_FDS_START
my $fd;
@@ -561,7 +560,7 @@ sub start_script {
my $io = $opt->{$fd} // next;
my $old = fileno($io);
if ($old == $fd) {
- fcntl($io, F_SETFD, 0) // die "F_SETFD: $!";
+ fcntl($io, F_SETFD, 0);
} else {
dup2($old, $fd) // die "dup2($old, $fd): $!";
}
@@ -572,7 +571,7 @@ sub start_script {
$ENV{LISTEN_PID} = $$;
$ENV{LISTEN_FDS} = $fds;
}
- if ($opt->{-C}) { chdir($opt->{-C}) or die "chdir: $!" }
+ if ($opt->{-C}) { chdir($opt->{-C}) }
$0 = join(' ', @$cmd);
local @SIG{keys %SIG} = map { undef } values %SIG;
local $SIG{FPE} = 'IGNORE'; # Perl default
@@ -657,7 +656,6 @@ sub need_scm_rights () {
# returns a pipe with FD_CLOEXEC disabled on the write-end
sub quit_waiter_pipe () {
- use autodie qw(fcntl pipe);
pipe(my $r, my $w);
fcntl($w, F_SETFD, fcntl($w, F_GETFD, 0) & ~FD_CLOEXEC);
($r, $w);
@@ -675,7 +673,7 @@ SKIP: {
my ($cb) = pop @_;
my $test_opt = shift // {};
local $lei_cwdfh;
- use autodie qw(mkdir open opendir);
+ use autodie qw(mkdir);
opendir $lei_cwdfh, '.';
require_git(2.6, 1);
my $mods = $test_opt->{mods} // [ 'lei' ];
@@ -766,7 +764,7 @@ sub setup_public_inboxes () {
'--newsgroup', "t.v$V", "t$V",
"$test_home/t$V", "http://example.com/t$V",
"t$V\@example.com" ]) or xbail "init v$V";
- unlink "$test_home/t$V/description" or xbail "unlink $!";
+ unlink "$test_home/t$V/description";
}
require PublicInbox::Config;
require PublicInbox::InboxWritable;
@@ -786,7 +784,7 @@ sub setup_public_inboxes () {
$im->done;
});
$seen or BAIL_OUT 'no imports';
- open my $fh, '>', $stamp or BAIL_OUT "open $stamp: $!";
+ open my $fh, '>', $stamp;
@ret;
}
@@ -815,13 +813,12 @@ sub create_coderepo ($$;@) {
my $scope = $lk->lock_for_scope;
my $tmpdir = delete $opt{tmpdir};
if (!-f "$dir/creat.stamp") {
- opendir(my $dfh, '.') or xbail "opendir .: $!";
- chdir($dir) or xbail "chdir($dir): $!";
+ opendir(my $dfh, '.');
+ chdir($dir);
local %ENV = (%ENV, %COMMIT_ENV);
$cb->($dir);
- chdir($dfh) or xbail "cd -: $!";
- open my $s, '>', "$dir/creat.stamp" or
- BAIL_OUT "error creating $dir/creat.stamp: $!";
+ chdir($dfh);
+ open my $s, '>', "$dir/creat.stamp";
}
return $dir if !defined($tmpdir);
xsys_e([qw(/bin/cp -Rp), $dir, $tmpdir]);
@@ -868,8 +865,7 @@ sub create_inbox ($$;@) {
xsys_e([ qw(git gc -q) ], { GIT_DIR => $dir });
}
}
- open my $s, '>', "$dir/creat.stamp" or
- BAIL_OUT "error creating $dir/creat.stamp: $!";
+ open my $s, '>', "$dir/creat.stamp";
}
if ($tmpdir) {
undef $ibx;
@@ -904,8 +900,8 @@ sub test_httpd ($$;$$) {
ua => $ua);
$cb->() if $cb;
$td->join('TERM');
- open my $fh, '<', $err or BAIL_OUT $!;
- my $e = do { local $/; <$fh> };
+ open my $fh, '<', $err;
+ my $e = read_all($fh);
if ($e =~ s/^Plack::Middleware::ReverseProxy missing,\n//gms) {
$e =~ s/^URL generation for redirects .*\n//gms;
}
@@ -934,7 +930,6 @@ sub no_pollerfd ($) {
sub cfg_new ($;@) {
my ($tmpdir, @body) = @_;
- use autodie;
require PublicInbox::Config;
my $f = "$tmpdir/tmp_cfg";
open my $fh, '>', $f;
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 20/30] test_common: only hide TCP port in messages
2023-10-17 23:37 [PATCH 00/30] autodie-ification and code simplifications Eric Wong
` (18 preceding siblings ...)
2023-10-17 23:38 ` [PATCH 19/30] test_common: use autodie and read_all where possible Eric Wong
@ 2023-10-17 23:38 ` Eric Wong
2023-10-17 23:38 ` [PATCH 21/30] test_common: use $cwdfh for every run_script command Eric Wong
` (10 subsequent siblings)
30 siblings, 0 replies; 32+ messages in thread
From: Eric Wong @ 2023-10-17 23:38 UTC (permalink / raw)
To: meta
v2:// lei outputs are on the filesystem, so putting $HOST:$PORT
is nonsensical. We'll also keep `127.0.0.1' or `[::1]' since
it's harmless and can point out obvious errors in system
configuration when testing with old Perls or libraries.
---
lib/PublicInbox/TestCommon.pm | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/PublicInbox/TestCommon.pm b/lib/PublicInbox/TestCommon.pm
index 3839ab45..96663731 100644
--- a/lib/PublicInbox/TestCommon.pm
+++ b/lib/PublicInbox/TestCommon.pm
@@ -619,7 +619,7 @@ sub lei_ok (@) {
my @msg = ref($_[0]) eq 'ARRAY' ? @{$_[0]} : @_;
if (!$lei_loud) {
for (@msg) {
- s!\A([a-z0-9]+://)[^/]+/!$1\$HOST_PORT/!;
+ s!(127\.0\.0\.1|\[::1\]):(?:\d+)!$1:\$PORT!g;
s!$tmpdir\b/(?:[^/]+/)?!\$TMPDIR/!g;
s!\Q$PWD\E\b!\$PWD!g;
}
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 21/30] test_common: use $cwdfh for every run_script command
2023-10-17 23:37 [PATCH 00/30] autodie-ification and code simplifications Eric Wong
` (19 preceding siblings ...)
2023-10-17 23:38 ` [PATCH 20/30] test_common: only hide TCP port in messages Eric Wong
@ 2023-10-17 23:38 ` Eric Wong
2023-10-17 23:38 ` [PATCH 22/30] init: drop extraneous `+' Eric Wong
` (9 subsequent siblings)
30 siblings, 0 replies; 32+ messages in thread
From: Eric Wong @ 2023-10-17 23:38 UTC (permalink / raw)
To: meta
It's not lei-specific since we have `-C' to perform chdir
for multiple admin commands.
---
lib/PublicInbox/TestCommon.pm | 13 ++++---------
1 file changed, 4 insertions(+), 9 deletions(-)
diff --git a/lib/PublicInbox/TestCommon.pm b/lib/PublicInbox/TestCommon.pm
index 96663731..5ad12942 100644
--- a/lib/PublicInbox/TestCommon.pm
+++ b/lib/PublicInbox/TestCommon.pm
@@ -15,7 +15,7 @@ use Carp ();
our @EXPORT;
my $lei_loud = $ENV{TEST_LEI_ERR_LOUD};
my $tail_cmd = $ENV{TAIL};
-our ($lei_opt, $lei_out, $lei_err, $lei_cwdfh);
+our ($lei_opt, $lei_out, $lei_err);
use autodie qw(chdir close fcntl open opendir seek unlink);
$_ = File::Spec->rel2abs($_) for (grep(!m!^/!, @INC));
@@ -410,15 +410,12 @@ sub run_script ($;$$) {
local $SIG{FPE} = 'IGNORE'; # Perl default
local $0 = join(' ', @$cmd);
my $orig_io = _prepare_redirects($fhref);
- my $cwdfh = $lei_cwdfh;
- if (my $d = $opt->{'-C'}) {
- $cwdfh or opendir $cwdfh, '.';
- chdir $d;
- }
+ opendir(my $cwdfh, '.');
+ chdir $opt->{-C} if defined $opt->{-C};
_run_sub($sub, $key, \@argv);
# n.b. all our uses of PublicInbox::DS should be fine
# with this and we can't Reset here.
- chdir($cwdfh) if $cwdfh;
+ chdir($cwdfh);
_undo_redirects($orig_io);
select STDOUT;
umask($umask);
@@ -672,9 +669,7 @@ sub test_lei {
SKIP: {
my ($cb) = pop @_;
my $test_opt = shift // {};
- local $lei_cwdfh;
use autodie qw(mkdir);
- opendir $lei_cwdfh, '.';
require_git(2.6, 1);
my $mods = $test_opt->{mods} // [ 'lei' ];
require_mods(@$mods, 2);
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 22/30] init: drop extraneous `+'
2023-10-17 23:37 [PATCH 00/30] autodie-ification and code simplifications Eric Wong
` (20 preceding siblings ...)
2023-10-17 23:38 ` [PATCH 21/30] test_common: use $cwdfh for every run_script command Eric Wong
@ 2023-10-17 23:38 ` Eric Wong
2023-10-17 23:38 ` [PATCH 23/30] init: use autodie to reduce distractions Eric Wong
` (8 subsequent siblings)
30 siblings, 0 replies; 32+ messages in thread
From: Eric Wong @ 2023-10-17 23:38 UTC (permalink / raw)
To: meta
It's actually valid Perl syntax, but still confusing to look at.
Fixes: add90b9504f4 ("support -C (chdir) for most non-daemon commands")
---
script/public-inbox-init | 2 +-
t/init.t | 7 +++++++
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/script/public-inbox-init b/script/public-inbox-init
index 33bee310..b3e94faf 100755
--- a/script/public-inbox-init
+++ b/script/public-inbox-init
@@ -60,7 +60,7 @@ my $inboxdir = shift @ARGV or $usage_cb->();
my $http_url = shift @ARGV or $usage_cb->();
my (@address) = @ARGV;
@address or $usage_cb->();
-+PublicInbox::Admin::do_chdir(\@chdir);
+PublicInbox::Admin::do_chdir(\@chdir);
@c_extra = map {
my ($k, $v) = split(/=/, $_, 2);
diff --git a/t/init.t b/t/init.t
index abe3a372..177c22b6 100644
--- a/t/init.t
+++ b/t/init.t
@@ -216,6 +216,13 @@ SKIP: {
is($n, 13, 'V1 NNTP article numbers skipped via --skip-artnum');
}
+{
+ my $cmd = [ qw(-init -C), "$tmpdir", qw(chdirlist chdirlist),
+ qw(http://example.com/chdirlist chdirlist@example.com)];
+ ok(run_script($cmd), '-init with -C (chdir)');
+ ok(-d "$tmpdir/chdirlist", '-C processed correctly');
+}
+
done_testing();
sub read_indexlevel {
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 23/30] init: use autodie to reduce distractions
2023-10-17 23:37 [PATCH 00/30] autodie-ification and code simplifications Eric Wong
` (21 preceding siblings ...)
2023-10-17 23:38 ` [PATCH 22/30] init: drop extraneous `+' Eric Wong
@ 2023-10-17 23:38 ` Eric Wong
2023-10-17 23:38 ` [PATCH 24/30] xt/mem-imapd-tls: remove unused/broken epoll imports Eric Wong
` (7 subsequent siblings)
30 siblings, 0 replies; 32+ messages in thread
From: Eric Wong @ 2023-10-17 23:38 UTC (permalink / raw)
To: meta
This hurts startup time a bit, but our tests use run_script
by default and I don't think normal users call -init enough
to care.
---
script/public-inbox-init | 27 +++++++++++----------------
1 file changed, 11 insertions(+), 16 deletions(-)
diff --git a/script/public-inbox-init b/script/public-inbox-init
index b3e94faf..6420db7e 100755
--- a/script/public-inbox-init
+++ b/script/public-inbox-init
@@ -4,6 +4,7 @@
use strict;
use v5.10.1;
use Getopt::Long qw/:config gnu_getopt no_ignore_case auto_abbrev/;
+use autodie qw(open chmod close rename);
use Fcntl qw(:DEFAULT);
my $help = <<EOF; # the following should fit w/o scrolling in 80x24 term:
usage: public-inbox-init NAME INBOX_DIR HTTP_URL ADDRESS [ADDRESS..]
@@ -126,15 +127,12 @@ my $perm = 0644 & ~umask;
my %seen;
if (-e $pi_config) {
require PublicInbox::Git;
- open(my $oh, '<', $pi_config) or die "unable to read $pi_config: $!\n";
- my @st = stat($oh);
+ open(my $oh, '<', $pi_config);
+ my @st = stat($oh) or die "(f)stat failed on $pi_config: $!\n";
$perm = $st[2];
- defined $perm or die "(f)stat failed on $pi_config: $!\n";
- chmod($perm & 07777, $fh) or
- die "(f)chmod failed on future $pi_config: $!\n";
- my $old = PublicInbox::Git::read_all($oh);
- print $fh $old or die "failed to write: $!\n";
- close $oh or die "failed to close $pi_config: $!\n";
+ chmod($perm & 07777, $fh);
+ print $fh PublicInbox::Git::read_all($oh);
+ close $oh;
# yes, this conflict checking is racy if multiple instances of this
# script are run by the same $PI_DIR
@@ -161,7 +159,7 @@ if (-e $pi_config) {
$indexlevel //= $ibx->{indexlevel} if $ibx;
}
my $pi_config_tmp = $fh->filename;
-close($fh) or die "failed to close $pi_config_tmp: $!\n";
+close($fh);
my $pfx = "publicinbox.$name";
my @x = (qw/git config/, "--file=$pi_config_tmp");
@@ -216,8 +214,8 @@ $ibx->init_inbox(0, $skip_epoch, $skip_artnum);
my $f = "$inboxdir/description";
if (sysopen $fh, $f, O_CREAT|O_EXCL|O_WRONLY) {
- print $fh "public inbox for $address[0]\n" or die "print($f): $!";
- close $fh or die "close($f): $!";
+ print $fh "public inbox for $address[0]\n";
+ close $fh;
}
# needed for git prior to v2.1.0
@@ -248,9 +246,6 @@ for my $kv (@c_extra) {
}
# needed for git prior to v2.1.0
-chmod($perm & 07777, $pi_config_tmp) or
- die "(f)chmod failed on future $pi_config: $!\n";
-
-rename $pi_config_tmp, $pi_config or
- die "failed to rename `$pi_config_tmp' to `$pi_config': $!\n";
+chmod($perm & 07777, $pi_config_tmp);
+rename $pi_config_tmp, $pi_config;
undef $auto_unlink; # trigger ->DESTROY
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 24/30] xt/mem-imapd-tls: remove unused/broken epoll imports
2023-10-17 23:37 [PATCH 00/30] autodie-ification and code simplifications Eric Wong
` (22 preceding siblings ...)
2023-10-17 23:38 ` [PATCH 23/30] init: use autodie to reduce distractions Eric Wong
@ 2023-10-17 23:38 ` Eric Wong
2023-10-17 23:38 ` [PATCH 25/30] xt/mem-imapd-tls: reduce FDs for lsof use Eric Wong
` (6 subsequent siblings)
30 siblings, 0 replies; 32+ messages in thread
From: Eric Wong @ 2023-10-17 23:38 UTC (permalink / raw)
To: meta
The `:epoll' tag has been gone for a few weeks, and EPOLLIN
isn't used in this file anywhere.
Fixes: 3005c1bc5d05 (ds: use object-oriented API for epoll)
---
xt/mem-imapd-tls.t | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/xt/mem-imapd-tls.t b/xt/mem-imapd-tls.t
index 58581017..bb2f6c35 100644
--- a/xt/mem-imapd-tls.t
+++ b/xt/mem-imapd-tls.t
@@ -7,7 +7,6 @@ use strict;
use v5.10.1;
use Socket qw(SOCK_STREAM IPPROTO_TCP SOL_SOCKET);
use PublicInbox::TestCommon;
-use PublicInbox::Syscall qw(:epoll);
use PublicInbox::DS;
require_mods(qw(-imapd));
my $inboxdir = $ENV{GIANT_INBOX_DIR};
@@ -134,7 +133,7 @@ package IMAPC;
use strict;
use parent qw(PublicInbox::DS);
# fields: step: state machine, zin: Zlib inflate context
-use PublicInbox::Syscall qw(EPOLLIN EPOLLOUT EPOLLONESHOT);
+use PublicInbox::Syscall qw(EPOLLOUT EPOLLONESHOT);
use Errno qw(EAGAIN);
# determines where we start event_step
use constant FIRST_STEP => ($ENV{TEST_COMPRESS} // 1) ? -2 : 0;
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 25/30] xt/mem-imapd-tls: reduce FDs for lsof use
2023-10-17 23:37 [PATCH 00/30] autodie-ification and code simplifications Eric Wong
` (23 preceding siblings ...)
2023-10-17 23:38 ` [PATCH 24/30] xt/mem-imapd-tls: remove unused/broken epoll imports Eric Wong
@ 2023-10-17 23:38 ` Eric Wong
2023-10-17 23:38 ` [PATCH 26/30] lei: use autodie where appropriate Eric Wong
` (5 subsequent siblings)
30 siblings, 0 replies; 32+ messages in thread
From: Eric Wong @ 2023-10-17 23:38 UTC (permalink / raw)
To: meta
We need some pipes in our parent process to capture the output
of lsof(1), so give us some more padding for temporary FDs.
---
xt/mem-imapd-tls.t | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/xt/mem-imapd-tls.t b/xt/mem-imapd-tls.t
index bb2f6c35..53adb11b 100644
--- a/xt/mem-imapd-tls.t
+++ b/xt/mem-imapd-tls.t
@@ -6,6 +6,7 @@
use strict;
use v5.10.1;
use Socket qw(SOCK_STREAM IPPROTO_TCP SOL_SOCKET);
+use PublicInbox::Spawn qw(which);
use PublicInbox::TestCommon;
use PublicInbox::DS;
require_mods(qw(-imapd));
@@ -71,7 +72,7 @@ if ($TEST_TLS) {
$ssl_opt{SSL_startHandshake} = 0;
}
chomp(my $nfd = `/bin/sh -c 'ulimit -n'`);
-$nfd -= 10;
+$nfd -= 20;
ok($nfd > 0, 'positive FD count');
my $MAX_FD = 10000;
$nfd = $MAX_FD if $nfd >= $MAX_FD;
@@ -118,10 +119,11 @@ if ($DONE != $nfd) {
PublicInbox::DS::event_loop();
}
is($nfd, $DONE, "$nfd/$DONE done");
-if ($^O eq 'linux' && open(my $f, '<', "/proc/$pid/status")) {
+my $lsof = which('lsof');
+if ($^O eq 'linux' && $lsof && open(my $f, '<', "/proc/$pid/status")) {
diag(grep(/RssAnon/, <$f>));
- diag " SELF lsof | wc -l ".`lsof -p $$ |wc -l`;
- diag "SERVER lsof | wc -l ".`lsof -p $pid |wc -l`;
+ diag " SELF lsof | wc -l ".`$lsof -p $$ |wc -l`;
+ diag "SERVER lsof | wc -l ".`$lsof -p $pid |wc -l`;
}
PublicInbox::DS->Reset;
$td->kill;
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 26/30] lei: use autodie where appropriate
2023-10-17 23:37 [PATCH 00/30] autodie-ification and code simplifications Eric Wong
` (24 preceding siblings ...)
2023-10-17 23:38 ` [PATCH 25/30] xt/mem-imapd-tls: reduce FDs for lsof use Eric Wong
@ 2023-10-17 23:38 ` Eric Wong
2023-10-17 23:38 ` [PATCH 27/30] lei_auth: update comments and use v5.12 Eric Wong
` (4 subsequent siblings)
30 siblings, 0 replies; 32+ messages in thread
From: Eric Wong @ 2023-10-17 23:38 UTC (permalink / raw)
To: meta
This makes us a bit harsher with misbehaving clients, but we
only have one client implementation at the moment.
---
lib/PublicInbox/LEI.pm | 48 ++++++++++++++++++------------------------
1 file changed, 20 insertions(+), 28 deletions(-)
diff --git a/lib/PublicInbox/LEI.pm b/lib/PublicInbox/LEI.pm
index 1ff6d67f..3ccdd4f7 100644
--- a/lib/PublicInbox/LEI.pm
+++ b/lib/PublicInbox/LEI.pm
@@ -9,6 +9,7 @@ package PublicInbox::LEI;
use v5.12;
use parent qw(PublicInbox::DS PublicInbox::LeiExternal
PublicInbox::LeiQuery);
+use autodie qw(bind chdir fork open socket socketpair unlink);
use Getopt::Long ();
use Socket qw(AF_UNIX SOCK_SEQPACKET pack_sockaddr_un);
use Errno qw(EPIPE EAGAIN ECONNREFUSED ENOENT ECONNRESET);
@@ -29,7 +30,6 @@ use File::Path ();
use File::Spec;
use Carp ();
use Sys::Syslog qw(openlog syslog closelog);
-use Scalar::Util qw(looks_like_number);
our $quit = \&CORE::exit;
our ($current_lei, $errors_log, $listener, $oldset, $dir_idle);
my $GLP = Getopt::Long::Parser->new;
@@ -574,12 +574,12 @@ sub _lei_atfork_child {
# we need to explicitly close things which are on stack
my $cfg = $self->{cfg};
if ($persist) {
- open $self->{3}, '<', '/' or die "open(/) $!";
+ open $self->{3}, '<', '/';
fchdir($self);
close($_) for (grep(defined, delete @$self{qw(0 1 2 sock)}));
delete @$cfg{qw(-lei_store -watches -lei_note_event)};
} else { # worker, Net::NNTP (Net::Cmd) uses STDERR directly
- open STDERR, '+>&='.fileno($self->{2}) or warn "open $!";
+ open STDERR, '+>&='.fileno($self->{2});
STDERR->autoflush(1);
POSIX::setpgid(0, $$) // die "setpgid(0, $$): $!";
delete @$cfg{qw(-watches -lei_note_event)};
@@ -813,10 +813,9 @@ sub dispatch {
if (my $chdir = $self->{opt}->{C}) {
for my $d (@$chdir) {
next if $d eq ''; # same as git(1)
- chdir $d or return fail($self, "cd $d: $!");
+ chdir $d;
}
- open $self->{3}, '<', '.' or
- return fail($self, "open . $!");
+ open($self->{3}, '<', '.');
}
$cb->($self, @argv);
} elsif (grep(/\A-/, $cmd, @argv)) { # --help or -h only
@@ -851,7 +850,7 @@ sub _lei_cfg ($;$) {
}
my ($cfg_dir) = ($f =~ m!(.*?/)[^/]+\z!);
File::Path::mkpath($cfg_dir);
- open my $fh, '>>', $f or die "open($f): $!\n";
+ open my $fh, '>>', $f;
@st = stat($fh) or die "fstat($f): $!\n";
$cur_st = pack('dd', $st[10], $st[7]);
qerr($self, "# $f created") if $self->{cmd} ne 'config';
@@ -1148,10 +1147,7 @@ sub accept_dispatch { # Listener {post_accept} callback
return send($sock, $msg, 0);
} else {
my $i = 0;
- for my $fd (@fds) {
- open($self->{$i++}, '+<&=', $fd) and next;
- send($sock, "open(+<&=$fd) (FD=$i): $!", 0);
- }
+ open($self->{$i++}, '+<&=', $_) for @fds;
$i == 4 or return send($sock, 'not enough FDs='.($i-1), 0)
}
# $ENV_STR = join('', map { "\0$_=$ENV{$_}" } keys %ENV);
@@ -1236,12 +1232,11 @@ sub dump_and_clear_log {
sub cfg2lei ($) {
my ($cfg) = @_;
my $lei = bless { env => { %{$cfg->{-env}} } }, __PACKAGE__;
- open($lei->{0}, '<&', \*STDIN) or die "dup 0: $!";
- open($lei->{1}, '>>&', \*STDOUT) or die "dup 1: $!";
- open($lei->{2}, '>>&', \*STDERR) or die "dup 2: $!";
- open($lei->{3}, '<', '/') or die "open /: $!";
- my ($x, $y);
- socketpair($x, $y, AF_UNIX, SOCK_SEQPACKET, 0) or die "socketpair: $!";
+ open($lei->{0}, '<&', \*STDIN);
+ open($lei->{1}, '>>&', \*STDOUT);
+ open($lei->{2}, '>>&', \*STDERR);
+ open($lei->{3}, '<', '/');
+ socketpair(my $x, my $y, AF_UNIX, SOCK_SEQPACKET, 0);
$lei->{sock} = $x;
require PublicInbox::LeiSelfSocket;
PublicInbox::LeiSelfSocket->new($y); # adds to event loop
@@ -1317,17 +1312,15 @@ sub lazy_start {
my $lk = PublicInbox::Lock->new($errors_log);
umask(077) // die("umask(077): $!");
$lk->lock_acquire;
- socket($listener, AF_UNIX, SOCK_SEQPACKET, 0) or die "socket: $!";
+ socket($listener, AF_UNIX, SOCK_SEQPACKET, 0);
if ($errno == ECONNREFUSED || $errno == ENOENT) {
return if connect($listener, $addr); # another process won
- if ($errno == ECONNREFUSED && -S $path) {
- unlink($path) or die "unlink($path): $!";
- }
+ unlink($path) if $errno == ECONNREFUSED && -S $path;
} else {
$! = $errno; # allow interpolation to stringify in die
die "connect($path): $!";
}
- bind($listener, $addr) or die "bind($path): $!";
+ bind($listener, $addr);
$lk->lock_release;
undef $lk;
my @st = stat($path) or die "stat($path): $!";
@@ -1340,11 +1333,11 @@ sub lazy_start {
require PublicInbox::Listener;
require PublicInbox::PktOp;
(-p STDOUT) or die "E: stdout must be a pipe\n";
- open(STDIN, '+>>', $errors_log) or die "open($errors_log): $!";
+ open(STDIN, '+>>', $errors_log);
STDIN->autoflush(1);
dump_and_clear_log();
POSIX::setsid() > 0 or die "setsid: $!";
- my $pid = fork // die "fork: $!";
+ my $pid = fork;
return if $pid;
$0 = "lei-daemon $path";
local %PATH2CFG;
@@ -1385,8 +1378,8 @@ sub lazy_start {
};
local $SIG{PIPE} = 'IGNORE';
local $SIG{ALRM} = 'IGNORE';
- open STDERR, '>&STDIN' or die "redirect stderr failed: $!";
- open STDOUT, '>&STDIN' or die "redirect stdout failed: $!";
+ open STDERR, '>&STDIN';
+ open STDOUT, '>&STDIN';
# $daemon pipe to `lei' closed, main loop begins:
eval { PublicInbox::DS::event_loop($sig, $oldset) };
warn "event loop error: $@\n" if $@;
@@ -1424,8 +1417,7 @@ sub wq_done_wait { # awaitpid cb (via wq_eof)
sub fchdir {
my ($lei) = @_;
- my $dh = $lei->{3} // die 'BUG: lei->{3} (CWD) gone';
- chdir($dh) || die "fchdir: $!";
+ chdir($lei->{3} // die 'BUG: lei->{3} (CWD) gone');
}
sub wq_eof { # EOF callback for main daemon
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 27/30] lei_auth: update comments and use v5.12
2023-10-17 23:37 [PATCH 00/30] autodie-ification and code simplifications Eric Wong
` (25 preceding siblings ...)
2023-10-17 23:38 ` [PATCH 26/30] lei: use autodie where appropriate Eric Wong
@ 2023-10-17 23:38 ` Eric Wong
2023-10-17 23:38 ` [PATCH 28/30] lei_config: drop redundant open check Eric Wong
` (3 subsequent siblings)
30 siblings, 0 replies; 32+ messages in thread
From: Eric Wong @ 2023-10-17 23:38 UTC (permalink / raw)
To: meta
There's nothing affected by the `unicode_strings' feature, so
it's safe to use v5.12, here.
---
lib/PublicInbox/LeiAuth.pm | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/lib/PublicInbox/LeiAuth.pm b/lib/PublicInbox/LeiAuth.pm
index 76a4410d..020dd125 100644
--- a/lib/PublicInbox/LeiAuth.pm
+++ b/lib/PublicInbox/LeiAuth.pm
@@ -1,8 +1,8 @@
-# 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>
# Authentication worker for anything that needs auth for read/write IMAP
-# (eventually for read-only NNTP access)
+# and read-only NNTP access
#
# timelines
# lei-daemon | LeiAuth worker #0 | other WQ workers
@@ -22,8 +22,7 @@
# |
# call net_merge_all_done ->-> do per-WQ-class defined actions
package PublicInbox::LeiAuth;
-use strict;
-use v5.10.1;
+use v5.12;
sub do_auth_atfork { # used by IPC WQ workers
my ($self, $wq) = @_;
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 28/30] lei_config: drop redundant open check
2023-10-17 23:37 [PATCH 00/30] autodie-ification and code simplifications Eric Wong
` (26 preceding siblings ...)
2023-10-17 23:38 ` [PATCH 27/30] lei_auth: update comments and use v5.12 Eric Wong
@ 2023-10-17 23:38 ` Eric Wong
2023-10-17 23:38 ` [PATCH 29/30] convert: use read_all to simplify error checks Eric Wong
` (2 subsequent siblings)
30 siblings, 0 replies; 32+ messages in thread
From: Eric Wong @ 2023-10-17 23:38 UTC (permalink / raw)
To: meta
autodie already takes care of checking for open failures.
---
lib/PublicInbox/LeiConfig.pm | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/PublicInbox/LeiConfig.pm b/lib/PublicInbox/LeiConfig.pm
index c47708d8..796bb4f5 100644
--- a/lib/PublicInbox/LeiConfig.pm
+++ b/lib/PublicInbox/LeiConfig.pm
@@ -22,7 +22,7 @@ sub cfg_do_edit ($;$) {
sub cfg_edit_done { # PktOp lei->do_env cb
my ($lei, $self) = @_;
- open my $fh, '+>', undef or die "open($!)";
+ open my $fh, '+>', undef;
my $cfg = do {
local $lei->{2} = $fh;
$lei->cfg_dump($self->{-f});
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 29/30] convert: use read_all to simplify error checks
2023-10-17 23:37 [PATCH 00/30] autodie-ification and code simplifications Eric Wong
` (27 preceding siblings ...)
2023-10-17 23:38 ` [PATCH 28/30] lei_config: drop redundant open check Eric Wong
@ 2023-10-17 23:38 ` Eric Wong
2023-10-17 23:38 ` [PATCH 30/30] idx_stack: use autodie + read_all Eric Wong
2023-10-19 1:14 ` [PATCH 31/30] lei: simplify startq/au_done wakeup notifications Eric Wong
30 siblings, 0 replies; 32+ messages in thread
From: Eric Wong @ 2023-10-17 23:38 UTC (permalink / raw)
To: meta
There wasn't a need to loop anyways with Perl `read' since
the default PerlIO layer will retry.
---
script/public-inbox-convert | 8 +-------
1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/script/public-inbox-convert b/script/public-inbox-convert
index 0cc52777..96f6d2ea 100755
--- a/script/public-inbox-convert
+++ b/script/public-inbox-convert
@@ -130,14 +130,8 @@ while (<$rd>) {
} elsif (/^commit /) {
$state = 'commit';
} elsif (/^data ([0-9]+)/) {
- my $len = $1;
print $io $_ or $im->wfail;
- while ($len) {
- my $n = read($rd, my $tmp, $len) or die "read: $!";
- warn "$n != $len\n" if $n != $len;
- $len -= $n;
- print $io $tmp or $im->wfail;
- }
+ print $io PublicInbox::Git::read_all($rd, $1) or $im->wfail;
next;
} elsif ($state eq 'commit') {
if (m{^M 100644 :([0-9]+) (${h}{2}/${h}{38})}o) {
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 30/30] idx_stack: use autodie + read_all
2023-10-17 23:37 [PATCH 00/30] autodie-ification and code simplifications Eric Wong
` (28 preceding siblings ...)
2023-10-17 23:38 ` [PATCH 29/30] convert: use read_all to simplify error checks Eric Wong
@ 2023-10-17 23:38 ` Eric Wong
2023-10-19 1:14 ` [PATCH 31/30] lei: simplify startq/au_done wakeup notifications Eric Wong
30 siblings, 0 replies; 32+ messages in thread
From: Eric Wong @ 2023-10-17 23:38 UTC (permalink / raw)
To: meta
We'll also add a note to support multi-hash git repos once git
itself gains that ability.
---
lib/PublicInbox/IdxStack.pm | 20 +++++++++-----------
1 file changed, 9 insertions(+), 11 deletions(-)
diff --git a/lib/PublicInbox/IdxStack.pm b/lib/PublicInbox/IdxStack.pm
index 54d480bd..cc9e0125 100644
--- a/lib/PublicInbox/IdxStack.pm
+++ b/lib/PublicInbox/IdxStack.pm
@@ -1,16 +1,18 @@
-# Copyright (C) 2020-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>
# temporary stack for public-inbox-index
+# FIXME: needs to support multi-hash in the same repo once git itself can
package PublicInbox::IdxStack;
-use v5.10.1;
-use strict;
+use v5.12;
use Fcntl qw(:seek);
use constant PACK_FMT => eval { pack('Q', 1) } ? 'A1QQH*H*' : 'A1IIH*H*';
+use autodie qw(open seek);
+use PublicInbox::Git qw(read_all);
# start off in write-only mode
sub new {
- open(my $io, '+>', undef) or die "open: $!";
+ open(my $io, '+>', undef);
# latest_cmt is still useful when the newest revision is a `d'(elete),
# otherwise we favor $sync->{latest_cmt} for checkpoints and {quit}
bless { wr => $io, latest_cmt => $_[1] }, __PACKAGE__
@@ -27,7 +29,7 @@ sub push_rec {
$self->{rec_size} = length($rec);
$self->{unpack_fmt} = $fmt;
};
- print { $self->{wr} } $rec or die "print: $!";
+ print { $self->{wr} } $rec;
$self->{tot_size} += length($rec);
}
@@ -49,12 +51,8 @@ sub pop_rec {
my $sz = $self->{rec_size} or return;
my $rec_pos = $self->{tot_size} -= $sz;
return if $rec_pos < 0;
- my $io = $self->{rd};
- seek($io, $rec_pos, SEEK_SET) or die "seek: $!";
- my $r = read($io, my $buf, $sz);
- defined($r) or die "read: $!";
- $r == $sz or die "read($r != $sz)";
- unpack($self->{unpack_fmt}, $buf);
+ seek($self->{rd}, $rec_pos, SEEK_SET);
+ unpack($self->{unpack_fmt}, read_all($self->{rd}, $sz));
}
1;
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 31/30] lei: simplify startq/au_done wakeup notifications
2023-10-17 23:37 [PATCH 00/30] autodie-ification and code simplifications Eric Wong
` (29 preceding siblings ...)
2023-10-17 23:38 ` [PATCH 30/30] idx_stack: use autodie + read_all Eric Wong
@ 2023-10-19 1:14 ` Eric Wong
30 siblings, 0 replies; 32+ messages in thread
From: Eric Wong @ 2023-10-19 1:14 UTC (permalink / raw)
To: meta
We only need to write one byte at MUA start instead of a byte
for every LeiXSearch worker. Also, make sure it succeeds by
enabling autodie for syswrite.
When reading, we can rely on `:perlio' layer `read' semantics
to retry on EINTR to avoid looping and other error checking.
---
lib/PublicInbox/LEI.pm | 9 +++++----
lib/PublicInbox/LeiXSearch.pm | 28 +++++++++-------------------
2 files changed, 14 insertions(+), 23 deletions(-)
diff --git a/lib/PublicInbox/LEI.pm b/lib/PublicInbox/LEI.pm
index 3ccdd4f7..56e4c001 100644
--- a/lib/PublicInbox/LEI.pm
+++ b/lib/PublicInbox/LEI.pm
@@ -9,7 +9,7 @@ package PublicInbox::LEI;
use v5.12;
use parent qw(PublicInbox::DS PublicInbox::LeiExternal
PublicInbox::LeiQuery);
-use autodie qw(bind chdir fork open socket socketpair unlink);
+use autodie qw(bind chdir fork open socket socketpair syswrite unlink);
use Getopt::Long ();
use Socket qw(AF_UNIX SOCK_SEQPACKET pack_sockaddr_un);
use Errno qw(EPIPE EAGAIN ECONNREFUSED ENOENT ECONNRESET);
@@ -1031,9 +1031,10 @@ sub start_mua {
$io->[0] = $self->{1} if $self->{opt}->{stdin} && -t $self->{1};
send_exec_cmd($self, $io, \@cmd, {});
}
- if ($self->{lxs} && $self->{au_done}) { # kick wait_startq
- syswrite($self->{au_done}, 'q' x ($self->{lxs}->{jobs} // 0));
- }
+
+ # kick wait_startq:
+ syswrite($self->{au_done}, 'q') if $self->{lxs} && $self->{au_done};
+
return unless -t $self->{2}; # XXX how to determine non-TUI MUAs?
$self->{opt}->{quiet} = 1;
delete $self->{-progress};
diff --git a/lib/PublicInbox/LeiXSearch.pm b/lib/PublicInbox/LeiXSearch.pm
index 25b66b3b..241b9dab 100644
--- a/lib/PublicInbox/LeiXSearch.pm
+++ b/lib/PublicInbox/LeiXSearch.pm
@@ -122,26 +122,16 @@ sub _mset_more ($$) {
$size >= $mo->{limit} && (($mo->{offset} += $size) < $mo->{total});
}
-# $startq will EOF when do_augment is done augmenting and allow
+# $startq will see `q' in do_post_augment -> start_mua if spawning MUA.
+# Otherwise $startq will EOF when do_augment is done augmenting and allow
# query_combined_mset and query_thread_mset to proceed.
sub wait_startq ($) {
my ($lei) = @_;
- my $startq = delete $lei->{startq} or return;
- while (1) {
- my $n = sysread($startq, my $do_augment_done, 1);
- if (defined $n) {
- return if $n == 0; # no MUA
- if ($do_augment_done eq 'q') {
- $lei->{opt}->{quiet} = 1;
- delete $lei->{opt}->{verbose};
- delete $lei->{-progress};
- } else {
- die "BUG: do_augment_done=`$do_augment_done'";
- }
- return;
- }
- die "wait_startq: $!" unless $!{EINTR};
- }
+ read(delete($lei->{startq}) // return, my $buf, 1) or return; # EOF
+ die "BUG: wrote `$buf' to au_done" if $buf ne 'q';
+ $lei->{opt}->{quiet} = 1;
+ delete $lei->{opt}->{verbose};
+ delete $lei->{-progress};
}
sub mset_progress {
@@ -451,10 +441,10 @@ sub do_post_augment {
$lei->fail("$err");
}
if (!$err && delete $lei->{early_mua}) { # non-augment case
- eval { $lei->start_mua };
+ eval { $lei->start_mua }; # may trigger wait_startq
$lei->fail($@) if $@;
}
- close(delete $lei->{au_done}); # triggers wait_startq in lei_xsearch
+ close(delete $lei->{au_done}); # trigger wait_startq if start_mua didn't
}
sub incr_post_augment { # called whenever an l2m shard finishes augment
^ permalink raw reply related [flat|nested] 32+ messages in thread
end of thread, other threads:[~2023-10-19 1:14 UTC | newest]
Thread overview: 32+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-10-17 23:37 [PATCH 00/30] autodie-ification and code simplifications Eric Wong
2023-10-17 23:37 ` [PATCH 01/30] lei_mirror: start converting to autodie Eric Wong
2023-10-17 23:37 ` [PATCH 02/30] lei_mirror: autodie most `close' calls Eric Wong
2023-10-17 23:37 ` [PATCH 03/30] lei_mirror: use autodie for most `open' calls Eric Wong
2023-10-17 23:37 ` [PATCH 04/30] git: introduce read_all function Eric Wong
2023-10-17 23:37 ` [PATCH 05/30] import: use read_all to detect short reads Eric Wong
2023-10-17 23:37 ` [PATCH 06/30] lei_mirror: use read_all Eric Wong
2023-10-17 23:37 ` [PATCH 07/30] use read_all in more places to improve safety Eric Wong
2023-10-17 23:37 ` [PATCH 08/30] xap_helper*: use autodie in more places Eric Wong
2023-10-17 23:37 ` [PATCH 09/30] xap_helper: die more easily in both implementations Eric Wong
2023-10-17 23:37 ` [PATCH 10/30] xap_helper: simplify SIGTERM exit checks Eric Wong
2023-10-17 23:37 ` [PATCH 11/30] xap_helper: autodie for getsockopt Eric Wong
2023-10-17 23:37 ` [PATCH 12/30] xap_client: autodie for pipe and socketpair Eric Wong
2023-10-17 23:37 ` [PATCH 13/30] xt/git-http-backend: remove Net::HTTP usage Eric Wong
2023-10-17 23:37 ` [PATCH 14/30] ds: introduce and use do_fork helper Eric Wong
2023-10-17 23:38 ` [PATCH 15/30] ds: get rid of SetLoopTimeout Eric Wong
2023-10-17 23:38 ` [PATCH 16/30] cindex: drop some unused functions Eric Wong
2023-10-17 23:38 ` [PATCH 17/30] syscall: common $F_SETPIPE_SZ definition Eric Wong
2023-10-17 23:38 ` [PATCH 18/30] t/lei-up: additional diagnostics for match failures Eric Wong
2023-10-17 23:38 ` [PATCH 19/30] test_common: use autodie and read_all where possible Eric Wong
2023-10-17 23:38 ` [PATCH 20/30] test_common: only hide TCP port in messages Eric Wong
2023-10-17 23:38 ` [PATCH 21/30] test_common: use $cwdfh for every run_script command Eric Wong
2023-10-17 23:38 ` [PATCH 22/30] init: drop extraneous `+' Eric Wong
2023-10-17 23:38 ` [PATCH 23/30] init: use autodie to reduce distractions Eric Wong
2023-10-17 23:38 ` [PATCH 24/30] xt/mem-imapd-tls: remove unused/broken epoll imports Eric Wong
2023-10-17 23:38 ` [PATCH 25/30] xt/mem-imapd-tls: reduce FDs for lsof use Eric Wong
2023-10-17 23:38 ` [PATCH 26/30] lei: use autodie where appropriate Eric Wong
2023-10-17 23:38 ` [PATCH 27/30] lei_auth: update comments and use v5.12 Eric Wong
2023-10-17 23:38 ` [PATCH 28/30] lei_config: drop redundant open check Eric Wong
2023-10-17 23:38 ` [PATCH 29/30] convert: use read_all to simplify error checks Eric Wong
2023-10-17 23:38 ` [PATCH 30/30] idx_stack: use autodie + read_all Eric Wong
2023-10-19 1:14 ` [PATCH 31/30] lei: simplify startq/au_done wakeup notifications Eric Wong
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).