* [PATCH 01/16] ipc: wq_do: support synchronous waits and responses
2021-09-19 12:50 [PATCH 00/16] lei IPC overhaul, NNTP fixes Eric Wong
@ 2021-09-19 12:50 ` Eric Wong
2021-09-19 12:50 ` [PATCH 02/16] ipc: allow disabling broadcast for wq_workers Eric Wong
` (14 subsequent siblings)
15 siblings, 0 replies; 19+ messages in thread
From: Eric Wong @ 2021-09-19 12:50 UTC (permalink / raw)
To: meta
This brings the wq_* SOCK_SEQPACKET API functionality
on par with the ipc_do (pipe-based) API.
---
lib/PublicInbox/IPC.pm | 36 ++++++++++++++++++++++++++++++++----
t/ipc.t | 6 ++++++
2 files changed, 38 insertions(+), 4 deletions(-)
diff --git a/lib/PublicInbox/IPC.pm b/lib/PublicInbox/IPC.pm
index 9efe551b..d5e37719 100644
--- a/lib/PublicInbox/IPC.pm
+++ b/lib/PublicInbox/IPC.pm
@@ -182,6 +182,13 @@ sub ipc_lock_init {
$self->{-ipc_lock} //= bless { lock_path => $f }, 'PublicInbox::Lock'
}
+sub _wait_return ($$) {
+ my ($r_res, $sub) = @_;
+ my $ret = _get_rec($r_res) // die "no response on $sub";
+ die $$ret if ref($ret) eq 'PublicInbox::IPC::Die';
+ wantarray ? @$ret : $$ret;
+}
+
# call $self->$sub(@args), on a worker if ipc_worker_spawn was used
sub ipc_do {
my ($self, $sub, @args) = @_;
@@ -191,9 +198,7 @@ sub ipc_do {
if (defined(wantarray)) {
my $r_res = $self->{-ipc_res} or die 'no ipc_res';
_send_rec($w_req, [ wantarray, $sub, @args ]);
- my $ret = _get_rec($r_res) // die "no response on $sub";
- die $$ret if ref($ret) eq 'PublicInbox::IPC::Die';
- wantarray ? @$ret : $$ret;
+ _wait_return($r_res, $sub);
} else { # likely, fire-and-forget into pipe
_send_rec($w_req, [ undef , $sub, @args ]);
}
@@ -298,7 +303,7 @@ sub wq_io_do { # always async
$!{ETOOMANYREFS} and
croak "sendmsg: $! (check RLIMIT_NOFILE)";
$!{EMSGSIZE} ? stream_in_full($s1, $fds, $buf) :
- croak("sendmsg: $!");
+ croak("sendmsg: $!");
}
} else {
@$self{0..$#$ios} = @$ios;
@@ -308,6 +313,29 @@ sub wq_io_do { # always async
}
}
+sub wq_sync_run {
+ my ($self, $wantarray, $sub, @args) = @_;
+ if ($wantarray) {
+ my @ret = eval { $self->$sub(@args) };
+ ipc_return($self->{0}, \@ret, $@);
+ } else { # '' => wantscalar
+ my $ret = eval { $self->$sub(@args) };
+ ipc_return($self->{0}, \$ret, $@);
+ }
+}
+
+sub wq_do {
+ my ($self, $sub, @args) = @_;
+ if (defined(wantarray)) {
+ pipe(my ($r, $w)) or die "pipe: $!";
+ wq_io_do($self, 'wq_sync_run', [ $w ], wantarray, $sub, @args);
+ undef $w;
+ _wait_return($r, $sub);
+ } else {
+ wq_io_do($self, $sub, [], @args);
+ }
+}
+
sub _wq_worker_start ($$$) {
my ($self, $oldset, $fields) = @_;
my ($bcast1, $bcast2);
diff --git a/t/ipc.t b/t/ipc.t
index 7983fdc0..202b1cc6 100644
--- a/t/ipc.t
+++ b/t/ipc.t
@@ -161,6 +161,12 @@ SKIP: {
is(waitpid($pid, 0), $pid, 'waitpid complete');
is($?, 0, 'child wq producer exited');
}
+ my @ary = $ipc->wq_do('test_array');
+ is_deeply(\@ary, [ qw(test array) ], 'wq_do wantarray');
+ is(my $s = $ipc->wq_do('test_scalar'), 'scalar', 'defined wantarray');
+ my $exp = bless ['blessed'], 'PublicInbox::WTF';
+ my $ret = eval { $ipc->wq_do('test_die', $exp) };
+ is_deeply($@, $exp, 'die with blessed ref');
}
$ipc->wq_close;
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 02/16] ipc: allow disabling broadcast for wq_workers
2021-09-19 12:50 [PATCH 00/16] lei IPC overhaul, NNTP fixes Eric Wong
2021-09-19 12:50 ` [PATCH 01/16] ipc: wq_do: support synchronous waits and responses Eric Wong
@ 2021-09-19 12:50 ` Eric Wong
2021-09-19 12:50 ` [PATCH 03/16] lei/store: use SOCK_SEQPACKET rather than pipe Eric Wong
` (13 subsequent siblings)
15 siblings, 0 replies; 19+ messages in thread
From: Eric Wong @ 2021-09-19 12:50 UTC (permalink / raw)
To: meta
Since some lei worker classes only use a single worker,
there's no sense in having broadcast for those cases.
---
lib/PublicInbox/IPC.pm | 16 ++++++++--------
lib/PublicInbox/WQWorker.pm | 9 ++++-----
2 files changed, 12 insertions(+), 13 deletions(-)
diff --git a/lib/PublicInbox/IPC.pm b/lib/PublicInbox/IPC.pm
index d5e37719..92f35189 100644
--- a/lib/PublicInbox/IPC.pm
+++ b/lib/PublicInbox/IPC.pm
@@ -245,10 +245,10 @@ sub recv_and_run {
$n;
}
-sub wq_worker_loop ($) {
- my ($self, $bcast_a) = @_;
- my $wqw = PublicInbox::WQWorker->new($self);
- PublicInbox::WQWorker->new($self, '-wq_bcast2');
+sub wq_worker_loop ($$) {
+ my ($self, $bcast2) = @_;
+ my $wqw = PublicInbox::WQWorker->new($self, $self->{-wq_s2});
+ PublicInbox::WQWorker->new($self, $bcast2) if $bcast2;
PublicInbox::DS->SetPostLoopCallback(sub { $wqw->{sock} });
PublicInbox::DS->EventLoop;
PublicInbox::DS->Reset;
@@ -339,8 +339,9 @@ sub wq_do {
sub _wq_worker_start ($$$) {
my ($self, $oldset, $fields) = @_;
my ($bcast1, $bcast2);
- socketpair($bcast1, $bcast2, AF_UNIX, $SEQPACKET, 0) or
- die "socketpair: $!";
+ $self->{-wq_no_bcast} or
+ socketpair($bcast1, $bcast2, AF_UNIX, $SEQPACKET, 0) or
+ die "socketpair: $!";
my $seed = rand(0xffffffff);
my $pid = fork // die "fork: $!";
if ($pid == 0) {
@@ -361,8 +362,7 @@ sub _wq_worker_start ($$$) {
my $on_destroy = $self->ipc_atfork_child;
local %SIG = %SIG;
PublicInbox::DS::sig_setmask($oldset);
- $self->{-wq_bcast2} = $bcast2;
- wq_worker_loop($self);
+ wq_worker_loop($self, $bcast2);
};
warn "worker $self->{-wq_ident} PID:$$ died: $@" if $@;
undef $end; # trigger exit
diff --git a/lib/PublicInbox/WQWorker.pm b/lib/PublicInbox/WQWorker.pm
index f7aa61c5..48b901bb 100644
--- a/lib/PublicInbox/WQWorker.pm
+++ b/lib/PublicInbox/WQWorker.pm
@@ -11,11 +11,10 @@ use Errno qw(EAGAIN ECONNRESET);
use IO::Handle (); # blocking
sub new {
- my ($cls, $wq, $field) = @_;
- my $s2 = $wq->{$field // '-wq_s2'} // die "BUG: no {$field}";
- $s2->blocking(0);
- my $self = bless { sock => $s2, wq => $wq }, $cls;
- $self->SUPER::new($s2, EPOLLEXCLUSIVE|EPOLLIN|EPOLLET);
+ my ($cls, $wq, $sock) = @_;
+ $sock->blocking(0);
+ my $self = bless { sock => $sock, wq => $wq }, $cls;
+ $self->SUPER::new($sock, EPOLLEXCLUSIVE|EPOLLIN|EPOLLET);
$self;
}
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 03/16] lei/store: use SOCK_SEQPACKET rather than pipe
2021-09-19 12:50 [PATCH 00/16] lei IPC overhaul, NNTP fixes Eric Wong
2021-09-19 12:50 ` [PATCH 01/16] ipc: wq_do: support synchronous waits and responses Eric Wong
2021-09-19 12:50 ` [PATCH 02/16] ipc: allow disabling broadcast for wq_workers Eric Wong
@ 2021-09-19 12:50 ` Eric Wong
2021-09-19 12:50 ` [PATCH 04/16] lei: simplify sto_done_request Eric Wong
` (12 subsequent siblings)
15 siblings, 0 replies; 19+ messages in thread
From: Eric Wong @ 2021-09-19 12:50 UTC (permalink / raw)
To: meta
This has several advantages:
* no need to use ipc.lock to protect a pipe for non-atomic writes
* ability to pass FDs. In another commit, this will let us
simplify lei->sto_done_request and pass newly-created
sockets to lei/store directly.
disadvantages:
- an extra pipe is required for rare messages over several
hundred KB, this is probably a non-issue, though
The performance delta is unknown, but I expect shards
(which remain pipes) to be the primary bottleneck IPC-wise
for lei/store.
---
lib/PublicInbox/LEI.pm | 4 ++--
lib/PublicInbox/LeiImport.pm | 2 +-
lib/PublicInbox/LeiImportKw.pm | 2 +-
lib/PublicInbox/LeiIndex.pm | 2 +-
lib/PublicInbox/LeiInput.pm | 2 +-
lib/PublicInbox/LeiNoteEvent.pm | 8 ++++----
lib/PublicInbox/LeiRemote.pm | 4 ++--
lib/PublicInbox/LeiRm.pm | 2 +-
lib/PublicInbox/LeiStore.pm | 10 ++++++++--
lib/PublicInbox/LeiTag.pm | 2 +-
lib/PublicInbox/LeiToMail.pm | 22 ++++++++++++----------
lib/PublicInbox/LeiXSearch.pm | 6 +++---
12 files changed, 37 insertions(+), 29 deletions(-)
diff --git a/lib/PublicInbox/LEI.pm b/lib/PublicInbox/LEI.pm
index 8b0614f2..549b855b 100644
--- a/lib/PublicInbox/LEI.pm
+++ b/lib/PublicInbox/LEI.pm
@@ -1501,9 +1501,9 @@ sub sto_done_request { # only call this from lei-daemon process (not workers)
eval {
if ($sock //= $lei->{sock}) { # issue, async wait
$LIVE_SOCK{"$sock"} = $sock;
- $lei->{sto}->ipc_do('done', "$sock");
+ $lei->{sto}->wq_do('done', "$sock");
} else { # forcibly wait
- my $wait = $lei->{sto}->ipc_do('done');
+ my $wait = $lei->{sto}->wq_do('done');
}
};
$lei->err($@) if $@;
diff --git a/lib/PublicInbox/LeiImport.pm b/lib/PublicInbox/LeiImport.pm
index 9084d771..40530914 100644
--- a/lib/PublicInbox/LeiImport.pm
+++ b/lib/PublicInbox/LeiImport.pm
@@ -16,7 +16,7 @@ sub input_eml_cb { # used by PublicInbox::LeiInput::input_fh
if (my $all_vmd = $self->{all_vmd}) {
@$vmd{keys %$all_vmd} = values %$all_vmd;
}
- $self->{lei}->{sto}->ipc_do('set_eml', $eml, $vmd, $xoids);
+ $self->{lei}->{sto}->wq_do('set_eml', $eml, $vmd, $xoids);
}
sub input_mbox_cb { # MboxReader callback
diff --git a/lib/PublicInbox/LeiImportKw.pm b/lib/PublicInbox/LeiImportKw.pm
index 402125cf..2863d17f 100644
--- a/lib/PublicInbox/LeiImportKw.pm
+++ b/lib/PublicInbox/LeiImportKw.pm
@@ -37,7 +37,7 @@ sub ck_update_kw { # via wq_io_do
$self->{lse}->kw_changed(undef, $kw, \@docids) or return;
$self->{verbose} and
$self->{lei}->qerr('# '.unpack('H*', $oidbin)." => @$kw\n");
- $self->{sto}->ipc_do('set_eml_vmd', undef, { kw => $kw }, \@docids);
+ $self->{sto}->wq_do('set_eml_vmd', undef, { kw => $kw }, \@docids);
}
sub ikw_done_wait {
diff --git a/lib/PublicInbox/LeiIndex.pm b/lib/PublicInbox/LeiIndex.pm
index 1b327a2c..b3f3e1a0 100644
--- a/lib/PublicInbox/LeiIndex.pm
+++ b/lib/PublicInbox/LeiIndex.pm
@@ -16,7 +16,7 @@ sub input_eml_cb { # used by input_maildir_cb and input_net_cb
if (my $all_vmd = $self->{all_vmd}) {
@$vmd{keys %$all_vmd} = values %$all_vmd;
}
- $self->{lei}->{sto}->ipc_do('index_eml_only', $eml, $vmd, $xoids);
+ $self->{lei}->{sto}->wq_do('index_eml_only', $eml, $vmd, $xoids);
}
sub input_fh { # overrides PublicInbox::LeiInput::input_fh
diff --git a/lib/PublicInbox/LeiInput.pm b/lib/PublicInbox/LeiInput.pm
index fe736981..22bedba6 100644
--- a/lib/PublicInbox/LeiInput.pm
+++ b/lib/PublicInbox/LeiInput.pm
@@ -378,7 +378,7 @@ sub process_inputs {
}
# always commit first, even on error partial work is acceptable for
# lei <import|tag|convert>
- my $wait = $self->{lei}->{sto}->ipc_do('done') if $self->{lei}->{sto};
+ my $wait = $self->{lei}->{sto}->wq_do('done') if $self->{lei}->{sto};
$self->{lei}->fail($err) if $err;
}
diff --git a/lib/PublicInbox/LeiNoteEvent.pm b/lib/PublicInbox/LeiNoteEvent.pm
index 18313359..5f692e75 100644
--- a/lib/PublicInbox/LeiNoteEvent.pm
+++ b/lib/PublicInbox/LeiNoteEvent.pm
@@ -36,18 +36,18 @@ sub eml_event ($$$$) {
my ($self, $eml, $vmd, $state) = @_;
my $sto = $self->{lei}->{sto};
if ($state =~ /\Aimport-(?:rw|ro)\z/) {
- $sto->ipc_do('set_eml', $eml, $vmd);
+ $sto->wq_do('set_eml', $eml, $vmd);
} elsif ($state =~ /\Aindex-(?:rw|ro)\z/) {
my $xoids = $self->{lei}->ale->xoids_for($eml);
- $sto->ipc_do('index_eml_only', $eml, $vmd, $xoids);
+ $sto->wq_do('index_eml_only', $eml, $vmd, $xoids);
} elsif ($state =~ /\Atag-(?:rw|ro)\z/) {
my $docids = [];
my $c = $self->{lse}->kw_changed($eml, $vmd->{kw}, $docids);
if (scalar @$docids) { # already in lei/store
- $sto->ipc_do('set_eml_vmd', undef, $vmd, $docids) if $c;
+ $sto->wq_do('set_eml_vmd', undef, $vmd, $docids) if $c;
} elsif (my $xoids = $self->{lei}->ale->xoids_for($eml)) {
# it's in an external, only set kw, here
- $sto->ipc_do('set_xvmd', $xoids, $eml, $vmd);
+ $sto->wq_do('set_xvmd', $xoids, $eml, $vmd);
} # else { totally unknown: ignore
} else {
warn "unknown state: $state (in $self->{lei}->{cfg}->{'-f'})\n";
diff --git a/lib/PublicInbox/LeiRemote.pm b/lib/PublicInbox/LeiRemote.pm
index 8d4ffed0..346aa6a4 100644
--- a/lib/PublicInbox/LeiRemote.pm
+++ b/lib/PublicInbox/LeiRemote.pm
@@ -28,7 +28,7 @@ sub _each_mboxrd_eml { # callback for MboxReader->mboxrd
my $xoids = $lei->{ale}->xoids_for($eml, 1);
my $smsg = bless {}, 'PublicInbox::Smsg';
if ($lei->{sto} && !$xoids) { # memoize locally
- my $res = $lei->{sto}->ipc_do('add_eml', $eml);
+ my $res = $lei->{sto}->wq_do('add_eml', $eml);
$smsg = $res if ref($res) eq ref($smsg);
}
$smsg->{blob} //= $xoids ? (keys(%$xoids))[0]
@@ -56,7 +56,7 @@ sub mset {
my $err = waitpid($pid, 0) == $pid ? undef
: "BUG: waitpid($cmd): $!";
@$reap = (); # cancel OnDestroy
- my $wait = $self->{lei}->{sto}->ipc_do('done');
+ my $wait = $self->{lei}->{sto}->wq_do('done');
die $err if $err;
$self; # we are the mset (and $ibx, and $self)
}
diff --git a/lib/PublicInbox/LeiRm.pm b/lib/PublicInbox/LeiRm.pm
index 3371f3ed..97b1c5c1 100644
--- a/lib/PublicInbox/LeiRm.pm
+++ b/lib/PublicInbox/LeiRm.pm
@@ -10,7 +10,7 @@ use parent qw(PublicInbox::IPC PublicInbox::LeiInput);
sub input_eml_cb { # used by PublicInbox::LeiInput::input_fh
my ($self, $eml) = @_;
- $self->{lei}->{sto}->ipc_do('remove_eml', $eml);
+ $self->{lei}->{sto}->wq_do('remove_eml', $eml);
}
sub input_mbox_cb { # MboxReader callback
diff --git a/lib/PublicInbox/LeiStore.pm b/lib/PublicInbox/LeiStore.pm
index 08add8f5..4ec63699 100644
--- a/lib/PublicInbox/LeiStore.pm
+++ b/lib/PublicInbox/LeiStore.pm
@@ -552,6 +552,12 @@ sub ipc_atfork_child {
$self->SUPER::ipc_atfork_child;
}
+sub recv_and_run {
+ my ($self, @args) = @_;
+ local $PublicInbox::DS::in_loop = 0; # waitpid synchronously
+ $self->SUPER::recv_and_run(@args);
+}
+
sub write_prepare {
my ($self, $lei) = @_;
$lei // die 'BUG: $lei not passed';
@@ -560,14 +566,14 @@ sub write_prepare {
require PublicInbox::PktOp;
my ($s2d_op_c, $s2d_op_p) = PublicInbox::PktOp->pair;
my $dir = $lei->store_path;
- $self->ipc_lock_init("$dir/ipc.lock");
substr($dir, -length('/lei/store'), 10, '');
pipe(my ($r, $w)) or die "pipe: $!";
$w->autoflush(1);
# Mail we import into lei are private, so headers filtered out
# by -mda for public mail are not appropriate
local @PublicInbox::MDA::BAD_HEADERS = ();
- $self->ipc_worker_spawn("lei/store $dir", $lei->oldset, {
+ $self->{-wq_no_bcast} = 1;
+ $self->wq_workers_start("lei/store $dir", 1, $lei->oldset, {
lei => $lei,
-err_wr => $w,
to_close => [ $r, $s2d_op_c->{sock} ],
diff --git a/lib/PublicInbox/LeiTag.pm b/lib/PublicInbox/LeiTag.pm
index c4f5ecff..9bbf0d79 100644
--- a/lib/PublicInbox/LeiTag.pm
+++ b/lib/PublicInbox/LeiTag.pm
@@ -12,7 +12,7 @@ sub input_eml_cb { # used by PublicInbox::LeiInput::input_fh
my ($self, $eml) = @_;
if (my $xoids = $self->{lse}->xoids_for($eml) // # tries LeiMailSync
$self->{lei}->{ale}->xoids_for($eml)) {
- $self->{lei}->{sto}->ipc_do('update_xvmd', $xoids, $eml,
+ $self->{lei}->{sto}->wq_do('update_xvmd', $xoids, $eml,
$self->{vmd_mod});
} else {
++$self->{unimported};
diff --git a/lib/PublicInbox/LeiToMail.pm b/lib/PublicInbox/LeiToMail.pm
index 9f7171fb..a419b83f 100644
--- a/lib/PublicInbox/LeiToMail.pm
+++ b/lib/PublicInbox/LeiToMail.pm
@@ -215,14 +215,14 @@ sub update_kw_maybe ($$$$) {
my $c = $lse->kw_changed($eml, $kw, my $docids = []);
my $vmd = { kw => $kw };
if (scalar @$docids) { # already in lei/store
- $lei->{sto}->ipc_do('set_eml_vmd', undef, $vmd, $docids) if $c;
+ $lei->{sto}->wq_do('set_eml_vmd', undef, $vmd, $docids) if $c;
} elsif (my $xoids = $lei->{ale}->xoids_for($eml)) {
# it's in an external, only set kw, here
- $lei->{sto}->ipc_do('set_xvmd', $xoids, $eml, $vmd);
+ $lei->{sto}->wq_do('set_xvmd', $xoids, $eml, $vmd);
} else { # never-before-seen, import the whole thing
# XXX this is critical in protecting against accidental
# data loss without --augment
- $lei->{sto}->ipc_do('set_eml', $eml, $vmd);
+ $lei->{sto}->wq_do('set_eml', $eml, $vmd);
}
}
@@ -296,7 +296,7 @@ sub _maildir_write_cb ($$) {
$lse->xsmsg_vmd($smsg) if $lse;
my $n = _buf2maildir($dst, $bref // \($eml->as_string),
$smsg, $dir);
- $sto->ipc_do('set_sync_info', $smsg->{blob}, $out, $n) if $sto;
+ $sto->wq_do('set_sync_info', $smsg->{blob}, $out, $n) if $sto;
++$lei->{-nr_write};
}
}
@@ -326,7 +326,7 @@ sub _imap_write_cb ($$) {
}
# imap_append returns UID if IMAP server has UIDPLUS extension
($sto && $uid =~ /\A[0-9]+\z/) and
- $sto->ipc_do('set_sync_info',
+ $sto->wq_do('set_sync_info',
$smsg->{blob}, $$uri, $uid + 0);
++$lei->{-nr_write};
}
@@ -360,7 +360,7 @@ sub _v2_write_cb ($$) {
my ($bref, $smsg, $eml) = @_;
$eml //= PublicInbox::Eml->new($bref);
return if $dedupe && $dedupe->is_dup($eml, $smsg);
- $lei->{v2w}->ipc_do('add', $eml); # V2Writable->add
+ $lei->{v2w}->wq_do('add', $eml); # V2Writable->add
++$lei->{-nr_write};
}
}
@@ -658,9 +658,10 @@ sub _pre_augment_v2 {
}
PublicInbox::InboxWritable->new($ibx, @creat);
$ibx->init_inbox if @creat;
- my $v2w = $lei->{v2w} = $ibx->importer;
- $v2w->ipc_lock_init("$dir/ipc.lock");
- $v2w->ipc_worker_spawn("lei/v2w $dir", $lei->oldset, { lei => $lei });
+ my $v2w = $ibx->importer;
+ $v2w->{-wq_no_bcast} = 1;
+ $v2w->wq_workers_start("lei/v2w $dir", 1, $lei->oldset, {lei => $lei});
+ $lei->{v2w} = $v2w;
return if !$lei->{opt}->{shared};
my $d = "$lei->{ale}->{git}->{git_dir}/objects";
my $al = "$dir/git/0.git/objects/info/alternates";
@@ -689,7 +690,7 @@ sub do_augment { # slow, runs in wq worker
sub post_augment {
my ($self, $lei, @args) = @_;
my $wait = $lei->{opt}->{'import-before'} ?
- $lei->{sto}->ipc_do('checkpoint', 1) : 0;
+ $lei->{sto}->wq_do('checkpoint', 1) : 0;
# _post_augment_mbox
my $m = $self->can("_post_augment_$self->{base_type}") or return;
$m->($self, $lei, @args);
@@ -774,6 +775,7 @@ sub write_mail { # via ->wq_io_do
sub wq_atexit_child {
my ($self) = @_;
+ local $PublicInbox::DS::in_loop = 0; # waitpid synchronously
my $lei = $self->{lei};
delete $self->{wcb};
$lei->{ale}->git->async_wait_all;
diff --git a/lib/PublicInbox/LeiXSearch.pm b/lib/PublicInbox/LeiXSearch.pm
index 1d49da3d..4583b067 100644
--- a/lib/PublicInbox/LeiXSearch.pm
+++ b/lib/PublicInbox/LeiXSearch.pm
@@ -269,7 +269,7 @@ sub each_remote_eml { # callback for MboxReader->mboxrd
my $xoids = $lei->{ale}->xoids_for($eml, 1);
my $smsg = bless {}, 'PublicInbox::Smsg';
if ($self->{import_sto} && !$xoids) {
- my $res = $self->{import_sto}->ipc_do('add_eml', $eml);
+ my $res = $self->{import_sto}->wq_do('add_eml', $eml);
if (ref($res) eq ref($smsg)) { # totally new message
$smsg = $res;
$smsg->{kw} = []; # short-circuit xsmsg_vmd
@@ -369,7 +369,7 @@ sub query_remote_mboxrd {
@$reap_curl = (); # cancel OnDestroy
die $err if $err;
my $nr = $lei->{-nr_remote_eml};
- my $wait = $lei->{sto}->ipc_do('done') if $nr && $lei->{sto};
+ my $wait = $lei->{sto}->wq_do('done') if $nr && $lei->{sto};
if ($? == 0) {
# don't update if no results, maybe MTA is down
$key && $nr and
@@ -413,7 +413,7 @@ sub query_done { # EOF callback for main daemon
warn "BUG: {sto} missing with --mail-sync";
}
$lei->sto_done_request if $lei->{sto};
- my $wait = $lei->{v2w} ? $lei->{v2w}->ipc_do('done') : undef;
+ my $wait = $lei->{v2w} ? $lei->{v2w}->wq_do('done') : undef;
$lei->{ovv}->ovv_end($lei);
my $start_mua;
if ($l2m) { # close() calls LeiToMail reap_compress
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 04/16] lei: simplify sto_done_request
2021-09-19 12:50 [PATCH 00/16] lei IPC overhaul, NNTP fixes Eric Wong
` (2 preceding siblings ...)
2021-09-19 12:50 ` [PATCH 03/16] lei/store: use SOCK_SEQPACKET rather than pipe Eric Wong
@ 2021-09-19 12:50 ` Eric Wong
2021-09-19 12:50 ` [PATCH 05/16] lei_xsearch: drop Data::Dumper use Eric Wong
` (11 subsequent siblings)
15 siblings, 0 replies; 19+ messages in thread
From: Eric Wong @ 2021-09-19 12:50 UTC (permalink / raw)
To: meta
With the switch from pipes to sockets for lei-daemon =>
lei/store IPC, we can send the script/lei client socket to the
lei/store process and rely on reference counting in both Perl
and the kernel to persist the script/lei.
---
lib/PublicInbox/LEI.pm | 13 ++-----------
lib/PublicInbox/LeiRefreshMailSync.pm | 2 +-
lib/PublicInbox/LeiStore.pm | 13 +------------
3 files changed, 4 insertions(+), 24 deletions(-)
diff --git a/lib/PublicInbox/LEI.pm b/lib/PublicInbox/LEI.pm
index 549b855b..f62e82dc 100644
--- a/lib/PublicInbox/LEI.pm
+++ b/lib/PublicInbox/LEI.pm
@@ -40,7 +40,6 @@ $GLP_PASS->configure(qw(gnu_getopt no_ignore_case auto_abbrev pass_through));
our %PATH2CFG; # persistent for socket daemon
our $MDIR2CFGPATH; # /path/to/maildir => { /path/to/config => [ ino watches ] }
-our %LIVE_SOCK; # "GLOB(0x....)" => $lei->{sock}
# TBD: this is a documentation mechanism to show a subcommand
# (may) pass options through to another command:
@@ -580,7 +579,6 @@ sub _lei_atfork_child {
$dir_idle->force_close if $dir_idle;
%PATH2CFG = ();
$MDIR2CFGPATH = {};
- %LIVE_SOCK = ();
eval 'no warnings; undef $PublicInbox::LeiNoteEvent::to_flush';
undef $errors_log;
$quit = \&CORE::exit;
@@ -619,7 +617,6 @@ sub pkt_ops {
$ops->{x_it} = [ \&x_it, $lei ];
$ops->{child_error} = [ \&child_error, $lei ];
$ops->{incr} = [ \&incr, $lei ];
- $ops->{sto_done_request} = [ \&sto_done_request, $lei, $lei->{sock} ];
$ops;
}
@@ -1496,12 +1493,11 @@ sub lms {
(-f $f || $rw) ? PublicInbox::LeiMailSync->new($f) : undef;
}
-sub sto_done_request { # only call this from lei-daemon process (not workers)
+sub sto_done_request {
my ($lei, $sock) = @_;
eval {
if ($sock //= $lei->{sock}) { # issue, async wait
- $LIVE_SOCK{"$sock"} = $sock;
- $lei->{sto}->wq_do('done', "$sock");
+ $lei->{sto}->wq_io_do('done', [ $sock ]);
} else { # forcibly wait
my $wait = $lei->{sto}->wq_do('done');
}
@@ -1509,9 +1505,4 @@ sub sto_done_request { # only call this from lei-daemon process (not workers)
$lei->err($@) if $@;
}
-sub sto_done_complete { # called in lei-daemon when LeiStore->done is complete
- my ($sock_str) = @_;
- delete $LIVE_SOCK{$sock_str}; # frees {sock} for waiting lei clients
-}
-
1;
diff --git a/lib/PublicInbox/LeiRefreshMailSync.pm b/lib/PublicInbox/LeiRefreshMailSync.pm
index 72b8fe63..2f105005 100644
--- a/lib/PublicInbox/LeiRefreshMailSync.pm
+++ b/lib/PublicInbox/LeiRefreshMailSync.pm
@@ -60,7 +60,7 @@ sub input_path_url { # overrides PublicInbox::LeiInput::input_path_url
$self->folder_missing($$uri);
}
} else { die "BUG: $input not supported" }
- $self->{lei}->{pkt_op_p}->pkt_do('sto_done_request');
+ $self->{lei}->sto_done_request;
}
sub lei_refresh_mail_sync {
diff --git a/lib/PublicInbox/LeiStore.pm b/lib/PublicInbox/LeiStore.pm
index 4ec63699..164a9f2d 100644
--- a/lib/PublicInbox/LeiStore.pm
+++ b/lib/PublicInbox/LeiStore.pm
@@ -534,10 +534,6 @@ sub done {
$self->{priv_eidx}->done; # V2Writable::done
xchg_stderr($self);
die $err if $err;
-
- # notify clients ->done has been issued
- defined($sock_ref) and
- $self->{s2d_op_p}->pkt_do('sto_done_complete', $sock_ref);
}
sub ipc_atfork_child {
@@ -562,9 +558,6 @@ sub write_prepare {
my ($self, $lei) = @_;
$lei // die 'BUG: $lei not passed';
unless ($self->{-ipc_req}) {
- # s2d => store-to-daemon messages
- require PublicInbox::PktOp;
- my ($s2d_op_c, $s2d_op_p) = PublicInbox::PktOp->pair;
my $dir = $lei->store_path;
substr($dir, -length('/lei/store'), 10, '');
pipe(my ($r, $w)) or die "pipe: $!";
@@ -576,14 +569,10 @@ sub write_prepare {
$self->wq_workers_start("lei/store $dir", 1, $lei->oldset, {
lei => $lei,
-err_wr => $w,
- to_close => [ $r, $s2d_op_c->{sock} ],
- s2d_op_p => $s2d_op_p,
+ to_close => [ $r ],
});
require PublicInbox::LeiStoreErr;
PublicInbox::LeiStoreErr->new($r, $lei);
- $s2d_op_c->{ops} = {
- sto_done_complete => [ $lei->can('sto_done_complete') ]
- };
}
$lei->{sto} = $self;
}
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 05/16] lei_xsearch: drop Data::Dumper use
2021-09-19 12:50 [PATCH 00/16] lei IPC overhaul, NNTP fixes Eric Wong
` (3 preceding siblings ...)
2021-09-19 12:50 ` [PATCH 04/16] lei: simplify sto_done_request Eric Wong
@ 2021-09-19 12:50 ` Eric Wong
2021-09-19 12:50 ` [PATCH 06/16] ipc: drop dynamic WQ process counts Eric Wong
` (10 subsequent siblings)
15 siblings, 0 replies; 19+ messages in thread
From: Eric Wong @ 2021-09-19 12:50 UTC (permalink / raw)
To: meta
We're not using Data::Dumper for JSON output.
---
lib/PublicInbox/LeiXSearch.pm | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/lib/PublicInbox/LeiXSearch.pm b/lib/PublicInbox/LeiXSearch.pm
index 4583b067..756183a9 100644
--- a/lib/PublicInbox/LeiXSearch.pm
+++ b/lib/PublicInbox/LeiXSearch.pm
@@ -633,7 +633,7 @@ sub _lcat2smsg { # git->cat_async callback
}
}
-sub lcat_dump {
+sub lcat_dump { # via wq_io_do
my ($self) = @_;
my $lei = $self->{lei};
my $each_smsg = $lei->{ovv}->ovv_each_smsg_cb($lei);
@@ -642,7 +642,6 @@ sub lcat_dump {
my $json_dump = $each_smsg;
$each_smsg = sub {
my ($smsg) = @_;
- use Data::Dumper;
$smsg->{-json_dump} = $json_dump;
$git->cat_async($smsg->{blob}, \&_lcat2smsg, $smsg);
};
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 06/16] ipc: drop dynamic WQ process counts
2021-09-19 12:50 [PATCH 00/16] lei IPC overhaul, NNTP fixes Eric Wong
` (4 preceding siblings ...)
2021-09-19 12:50 ` [PATCH 05/16] lei_xsearch: drop Data::Dumper use Eric Wong
@ 2021-09-19 12:50 ` Eric Wong
2021-09-19 12:50 ` [PATCH 07/16] lei: clamp internal worker processes to 4 Eric Wong
` (9 subsequent siblings)
15 siblings, 0 replies; 19+ messages in thread
From: Eric Wong @ 2021-09-19 12:50 UTC (permalink / raw)
To: meta
In retrospect, I don't think it's needed; and trying to wire up
a user interface for lei to manage process counts doesn't seem
worthwhile. It could be resurrected for public-facing daemon
use in the future, but that's what version control systems are for.
This also lets us automatically avoid setting up broadcast
sockets
Followup-to: 7b7939d47b336fb7 ("lei: lock worker counts")
---
lib/PublicInbox/IPC.pm | 69 ++++++-------------------------------
lib/PublicInbox/LEI.pm | 1 -
lib/PublicInbox/LeiStore.pm | 1 -
t/ipc.t | 13 +------
4 files changed, 11 insertions(+), 73 deletions(-)
diff --git a/lib/PublicInbox/IPC.pm b/lib/PublicInbox/IPC.pm
index 92f35189..add5f3df 100644
--- a/lib/PublicInbox/IPC.pm
+++ b/lib/PublicInbox/IPC.pm
@@ -262,9 +262,10 @@ sub do_sock_stream { # via wq_io_do, for big requests
sub wq_broadcast {
my ($self, $sub, @args) = @_;
if (my $wkr = $self->{-wq_workers}) {
+ my $buf = ipc_freeze([$sub, @args]);
for my $bcast1 (values %$wkr) {
- my $buf = ipc_freeze([$sub, @args]);
- send($bcast1, $buf, MSG_EOR) // croak "send: $!";
+ my $sock = $bcast1 // $self->{-wq_s1} // next;
+ send($sock, $buf, MSG_EOR) // croak "send: $!";
# XXX shouldn't have to deal with EMSGSIZE here...
}
} else {
@@ -336,11 +337,10 @@ sub wq_do {
}
}
-sub _wq_worker_start ($$$) {
- my ($self, $oldset, $fields) = @_;
+sub _wq_worker_start ($$$$) {
+ my ($self, $oldset, $fields, $one) = @_;
my ($bcast1, $bcast2);
- $self->{-wq_no_bcast} or
- socketpair($bcast1, $bcast2, AF_UNIX, $SEQPACKET, 0) or
+ $one or socketpair($bcast1, $bcast2, AF_UNIX, $SEQPACKET, 0) or
die "socketpair: $!";
my $seed = rand(0xffffffff);
my $pid = fork // die "fork: $!";
@@ -380,66 +380,17 @@ sub wq_workers_start {
socketpair($self->{-wq_s1}, $self->{-wq_s2}, AF_UNIX, $SEQPACKET, 0) or
die "socketpair: $!";
$self->ipc_atfork_prepare;
- $nr_workers //= $self->{-wq_nr_workers};
+ $nr_workers //= $self->{-wq_nr_workers}; # was set earlier
my $sigset = $oldset // PublicInbox::DS::block_signals();
$self->{-wq_workers} = {};
$self->{-wq_ident} = $ident;
- _wq_worker_start($self, $sigset, $fields) for (1..$nr_workers);
+ my $one = $nr_workers == 1;
+ $self->{-wq_nr_workers} = $nr_workers;
+ _wq_worker_start($self, $sigset, $fields, $one) for (1..$nr_workers);
PublicInbox::DS::sig_setmask($sigset) unless $oldset;
$self->{-wq_ppid} = $$;
}
-sub wq_worker_incr { # SIGTTIN handler
- my ($self, $oldset, $fields) = @_;
- $self->{-wq_s2} or return;
- die "-wq_nr_workers locked" if defined $self->{-wq_nr_workers};
- $self->ipc_atfork_prepare;
- my $sigset = $oldset // PublicInbox::DS::block_signals();
- _wq_worker_start($self, $sigset, $fields);
- PublicInbox::DS::sig_setmask($sigset) unless $oldset;
-}
-
-sub wq_exit { # wakes up wq_worker_decr_wait
- send($_[0]->{-wq_s2}, $$, MSG_EOR) // die "$$ send: $!";
- exit;
-}
-
-sub wq_worker_decr { # SIGTTOU handler, kills first idle worker
- my ($self) = @_;
- return unless wq_workers($self);
- die "-wq_nr_workers locked" if defined $self->{-wq_nr_workers};
- $self->wq_io_do('wq_exit');
- # caller must call wq_worker_decr_wait in main loop
-}
-
-sub wq_worker_decr_wait {
- my ($self, $timeout, $cb, @args) = @_;
- return if $self->{-wq_ppid} != $$; # can't reap siblings or parents
- die "-wq_nr_workers locked" if defined $self->{-wq_nr_workers};
- my $s1 = $self->{-wq_s1} // croak 'BUG: no wq_s1';
- vec(my $rin = '', fileno($s1), 1) = 1;
- select(my $rout = $rin, undef, undef, $timeout) or
- croak 'timed out waiting for wq_exit';
- recv($s1, my $pid, 64, 0) // croak "recv: $!";
- my $workers = $self->{-wq_workers} // croak 'BUG: no wq_workers';
- delete $workers->{$pid} // croak "BUG: PID:$pid invalid";
- dwaitpid($pid, $cb // \&ipc_worker_reap, [ $self, @args ]);
-}
-
-# set or retrieve number of workers
-sub wq_workers {
- my ($self, $nr, $cb, @args) = @_;
- my $cur = $self->{-wq_workers} or return;
- if (defined $nr) {
- while (scalar(keys(%$cur)) > $nr) {
- $self->wq_worker_decr;
- $self->wq_worker_decr_wait(undef, $cb, @args);
- }
- $self->wq_worker_incr while scalar(keys(%$cur)) < $nr;
- }
- scalar(keys(%$cur));
-}
-
sub wq_close {
my ($self, $nohang, $cb, @args) = @_;
delete @$self{qw(-wq_s1 -wq_s2)} or return;
diff --git a/lib/PublicInbox/LEI.pm b/lib/PublicInbox/LEI.pm
index f62e82dc..def85ef1 100644
--- a/lib/PublicInbox/LEI.pm
+++ b/lib/PublicInbox/LEI.pm
@@ -627,7 +627,6 @@ sub workers_start {
my $end = $lei->pkt_op_pair;
my $ident = $wq->{-wq_ident} // "lei-$lei->{cmd} worker";
$flds->{lei} = $lei;
- $wq->{-wq_nr_workers} //= $jobs; # lock, no incrementing
$wq->wq_workers_start($ident, $jobs, $lei->oldset, $flds);
delete $lei->{pkt_op_p};
my $op_c = delete $lei->{pkt_op_c};
diff --git a/lib/PublicInbox/LeiStore.pm b/lib/PublicInbox/LeiStore.pm
index 164a9f2d..b4f40912 100644
--- a/lib/PublicInbox/LeiStore.pm
+++ b/lib/PublicInbox/LeiStore.pm
@@ -565,7 +565,6 @@ sub write_prepare {
# Mail we import into lei are private, so headers filtered out
# by -mda for public mail are not appropriate
local @PublicInbox::MDA::BAD_HEADERS = ();
- $self->{-wq_no_bcast} = 1;
$self->wq_workers_start("lei/store $dir", 1, $lei->oldset, {
lei => $lei,
-err_wr => $w,
diff --git a/t/ipc.t b/t/ipc.t
index 202b1cc6..ce89f94b 100644
--- a/t/ipc.t
+++ b/t/ipc.t
@@ -180,24 +180,13 @@ SKIP: {
is($warn[2], $warn[1], 'worker did not die');
$SIG{__WARN__} = 'DEFAULT';
- is($ipc->wq_workers_start('wq', 1), $$, 'workers started again');
- is($ipc->wq_workers, 1, '1 worker started');
-
- $ipc->wq_worker_incr;
- is($ipc->wq_workers, 2, 'worker count bumped');
- $ipc->wq_worker_decr;
- $ipc->wq_worker_decr_wait(10);
- is($ipc->wq_workers, 1, 'worker count lowered');
- is($ipc->wq_workers(2), 2, 'worker count set');
- is($ipc->wq_workers, 2, 'worker count stayed set');
-
+ is($ipc->wq_workers_start('wq', 2), $$, 'workers started again');
$ipc->wq_broadcast('test_append_pid', "$tmpdir/append_pid");
$ipc->wq_close;
open my $fh, '<', "$tmpdir/append_pid" or BAIL_OUT "open: $!";
chomp(my @pids = <$fh>);
my %pids = map { $_ => 1 } grep(/\A[0-9]+\z/, @pids);
is(scalar keys %pids, 2, 'broadcast hit both PIDs');
- is($ipc->wq_workers, undef, 'workers undef after close');
}
done_testing;
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 07/16] lei: clamp internal worker processes to 4
2021-09-19 12:50 [PATCH 00/16] lei IPC overhaul, NNTP fixes Eric Wong
` (5 preceding siblings ...)
2021-09-19 12:50 ` [PATCH 06/16] ipc: drop dynamic WQ process counts Eric Wong
@ 2021-09-19 12:50 ` Eric Wong
2021-09-19 12:50 ` [PATCH 08/16] lei ls-mail-source: use "high"/"low" for NNTP Eric Wong
` (8 subsequent siblings)
15 siblings, 0 replies; 19+ messages in thread
From: Eric Wong @ 2021-09-19 12:50 UTC (permalink / raw)
To: meta
"All" my CPUs is only 4, but it's probably ridiculous for
somebody with a 16-core system to have 16 processes for
accessing SQLite DBs.
We do the same thing in Pmdir for parallel Maildir access
(and V2Writable).
---
lib/PublicInbox/LeiImportKw.pm | 4 +++-
lib/PublicInbox/LeiNoteEvent.pm | 3 ++-
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/lib/PublicInbox/LeiImportKw.pm b/lib/PublicInbox/LeiImportKw.pm
index 2863d17f..379101c2 100644
--- a/lib/PublicInbox/LeiImportKw.pm
+++ b/lib/PublicInbox/LeiImportKw.pm
@@ -11,7 +11,9 @@ use parent qw(PublicInbox::IPC);
sub new {
my ($cls, $lei) = @_;
my $self = bless { -wq_ident => 'lei import_kw worker' }, $cls;
- my ($op_c, $ops) = $lei->workers_start($self, $self->detect_nproc);
+ my $j = $self->detect_nproc // 4;
+ $j = 4 if $j > 4;
+ my ($op_c, $ops) = $lei->workers_start($self, $j);
$op_c->{ops} = $ops; # for PktOp->event_step
$self->{lei_sock} = $lei->{sock};
$lei->{ikw} = $self;
diff --git a/lib/PublicInbox/LeiNoteEvent.pm b/lib/PublicInbox/LeiNoteEvent.pm
index 5f692e75..a0591a09 100644
--- a/lib/PublicInbox/LeiNoteEvent.pm
+++ b/lib/PublicInbox/LeiNoteEvent.pm
@@ -80,8 +80,9 @@ sub lei_note_event {
my $self = $cfg->{-lei_note_event} //= do {
my $wq = bless { lms => $lms }, __PACKAGE__;
# MUAs such as mutt can trigger massive rename() storms so
- # use all CPU power available:
+ # use some CPU, but don't overwhelm slower storage, either
my $jobs = $wq->detect_nproc // 1;
+ $jobs = 4 if $jobs > 4; # same default as V2Writable
my ($op_c, $ops) = $lei->workers_start($wq, $jobs);
$lei->wait_wq_events($op_c, $ops);
note_event_arm_done($lei);
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 08/16] lei ls-mail-source: use "high"/"low" for NNTP
2021-09-19 12:50 [PATCH 00/16] lei IPC overhaul, NNTP fixes Eric Wong
` (6 preceding siblings ...)
2021-09-19 12:50 ` [PATCH 07/16] lei: clamp internal worker processes to 4 Eric Wong
@ 2021-09-19 12:50 ` Eric Wong
2021-09-19 12:50 ` [PATCH 09/16] lei ls-mail-source: pretty JSON support Eric Wong
` (7 subsequent siblings)
15 siblings, 0 replies; 19+ messages in thread
From: Eric Wong @ 2021-09-19 12:50 UTC (permalink / raw)
To: meta
The meanings of "hwm" and "lwm" may not be obvious abbreviations
for (high|low) water mark descriptions used by RFC 3977.
"high" and "low" should be obvious to anyone.
---
lib/PublicInbox/LeiLsMailSource.pm | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/lib/PublicInbox/LeiLsMailSource.pm b/lib/PublicInbox/LeiLsMailSource.pm
index bcb1838e..a2e75e94 100644
--- a/lib/PublicInbox/LeiLsMailSource.pm
+++ b/lib/PublicInbox/LeiLsMailSource.pm
@@ -59,10 +59,10 @@ sub input_path_url { # overrides LeiInput version
# <https://rt.cpan.org/Ticket/Display.html?id=129966>
$desc =~ s/\r\z//;
- my ($hwm, $lwm, $status) = @{$all->{$ng}};
+ my ($high, $low, $status) = @{$all->{$ng}};
push @x, { name => $ng, url => "$sec/$ng",
- lwm => $lwm + 0,
- hwm => $hwm + 0, status => $status,
+ low => $low + 0,
+ high => $high + 0, status => $status,
description => $desc };
}
@f = map { "$sec/$_" } keys %$all;
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 09/16] lei ls-mail-source: pretty JSON support
2021-09-19 12:50 [PATCH 00/16] lei IPC overhaul, NNTP fixes Eric Wong
` (7 preceding siblings ...)
2021-09-19 12:50 ` [PATCH 08/16] lei ls-mail-source: use "high"/"low" for NNTP Eric Wong
@ 2021-09-19 12:50 ` Eric Wong
2021-09-19 12:50 ` [PATCH 10/16] net_reader: fix single NNTP article fetch, test ranges Eric Wong
` (6 subsequent siblings)
15 siblings, 0 replies; 19+ messages in thread
From: Eric Wong @ 2021-09-19 12:50 UTC (permalink / raw)
To: meta
As with other commands, we enable pretty JSON by default if
stdout is a terminal or if --pretty is specified. While the
->pretty JSON output has excessive vertical whitespace, too many
lines is preferable to having everything on one line.
---
lib/PublicInbox/LEI.pm | 2 +-
lib/PublicInbox/LeiLsMailSource.pm | 19 ++++++++++---------
2 files changed, 11 insertions(+), 10 deletions(-)
diff --git a/lib/PublicInbox/LEI.pm b/lib/PublicInbox/LEI.pm
index def85ef1..b468a32c 100644
--- a/lib/PublicInbox/LEI.pm
+++ b/lib/PublicInbox/LEI.pm
@@ -214,7 +214,7 @@ our %CMD = ( # sorted in order of importance/use:
'ls-mail-sync' => [ '[FILTER]', 'list mail sync folders',
qw(z|0 globoff|g invert-match|v local remote), @c_opt ],
'ls-mail-source' => [ 'URL', 'list IMAP or NNTP mail source folders',
- qw(z|0 ascii l url), @c_opt ],
+ qw(z|0 ascii l pretty url), @c_opt ],
'forget-external' => [ 'LOCATION...|--prune',
'exclude further results from a publicinbox|extindex',
qw(prune), @c_opt ],
diff --git a/lib/PublicInbox/LeiLsMailSource.pm b/lib/PublicInbox/LeiLsMailSource.pm
index a2e75e94..2265969a 100644
--- a/lib/PublicInbox/LeiLsMailSource.pm
+++ b/lib/PublicInbox/LeiLsMailSource.pm
@@ -12,15 +12,9 @@ use parent qw(PublicInbox::IPC PublicInbox::LeiInput);
sub input_path_url { # overrides LeiInput version
my ($self, $url) = @_;
# TODO: support ndjson and other JSONs we support elsewhere
- my $json;
my $lei = $self->{lei};
- my $ORS = "\n";
- if ($self->{lei}->{opt}->{l}) {
- $json = ref(PublicInbox::Config->json)->new->utf8->canonical;
- $json->ascii(1) if $lei->{opt}->{ascii};
- } elsif ($self->{lei}->{opt}->{z}) {
- $ORS = "\0";
- }
+ my $json = $lei->{json};
+ my $ORS = $self->{lei}->{opt}->{z} ? "\0" : "\n";
my @f;
if ($url =~ m!\Aimaps?://!i) {
my $uri = PublicInbox::URIimap->new($url);
@@ -93,7 +87,14 @@ sub lei_ls_mail_source {
my $self = bless { pfx => $pfx, -ls_ok => 1 }, __PACKAGE__;
$self->{cfg} = $lei->_lei_cfg; # may be undef
$self->prepare_inputs($lei, [ $url ]) or return;
- $lei->start_pager if -t $lei->{1};
+ my $isatty = -t $lei->{1};
+ if ($lei->{opt}->{l}) {
+ my $json = ref(PublicInbox::Config->json)->new->utf8->canonical;
+ $lei->{json} = $json;
+ $json->ascii(1) if $lei->{opt}->{ascii};
+ $json->pretty(1)->indent(2) if $isatty || $lei->{opt}->{pretty};
+ }
+ $lei->start_pager if $isatty;
my $ops = {};
$lei->{auth}->op_merge($ops, $self);
(my $op_c, $ops) = $lei->workers_start($self, 1, $ops);
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 10/16] net_reader: fix single NNTP article fetch, test ranges
2021-09-19 12:50 [PATCH 00/16] lei IPC overhaul, NNTP fixes Eric Wong
` (8 preceding siblings ...)
2021-09-19 12:50 ` [PATCH 09/16] lei ls-mail-source: pretty JSON support Eric Wong
@ 2021-09-19 12:50 ` Eric Wong
2021-09-19 12:50 ` [PATCH 11/16] xt: add fsck script over over.sqlite3 Eric Wong
` (5 subsequent siblings)
15 siblings, 0 replies; 19+ messages in thread
From: Eric Wong @ 2021-09-19 12:50 UTC (permalink / raw)
To: meta
While NNTP ranges was already working, fetching a single message
was broken. We'll also simplify the code a bit and ensure
incremental synchronization is ignored when ranges are
specified.
---
lib/PublicInbox/NetReader.pm | 15 +++++++++------
t/lei-import-nntp.t | 26 ++++++++++++++++++++++++++
t/uri_nntps.t | 3 +++
3 files changed, 38 insertions(+), 6 deletions(-)
diff --git a/lib/PublicInbox/NetReader.pm b/lib/PublicInbox/NetReader.pm
index 30b8f810..236e824c 100644
--- a/lib/PublicInbox/NetReader.pm
+++ b/lib/PublicInbox/NetReader.pm
@@ -725,21 +725,24 @@ sub _nntp_fetch_all ($$$) {
my $msg = ndump($nn->message);
return "E: GROUP $group <$sec> $msg";
}
+ (defined($num_a) && defined($num_b) && $num_a > $num_b) and
+ return "E: $uri: backwards range: $num_a > $num_b";
# IMAPTracker is also used for tracking NNTP, UID == article number
# LIST.ACTIVE can get the equivalent of UIDVALIDITY, but that's
# expensive. So we assume newsgroups don't change:
my ($itrk, $l_art) = itrk_last($self, $uri);
- # allow users to specify articles to refetch
- # cf. https://tools.ietf.org/id/draft-gilman-news-url-01.txt
- # nntp://example.com/inbox.foo/$num_a-$num_b
- $beg = $num_a if defined($num_a) && $num_a < $beg;
- $end = $num_b if defined($num_b) && $num_b < $end;
- if (defined $l_art) {
+ if (defined($l_art) && !defined($num_a)) {
return if $l_art >= $end; # nothing to do
$beg = $l_art + 1;
}
+ # allow users to specify articles to refetch
+ # cf. https://tools.ietf.org/id/draft-gilman-news-url-01.txt
+ # nntp://example.com/inbox.foo/$num_a-$num_b
+ $beg = $num_a if defined($num_a) && $num_a > $beg && $num_a <= $end;
+ $end = $num_b if defined($num_b) && $num_b >= $beg && $num_b < $end;
+ $end = $beg if defined($num_a) && !defined($num_b);
my ($err, $art, $last_art, $kw); # kw stays undef, no keywords in NNTP
unless ($self->{quiet}) {
warn "# $uri fetching ARTICLE $beg..$end\n";
diff --git a/t/lei-import-nntp.t b/t/lei-import-nntp.t
index f2c35406..1eb41e0e 100644
--- a/t/lei-import-nntp.t
+++ b/t/lei-import-nntp.t
@@ -37,5 +37,31 @@ test_lei({ tmpdir => $tmpdir }, sub {
ok(-s $f, 'mail_sync exists tracked for redundant imports');
lei_ok 'ls-mail-sync';
like($lei_out, qr!\A\Q$url\E\n\z!, 'ls-mail-sync output as-expected');
+
+ ok(!lei(qw(import), "$url/12-1"), 'backwards range rejected');
+
+ # new home
+ local $ENV{HOME} = "$tmpdir/h2";
+ lei_ok(qw(ls-mail-source -l), $url);
+ my $ls = json_utf8->decode($lei_out);
+ my ($high, $low) = @{$ls->[0]}{qw(high low)};
+ ok($high > $low, 'high > low');
+
+ my $end = $high - 1;
+ lei_ok qw(import), "$url/$high";
+ lei_ok qw(q z:0..); my $one = json_utf8->decode($lei_out);
+ pop @$one; # trailing null
+ is(scalar(@$one), 1, 'only 1 result');
+
+ local $ENV{HOME} = "$tmpdir/h3";
+ lei_ok qw(import), "$url/$low-$end";
+ lei_ok qw(q z:0..); my $start = json_utf8->decode($lei_out);
+ pop @$start; # trailing null
+ is(scalar(@$start), scalar(map { $_ } ($low..$end)),
+ 'range worked as expected');
+ my %seen;
+ for (@$start, @$one) {
+ is($seen{$_->{blob}}++, 0, "blob $_->{blob} seen once");
+ }
});
done_testing;
diff --git a/t/uri_nntps.t b/t/uri_nntps.t
index babd8088..6b123a9b 100644
--- a/t/uri_nntps.t
+++ b/t/uri_nntps.t
@@ -37,4 +37,7 @@ is(PublicInbox::URInntps->new('nntps://0:563/')->canonical->as_string,
$uri = PublicInbox::URInntps->new('nntps://NSA:Hunter2@0/inbox');
is($uri->userinfo, 'NSA:Hunter2', 'userinfo accepted w/ pass');
+$uri = PublicInbox::URInntps->new('nntps://NSA:Hunter2@0/inbox.test/9-10');
+is_deeply([$uri->group], [ 'inbox.test', 9, 10 ], 'ranges work');
+
done_testing;
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 11/16] xt: add fsck script over over.sqlite3
2021-09-19 12:50 [PATCH 00/16] lei IPC overhaul, NNTP fixes Eric Wong
` (9 preceding siblings ...)
2021-09-19 12:50 ` [PATCH 10/16] net_reader: fix single NNTP article fetch, test ranges Eric Wong
@ 2021-09-19 12:50 ` Eric Wong
2021-09-19 12:50 ` [PATCH 12/16] watch: use net_reader->mic_new wrapper for SOCKS+TLS Eric Wong
` (4 subsequent siblings)
15 siblings, 0 replies; 19+ messages in thread
From: Eric Wong @ 2021-09-19 12:50 UTC (permalink / raw)
To: meta
I'm not sure what caused it, but I've noticed two missing
messages that failed from "lei up" on an https:// external;
and I've also seen some duplicates in the past (which I
think I fixed...).
---
MANIFEST | 1 +
xt/over-fsck.perl | 44 ++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 45 insertions(+)
create mode 100644 xt/over-fsck.perl
diff --git a/MANIFEST b/MANIFEST
index 218e20e9..2df743f8 100644
--- a/MANIFEST
+++ b/MANIFEST
@@ -568,6 +568,7 @@ xt/msgtime_cmp.t
xt/net_nntp_socks.t
xt/net_writer-imap.t
xt/nntpd-validate.t
+xt/over-fsck.perl
xt/perf-msgview.t
xt/perf-nntpd.t
xt/perf-obfuscate.t
diff --git a/xt/over-fsck.perl b/xt/over-fsck.perl
new file mode 100644
index 00000000..053204fe
--- /dev/null
+++ b/xt/over-fsck.perl
@@ -0,0 +1,44 @@
+#!perl -w
+# Copyright (C) all contributors <meta@public-inbox.org>
+# License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
+# unstable dev script, chasing a bug which may be in LeiSavedSearch->is_dup
+use v5.12;
+use Data::Dumper;
+use PublicInbox::OverIdx;
+@ARGV == 1 or die "Usage: $0 /path/to/over.sqlite3\n";
+my $over = PublicInbox::OverIdx->new($ARGV[0]);
+my $dbh = $over->dbh;
+$dbh->do('PRAGMA mmap_size = '.(2 ** 48));
+my $num = 0;
+my ($err, $none, $nr, $ids);
+$Data::Dumper::Useqq = $Data::Dumper::Sortkeys = 1;
+do {
+ $ids = $over->ids_after(\$num);
+ $nr += @$ids;
+ for my $n (@$ids) {
+ my $smsg = $over->get_art($n);
+ if (!$smsg) {
+ warn "#$n article missing\n";
+ ++$err;
+ next;
+ }
+ my $exp = $smsg->{blob};
+ if ($exp eq '') {
+ ++$none if $smsg->{bytes};
+ next;
+ }
+ my $xr3 = $over->get_xref3($n, 1);
+ my $found;
+ for my $r (@$xr3) {
+ $r->[2] = unpack('H*', $r->[2]);
+ $found = 1 if $r->[2] eq $exp;
+ }
+ if (!$found) {
+ warn Dumper([$smsg, $xr3 ]);
+ ++$err;
+ }
+ }
+} while (@$ids);
+warn "$none/$nr had no blob (external?)\n" if $none;
+warn "$err errors\n" if $err;
+exit($err ? 1 : 0);
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 12/16] watch: use net_reader->mic_new wrapper for SOCKS+TLS
2021-09-19 12:50 [PATCH 00/16] lei IPC overhaul, NNTP fixes Eric Wong
` (10 preceding siblings ...)
2021-09-19 12:50 ` [PATCH 11/16] xt: add fsck script over over.sqlite3 Eric Wong
@ 2021-09-19 12:50 ` Eric Wong
2021-09-19 12:50 ` [PATCH 13/16] net_reader: no STARTTLS for IMAP localhost or onions Eric Wong
` (3 subsequent siblings)
15 siblings, 0 replies; 19+ messages in thread
From: Eric Wong @ 2021-09-19 12:50 UTC (permalink / raw)
To: meta
This brings -watch up to feature parity with lei with
SOCKS support.
---
lib/PublicInbox/Watch.pm | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/lib/PublicInbox/Watch.pm b/lib/PublicInbox/Watch.pm
index 43ee0714..387eb6d2 100644
--- a/lib/PublicInbox/Watch.pm
+++ b/lib/PublicInbox/Watch.pm
@@ -358,7 +358,8 @@ sub watch_imap_idle_1 ($$$) {
my $mic;
local $0 = $uri->mailbox." $sec";
until ($self->{quit}) {
- $mic //= PublicInbox::IMAPClient->new(%$mic_arg,Keepalive => 1);
+ $mic //= PublicInbox::NetReader::mic_new(
+ $self, $mic_arg, $sec, $uri);
my $err;
if ($mic && $mic->IsConnected) {
local $self->{mics_cached}->{$sec} = $mic;
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 13/16] net_reader: no STARTTLS for IMAP localhost or onions
2021-09-19 12:50 [PATCH 00/16] lei IPC overhaul, NNTP fixes Eric Wong
` (11 preceding siblings ...)
2021-09-19 12:50 ` [PATCH 12/16] watch: use net_reader->mic_new wrapper for SOCKS+TLS Eric Wong
@ 2021-09-19 12:50 ` Eric Wong
2021-09-19 12:50 ` [PATCH 14/16] lei config --edit: use controlling terminal Eric Wong
` (2 subsequent siblings)
15 siblings, 0 replies; 19+ messages in thread
From: Eric Wong @ 2021-09-19 12:50 UTC (permalink / raw)
To: meta
At least not by default, to match existing NNTP behavior.
Tor .onions are already encrypted, and there's no point
in encrypting traffic on localhost outside of testing.
---
lib/PublicInbox/NetReader.pm | 20 +++++++++++---------
t/imapd-tls.t | 11 +++++++++--
t/nntpd-tls.t | 8 ++++++++
3 files changed, 28 insertions(+), 11 deletions(-)
diff --git a/lib/PublicInbox/NetReader.pm b/lib/PublicInbox/NetReader.pm
index 236e824c..e305523e 100644
--- a/lib/PublicInbox/NetReader.pm
+++ b/lib/PublicInbox/NetReader.pm
@@ -91,6 +91,16 @@ try configuring a socks5h:// proxy:
EOM
}
+# Net::NNTP doesn't support CAPABILITIES, yet; and both IMAP+NNTP
+# servers may have multiple listen sockets.
+sub try_starttls ($) {
+ my ($host) = @_;
+ return if $host =~ /\.onion\z/si;
+ return if $host =~ /\A127\.[0-9]+\.[0-9]+\.[0-9]+\z/s;
+ return if $host eq '::1';
+ 1;
+}
+
# mic_for may prompt the user and store auth info, prepares mic_get
sub mic_for ($$$$) { # mic = Mail::IMAPClient
my ($self, $uri, $mic_common, $lei) = @_;
@@ -122,6 +132,7 @@ sub mic_for ($$$$) { # mic = Mail::IMAPClient
# it to be disabled since I usually connect to localhost
if (!$mic_arg->{Ssl} && !defined($mic_arg->{Starttls}) &&
$mic->has_capability('STARTTLS') &&
+ try_starttls($host) &&
$mic->can('starttls')) {
$mic->starttls or die "E: <$uri> STARTTLS: $@\n";
}
@@ -164,15 +175,6 @@ sub mic_for ($$$$) { # mic = Mail::IMAPClient
$mic;
}
-# Net::NNTP doesn't support CAPABILITIES, yet
-sub try_starttls ($) {
- my ($host) = @_;
- return if $host =~ /\.onion\z/s;
- return if $host =~ /\A127\.[0-9]+\.[0-9]+\.[0-9]+\z/s;
- return if $host eq '::1';
- 1;
-}
-
sub nn_new ($$$) {
my ($nn_arg, $nntp_cfg, $uri) = @_;
my $nn;
diff --git a/t/imapd-tls.t b/t/imapd-tls.t
index 72ba8769..73f5112f 100644
--- a/t/imapd-tls.t
+++ b/t/imapd-tls.t
@@ -1,8 +1,8 @@
+#!perl -w
# Copyright (C) 2020-2021 all contributors <meta@public-inbox.org>
# License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
use strict;
-use warnings;
-use Test::More;
+use v5.10.1;
use Socket qw(IPPROTO_TCP SOL_SOCKET);
use PublicInbox::TestCommon;
# IO::Poll is part of the standard library, but distros may split it off...
@@ -155,6 +155,13 @@ for my $args (
ok(sysread($slow, my $end, 4096) > 0, 'got end');
is(sysread($slow, my $eof, 4096), 0, 'got EOF');
+ test_lei(sub {
+ lei_ok qw(ls-mail-source), "imap://$starttls_addr",
+ \'STARTTLS not used by default';
+ ok(!lei(qw(ls-mail-source -c imap.starttls=true),
+ "imap://$starttls_addr"), 'STARTTLS verify fails');
+ });
+
SKIP: {
skip 'TCP_DEFER_ACCEPT is Linux-only', 2 if $^O ne 'linux';
my $var = eval { Socket::TCP_DEFER_ACCEPT() } // 9;
diff --git a/t/nntpd-tls.t b/t/nntpd-tls.t
index 2c09d34e..9af6c254 100644
--- a/t/nntpd-tls.t
+++ b/t/nntpd-tls.t
@@ -146,6 +146,14 @@ for my $args (
is(sysread($slow, my $eof, 4096), 0, 'got EOF');
$slow = undef;
+ test_lei(sub {
+ lei_ok qw(ls-mail-source), "nntp://$starttls_addr",
+ \'STARTTLS not used by default';
+ ok(!lei(qw(ls-mail-source -c nntp.starttls=true),
+ "nntp://$starttls_addr"), 'STARTTLS verify fails');
+ diag $lei_err;
+ });
+
SKIP: {
skip 'TCP_DEFER_ACCEPT is Linux-only', 2 if $^O ne 'linux';
my $var = eval { Socket::TCP_DEFER_ACCEPT() } // 9;
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 14/16] lei config --edit: use controlling terminal
2021-09-19 12:50 [PATCH 00/16] lei IPC overhaul, NNTP fixes Eric Wong
` (12 preceding siblings ...)
2021-09-19 12:50 ` [PATCH 13/16] net_reader: no STARTTLS for IMAP localhost or onions Eric Wong
@ 2021-09-19 12:50 ` Eric Wong
2021-09-19 12:50 ` [PATCH 15/16] net_reader: disallow imap.fetchBatchSize=0 Eric Wong
2021-09-19 12:50 ` [PATCH 16/16] doc: lei-config: document various knobs Eric Wong
15 siblings, 0 replies; 19+ messages in thread
From: Eric Wong @ 2021-09-19 12:50 UTC (permalink / raw)
To: meta
As with "lei edit-search", "lei config --edit" may
spawn an interactive editor which works best from
the terminal running script/lei.
So implement LeiConfig as a superclass of LeiEditSearch
so the two commands can share the same verification
hooks and retry logic.
---
MANIFEST | 1 +
lib/PublicInbox/LEI.pm | 18 +++++-----
lib/PublicInbox/LeiConfig.pm | 42 ++++++++++++++++++++++
lib/PublicInbox/LeiEditSearch.pm | 60 +++++++++++--------------------
lib/PublicInbox/LeiExternal.pm | 2 +-
lib/PublicInbox/LeiInit.pm | 4 +--
lib/PublicInbox/LeiSavedSearch.pm | 16 +++------
t/lei.t | 3 ++
8 files changed, 82 insertions(+), 64 deletions(-)
create mode 100644 lib/PublicInbox/LeiConfig.pm
diff --git a/MANIFEST b/MANIFEST
index 2df743f8..8c2e964b 100644
--- a/MANIFEST
+++ b/MANIFEST
@@ -208,6 +208,7 @@ lib/PublicInbox/LeiALE.pm
lib/PublicInbox/LeiAddWatch.pm
lib/PublicInbox/LeiAuth.pm
lib/PublicInbox/LeiBlob.pm
+lib/PublicInbox/LeiConfig.pm
lib/PublicInbox/LeiConvert.pm
lib/PublicInbox/LeiCurl.pm
lib/PublicInbox/LeiDedupe.pm
diff --git a/lib/PublicInbox/LEI.pm b/lib/PublicInbox/LEI.pm
index b468a32c..148a5b1e 100644
--- a/lib/PublicInbox/LEI.pm
+++ b/lib/PublicInbox/LEI.pm
@@ -278,7 +278,7 @@ our %CMD = ( # sorted in order of importance/use:
'config' => [ '[...]', sub {
'git-config(1) wrapper for '._config_path($_[0]);
}, qw(config-file|system|global|file|f=s), # for conflict detection
- qw(c=s@ C=s@), pass_through('git config') ],
+ qw(edit|e c=s@ C=s@), pass_through('git config') ],
'inspect' => [ 'ITEMS...|--stdin', 'inspect lei/store and/or local external',
qw(stdin| pretty ascii dir=s), @c_opt ],
@@ -870,14 +870,6 @@ sub _config {
waitpid(spawn($cmd, \%env, \%rdr), 0);
}
-sub lei_config {
- my ($self, @argv) = @_;
- $self->{opt}->{'config-file'} and return fail $self,
- "config file switches not supported by `lei config'";
- _config(@_);
- x_it($self, $?) if $?;
-}
-
sub lei_daemon_pid { puts shift, $$ }
sub lei_daemon_kill {
@@ -1504,4 +1496,12 @@ sub sto_done_request {
$lei->err($@) if $@;
}
+sub cfg_dump ($$) {
+ my ($lei, $f) = @_;
+ my $ret = eval { PublicInbox::Config->git_config_dump($f, $lei->{2}) };
+ return $ret if !$@;
+ $lei->err($@);
+ undef;
+}
+
1;
diff --git a/lib/PublicInbox/LeiConfig.pm b/lib/PublicInbox/LeiConfig.pm
new file mode 100644
index 00000000..23be9aaf
--- /dev/null
+++ b/lib/PublicInbox/LeiConfig.pm
@@ -0,0 +1,42 @@
+# Copyright (C) 2021 all contributors <meta@public-inbox.org>
+# License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
+package PublicInbox::LeiConfig;
+use strict;
+use v5.10.1;
+use PublicInbox::PktOp;
+
+sub cfg_do_edit ($;$) {
+ my ($self, $reason) = @_;
+ my $lei = $self->{lei};
+ $lei->pgr_err($reason) if defined $reason;
+ my $cmd = [ qw(git config --edit -f), $self->{-f} ];
+ my $env = { GIT_CONFIG => $self->{-f} };
+ $self->cfg_edit_begin if $self->can('cfg_edit_begin');
+ # run in script/lei foreground
+ my ($op_c, $op_p) = PublicInbox::PktOp->pair;
+ # $op_p will EOF when $EDITOR is done
+ $op_c->{ops} = { '' => [\&cfg_edit_done, $self] };
+ $lei->send_exec_cmd([ @$lei{qw(0 1 2)}, $op_p->{op_p} ], $cmd, $env);
+}
+
+sub cfg_edit_done { # PktOp
+ my ($self) = @_;
+ eval {
+ my $cfg = $self->{lei}->cfg_dump($self->{-f}, $self->{lei}->{2})
+ // return cfg_do_edit($self, "\n");
+ $self->cfg_verify($cfg) if $self->can('cfg_verify');
+ };
+ $self->{lei}->fail($@) if $@;
+}
+
+sub lei_config {
+ my ($lei, @argv) = @_;
+ $lei->{opt}->{'config-file'} and return $lei->fail(
+ "config file switches not supported by `lei config'");
+ return $lei->_config(@argv) unless $lei->{opt}->{edit};
+ my $f = $lei->_lei_cfg(1)->{-f};
+ my $self = bless { lei => $lei, -f => $f }, __PACKAGE__;
+ cfg_do_edit($self);
+}
+
+1;
diff --git a/lib/PublicInbox/LeiEditSearch.pm b/lib/PublicInbox/LeiEditSearch.pm
index 47166ce7..bcf7c105 100644
--- a/lib/PublicInbox/LeiEditSearch.pm
+++ b/lib/PublicInbox/LeiEditSearch.pm
@@ -7,48 +7,32 @@ use strict;
use v5.10.1;
use PublicInbox::LeiSavedSearch;
use PublicInbox::LeiUp;
+use parent qw(PublicInbox::LeiConfig);
-sub edit_begin {
- my ($lss, $lei) = @_;
- if (ref($lss->{-cfg}->{'lei.q.output'})) {
- delete $lss->{-cfg}->{'lei.q.output'}; # invalid
- $lei->pgr_err(<<EOM);
-$lss->{-f} has multiple values of lei.q.output
+sub cfg_edit_begin {
+ my ($self) = @_;
+ if (ref($self->{lss}->{-cfg}->{'lei.q.output'})) {
+ delete $self->{lss}->{-cfg}->{'lei.q.output'}; # invalid
+ $self->{lei}->pgr_err(<<EOM);
+$self->{lss}->{-f} has multiple values of lei.q.output
please remove redundant ones
EOM
}
- $lei->{-lss_for_edit} = $lss;
}
-sub do_edit ($$;$) {
- my ($lss, $lei, $reason) = @_;
- $lei->pgr_err($reason) if defined $reason;
- my @cmd = (qw(git config --edit -f), $lss->{'-f'});
- $lei->qerr("# spawning @cmd");
- edit_begin($lss, $lei);
- # run in script/lei foreground
- require PublicInbox::PktOp;
- my ($op_c, $op_p) = PublicInbox::PktOp->pair;
- # $op_p will EOF when $EDITOR is done
- $op_c->{ops} = { '' => [\&op_edit_done, $lss, $lei] };
- $lei->send_exec_cmd([ @$lei{qw(0 1 2)}, $op_p->{op_p} ], \@cmd, {});
-}
-
-sub _edit_done {
- my ($lss, $lei) = @_;
- my $cfg = $lss->can('cfg_dump')->($lei, $lss->{'-f'}) //
- return do_edit($lss, $lei, <<EOM);
-$lss->{-f} is unparseable
-EOM
+sub cfg_verify {
+ my ($self, $cfg) = @_;
my $new_out = $cfg->{'lei.q.output'} // '';
- return do_edit($lss, $lei, <<EOM) if ref $new_out;
-$lss->{-f} has multiple values of lei.q.output
+ return $self->cfg_do_edit(<<EOM) if ref $new_out;
+$self->{-f} has multiple values of lei.q.output
EOM
- return do_edit($lss, $lei, <<EOM) if $new_out eq '';
-$lss->{-f} needs lei.q.output
+ return $self->cfg_do_edit(<<EOM) if $new_out eq '';
+$self->{-f} needs lei.q.output
EOM
+ my $lss = $self->{lss};
my $old_out = $lss->{-cfg}->{'lei.q.output'} // return;
return if $old_out eq $new_out;
+ my $lei = $self->{lei};
my $old_path = $old_out;
my $new_path = $new_out;
s!$PublicInbox::LeiSavedSearch::LOCAL_PFX!! for ($old_path, $new_path);
@@ -57,10 +41,10 @@ EOM
return if $dir_new eq $dir_old;
($old_out =~ m!\Av2:!i || $new_out =~ m!\Av2:!) and
- return do_edit($lss, $lei, <<EOM);
+ return $self->cfg_do_edit(<<EOM);
conversions from/to v2 inboxes not supported at this time
EOM
- return do_edit($lss, $lei, <<EOM) if -e $dir_new;
+ return $self->cfg_do_edit(<<EOM) if -e $dir_new;
lei.q.output changed from `$old_out' to `$new_out'
However, $dir_new exists
EOM
@@ -79,16 +63,12 @@ E: rename($dir_old, $dir_new) error: $!
EOM
}
-sub op_edit_done { # PktOp
- my ($lss, $lei) = @_;
- eval { _edit_done($lss, $lei) };
- $lei->fail($@) if $@;
-}
-
sub lei_edit_search {
my ($lei, $out) = @_;
my $lss = PublicInbox::LeiSavedSearch->up($lei, $out) or return;
- do_edit($lss, $lei);
+ my $f = $lss->{-f};
+ my $self = bless { lei => $lei, lss => $lss, -f => $f }, __PACKAGE__;
+ $self->cfg_do_edit;
}
*_complete_edit_search = \&PublicInbox::LeiUp::_complete_up;
diff --git a/lib/PublicInbox/LeiExternal.pm b/lib/PublicInbox/LeiExternal.pm
index 6fd3efef..d802f0e2 100644
--- a/lib/PublicInbox/LeiExternal.pm
+++ b/lib/PublicInbox/LeiExternal.pm
@@ -139,7 +139,7 @@ sub add_external_finish {
my $key = "external.$location.boost";
my $cur_boost = $cfg->{$key};
return if defined($cur_boost) && $cur_boost == $new_boost; # idempotent
- $self->lei_config($key, $new_boost);
+ $self->_config($key, $new_boost);
}
sub lei_add_external {
diff --git a/lib/PublicInbox/LeiInit.pm b/lib/PublicInbox/LeiInit.pm
index 6558ac0a..27ce8169 100644
--- a/lib/PublicInbox/LeiInit.pm
+++ b/lib/PublicInbox/LeiInit.pm
@@ -23,7 +23,7 @@ sub lei_init {
# some folks like symlinks and bind mounts :P
if (@dir && "@cur[1,0]" eq "@dir[1,0]") {
- $self->lei_config('leistore.dir', $dir);
+ $self->_config('leistore.dir', $dir);
$self->_lei_store(1)->done;
return $self->qerr("$exists (as $cur)");
}
@@ -31,7 +31,7 @@ sub lei_init {
E: leistore.dir=$cur already initialized and it is not $dir
}
- $self->lei_config('leistore.dir', $dir);
+ $self->_config('leistore.dir', $dir);
$self->_lei_store(1)->done;
$exists //= "# leistore.dir=$dir newly initialized";
$self->qerr($exists);
diff --git a/lib/PublicInbox/LeiSavedSearch.pm b/lib/PublicInbox/LeiSavedSearch.pm
index 754f8294..89f5c359 100644
--- a/lib/PublicInbox/LeiSavedSearch.pm
+++ b/lib/PublicInbox/LeiSavedSearch.pm
@@ -29,14 +29,6 @@ sub BOOL_FIELDS () {
qw(external local remote import-remote import-before threads)
}
-sub cfg_dump ($$) {
- my ($lei, $f) = @_;
- my $ret = eval { PublicInbox::Config->git_config_dump($f, $lei->{2}) };
- return $ret if !$@;
- $lei->err($@);
- undef;
-}
-
sub lss_dir_for ($$;$) {
my ($lei, $dstref, $on_fs) = @_;
my $pfx;
@@ -64,7 +56,7 @@ sub lss_dir_for ($$;$) {
for my $g ("$pfx-*", '*') {
my @maybe = glob("$lss_dir$g/lei.saved-search");
for my $f (@maybe) {
- $c = cfg_dump($lei, $f) // next;
+ $c = $lei->cfg_dump($f) // next;
$o = $c->{'lei.q.output'} // next;
$o =~ s!$LOCAL_PFX!! or next;
@st = stat($o) or next;
@@ -88,7 +80,7 @@ sub list {
print $fh "\tpath = ", cquote_val($p), "\n";
}
close $fh or die "close $f: $!";
- my $cfg = cfg_dump($lei, $f);
+ my $cfg = $lei->cfg_dump($f);
unlink($f);
my $out = $cfg ? $cfg->get_all('lei.q.output') : [];
map {;
@@ -113,7 +105,7 @@ sub up { # updating existing saved search via "lei up"
output2lssdir($self, $lei, \$dir, \$f) or
return $lei->fail("--save was not used with $dst cwd=".
$lei->rel2abs('.'));
- $self->{-cfg} = cfg_dump($lei, $f) // return $lei->fail;
+ $self->{-cfg} = $lei->cfg_dump($f) // return $lei->fail;
$self->{-ovf} = "$dir/over.sqlite3";
$self->{'-f'} = $f;
$self->{lock_path} = "$self->{-f}.flock";
@@ -276,7 +268,7 @@ sub output2lssdir {
my $dir = lss_dir_for($lei, \$dst, 1);
my $f = "$dir/lei.saved-search";
if (-f $f && -r _) {
- $self->{-cfg} = cfg_dump($lei, $f) // return;
+ $self->{-cfg} = $lei->cfg_dump($f) // return;
$$dir_ref = $dir;
$$fn_ref = $f;
return 1;
diff --git a/t/lei.t b/t/lei.t
index c8f47576..53fc43fb 100644
--- a/t/lei.t
+++ b/t/lei.t
@@ -100,6 +100,9 @@ my $test_config = sub {
is($lei_out, "tr00\n", "-c string value passed as-is");
lei_ok(qw(-c imap.debug=a -c imap.debug=b config --get-all imap.debug));
is($lei_out, "a\nb\n", '-c and --get-all work together');
+
+ lei_ok([qw(config -e)], { VISUAL => 'cat' });
+ is($lei_out, "[a]\n\tb = c\n", '--edit works');
};
my $test_completion = sub {
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 15/16] net_reader: disallow imap.fetchBatchSize=0
2021-09-19 12:50 [PATCH 00/16] lei IPC overhaul, NNTP fixes Eric Wong
` (13 preceding siblings ...)
2021-09-19 12:50 ` [PATCH 14/16] lei config --edit: use controlling terminal Eric Wong
@ 2021-09-19 12:50 ` Eric Wong
2021-09-19 12:50 ` [PATCH 16/16] doc: lei-config: document various knobs Eric Wong
15 siblings, 0 replies; 19+ messages in thread
From: Eric Wong @ 2021-09-19 12:50 UTC (permalink / raw)
To: meta
A batch size of zero is nonsensical and causes infinite loops.
---
lib/PublicInbox/NetReader.pm | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lib/PublicInbox/NetReader.pm b/lib/PublicInbox/NetReader.pm
index e305523e..fbe1ac4f 100644
--- a/lib/PublicInbox/NetReader.pm
+++ b/lib/PublicInbox/NetReader.pm
@@ -344,10 +344,10 @@ sub imap_common_init ($;$) {
}
my $k = 'imap.fetchBatchSize';
my $bs = $cfg->urlmatch($k, $$uri) // next;
- if ($bs =~ /\A([0-9]+)\z/) {
+ if ($bs =~ /\A([0-9]+)\z/ && $bs > 0) {
$self->{cfg_opt}->{$sec}->{batch_size} = $bs;
} else {
- warn "$k=$bs is not an integer\n";
+ warn "$k=$bs is not a positive integer\n";
}
}
# make sure we can connect and cache the credentials in memory
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 16/16] doc: lei-config: document various knobs
2021-09-19 12:50 [PATCH 00/16] lei IPC overhaul, NNTP fixes Eric Wong
` (14 preceding siblings ...)
2021-09-19 12:50 ` [PATCH 15/16] net_reader: disallow imap.fetchBatchSize=0 Eric Wong
@ 2021-09-19 12:50 ` Eric Wong
2021-09-19 16:14 ` Kyle Meyer
15 siblings, 1 reply; 19+ messages in thread
From: Eric Wong @ 2021-09-19 12:50 UTC (permalink / raw)
To: meta
It's still a work-in-progress, but the basic debug knob
comes in handy for new users; as does proxy support.
---
Documentation/lei-config.pod | 91 +++++++++++++++++++++++++++++++++++-
1 file changed, 90 insertions(+), 1 deletion(-)
diff --git a/Documentation/lei-config.pod b/Documentation/lei-config.pod
index a64045ef..daf66fe4 100644
--- a/Documentation/lei-config.pod
+++ b/Documentation/lei-config.pod
@@ -8,10 +8,99 @@ lei config [OPTIONS]
=head1 DESCRIPTION
-Call git-config(1) with C<$XDG_CONFIG_HOME/lei/config> as the
+Call L<git-config(1> with C<$XDG_CONFIG_HOME/lei/config> as the
configuration file. All C<OPTIONS> are passed through, but those that
override the configuration file are not permitted.
+All C<imap> and C<nntp> options may be specified per-host or
+(if using git 2.26+) with wildcards:
+
+ [imap "imap://*.onion"]
+ proxy = socks5h://127.0.0.1:9050
+
+ [nntp "nntp://example.com"]
+ proxy = socks5h://127.0.0.1:1080
+
+=head2 VARIABLES
+
+=over 8
+
+=item external.*
+
+Managed by L<lei-add-external(1)> and L<lei-forget-external(1)>
+
+=item imap.proxy
+
+=item nntp.proxy
+
+The C<socks5h://> proxy address. Older versions of SOCKS may
+be supported if there is user demand.
+
+=item imap.starttls
+
+=item nntp.starttls
+
+Enable or disable STARTTLS on non-imaps:// and non-nntps://
+hosts. By default, STARTTLS is enabled if available unless
+connecting to a Tor .onion or localhost.
+
+=item imap.compress
+
+=item nntp.compress
+
+Enable protocol-level compression, this may be incompatible
+or broken with some servers.
+
+Note: L<Net::NNTP> compression support is pending:
+L<https://rt.cpan.org/Ticket/Display.html?id=129967>
+
+=item imap.debug
+
+=item nntp.debug
+
+Enable debugging output of underlying IMAP and NNTP libraries,
+currently L<Mail::IMAPClient> and L<Net::NNTP>, respectively.
+If using L<imap.proxy> or L<nntp.proxy> point to a SOCKS proxy,
+debugging output for L<IO::Socket::Socks> will be enabled, as
+well.
+
+Disabling L<imap.compress> may be required to improve output.
+
+=item imap.timeout
+
+=item nntp.timeout
+
+The read timeout for responses.
+
+Default: 600 seconds (IMAP); 120 seconds (NNTP)
+
+=item imap.fetchBatchSize
+
+Number of full messages to fetch at once. Larger values reduce
+network round trips at the cost of higher memory use, especially
+when retrieving large messages.
+
+Small responses for IMAP flags are fetched at 10000 times this value.
+
+Default: 1
+
+=item imap.ignoreSizeErrors
+
+Ignore size mismatches from broken IMAP server implementations.
+
+Default: false
+
+=item color.SLOT
+
+C<quoted>, C<hdrdefault>, C<status>, C<attachment> color slots
+are supported for the C<-f text> and C<-f reply> output formats
+of L<lei-lcat(1)> and L<lei-q(1)>.
+
+The any per-project .git/config, and global ~/.gitconfig files
+will also be parsed for diff coloring. git diff color slots
+(C<color.diff.SLOT>) supported are C<new>, C<old>, C<meta>,
+C<frag>, C<func>, and C<context>.
+
=head1 CONTACT
Feedback welcome via plain-text mail to L<mailto:meta@public-inbox.org>
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH 16/16] doc: lei-config: document various knobs
2021-09-19 12:50 ` [PATCH 16/16] doc: lei-config: document various knobs Eric Wong
@ 2021-09-19 16:14 ` Kyle Meyer
2021-09-19 20:00 ` Eric Wong
0 siblings, 1 reply; 19+ messages in thread
From: Kyle Meyer @ 2021-09-19 16:14 UTC (permalink / raw)
To: Eric Wong; +Cc: meta
Eric Wong writes:
> diff --git a/Documentation/lei-config.pod b/Documentation/lei-config.pod
> index a64045ef..daf66fe4 100644
> --- a/Documentation/lei-config.pod
> +++ b/Documentation/lei-config.pod
> @@ -8,10 +8,99 @@ lei config [OPTIONS]
>
> =head1 DESCRIPTION
>
> -Call git-config(1) with C<$XDG_CONFIG_HOME/lei/config> as the
> +Call L<git-config(1> with C<$XDG_CONFIG_HOME/lei/config> as the
Looks like the ')' was unintentionally dropped.
> +=item imap.compress
> +
> +=item nntp.compress
> +
> +Enable protocol-level compression, this may be incompatible
> +or broken with some servers.
nit: s/, /. This/
> +=item color.SLOT
> +
> +C<quoted>, C<hdrdefault>, C<status>, C<attachment> color slots
> +are supported for the C<-f text> and C<-f reply> output formats
> +of L<lei-lcat(1)> and L<lei-q(1)>.
> +
> +The any per-project .git/config, and global ~/.gitconfig files
s/The any/Any/ ?
> +will also be parsed for diff coloring. git diff color slots
> +(C<color.diff.SLOT>) supported are C<new>, C<old>, C<meta>,
> +C<frag>, C<func>, and C<context>.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 16/16] doc: lei-config: document various knobs
2021-09-19 16:14 ` Kyle Meyer
@ 2021-09-19 20:00 ` Eric Wong
0 siblings, 0 replies; 19+ messages in thread
From: Eric Wong @ 2021-09-19 20:00 UTC (permalink / raw)
To: Kyle Meyer; +Cc: meta
Kyle Meyer <kyle@kyleam.com> wrote:
> Eric Wong writes:
> > +Call L<git-config(1> with C<$XDG_CONFIG_HOME/lei/config> as the
>
> Looks like the ')' was unintentionally dropped.
> > +Enable protocol-level compression, this may be incompatible
> > +or broken with some servers.
>
> nit: s/, /. This/
> > +The any per-project .git/config, and global ~/.gitconfig files
>
> s/The any/Any/ ?
Thanks, will squash the below. I'll also clarify the sentence
on imap.debug vs imap.compress
diff --git a/Documentation/lei-config.pod b/Documentation/lei-config.pod
index daf66fe4..c91f36ee 100644
--- a/Documentation/lei-config.pod
+++ b/Documentation/lei-config.pod
@@ -8,7 +8,7 @@ lei config [OPTIONS]
=head1 DESCRIPTION
-Call L<git-config(1> with C<$XDG_CONFIG_HOME/lei/config> as the
+Call L<git-config(1)> with C<$XDG_CONFIG_HOME/lei/config> as the
configuration file. All C<OPTIONS> are passed through, but those that
override the configuration file are not permitted.
@@ -48,7 +48,7 @@ connecting to a Tor .onion or localhost.
=item nntp.compress
-Enable protocol-level compression, this may be incompatible
+Enable protocol-level compression. This may be incompatible
or broken with some servers.
Note: L<Net::NNTP> compression support is pending:
@@ -64,7 +64,7 @@ If using L<imap.proxy> or L<nntp.proxy> point to a SOCKS proxy,
debugging output for L<IO::Socket::Socks> will be enabled, as
well.
-Disabling L<imap.compress> may be required to improve output.
+Disabling L<imap.compress> may be required for readability.
=item imap.timeout
@@ -96,7 +96,7 @@ C<quoted>, C<hdrdefault>, C<status>, C<attachment> color slots
are supported for the C<-f text> and C<-f reply> output formats
of L<lei-lcat(1)> and L<lei-q(1)>.
-The any per-project .git/config, and global ~/.gitconfig files
+Any per-project .git/config, and global ~/.gitconfig files
will also be parsed for diff coloring. git diff color slots
(C<color.diff.SLOT>) supported are C<new>, C<old>, C<meta>,
C<frag>, C<func>, and C<context>.
^ permalink raw reply related [flat|nested] 19+ messages in thread