unofficial mirror of meta@public-inbox.org
 help / color / mirror / Atom feed
* [PATCH 00/16] lei IPC overhaul, NNTP fixes
@ 2021-09-19 12:50 Eric Wong
  2021-09-19 12:50 ` [PATCH 01/16] ipc: wq_do: support synchronous waits and responses Eric Wong
                   ` (15 more replies)
  0 siblings, 16 replies; 19+ messages in thread
From: Eric Wong @ 2021-09-19 12:50 UTC (permalink / raw)
  To: meta

11/16 is a bit worrying for saved search dedupe over HTTP(S),
and I can't seem to reproduce it reliably, either..

ls-mail-source and import use is far nicer, as it provides a
good avenue for doing partial fetches.

lei/store IPC got a massive overhaul, and the sto_done_request
simplification is nice.  This will probably simplify automatic
export-kw support to IMAP folders.

I also noticed "lei config --edit" was wonky, so
I made it share code with "lei edit-search".

Starting to document config knobs, too.

Eric Wong (16):
  ipc: wq_do: support synchronous waits and responses
  ipc: allow disabling broadcast for wq_workers
  lei/store: use SOCK_SEQPACKET rather than pipe
  lei: simplify sto_done_request
  lei_xsearch: drop Data::Dumper use
  ipc: drop dynamic WQ process counts
  lei: clamp internal worker processes to 4
  lei ls-mail-source: use "high"/"low" for NNTP
  lei ls-mail-source: pretty JSON support
  net_reader: fix single NNTP article fetch, test ranges
  xt: add fsck script over over.sqlite3
  watch: use net_reader->mic_new wrapper for SOCKS+TLS
  net_reader: no STARTTLS for IMAP localhost or onions
  lei config --edit: use controlling terminal
  net_reader: disallow imap.fetchBatchSize=0
  doc: lei-config: document various knobs

 Documentation/lei-config.pod          |  91 +++++++++++++++++++-
 MANIFEST                              |   2 +
 lib/PublicInbox/IPC.pm                | 117 +++++++++++---------------
 lib/PublicInbox/LEI.pm                |  32 +++----
 lib/PublicInbox/LeiConfig.pm          |  42 +++++++++
 lib/PublicInbox/LeiEditSearch.pm      |  60 +++++--------
 lib/PublicInbox/LeiExternal.pm        |   2 +-
 lib/PublicInbox/LeiImport.pm          |   2 +-
 lib/PublicInbox/LeiImportKw.pm        |   6 +-
 lib/PublicInbox/LeiIndex.pm           |   2 +-
 lib/PublicInbox/LeiInit.pm            |   4 +-
 lib/PublicInbox/LeiInput.pm           |   2 +-
 lib/PublicInbox/LeiLsMailSource.pm    |  25 +++---
 lib/PublicInbox/LeiNoteEvent.pm       |  11 +--
 lib/PublicInbox/LeiRefreshMailSync.pm |   2 +-
 lib/PublicInbox/LeiRemote.pm          |   4 +-
 lib/PublicInbox/LeiRm.pm              |   2 +-
 lib/PublicInbox/LeiSavedSearch.pm     |  16 +---
 lib/PublicInbox/LeiStore.pm           |  22 ++---
 lib/PublicInbox/LeiTag.pm             |   2 +-
 lib/PublicInbox/LeiToMail.pm          |  22 ++---
 lib/PublicInbox/LeiXSearch.pm         |   9 +-
 lib/PublicInbox/NetReader.pm          |  39 +++++----
 lib/PublicInbox/WQWorker.pm           |   9 +-
 lib/PublicInbox/Watch.pm              |   3 +-
 t/imapd-tls.t                         |  11 ++-
 t/ipc.t                               |  19 ++---
 t/lei-import-nntp.t                   |  26 ++++++
 t/lei.t                               |   3 +
 t/nntpd-tls.t                         |   8 ++
 t/uri_nntps.t                         |   3 +
 xt/over-fsck.perl                     |  44 ++++++++++
 32 files changed, 403 insertions(+), 239 deletions(-)
 create mode 100644 lib/PublicInbox/LeiConfig.pm
 create mode 100644 xt/over-fsck.perl


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [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

end of thread, other threads:[~2021-09-19 20:00 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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 ` [PATCH 03/16] lei/store: use SOCK_SEQPACKET rather than pipe Eric Wong
2021-09-19 12:50 ` [PATCH 04/16] lei: simplify sto_done_request Eric Wong
2021-09-19 12:50 ` [PATCH 05/16] lei_xsearch: drop Data::Dumper use Eric Wong
2021-09-19 12:50 ` [PATCH 06/16] ipc: drop dynamic WQ process counts Eric Wong
2021-09-19 12:50 ` [PATCH 07/16] lei: clamp internal worker processes to 4 Eric Wong
2021-09-19 12:50 ` [PATCH 08/16] lei ls-mail-source: use "high"/"low" for NNTP Eric Wong
2021-09-19 12:50 ` [PATCH 09/16] lei ls-mail-source: pretty JSON support Eric Wong
2021-09-19 12:50 ` [PATCH 10/16] net_reader: fix single NNTP article fetch, test ranges Eric Wong
2021-09-19 12:50 ` [PATCH 11/16] xt: add fsck script over over.sqlite3 Eric Wong
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 ` [PATCH 13/16] net_reader: no STARTTLS for IMAP localhost or onions Eric Wong
2021-09-19 12:50 ` [PATCH 14/16] lei config --edit: use controlling terminal 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
2021-09-19 16:14   ` Kyle Meyer
2021-09-19 20:00     ` 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).