unofficial mirror of meta@public-inbox.org
 help / color / mirror / Atom feed
* [PATCH 00/13] lei approxidate, startup fix, --alert
@ 2021-02-08  9:05 Eric Wong
  2021-02-08  9:05 ` [PATCHv2 01/13] lei q: improve remote mboxrd UX + MUA Eric Wong
                   ` (12 more replies)
  0 siblings, 13 replies; 15+ messages in thread
From: Eric Wong @ 2021-02-08  9:05 UTC (permalink / raw)
  To: meta

I've redone and squashed some changes into PATCH 1/13 which
was posted yesterday.

3/13 (SIGWINCH) is rebase necessary after 1/13,
4/13 (--alert=CMD) is a generalized take on 3/13.

12/13 is...

Eric Wong (13):
  lei q: improve remote mboxrd UX + MUA
  lei_xsearch: quiet Eml warnings from remote mboxrds
  lei q: SIGWINCH process group with the terminal
  lei q: support --alert=CMD for early MUA users
  tests: favor IPv6
  ds: improve add_timer usability
  lei: start_pager: drop COLUMNS default
  lei: avoid racing on unlink + bind + listen
  lei: drop BSD::Resource usage
  git: implement date_parse method
  lei q: use git approxidate with d:, dt: and rt: ranges
  search: use one git-rev-parse process for all dates
  spawnpp: raise exception on E2BIG errors

 lib/PublicInbox/DS.pm           |  10 ++--
 lib/PublicInbox/ExtSearchIdx.pm |   5 +-
 lib/PublicInbox/FakeInotify.pm  |   4 +-
 lib/PublicInbox/Git.pm          |  10 +++-
 lib/PublicInbox/IPC.pm          |   8 +--
 lib/PublicInbox/LEI.pm          | 100 ++++++++++++++++++++++----------
 lib/PublicInbox/LeiCurl.pm      |  11 +++-
 lib/PublicInbox/LeiMirror.pm    |   5 +-
 lib/PublicInbox/LeiOverview.pm  |   6 +-
 lib/PublicInbox/LeiQuery.pm     |  12 ++--
 lib/PublicInbox/LeiToMail.pm    |  24 ++++----
 lib/PublicInbox/LeiXSearch.pm   |  97 ++++++++++++++++++++-----------
 lib/PublicInbox/Search.pm       |  86 +++++++++++++++++++++++++++
 lib/PublicInbox/SpawnPP.pm      |  23 ++++++--
 lib/PublicInbox/TestCommon.pm   |  30 ++++++++--
 lib/PublicInbox/Watch.pm        |  19 +++---
 script/lei                      |  16 ++---
 t/extsearch.t                   |   2 +-
 t/git.t                         |  17 +++++-
 t/httpd-corner.psgi             |   2 +-
 t/httpd-corner.t                |  12 ++--
 t/httpd-https.t                 |   2 +-
 t/httpd-unix.t                  |   7 +--
 t/httpd.t                       |   8 +--
 t/imapd-tls.t                   |   4 +-
 t/imapd.t                       |   8 +--
 t/lei-mirror.t                  |   2 +-
 t/nntpd-tls.t                   |   4 +-
 t/nntpd.t                       |  11 ++--
 t/psgi_attach.t                 |   2 +-
 t/psgi_v2.t                     |   2 +-
 t/search.t                      |  51 ++++++++++++++++
 t/solver_git.t                  |   2 +-
 t/v2mirror.t                    |   3 +-
 t/v2writable.t                  |   3 +-
 t/www_altid.t                   |   2 +-
 t/www_listing.t                 |   3 +-
 xt/git-http-backend.t           |   4 +-
 xt/httpd-async-stream.t         |   2 +-
 xt/imapd-mbsync-oimap.t         |   4 +-
 xt/imapd-validate.t             |   4 +-
 xt/mem-imapd-tls.t              |   2 +-
 xt/nntpd-validate.t             |   3 +-
 xt/perf-nntpd.t                 |  16 ++---
 xt/solver.t                     |   3 +-
 45 files changed, 441 insertions(+), 210 deletions(-)


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

* [PATCHv2 01/13] lei q: improve remote mboxrd UX + MUA
  2021-02-08  9:05 [PATCH 00/13] lei approxidate, startup fix, --alert Eric Wong
@ 2021-02-08  9:05 ` Eric Wong
  2021-02-08  9:05 ` [PATCH 02/13] lei_xsearch: quiet Eml warnings from remote mboxrds Eric Wong
                   ` (11 subsequent siblings)
  12 siblings, 0 replies; 15+ messages in thread
From: Eric Wong @ 2021-02-08  9:05 UTC (permalink / raw)
  To: meta

For early MUA spawners using lock-free outputs, we we need to
on the startq pipe to silence progress reporting.  For
--augment users, we can start the MUA even earlier by
creating Maildirs in the pre-augment phase.

To improve progress reporting for non-MUA (or late-MUA)
spawners, we'll no longer blindly append "--compressed" to the
curl(1) command when POST-ing for the gzipped mboxrd.
Furthermore, we'll overload stringify ('""') in LeiCurl to
ensure the empty -d '' string shows up properly.

v2: fix startq waiting with --threads
    mset_progress is never shown with early MUA spawning,
    The plan is to still show progress when augmenting and
    deduping.  This fixes all local search cases.
    A leftover debug bit is dropped, too
---
 lib/PublicInbox/IPC.pm         |  8 ++--
 lib/PublicInbox/LEI.pm         |  4 +-
 lib/PublicInbox/LeiCurl.pm     | 11 +++--
 lib/PublicInbox/LeiMirror.pm   |  5 +-
 lib/PublicInbox/LeiOverview.pm |  3 +-
 lib/PublicInbox/LeiToMail.pm   | 24 +++++-----
 lib/PublicInbox/LeiXSearch.pm  | 88 +++++++++++++++++++++-------------
 7 files changed, 86 insertions(+), 57 deletions(-)

diff --git a/lib/PublicInbox/IPC.pm b/lib/PublicInbox/IPC.pm
index c8673e26..9331233a 100644
--- a/lib/PublicInbox/IPC.pm
+++ b/lib/PublicInbox/IPC.pm
@@ -109,7 +109,6 @@ sub ipc_worker_spawn {
 		$w_res->autoflush(1);
 		$SIG{$_} = 'IGNORE' for (qw(TERM INT QUIT));
 		local $0 = $ident;
-		PublicInbox::DS::sig_setmask($sigset);
 		# ensure we properly exit even if warn() dies:
 		my $end = PublicInbox::OnDestroy->new($$, sub { exit(!!$@) });
 		eval {
@@ -117,6 +116,7 @@ sub ipc_worker_spawn {
 			local @$self{keys %$fields} = values(%$fields);
 			my $on_destroy = $self->ipc_atfork_child;
 			local %SIG = %SIG;
+			PublicInbox::DS::sig_setmask($sigset);
 			ipc_worker_loop($self, $r_req, $w_res);
 		};
 		warn "worker $ident PID:$$ died: $@\n" if $@;
@@ -293,7 +293,6 @@ sub _wq_worker_start ($$$) {
 		$SIG{$_} = 'IGNORE' for (qw(PIPE));
 		$SIG{$_} = 'DEFAULT' for (qw(TTOU TTIN TERM QUIT INT CHLD));
 		local $0 = $self->{-wq_ident};
-		PublicInbox::DS::sig_setmask($oldset);
 		# ensure we properly exit even if warn() dies:
 		my $end = PublicInbox::OnDestroy->new($$, sub { exit(!!$@) });
 		eval {
@@ -301,6 +300,7 @@ sub _wq_worker_start ($$$) {
 			local @$self{keys %$fields} = values(%$fields);
 			my $on_destroy = $self->ipc_atfork_child;
 			local %SIG = %SIG;
+			PublicInbox::DS::sig_setmask($oldset);
 			wq_worker_loop($self);
 		};
 		warn "worker $self->{-wq_ident} PID:$$ died: $@" if $@;
@@ -395,9 +395,9 @@ sub wq_close {
 }
 
 sub wq_kill_old {
-	my ($self) = @_;
+	my ($self, $sig) = @_;
 	my $pids = $self->{"-wq_old_pids.$$"} or return;
-	kill 'TERM', @$pids;
+	kill($sig // 'TERM', @$pids);
 }
 
 sub wq_kill {
diff --git a/lib/PublicInbox/LEI.pm b/lib/PublicInbox/LEI.pm
index dce80762..c3645698 100644
--- a/lib/PublicInbox/LEI.pm
+++ b/lib/PublicInbox/LEI.pm
@@ -741,7 +741,9 @@ sub start_mua {
 	} elsif ($self->{oneshot}) {
 		$self->{"mua.pid.$self.$$"} = spawn(\@cmd);
 	}
-	delete $self->{-progress};
+	if ($self->{lxs} && $self->{au_done}) { # kick wait_startq
+		syswrite($self->{au_done}, 'q' x ($self->{lxs}->{jobs} // 0));
+	}
 }
 
 # caller needs to "-t $self->{1}" to check if tty
diff --git a/lib/PublicInbox/LeiCurl.pm b/lib/PublicInbox/LeiCurl.pm
index 38b17c78..f346a1b4 100644
--- a/lib/PublicInbox/LeiCurl.pm
+++ b/lib/PublicInbox/LeiCurl.pm
@@ -8,6 +8,12 @@ use v5.10.1;
 use PublicInbox::Spawn qw(which);
 use PublicInbox::Config;
 
+# Ensures empty strings are quoted, we don't need more
+# sophisticated quoting than for empty strings: curl -d ''
+use overload '""' => sub {
+	join(' ', map { $_ eq '' ?  "''" : $_ } @{$_[0]});
+};
+
 my %lei2curl = (
 	'curl-config=s@' => 'config|K=s@',
 );
@@ -63,10 +69,9 @@ EOM
 
 # completes the result of cmd() for $uri
 sub for_uri {
-	my ($self, $lei, $uri) = @_;
+	my ($self, $lei, $uri, @opt) = @_;
 	my $pfx = torsocks($self, $lei, $uri) or return; # error
-	[ @$pfx, @$self, substr($uri->path, -3) eq '.gz' ? () : '--compressed',
-		$uri->as_string ]
+	bless [ @$pfx, @$self, @opt, $uri->as_string ], ref($self);
 }
 
 1;
diff --git a/lib/PublicInbox/LeiMirror.pm b/lib/PublicInbox/LeiMirror.pm
index 5ba69287..c5153148 100644
--- a/lib/PublicInbox/LeiMirror.pm
+++ b/lib/PublicInbox/LeiMirror.pm
@@ -31,7 +31,7 @@ sub try_scrape {
 	my $uri = URI->new($self->{src});
 	my $lei = $self->{lei};
 	my $curl = $self->{curl} //= PublicInbox::LeiCurl->new($lei) or return;
-	my $cmd = $curl->for_uri($lei, $uri);
+	my $cmd = $curl->for_uri($lei, $uri, '--compressed');
 	my $opt = { 0 => $lei->{0}, 2 => $lei->{2} };
 	my $fh = popen_rd($cmd, $lei->{env}, $opt);
 	my $html = do { local $/; <$fh> } // die "read(curl $uri): $!";
@@ -93,8 +93,7 @@ sub _try_config {
 	my $path = $uri->path;
 	chop($path) eq '/' or die "BUG: $uri not canonicalized";
 	$uri->path($path . '/_/text/config/raw');
-	my $cmd = $self->{curl}->for_uri($lei, $uri);
-	push @$cmd, '--compressed'; # curl decompresses for us
+	my $cmd = $self->{curl}->for_uri($lei, $uri, '--compressed');
 	my $ce = "$dst/inbox.config.example";
 	my $f = "$ce-$$.tmp";
 	open(my $fh, '+>', $f) or return $lei->err("open $f: $! (non-fatal)");
diff --git a/lib/PublicInbox/LeiOverview.pm b/lib/PublicInbox/LeiOverview.pm
index dcfb9cc7..f0ac4684 100644
--- a/lib/PublicInbox/LeiOverview.pm
+++ b/lib/PublicInbox/LeiOverview.pm
@@ -95,9 +95,10 @@ sub new {
 		$lei->{dedupe} //= PublicInbox::LeiDedupe->new($lei);
 	} else {
 		# default to the cheapest sort since MUA usually resorts
-		$lei->{opt}->{'sort'} //= 'docid' if $dst ne '/dev/stdout';
+		$opt->{'sort'} //= 'docid' if $dst ne '/dev/stdout';
 		$lei->{l2m} = eval { PublicInbox::LeiToMail->new($lei) };
 		return $lei->fail($@) if $@;
+		$lei->{early_mua} = 1 if $opt->{mua} && $lei->{l2m}->lock_free;
 	}
 	$self;
 }
diff --git a/lib/PublicInbox/LeiToMail.pm b/lib/PublicInbox/LeiToMail.pm
index 4c5a5685..a5a196db 100644
--- a/lib/PublicInbox/LeiToMail.pm
+++ b/lib/PublicInbox/LeiToMail.pm
@@ -371,7 +371,17 @@ sub new {
 	$self;
 }
 
-sub _pre_augment_maildir {} # noop
+sub _pre_augment_maildir {
+	my ($self, $lei) = @_;
+	my $dst = $lei->{ovv}->{dst};
+	for my $x (qw(tmp new cur)) {
+		my $d = $dst.$x;
+		next if -d $d;
+		require File::Path;
+		File::Path::mkpath($d);
+		-d $d or die "$d is not a directory";
+	}
+}
 
 sub _do_augment_maildir {
 	my ($self, $lei) = @_;
@@ -388,17 +398,7 @@ sub _do_augment_maildir {
 	}
 }
 
-sub _post_augment_maildir {
-	my ($self, $lei) = @_;
-	my $dst = $lei->{ovv}->{dst};
-	for my $x (qw(tmp new cur)) {
-		my $d = $dst.$x;
-		next if -d $d;
-		require File::Path;
-		File::Path::mkpath($d);
-		-d $d or die "$d is not a directory";
-	}
-}
+sub _post_augment_maildir {} # noop
 
 sub _pre_augment_mbox {
 	my ($self, $lei) = @_;
diff --git a/lib/PublicInbox/LeiXSearch.pm b/lib/PublicInbox/LeiXSearch.pm
index 2794140a..db089a67 100644
--- a/lib/PublicInbox/LeiXSearch.pm
+++ b/lib/PublicInbox/LeiXSearch.pm
@@ -101,20 +101,34 @@ sub _mset_more ($$) {
 # $startq will EOF when query_prepare is done augmenting and allow
 # query_mset and query_thread_mset to proceed.
 sub wait_startq ($) {
-	my ($startq) = @_;
-	$_[0] = undef;
-	read($startq, my $query_prepare_done, 1);
+	my ($lei) = @_;
+	my $startq = delete $lei->{startq} or return;
+	while (1) {
+		my $n = sysread($startq, my $query_prepare_done, 1);
+		if (defined $n) {
+			return if $n == 0; # no MUA
+			if ($query_prepare_done eq 'q') {
+				$lei->{opt}->{quiet} = 1;
+				delete $lei->{opt}->{verbose};
+				delete $lei->{-progress};
+			} else {
+				$lei->fail("$$ WTF `$query_prepare_done'");
+			}
+			return;
+		}
+		return $lei->fail("$$ wait_startq: $!") unless $!{EINTR};
+	}
 }
 
 sub mset_progress {
 	my $lei = shift;
-	return unless $lei->{-progress};
+	return if $lei->{early_mua} || !$lei->{-progress};
 	if ($lei->{pkt_op_p}) {
 		pkt_do($lei->{pkt_op_p}, 'mset_progress', @_);
 	} else { # single lei-daemon consumer
 		my ($desc, $mset_size, $mset_total_est) = @_;
 		$lei->{-mset_total} += $mset_size;
-		$lei->err("# $desc $mset_size/$mset_total_est");
+		$lei->qerr("# $desc $mset_size/$mset_total_est");
 	}
 }
 
@@ -122,7 +136,6 @@ sub query_thread_mset { # for --threads
 	my ($self, $ibxish) = @_;
 	local $0 = "$0 query_thread_mset";
 	my $lei = $self->{lei};
-	my $startq = delete $lei->{startq};
 	my ($srch, $over) = ($ibxish->search, $ibxish->over);
 	my $desc = $ibxish->{inboxdir} // $ibxish->{topdir};
 	return warn("$desc not indexed by Xapian\n") unless ($srch && $over);
@@ -140,7 +153,7 @@ sub query_thread_mset { # for --threads
 		while ($over->expand_thread($ctx)) {
 			for my $n (@{$ctx->{xids}}) {
 				my $smsg = $over->get_art($n) or next;
-				wait_startq($startq) if $startq;
+				wait_startq($lei);
 				my $mitem = delete $n2item{$smsg->{num}};
 				$each_smsg->($smsg, $mitem);
 			}
@@ -155,7 +168,6 @@ sub query_mset { # non-parallel for non-"--threads" users
 	my ($self) = @_;
 	local $0 = "$0 query_mset";
 	my $lei = $self->{lei};
-	my $startq = delete $lei->{startq};
 	my $mo = { %{$lei->{mset_opt}} };
 	my $mset;
 	for my $loc (locals($self)) {
@@ -168,7 +180,7 @@ sub query_mset { # non-parallel for non-"--threads" users
 				$mset->size, $mset->get_matches_estimated);
 		for my $mitem ($mset->items) {
 			my $smsg = smsg_for($self, $mitem) or next;
-			wait_startq($startq) if $startq;
+			wait_startq($lei);
 			$each_smsg->($smsg, $mitem);
 		}
 	} while (_mset_more($mset, $mo));
@@ -183,7 +195,7 @@ sub each_eml { # callback for MboxReader->mboxrd
 	$smsg->parse_references($eml, mids($eml));
 	$smsg->{$_} //= '' for qw(from to cc ds subject references mid);
 	delete @$smsg{qw(From Subject -ds -ts)};
-	if (my $startq = delete($lei->{startq})) { wait_startq($startq) }
+	wait_startq($lei);
 	if ($lei->{-progress}) {
 		++$lei->{-nr_remote_eml};
 		my $now = now();
@@ -210,7 +222,6 @@ sub query_remote_mboxrd {
 	my $cerr = File::Temp->new(TEMPLATE => 'curl.err-XXXX', TMPDIR => 1);
 	fcntl($cerr, F_SETFL, O_APPEND|O_RDWR) or warn "set O_APPEND: $!";
 	my $rdr = { 2 => $cerr, pgid => 0 };
-	my $coff = 0;
 	my $sigint_reap = $lei->can('sigint_reap');
 	if ($verbose) {
 		# spawn a process to force line-buffering, otherwise curl
@@ -228,13 +239,14 @@ sub query_remote_mboxrd {
 		$lei->{-nr_remote_eml} = 0;
 		$uri->query_form(@qform);
 		my $cmd = $curl->for_uri($lei, $uri);
-		$lei->err("# @$cmd") if $verbose;
+		$lei->qerr("# $cmd");
 		my ($fh, $pid) = popen_rd($cmd, $env, $rdr);
 		$reap_curl = PublicInbox::OnDestroy->new($sigint_reap, $pid);
 		$fh = IO::Uncompress::Gunzip->new($fh);
 		PublicInbox::MboxReader->mboxrd($fh, \&each_eml, $self,
 						$lei, $each_smsg);
-		my $err = waitpid($pid, 0) == $pid ? undef : "BUG: waitpid: $!";
+		my $err = waitpid($pid, 0) == $pid ? undef
+						: "BUG: waitpid($cmd): $!";
 		@$reap_curl = (); # cancel OnDestroy
 		die $err if $err;
 		if ($? == 0) {
@@ -242,16 +254,18 @@ sub query_remote_mboxrd {
 			mset_progress($lei, $lei->{-current_url}, $nr, $nr);
 			next;
 		}
-		seek($cerr, $coff, SEEK_SET) or warn "seek(curl stderr): $!\n";
-		my $e = do { local $/; <$cerr> } //
-				die "read(curl stderr): $!\n";
-		$coff += length($e);
-		truncate($cerr, 0);
-		next if (($? >> 8) == 22 && $e =~ /\b404\b/);
-		$lei->child_error($?);
+		$err = '';
+		if (-s $cerr) {
+			seek($cerr, 0, SEEK_SET) or
+					$lei->err("seek($cmd stderr): $!");
+			$err = do { local $/; <$cerr> } //
+					"read($cmd stderr): $!";
+			truncate($cerr, 0) or
+					$lei->err("truncate($cmd stderr): $!");
+		}
+		next if (($? >> 8) == 22 && $err =~ /\b404\b/);
 		$uri->query_form(q => $lei->{mset_opt}->{qstr});
-		# --verbose already showed the error via tail(1)
-		$lei->err("E: $uri \$?=$?\n", $verbose ? () : $e);
+		$lei->child_error($?, "E: <$uri> $err");
 	}
 	undef $each_smsg;
 	$lei->{ovv}->ovv_atexit_child($lei);
@@ -311,15 +325,23 @@ Error closing $lei->{ovv}->{dst}: $!
 
 sub do_post_augment {
 	my ($lei) = @_;
-	eval { $lei->{l2m}->post_augment($lei) };
-	if (my $err = $@) {
-		if (my $lxs = delete $lei->{lxs}) {
-			$lxs->wq_kill;
-			$lxs->wq_close(0, undef, $lei);
+	my $l2m = $lei->{l2m};
+	my $err;
+	if ($l2m) {
+		eval { $l2m->post_augment($lei) };
+		$err = $@;
+		if ($err) {
+			if (my $lxs = delete $lei->{lxs}) {
+				$lxs->wq_kill;
+				$lxs->wq_close(0, undef, $lei);
+			}
+			$lei->fail("$err");
 		}
-		$lei->fail("$err");
 	}
-	close(delete $lei->{au_done}); # triggers wait_startq
+	if (!$err && delete $lei->{early_mua}) { # non-augment case
+		$lei->start_mua;
+	}
+	close(delete $lei->{au_done}); # triggers wait_startq in lei_xsearch
 }
 
 my $MAX_PER_HOST = 4;
@@ -334,9 +356,6 @@ sub concurrency {
 
 sub start_query { # always runs in main (lei-daemon) process
 	my ($self, $lei) = @_;
-	if (my $l2m = $lei->{l2m}) {
-		$lei->start_mua if $l2m->lock_free;
-	}
 	if ($lei->{opt}->{threads}) {
 		for my $ibxish (locals($self)) {
 			$self->wq_io_do('query_thread_mset', [], $ibxish);
@@ -387,6 +406,9 @@ sub do_query {
 	my $l2m = $lei->{l2m};
 	if ($l2m) {
 		$l2m->pre_augment($lei);
+		if ($lei->{opt}->{augment} && delete $lei->{early_mua}) {
+			$lei->start_mua;
+		}
 		$l2m->wq_workers_start('lei2mail', $l2m->{jobs},
 					$lei->oldset, { lei => $lei });
 		pipe($lei->{startq}, $lei->{au_done}) or die "pipe: $!";
@@ -404,7 +426,7 @@ sub do_query {
 	delete $lei->{pkt_op_p};
 	$l2m->wq_close(1) if $l2m;
 	$lei->event_step_init; # wait for shutdowns
-	$self->wq_io_do('query_prepare', []) if $l2m;
+	$self->wq_io_do('query_prepare', []) if $l2m; # for augment/dedupe
 	start_query($self, $lei);
 	$self->wq_close(1); # lei_xsearch workers stop when done
 	if ($lei->{oneshot}) {

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

* [PATCH 02/13] lei_xsearch: quiet Eml warnings from remote mboxrds
  2021-02-08  9:05 [PATCH 00/13] lei approxidate, startup fix, --alert Eric Wong
  2021-02-08  9:05 ` [PATCHv2 01/13] lei q: improve remote mboxrd UX + MUA Eric Wong
@ 2021-02-08  9:05 ` Eric Wong
  2021-02-08  9:05 ` [PATCH 03/13] lei q: SIGWINCH process group with the terminal Eric Wong
                   ` (10 subsequent siblings)
  12 siblings, 0 replies; 15+ messages in thread
From: Eric Wong @ 2021-02-08  9:05 UTC (permalink / raw)
  To: meta

This will probably cover full Atom/HTML feed generation or any
outputs which are order-dependent, but those aren't prioritized
at the moment.
---
 lib/PublicInbox/LeiXSearch.pm | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/PublicInbox/LeiXSearch.pm b/lib/PublicInbox/LeiXSearch.pm
index db089a67..588df3a4 100644
--- a/lib/PublicInbox/LeiXSearch.pm
+++ b/lib/PublicInbox/LeiXSearch.pm
@@ -16,6 +16,7 @@ use PublicInbox::Search qw(xap_terms);
 use PublicInbox::Spawn qw(popen_rd spawn which);
 use PublicInbox::MID qw(mids);
 use PublicInbox::Smsg;
+use PublicInbox::Eml;
 use Fcntl qw(SEEK_SET F_SETFL O_APPEND O_RDWR);
 
 sub new {
@@ -376,6 +377,7 @@ sub start_query { # always runs in main (lei-daemon) process
 sub ipc_atfork_child {
 	my ($self) = @_;
 	$self->{lei}->lei_atfork_child;
+	$SIG{__WARN__} = PublicInbox::Eml::warn_ignore_cb();
 	$self->SUPER::ipc_atfork_child;
 }
 

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

* [PATCH 03/13] lei q: SIGWINCH process group with the terminal
  2021-02-08  9:05 [PATCH 00/13] lei approxidate, startup fix, --alert Eric Wong
  2021-02-08  9:05 ` [PATCHv2 01/13] lei q: improve remote mboxrd UX + MUA Eric Wong
  2021-02-08  9:05 ` [PATCH 02/13] lei_xsearch: quiet Eml warnings from remote mboxrds Eric Wong
@ 2021-02-08  9:05 ` Eric Wong
  2021-02-08  9:05 ` [PATCH 04/13] lei q: support --alert=CMD for early MUA users Eric Wong
                   ` (9 subsequent siblings)
  12 siblings, 0 replies; 15+ messages in thread
From: Eric Wong @ 2021-02-08  9:05 UTC (permalink / raw)
  To: meta

While using utime on the destination Maildir is enough for mutt
to eventually notice new mail, "eventually" isn't good enough.

Send a SIGWINCH to wake mutt (and likely other MUAs)
immediately.  This is more portable than relying on MUAs to
support inotify or EVFILT_VNODE.
---
 resent after rebasing due to 1/13 squashes

 lib/PublicInbox/LEI.pm        | 11 +++++++++++
 lib/PublicInbox/LeiXSearch.pm |  7 ++++++-
 script/lei                    |  8 +++++---
 3 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/lib/PublicInbox/LEI.pm b/lib/PublicInbox/LEI.pm
index c3645698..e95a674b 100644
--- a/lib/PublicInbox/LEI.pm
+++ b/lib/PublicInbox/LEI.pm
@@ -746,6 +746,17 @@ sub start_mua {
 	}
 }
 
+sub poke_mua { # forces terminal MUAs to wake up and hopefully notice new mail
+	my ($self) = @_;
+	return unless $self->{opt}->{mua} && -t $self->{1};
+	# hit the process group that started the MUA
+	if (my $s = $self->{sock}) {
+		send($s, '-WINCH', MSG_EOR);
+	} elsif ($self->{oneshot}) {
+		kill('-WINCH', $$);
+	}
+}
+
 # caller needs to "-t $self->{1}" to check if tty
 sub start_pager {
 	my ($self) = @_;
diff --git a/lib/PublicInbox/LeiXSearch.pm b/lib/PublicInbox/LeiXSearch.pm
index 588df3a4..10485220 100644
--- a/lib/PublicInbox/LeiXSearch.pm
+++ b/lib/PublicInbox/LeiXSearch.pm
@@ -317,7 +317,12 @@ Error closing $lei->{ovv}->{dst}: $!
 			}
 			$lei->{1} = $out;
 		}
-		$l2m->lock_free ? $l2m->poke_dst : $lei->start_mua;
+		if ($l2m->lock_free) {
+			$l2m->poke_dst;
+			$lei->poke_mua;
+		} else { # mbox users
+			$lei->start_mua;
+		}
 	}
 	$lei->{-progress} and
 		$lei->err('# ', $lei->{-mset_total} // 0, " matches");
diff --git a/script/lei b/script/lei
index b7f21f14..0b0e2976 100755
--- a/script/lei
+++ b/script/lei
@@ -105,13 +105,15 @@ Falling back to (slow) one-shot mode
 			die "recvmsg: $!";
 		}
 		last if $buf eq '';
-		if ($buf =~ /\Ax_it ([0-9]+)\z/) {
+		if ($buf =~ /\Aexec (.+)\z/) {
+			$exec_cmd->(\@fds, split(/\0/, $1));
+		} elsif ($buf eq '-WINCH') {
+			kill($buf, $$); # for MUA
+		} elsif ($buf =~ /\Ax_it ([0-9]+)\z/) {
 			$x_it_code = $1 + 0;
 			last;
 		} elsif ($buf =~ /\Achild_error ([0-9]+)\z/) {
 			$x_it_code = $1 + 0;
-		} elsif ($buf =~ /\Aexec (.+)\z/) {
-			$exec_cmd->(\@fds, split(/\0/, $1));
 		} else {
 			$sigchld->();
 			die $buf;

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

* [PATCH 04/13] lei q: support --alert=CMD for early MUA users
  2021-02-08  9:05 [PATCH 00/13] lei approxidate, startup fix, --alert Eric Wong
                   ` (2 preceding siblings ...)
  2021-02-08  9:05 ` [PATCH 03/13] lei q: SIGWINCH process group with the terminal Eric Wong
@ 2021-02-08  9:05 ` Eric Wong
  2021-02-08  9:05 ` [PATCH 05/13] tests: favor IPv6 Eric Wong
                   ` (8 subsequent siblings)
  12 siblings, 0 replies; 15+ messages in thread
From: Eric Wong @ 2021-02-08  9:05 UTC (permalink / raw)
  To: meta

For --mua users writing to lock-free -o MFOLDER destinations;
we'll keep -WINCH and send an ASCII terminal bell when results
are complete.  This is intended to let early MUA spawners know
when lei2mail is done writing results.

We'll also support running arbitrary commands.  It may be used
to run play(1) (from SoX), handle pipelines+redirects
(e.g. "/bin/sh -c 'echo search done | wall'") or other commands.
---
 lib/PublicInbox/LEI.pm         | 54 ++++++++++++++++++++++++----------
 lib/PublicInbox/LeiOverview.pm |  5 +++-
 2 files changed, 43 insertions(+), 16 deletions(-)

diff --git a/lib/PublicInbox/LEI.pm b/lib/PublicInbox/LEI.pm
index e95a674b..7b2a3e6f 100644
--- a/lib/PublicInbox/LEI.pm
+++ b/lib/PublicInbox/LEI.pm
@@ -112,7 +112,7 @@ our %CMD = ( # sorted in order of importance/use:
 	save-as=s output|mfolder|o=s format|f=s dedupe|d=s threads|t augment|a
 	sort|s=s reverse|r offset=i remote! local! external! pretty
 	include|I=s@ exclude=s@ only=s@ jobs|j=s globoff|g stdin|
-	mua=s no-torsocks torsocks=s verbose|v+ quiet|q),
+	alert=s@ mua=s no-torsocks torsocks=s verbose|v+ quiet|q),
 	PublicInbox::LeiQuery::curl_opt(), opt_dash('limit|n=i', '[0-9]+') ],
 
 'show' => [ 'MID|OID', 'show a given object (Message-ID or object ID)',
@@ -227,6 +227,11 @@ my %OPTDESC = (
 'show	threads|t' => 'display entire thread a message belongs to',
 'q	threads|t' =>
 	'return all messages in the same threads as the actual match(es)',
+'alert=s@' => ['CMD,-WINCH,-bell,<any command>',
+	'run command(s) or perform ops when done writing to output ' .
+	'(default: "-WINCH,-bell" with --mua and Maildir/IMAP output, ' .
+	'nothing otherwise)' ],
+
 'augment|a' => 'augment --output destination instead of clobbering',
 
 'output|mfolder|o=s' => [ 'MFOLDER',
@@ -739,21 +744,43 @@ sub start_mua {
 	if (my $sock = $self->{sock}) { # lei(1) client process runs it
 		send($sock, exec_buf(\@cmd, {}), MSG_EOR);
 	} elsif ($self->{oneshot}) {
-		$self->{"mua.pid.$self.$$"} = spawn(\@cmd);
+		$self->{"pid.$self.$$"}->{spawn(\@cmd)} = \@cmd;
 	}
 	if ($self->{lxs} && $self->{au_done}) { # kick wait_startq
 		syswrite($self->{au_done}, 'q' x ($self->{lxs}->{jobs} // 0));
 	}
+	$self->{opt}->{quiet} = 1;
+	delete $self->{-progress};
+	delete $self->{opt}->{verbose};
 }
 
 sub poke_mua { # forces terminal MUAs to wake up and hopefully notice new mail
 	my ($self) = @_;
-	return unless $self->{opt}->{mua} && -t $self->{1};
-	# hit the process group that started the MUA
-	if (my $s = $self->{sock}) {
-		send($s, '-WINCH', MSG_EOR);
-	} elsif ($self->{oneshot}) {
-		kill('-WINCH', $$);
+	my $alerts = $self->{opt}->{alert} // return;
+	while (my $op = shift(@$alerts)) {
+		if ($op eq '-WINCH') {
+			# hit the process group that started the MUA
+			if ($self->{sock}) {
+				send($self->{sock}, '-WINCH', MSG_EOR);
+			} elsif ($self->{oneshot}) {
+				kill('-WINCH', $$);
+			}
+		} elsif ($op eq '-bell') {
+			out($self, "\a");
+		} elsif ($op =~ /(?<!\\),/) { # bare ',' (not ',,')
+			push @$alerts, split(/(?<!\\),/, $op);
+		} elsif ($op =~ m!\A([/a-z0-9A-Z].+)!) {
+			my $cmd = $1; # run an arbitrary command
+			require Text::ParseWords;
+			$cmd = [ Text::ParseWords::shellwords($cmd) ];
+			if (my $s = $self->{sock}) {
+				send($s, exec_buf($cmd, {}), MSG_EOR);
+			} elsif ($self->{oneshot}) {
+				$self->{"pid.$self.$$"}->{spawn($cmd)} = $cmd;
+			}
+		} else {
+			err($self, "W: unsupported --alert=$op"); # non-fatal
+		}
 	}
 }
 
@@ -776,8 +803,8 @@ sub start_pager {
 		my $fds = [ map { fileno($_) } @$rdr{0..2} ];
 		$send_cmd->($sock, $fds, exec_buf([$pager], $new_env), MSG_EOR);
 	} elsif ($self->{oneshot}) {
-		$pgr->[0] = spawn([$pager], $new_env, $rdr);
-		$pgr->[3] = $$; # ew'll reap it
+		my $cmd = [$pager];
+		$self->{"pid.$self.$$"}->{spawn($cmd, $new_env, $rdr)} = $cmd;
 	} else {
 		die 'BUG: start_pager w/o socket';
 	}
@@ -793,8 +820,6 @@ sub stop_pager {
 	$self->{2} = $pgr->[2];
 	# do not restore original stdout, just close it so we error out
 	close(delete($self->{1})) if $self->{1};
-	my $pid = $pgr->[0];
-	dwaitpid($pid) if $pid && ($pgr->[3] // 0) == $$;
 }
 
 sub accept_dispatch { # Listener {post_accept} callback
@@ -1044,9 +1069,8 @@ sub DESTROY {
 	my ($self) = @_;
 	$self->{1}->autoflush(1) if $self->{1};
 	stop_pager($self);
-	if (my $mua_pid = delete $self->{"mua.pid.$self.$$"}) {
-		waitpid($mua_pid, 0);
-	}
+	my $oneshot_pids = delete $self->{"pid.$self.$$"} or return;
+	waitpid($_, 0) for keys %$oneshot_pids;
 }
 
 1;
diff --git a/lib/PublicInbox/LeiOverview.pm b/lib/PublicInbox/LeiOverview.pm
index f0ac4684..98c89d12 100644
--- a/lib/PublicInbox/LeiOverview.pm
+++ b/lib/PublicInbox/LeiOverview.pm
@@ -98,7 +98,10 @@ sub new {
 		$opt->{'sort'} //= 'docid' if $dst ne '/dev/stdout';
 		$lei->{l2m} = eval { PublicInbox::LeiToMail->new($lei) };
 		return $lei->fail($@) if $@;
-		$lei->{early_mua} = 1 if $opt->{mua} && $lei->{l2m}->lock_free;
+		if ($opt->{mua} && $lei->{l2m}->lock_free) {
+			$lei->{early_mua} = 1;
+			$opt->{alert} //= [ '-WINCH,-bell' ] if -t $lei->{1};
+		}
 	}
 	$self;
 }

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

* [PATCH 05/13] tests: favor IPv6
  2021-02-08  9:05 [PATCH 00/13] lei approxidate, startup fix, --alert Eric Wong
                   ` (3 preceding siblings ...)
  2021-02-08  9:05 ` [PATCH 04/13] lei q: support --alert=CMD for early MUA users Eric Wong
@ 2021-02-08  9:05 ` Eric Wong
  2021-02-08  9:05 ` [PATCH 06/13] ds: improve add_timer usability Eric Wong
                   ` (7 subsequent siblings)
  12 siblings, 0 replies; 15+ messages in thread
From: Eric Wong @ 2021-02-08  9:05 UTC (permalink / raw)
  To: meta

IPv4 gets plenty of real-world coverage, and apparently there's
Debian buildd hosts which lack IPv4(*).  So ensure everything
can work on IPv6 and not cause problems for odd setups.

(*) https://bugs.debian.org/979432
---
 lib/PublicInbox/TestCommon.pm | 30 ++++++++++++++++++++++++------
 t/extsearch.t                 |  2 +-
 t/httpd-corner.psgi           |  2 +-
 t/httpd-corner.t              | 12 +++++-------
 t/httpd-https.t               |  2 +-
 t/httpd-unix.t                |  7 +++----
 t/httpd.t                     |  8 +++-----
 t/imapd-tls.t                 |  4 ++--
 t/imapd.t                     |  8 ++------
 t/lei-mirror.t                |  2 +-
 t/nntpd-tls.t                 |  4 ++--
 t/nntpd.t                     | 11 ++++-------
 t/psgi_attach.t               |  2 +-
 t/psgi_v2.t                   |  2 +-
 t/solver_git.t                |  2 +-
 t/v2mirror.t                  |  3 +--
 t/v2writable.t                |  3 +--
 t/www_altid.t                 |  2 +-
 t/www_listing.t               |  3 +--
 xt/git-http-backend.t         |  4 +---
 xt/httpd-async-stream.t       |  2 +-
 xt/imapd-mbsync-oimap.t       |  4 +++-
 xt/imapd-validate.t           |  4 ++--
 xt/mem-imapd-tls.t            |  2 +-
 xt/nntpd-validate.t           |  3 +--
 xt/perf-nntpd.t               | 16 ++++++----------
 xt/solver.t                   |  3 +--
 27 files changed, 72 insertions(+), 75 deletions(-)

diff --git a/lib/PublicInbox/TestCommon.pm b/lib/PublicInbox/TestCommon.pm
index 5cce44e4..2f4ca622 100644
--- a/lib/PublicInbox/TestCommon.pm
+++ b/lib/PublicInbox/TestCommon.pm
@@ -14,7 +14,7 @@ BEGIN {
 	@EXPORT = qw(tmpdir tcp_server tcp_connect require_git require_mods
 		run_script start_script key2sub xsys xsys_e xqx eml_load tick
 		have_xapian_compact json_utf8 setup_public_inboxes
-		test_lei $lei $lei_out $lei_err $lei_opt);
+		tcp_host_port test_lei $lei $lei_out $lei_err $lei_opt);
 	require Test::More;
 	my @methods = grep(!/\W/, @Test::More::EXPORT);
 	eval(join('', map { "*$_=\\&Test::More::$_;" } @methods));
@@ -40,20 +40,38 @@ sub tmpdir (;$) {
 }
 
 sub tcp_server () {
-	IO::Socket::INET->new(
-		LocalAddr => '127.0.0.1',
+	my %opt = (
 		ReuseAddr => 1,
 		Proto => 'tcp',
 		Type => Socket::SOCK_STREAM(),
 		Listen => 1024,
 		Blocking => 0,
-	) or BAIL_OUT "failed to create TCP server: $!";
+	);
+	eval {
+		die 'IPv4-only' if $ENV{TEST_IPV4_ONLY};
+		require IO::Socket::INET6;
+		IO::Socket::INET6->new(%opt, LocalAddr => '[::1]')
+	} || eval {
+		die 'IPv6-only' if $ENV{TEST_IPV6_ONLY};
+		IO::Socket::INET->new(%opt, LocalAddr => '127.0.0.1')
+	} || BAIL_OUT "failed to create TCP server: $! ($@)";
+}
+
+sub tcp_host_port ($) {
+	my ($s) = @_;
+	my ($h, $p) = ($s->sockhost, $s->sockport);
+	my $ipv4 = $s->sockdomain == Socket::AF_INET();
+	if (wantarray) {
+		$ipv4 ? ($h, $p) : ("[$h]", $p);
+	} else {
+		$ipv4 ? "$h:$p" : "[$h]:$p";
+	}
 }
 
 sub tcp_connect {
 	my ($dest, %opt) = @_;
-	my $addr = $dest->sockhost . ':' . $dest->sockport;
-	my $s = IO::Socket::INET->new(
+	my $addr = tcp_host_port($dest);
+	my $s = ref($dest)->new(
 		Proto => 'tcp',
 		Type => Socket::SOCK_STREAM(),
 		PeerAddr => $addr,
diff --git a/t/extsearch.t b/t/extsearch.t
index 26c3d4ae..d199fc7b 100644
--- a/t/extsearch.t
+++ b/t/extsearch.t
@@ -14,7 +14,7 @@ use_ok 'PublicInbox::ExtSearch';
 use_ok 'PublicInbox::ExtSearchIdx';
 use_ok 'PublicInbox::OverIdx';
 my $sock = tcp_server();
-my $host_port = $sock->sockhost . ':' . $sock->sockport;
+my $host_port = tcp_host_port($sock);
 my ($home, $for_destroy) = tmpdir();
 local $ENV{HOME} = $home;
 mkdir "$home/.public-inbox" or BAIL_OUT $!;
diff --git a/t/httpd-corner.psgi b/t/httpd-corner.psgi
index 5436a74d..5fab2ba4 100644
--- a/t/httpd-corner.psgi
+++ b/t/httpd-corner.psgi
@@ -48,7 +48,7 @@ my $app = sub {
 		}
 	} elsif ($path eq '/host-port') {
 		$code = 200;
-		push @$body, "$env->{REMOTE_ADDR}:$env->{REMOTE_PORT}";
+		push @$body, "$env->{REMOTE_ADDR} $env->{REMOTE_PORT}";
 	} elsif ($path eq '/callback') {
 		return sub {
 			my ($res) = @_;
diff --git a/t/httpd-corner.t b/t/httpd-corner.t
index c3f80530..794d8aeb 100644
--- a/t/httpd-corner.t
+++ b/t/httpd-corner.t
@@ -11,7 +11,6 @@ use PublicInbox::TestCommon;
 require_mods(qw(Plack::Util Plack::Builder HTTP::Date HTTP::Status));
 use Digest::SHA qw(sha1_hex);
 use IO::Handle ();
-use IO::Socket;
 use IO::Socket::UNIX;
 use Fcntl qw(:seek);
 use Socket qw(IPPROTO_TCP TCP_NODELAY SOL_SOCKET);
@@ -50,14 +49,13 @@ sub unix_server ($) {
 		Listen => 1024,
 		Type => Socket::SOCK_STREAM(),
 		Local => $_[0],
-	);
+	) or BAIL_OUT "bind + listen $_[0]: $!";
 	$s->blocking(0);
 	$s;
 }
 
 my $upath = "$tmpdir/s";
 my $unix = unix_server($upath);
-ok($unix, 'UNIX socket created');
 my $td;
 my $spawn_httpd = sub {
 	my (@args) = @_;
@@ -219,7 +217,7 @@ sub check_400 {
 	ok($u, 'unix socket connected');
 	$u->write("GET /host-port HTTP/1.0\r\n\r\n");
 	$u->read(my $buf, 4096);
-	like($buf, qr!\r\n\r\n127\.0\.0\.1:0\z!,
+	like($buf, qr!\r\n\r\n127\.0\.0\.1 0\z!,
 		'set REMOTE_ADDR and REMOTE_PORT for Unix socket');
 }
 
@@ -236,8 +234,8 @@ sub conn_for {
 	$conn->write("GET /host-port HTTP/1.0\r\n\r\n");
 	$conn->read(my $buf, 4096);
 	my ($head, $body) = split(/\r\n\r\n/, $buf);
-	my ($addr, $port) = split(/:/, $body);
-	is($addr, $conn->sockhost, 'host matches addr');
+	my ($addr, $port) = split(/ /, $body);
+	is($addr, (tcp_host_port($conn))[0], 'host matches addr');
 	is($port, $conn->sockport, 'port matches');
 }
 
@@ -306,7 +304,7 @@ my $check_self = sub {
 
 SKIP: {
 	my $curl = which('curl') or skip('curl(1) missing', 4);
-	my $base = 'http://' . $sock->sockhost . ':' . $sock->sockport;
+	my $base = 'http://'.tcp_host_port($sock);
 	my $url = "$base/sha1";
 	my ($r, $w);
 	pipe($r, $w) or die "pipe: $!";
diff --git a/t/httpd-https.t b/t/httpd-https.t
index a2166ce6..b37492eb 100644
--- a/t/httpd-https.t
+++ b/t/httpd-https.t
@@ -21,7 +21,7 @@ my $err = "$tmpdir/stderr.log";
 my $out = "$tmpdir/stdout.log";
 my $https = tcp_server();
 my $td;
-my $https_addr = $https->sockhost . ':' . $https->sockport;
+my $https_addr = tcp_host_port($https);
 
 for my $args (
 	[ "-lhttps://$https_addr/?key=$key,cert=$cert" ],
diff --git a/t/httpd-unix.t b/t/httpd-unix.t
index 2d3cecc1..fe4a2161 100644
--- a/t/httpd-unix.t
+++ b/t/httpd-unix.t
@@ -42,13 +42,12 @@ for (1..1000) {
 ok(-S $unix, 'UNIX socket was bound by -httpd');
 sub check_sock ($) {
 	my ($unix) = @_;
-	my $sock = IO::Socket::UNIX->new(Peer => $unix, Type => SOCK_STREAM);
-	warn "E: $! connecting to $unix\n" unless defined $sock;
-	ok($sock, 'client UNIX socket connected');
+	my $sock = IO::Socket::UNIX->new(Peer => $unix, Type => SOCK_STREAM)
+		// BAIL_OUT "E: $! connecting to $unix";
 	ok($sock->write("GET /host-port HTTP/1.0\r\n\r\n"),
 		'wrote req to server');
 	ok($sock->read(my $buf, 4096), 'read response');
-	like($buf, qr!\r\n\r\n127\.0\.0\.1:0\z!,
+	like($buf, qr!\r\n\r\n127\.0\.0\.1 0\z!,
 		'set REMOTE_ADDR and REMOTE_PORT for Unix socket');
 }
 
diff --git a/t/httpd.t b/t/httpd.t
index 2fc28355..af9fbfeb 100644
--- a/t/httpd.t
+++ b/t/httpd.t
@@ -44,11 +44,9 @@ EOF
 		$im->add($mime);
 		$im->done($mime);
 	}
-	ok($sock, 'sock created');
 	$cmd = [ '-httpd', '-W0', "--stdout=$out", "--stderr=$err" ];
 	$td = start_script($cmd, undef, { 3 => $sock });
-	my $host = $sock->sockhost;
-	my $port = $sock->sockport;
+	my $http_pfx = 'http://'.tcp_host_port($sock);
 	{
 		my $bad = tcp_connect($sock);
 		print $bad "GETT / HTTP/1.0\r\n\r\n" or die;
@@ -65,7 +63,7 @@ EOF
 	}
 
 	is(xsys(qw(git clone -q --mirror),
-			"http://$host:$port/$group", "$tmpdir/clone.git"),
+			"$http_pfx/$group", "$tmpdir/clone.git"),
 		0, 'smart clone successful');
 
 	# ensure dumb cloning works, too:
@@ -73,7 +71,7 @@ EOF
 		qw(config http.uploadpack false)),
 		0, 'disable http.uploadpack');
 	is(xsys(qw(git clone -q --mirror),
-			"http://$host:$port/$group", "$tmpdir/dumb.git"),
+			"$http_pfx/$group", "$tmpdir/dumb.git"),
 		0, 'clone successful');
 
 	ok($td->kill, 'killed httpd');
diff --git a/t/imapd-tls.t b/t/imapd-tls.t
index e40ae1e8..ab90ddec 100644
--- a/t/imapd-tls.t
+++ b/t/imapd-tls.t
@@ -70,8 +70,8 @@ EOF
 	}
 }
 
-my $imaps_addr = $imaps->sockhost . ':' . $imaps->sockport;
-my $starttls_addr = $starttls->sockhost . ':' . $starttls->sockport;
+my $imaps_addr = tcp_host_port($imaps);
+my $starttls_addr = tcp_host_port($starttls);
 my $env = { PI_CONFIG => $pi_config };
 my $td;
 
diff --git a/t/imapd.t b/t/imapd.t
index 1df9d26e..0583dfdd 100644
--- a/t/imapd.t
+++ b/t/imapd.t
@@ -60,11 +60,8 @@ my $err = "$tmpdir/stderr.log";
 my $out = "$tmpdir/stdout.log";
 my $cmd = [ '-imapd', '-W0', "--stdout=$out", "--stderr=$err" ];
 my $td = start_script($cmd, undef, { 3 => $sock }) or BAIL_OUT("-imapd: $?");
-my %mic_opt = (
-	Server => $sock->sockhost,
-	Port => $sock->sockport,
-	Uid => 1,
-);
+my ($ihost, $iport) = tcp_host_port($sock);
+my %mic_opt = ( Server => $ihost, Port => $iport, Uid => 1 );
 my $mic = $imap_client->new(%mic_opt);
 my $pre_login_capa = $mic->capability;
 is(grep(/\AAUTH=ANONYMOUS\z/, @$pre_login_capa), 1,
@@ -456,7 +453,6 @@ SKIP: {
 	my $url = "http://example.com/i1";
 	my $inboxdir = "$tmpdir/watchimap";
 	my $cmd = ['-init', '-V2', '-Lbasic', $name, $inboxdir, $url, $addr];
-	my ($ihost, $iport) = ($sock->sockhost, $sock->sockport);
 	my $imapurl = "imap://$ihost:$iport/inbox.i1.0";
 	run_script($cmd) or BAIL_OUT("init $name");
 	xsys(qw(git config), "--file=$home/.public-inbox/config",
diff --git a/t/lei-mirror.t b/t/lei-mirror.t
index 667284fd..e3707979 100644
--- a/t/lei-mirror.t
+++ b/t/lei-mirror.t
@@ -6,7 +6,7 @@ require_git 2.6;
 require_mods(qw(DBD::SQLite Search::Xapian));
 my $sock = tcp_server();
 my ($tmpdir, $for_destroy) = tmpdir();
-my $http = 'http://'.$sock->sockhost.':'.$sock->sockport.'/';
+my $http = 'http://'.tcp_host_port($sock);
 my ($ro_home, $cfg_path) = setup_public_inboxes;
 my $cmd = [ qw(-httpd -W0), "--stdout=$tmpdir/out", "--stderr=$tmpdir/err" ];
 my $td = start_script($cmd, { PI_CONFIG => $cfg_path }, { 3 => $sock });
diff --git a/t/nntpd-tls.t b/t/nntpd-tls.t
index 1194be6f..8dab4ca8 100644
--- a/t/nntpd-tls.t
+++ b/t/nntpd-tls.t
@@ -70,8 +70,8 @@ EOF
 	}
 }
 
-my $nntps_addr = $nntps->sockhost . ':' . $nntps->sockport;
-my $starttls_addr = $starttls->sockhost . ':' . $starttls->sockport;
+my $nntps_addr = tcp_host_port($nntps);
+my $starttls_addr = tcp_host_port($starttls);
 my $env = { PI_CONFIG => $pi_config };
 my $td;
 
diff --git a/t/nntpd.t b/t/nntpd.t
index 63287d43..6c100138 100644
--- a/t/nntpd.t
+++ b/t/nntpd.t
@@ -8,7 +8,6 @@ use PublicInbox::Spawn qw(which);
 require_mods(qw(DBD::SQLite));
 require PublicInbox::InboxWritable;
 use PublicInbox::Eml;
-use IO::Socket;
 use Socket qw(IPPROTO_TCP TCP_NODELAY);
 use Net::NNTP;
 use Sys::Hostname;
@@ -34,6 +33,7 @@ my $addr = $group . '@example.com';
 
 my %opts;
 my $sock = tcp_server();
+my $host_port = tcp_host_port($sock);
 my $td;
 my $len;
 
@@ -96,10 +96,8 @@ EOF
 		}
 	}
 
-	ok($sock, 'sock created');
 	my $cmd = [ '-nntpd', '-W0', "--stdout=$out", "--stderr=$err" ];
 	$td = start_script($cmd, undef, { 3 => $sock });
-	my $host_port = $sock->sockhost . ':' . $sock->sockport;
 	my $n = Net::NNTP->new($host_port);
 	my $list = $n->list;
 	ok(delete $list->{'x.y.z'}, 'deleted x.y.z group');
@@ -376,7 +374,7 @@ Date: Fri, 02 Oct 1993 00:00:00 +0000
 		my @of = xqx([$lsof, '-p', $td->{pid}], undef, $noerr);
 		is(scalar(grep(/\(deleted\)/, @of)), 0, 'no deleted files');
 	};
-	SKIP: { test_watch($tmpdir, $sock, $group) };
+	SKIP: { test_watch($tmpdir, $host_port, $group) };
 	{
 		setsockopt($s, IPPROTO_TCP, TCP_NODELAY, 1);
 		syswrite($s, 'HDR List-id 1-');
@@ -417,7 +415,7 @@ sub read_til_dot {
 }
 
 sub test_watch {
-	my ($tmpdir, $sock, $group) = @_;
+	my ($tmpdir, $host_port, $group) = @_;
 	use_ok 'PublicInbox::Watch';
 	use_ok 'PublicInbox::InboxIdle';
 	use_ok 'PublicInbox::Config';
@@ -432,8 +430,7 @@ sub test_watch {
 	my $url = "http://example.com/i1";
 	my $inboxdir = "$tmpdir/watchnntp";
 	my $cmd = ['-init', '-V1', '-Lbasic', $name, $inboxdir, $url, $addr];
-	my ($ihost, $iport) = ($sock->sockhost, $sock->sockport);
-	my $nntpurl = "nntp://$ihost:$iport/$group";
+	my $nntpurl = "nntp://$host_port/$group";
 	run_script($cmd) or BAIL_OUT("init $name");
 	xsys(qw(git config), "--file=$home/.public-inbox/config",
 			"publicinbox.$name.watch",
diff --git a/t/psgi_attach.t b/t/psgi_attach.t
index 65d704bf..f53b7510 100644
--- a/t/psgi_attach.t
+++ b/t/psgi_attach.t
@@ -116,7 +116,7 @@ SKIP: {
 	my ($out, $err) = map { "$inboxdir/std$_.log" } qw(out err);
 	my $cmd = [ qw(-httpd -W0), "--stdout=$out", "--stderr=$err" ];
 	my $td = start_script($cmd, $env, { 3 => $sock });
-	my ($h, $p) = ($sock->sockhost, $sock->sockport);
+	my ($h, $p) = tcp_host_port($sock);
 	local $ENV{PLACK_TEST_EXTERNALSERVER_URI} = "http://$h:$p";
 	Plack::Test::ExternalServer::test_psgi(client => $client);
 }
diff --git a/t/psgi_v2.t b/t/psgi_v2.t
index 7ab60adc..81355f04 100644
--- a/t/psgi_v2.t
+++ b/t/psgi_v2.t
@@ -35,7 +35,7 @@ my $run_httpd = sub {
 		my ($out, $err) = map { "$inboxdir/std$_.log" } qw(out err);
 		my $cmd = [ qw(-httpd -W0), "--stdout=$out", "--stderr=$err" ];
 		my $td = start_script($cmd, $env, { 3 => $sock });
-		my ($h, $p) = ($sock->sockhost, $sock->sockport);
+		my ($h, $p) = tcp_host_port($sock);
 		local $ENV{PLACK_TEST_EXTERNALSERVER_URI} = "http://$h:$p";
 		Plack::Test::ExternalServer::test_psgi(client => $client);
 		$td->join('TERM');
diff --git a/t/solver_git.t b/t/solver_git.t
index d03a6f38..3ae7259a 100644
--- a/t/solver_git.t
+++ b/t/solver_git.t
@@ -196,7 +196,7 @@ EOF
 		my ($out, $err) = map { "$inboxdir/std$_.log" } qw(out err);
 		my $cmd = [ qw(-httpd -W0), "--stdout=$out", "--stderr=$err" ];
 		my $td = start_script($cmd, $env, { 3 => $sock });
-		my ($h, $p) = ($sock->sockhost, $sock->sockport);
+		my ($h, $p) = tcp_host_port($sock);
 		local $ENV{PLACK_TEST_EXTERNALSERVER_URI} = "http://$h:$p";
 		Plack::Test::ExternalServer::test_psgi(client => $client);
 	}
diff --git a/t/v2mirror.t b/t/v2mirror.t
index ebad2566..12e3fcd0 100644
--- a/t/v2mirror.t
+++ b/t/v2mirror.t
@@ -64,10 +64,9 @@ $v2w->done;
 $ibx->cleanup;
 
 my $sock = tcp_server();
-ok($sock, 'sock created');
 my $cmd = [ '-httpd', '-W0', "--stdout=$tmpdir/out", "--stderr=$tmpdir/err" ];
 my $td = start_script($cmd, undef, { 3 => $sock });
-my ($host, $port) = ($sock->sockhost, $sock->sockport);
+my ($host, $port) = tcp_host_port($sock);
 $sock = undef;
 
 my @cmd;
diff --git a/t/v2writable.t b/t/v2writable.t
index f5c8313a..f0fa8a79 100644
--- a/t/v2writable.t
+++ b/t/v2writable.t
@@ -164,12 +164,11 @@ EOF
 	;
 	close $fh or die "close: $!\n";
 	my $sock = tcp_server();
-	ok($sock, 'sock created');
 	my $len;
 	my $cmd = [ '-nntpd', '-W0', "--stdout=$out", "--stderr=$err" ];
 	my $env = { PI_CONFIG => $pi_config };
 	my $td = start_script($cmd, $env, { 3 => $sock });
-	my $host_port = $sock->sockhost . ':' . $sock->sockport;
+	my $host_port = tcp_host_port($sock);
 	my $n = Net::NNTP->new($host_port);
 	$n->group($group);
 	my $x = $n->xover('1-');
diff --git a/t/www_altid.t b/t/www_altid.t
index 14eda030..784acc8b 100644
--- a/t/www_altid.t
+++ b/t/www_altid.t
@@ -76,7 +76,7 @@ SKIP: {
 	my ($out, $err) = map { "$inboxdir/std$_.log" } qw(out err);
 	my $cmd = [ qw(-httpd -W0), "--stdout=$out", "--stderr=$err" ];
 	my $td = start_script($cmd, $env, { 3 => $sock });
-	my ($h, $p) = ($sock->sockhost, $sock->sockport);
+	my ($h, $p) = tcp_host_port($sock);
 	local $ENV{PLACK_TEST_EXTERNALSERVER_URI} = "http://$h:$p";
 	Plack::Test::ExternalServer::test_psgi(client => $client);
 }
diff --git a/t/www_listing.t b/t/www_listing.t
index 1bcbaefb..63ef2eac 100644
--- a/t/www_listing.t
+++ b/t/www_listing.t
@@ -79,8 +79,7 @@ SKIP: {
 	my $cfgfile = "$tmpdir/config";
 	my $v2 = "$tmpdir/v2";
 	my $sock = tcp_server();
-	ok($sock, 'sock created');
-	my ($host, $port) = ($sock->sockhost, $sock->sockport);
+	my ($host, $port) = tcp_host_port($sock);
 	my @clone = qw(git clone -q -s --bare);
 	is(xsys(@clone, $bare->{git_dir}, $alt), 0, 'clone shared repo');
 
diff --git a/xt/git-http-backend.t b/xt/git-http-backend.t
index dcff72cc..adadebb0 100644
--- a/xt/git-http-backend.t
+++ b/xt/git-http-backend.t
@@ -19,8 +19,7 @@ my ($tmpdir, $for_destroy) = tmpdir();
 my $err = "$tmpdir/stderr.log";
 my $out = "$tmpdir/stdout.log";
 my $sock = tcp_server();
-my $host = $sock->sockhost;
-my $port = $sock->sockport;
+my ($host, $port) = tcp_host_port($sock);
 my $td;
 
 my $get_maxrss = sub {
@@ -37,7 +36,6 @@ my $get_maxrss = sub {
 };
 
 {
-	ok($sock, 'sock created');
 	my $cmd = [ '-httpd', '-W0', "--stdout=$out", "--stderr=$err", $psgi ];
 	$td = start_script($cmd, undef, { 3 => $sock });
 }
diff --git a/xt/httpd-async-stream.t b/xt/httpd-async-stream.t
index f6715c58..c7039f3e 100644
--- a/xt/httpd-async-stream.t
+++ b/xt/httpd-async-stream.t
@@ -41,7 +41,7 @@ address = test\@example.com
 	# not using multiple workers, here, since we want to increase
 	# the chance of tripping concurrency bugs within PublicInbox/HTTP*.pm
 	my $cmd = [ '-httpd', "--stdout=$out", "--stderr=$err", '-W0' ];
-	my $host_port = $http->sockhost.':'.$http->sockport;
+	my $host_port = tcp_host_port($http);
 	push @$cmd, "-lhttp://$host_port";
 	my $url = "$host_port/test/$endpoint";
 	print STDERR "# CMD ". join(' ', @$cmd). "\n";
diff --git a/xt/imapd-mbsync-oimap.t b/xt/imapd-mbsync-oimap.t
index 5f671fc8..6635e2b4 100644
--- a/xt/imapd-mbsync-oimap.t
+++ b/xt/imapd-mbsync-oimap.t
@@ -35,6 +35,8 @@ my $td = start_script($cmd, $env, { 3 => $sock }) or BAIL_OUT "-imapd: $?";
 	my $c = tcp_connect($sock);
 	like(readline($c), qr/CAPABILITY /, 'got greeting');
 }
+
+my $host_port = tcp_host_port($sock);
 my ($host, $port) = ($sock->sockhost, $sock->sockport);
 my %pids;
 
@@ -120,7 +122,7 @@ while (scalar keys %pids) {
 my $sec = $ENV{TEST_PERSIST} // 0;
 diag "TEST_PERSIST=$sec";
 if ($sec) {
-	diag "sleeping ${sec}s, imap://$host:$port/$mailbox available";
+	diag "sleeping ${sec}s, imap://$host_port/$mailbox available";
 	diag "tmpdir=$tmpdir (Maildirs available)";
 	diag "stdout=$out";
 	diag "stderr=$err";
diff --git a/xt/imapd-validate.t b/xt/imapd-validate.t
index b6ac3e21..3a229883 100644
--- a/xt/imapd-validate.t
+++ b/xt/imapd-validate.t
@@ -152,11 +152,11 @@ $make_local_server = sub {
 	# not using multiple workers, here, since we want to increase
 	# the chance of tripping concurrency bugs within PublicInbox/IMAP*.pm
 	my $cmd = [ '-imapd', "--stdout=$out", "--stderr=$err", '-W0' ];
-	push @$cmd, '-limap://'.$imap->sockhost.':'.$imap->sockport;
+	push @$cmd, '-limap://'.tcp_host_port($imap);
 	if ($test_tls) {
 		my $imaps = tcp_server();
 		$rdr->{4} = $imaps;
-		push @$cmd, '-limaps://'.$imaps->sockhost.':'.$imaps->sockport;
+		push @$cmd, '-limaps://'.tcp_host_port($imaps);
 		push @$cmd, "--cert=$cert", "--key=$key";
 		my $tls_opt = [
 			SSL_hostname => 'server.local',
diff --git a/xt/mem-imapd-tls.t b/xt/mem-imapd-tls.t
index e4b3b8cd..99d8cb0d 100644
--- a/xt/mem-imapd-tls.t
+++ b/xt/mem-imapd-tls.t
@@ -45,7 +45,7 @@ my $imaps = tcp_server();
 EOF
 	close $fh or die "close: $!\n";
 }
-my $imaps_addr = $imaps->sockhost . ':' . $imaps->sockport;
+my $imaps_addr = tcp_host_port($imaps);
 my $env = { PI_CONFIG => $pi_config };
 my $arg = $TEST_TLS ? [ "-limaps://$imaps_addr/?cert=$cert,key=$key" ] : [];
 my $cmd = [ '-imapd', '-W0', @$arg, "--stdout=$out", "--stderr=$err" ];
diff --git a/xt/nntpd-validate.t b/xt/nntpd-validate.t
index efe97c02..83f024f9 100644
--- a/xt/nntpd-validate.t
+++ b/xt/nntpd-validate.t
@@ -169,8 +169,7 @@ sub make_local_server {
 		open my $fh, '>', $_ or die "truncate: $!";
 	}
 	my $sock = tcp_server();
-	ok($sock, 'sock created');
-	$host_port = $sock->sockhost . ':' . $sock->sockport;
+	$host_port = tcp_host_port($sock);
 
 	# not using multiple workers, here, since we want to increase
 	# the chance of tripping concurrency bugs within PublicInbox/NNTP*.pm
diff --git a/xt/perf-nntpd.t b/xt/perf-nntpd.t
index cd0d4938..85db036c 100644
--- a/xt/perf-nntpd.t
+++ b/xt/perf-nntpd.t
@@ -8,12 +8,15 @@ use PublicInbox::Inbox;
 use Net::NNTP;
 my $inboxdir = $ENV{GIANT_INBOX_DIR} // $ENV{GIANT_PI_DIR};
 plan skip_all => "GIANT_INBOX_DIR not defined for $0" unless defined($inboxdir);
-my ($host_port, $group, %opts, $s, $td, $tmp_obj);
+my ($host_port, $group, $s, $td, $tmp_obj);
 use PublicInbox::TestCommon;
 
 if (($ENV{NNTP_TEST_URL} || '') =~ m!\Anntp://([^/]+)/([^/]+)\z!) {
 	($host_port, $group) = ($1, $2);
 	$host_port .= ":119" unless index($host_port, ':') > 0;
+	my $six = substr($host_port, 0, 1) eq '[' ? '6' : '';
+	my $cls = "IO::Socket::INET$six";
+	$cls->new(Proto => 'tcp', Timeout => 1, PeerAddr => $host_port);
 } else {
 	$group = 'inbox.test.perf.nntpd';
 	my $ibx = { inboxdir => $inboxdir, newsgroup => $group };
@@ -34,18 +37,11 @@ if (($ENV{NNTP_TEST_URL} || '') =~ m!\Anntp://([^/]+)/([^/]+)\z!) {
 	}
 
 	my $sock = tcp_server();
-	ok($sock, 'sock created');
 	my $cmd = [ '-nntpd', '-W0' ];
 	$td = start_script($cmd, { PI_CONFIG => $pi_config }, { 3 => $sock });
-	$host_port = $sock->sockhost . ':' . $sock->sockport;
+	$host_port = tcp_host_port($sock);
+	$s = tcp_connect($sock);
 }
-%opts = (
-	PeerAddr => $host_port,
-	Proto => 'tcp',
-	Timeout => 1,
-);
-$s = IO::Socket::INET->new(%opts);
-$s->autoflush(1);
 my $buf = $s->getline;
 like($buf, qr/\A201 .* ready - post via email\r\n/s, 'got greeting');
 
diff --git a/xt/solver.t b/xt/solver.t
index 2f2fcc44..880458fb 100644
--- a/xt/solver.t
+++ b/xt/solver.t
@@ -67,8 +67,7 @@ SKIP: {
 	my ($out, $err) = map { "$tmpdir/std$_.log" } qw(out err);
 	my $cmd = [ qw(-httpd -W0), "--stdout=$out", "--stderr=$err" ];
 	my $td = start_script($cmd, undef, { 3 => $sock });
-	my ($h, $p) = ($sock->sockhost, $sock->sockport);
-
+	my ($h, $p) = tcp_host_port($sock);
 	local $ENV{PLACK_TEST_EXTERNALSERVER_URI} = "http://$h:$p";
 	while (($ibx_name, $urls) = each %$todo) {
 		Plack::Test::ExternalServer::test_psgi(client => $client);

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

* [PATCH 06/13] ds: improve add_timer usability
  2021-02-08  9:05 [PATCH 00/13] lei approxidate, startup fix, --alert Eric Wong
                   ` (4 preceding siblings ...)
  2021-02-08  9:05 ` [PATCH 05/13] tests: favor IPv6 Eric Wong
@ 2021-02-08  9:05 ` Eric Wong
  2021-02-08  9:05 ` [PATCH 07/13] lei: start_pager: drop COLUMNS default Eric Wong
                   ` (6 subsequent siblings)
  12 siblings, 0 replies; 15+ messages in thread
From: Eric Wong @ 2021-02-08  9:05 UTC (permalink / raw)
  To: meta

Packing args into an arrayref is awkward and we may be using
this API more in lei.
---
 lib/PublicInbox/DS.pm           | 10 +++++-----
 lib/PublicInbox/ExtSearchIdx.pm |  5 ++---
 lib/PublicInbox/FakeInotify.pm  |  4 ++--
 lib/PublicInbox/Watch.pm        | 19 ++++++++-----------
 4 files changed, 17 insertions(+), 21 deletions(-)

diff --git a/lib/PublicInbox/DS.pm b/lib/PublicInbox/DS.pm
index ec965abe..e5f36bc5 100644
--- a/lib/PublicInbox/DS.pm
+++ b/lib/PublicInbox/DS.pm
@@ -33,7 +33,7 @@ use PublicInbox::Syscall qw(:epoll);
 use PublicInbox::Tmpfile;
 use Errno qw(EAGAIN EINVAL);
 use Carp qw(carp);
-our @EXPORT_OK = qw(now msg_more dwaitpid);
+our @EXPORT_OK = qw(now msg_more dwaitpid add_timer);
 
 my $nextq; # queue for next_tick
 my $wait_pids; # list of [ pid, callback, callback_arg ]
@@ -96,12 +96,12 @@ Add a timer to occur $seconds from now. $seconds may be fractional, but timers
 are not guaranteed to fire at the exact time you ask for.
 
 =cut
-sub add_timer ($$;$) {
-    my ($secs, $coderef, $arg) = @_;
+sub add_timer ($$;@) {
+    my ($secs, $coderef, @args) = @_;
 
     my $fire_time = now() + $secs;
 
-    my $timer = [$fire_time, $coderef, $arg];
+    my $timer = [$fire_time, $coderef, @args];
 
     if (!@Timers || $fire_time >= $Timers[-1][0]) {
         push @Timers, $timer;
@@ -184,7 +184,7 @@ sub RunTimers {
     # Run expired timers
     while (@Timers && $Timers[0][0] <= $now) {
         my $to_run = shift(@Timers);
-        $to_run->[1]->($to_run->[2]);
+        $to_run->[1]->(@$to_run[2..$#$to_run]);
     }
 
     # timers may enqueue into nextq:
diff --git a/lib/PublicInbox/ExtSearchIdx.pm b/lib/PublicInbox/ExtSearchIdx.pm
index 9b7340df..a4b3bbd5 100644
--- a/lib/PublicInbox/ExtSearchIdx.pm
+++ b/lib/PublicInbox/ExtSearchIdx.pm
@@ -29,7 +29,7 @@ use PublicInbox::V2Writable;
 use PublicInbox::InboxWritable;
 use PublicInbox::ContentHash qw(content_hash);
 use PublicInbox::Eml;
-use PublicInbox::DS qw(now);
+use PublicInbox::DS qw(now add_timer);
 use DBI qw(:sql_types); # SQL_BLOB
 
 sub new {
@@ -1027,8 +1027,7 @@ sub on_inbox_unlock { # called by PublicInbox::InboxIdle
 	$pr->("indexing $ekey\n") if $pr;
 	$self->idx_init($opt);
 	sync_inbox($self, $self->{-watch_sync}, $ibx);
-	$self->{-commit_timer} //= PublicInbox::DS::add_timer(
-					$opt->{'commit-interval'} // 10,
+	$self->{-commit_timer} //= add_timer($opt->{'commit-interval'} // 10,
 					\&_watch_commit, $self);
 }
 
diff --git a/lib/PublicInbox/FakeInotify.pm b/lib/PublicInbox/FakeInotify.pm
index 326b2391..25818e07 100644
--- a/lib/PublicInbox/FakeInotify.pm
+++ b/lib/PublicInbox/FakeInotify.pm
@@ -6,7 +6,7 @@
 package PublicInbox::FakeInotify;
 use strict;
 use Time::HiRes qw(stat);
-use PublicInbox::DS;
+use PublicInbox::DS qw(add_timer);
 sub IN_MODIFY () { 0x02 } # match Linux inotify
 # my $IN_MOVED_TO = 0x80;
 # my $IN_CREATE = 0x100;
@@ -66,7 +66,7 @@ sub read {
 sub poll_once {
 	my ($obj) = @_;
 	$obj->event_step; # PublicInbox::InboxIdle::event_step
-	PublicInbox::DS::add_timer($poll_intvl, \&poll_once, $obj);
+	add_timer($poll_intvl, \&poll_once, $obj);
 }
 
 package PublicInbox::FakeInotify::Watch;
diff --git a/lib/PublicInbox/Watch.pm b/lib/PublicInbox/Watch.pm
index 185e5da8..1835fa0e 100644
--- a/lib/PublicInbox/Watch.pm
+++ b/lib/PublicInbox/Watch.pm
@@ -11,7 +11,7 @@ use PublicInbox::InboxWritable qw(eml_from_path);
 use PublicInbox::Filter::Base qw(REJECT);
 use PublicInbox::Spamcheck;
 use PublicInbox::Sigfd;
-use PublicInbox::DS qw(now);
+use PublicInbox::DS qw(now add_timer);
 use PublicInbox::MID qw(mids);
 use PublicInbox::ContentHash qw(content_hash);
 use PublicInbox::EOFpipe;
@@ -590,8 +590,8 @@ sub watch_atfork_parent ($) {
 	PublicInbox::DS::block_signals();
 }
 
-sub imap_idle_requeue ($) { # DS::add_timer callback
-	my ($self, $url_intvl) = @{$_[0]};
+sub imap_idle_requeue { # DS::add_timer callback
+	my ($self, $url_intvl) = @_;
 	return if $self->{quit};
 	push @{$self->{idle_todo}}, $url_intvl;
 	event_step($self);
@@ -605,8 +605,7 @@ sub imap_idle_reap { # PublicInbox::DS::dwaitpid callback
 	my ($url, $intvl) = @$url_intvl;
 	return if $self->{quit};
 	warn "W: PID=$pid on $url died: \$?=$?\n" if $?;
-	PublicInbox::DS::add_timer(60,
-				\&imap_idle_requeue, [ $self, $url_intvl ]);
+	add_timer(60, \&imap_idle_requeue, $self, $url_intvl);
 }
 
 sub reap { # callback for EOFpipe
@@ -700,8 +699,8 @@ sub watch_nntp_fetch_all ($$) {
 	}
 }
 
-sub poll_fetch_fork ($) { # DS::add_timer callback
-	my ($self, $intvl, $urls) = @{$_[0]};
+sub poll_fetch_fork { # DS::add_timer callback
+	my ($self, $intvl, $urls) = @_;
 	return if $self->{quit};
 	pipe(my ($r, $w)) or die "pipe: $!";
 	my $oldset = watch_atfork_parent($self);
@@ -736,8 +735,7 @@ sub poll_fetch_reap {
 		warn "W: PID=$pid died: \$?=$?\n", map { "$_\n" } @$urls;
 	}
 	warn("I: will check $_ in ${intvl}s\n") for @$urls;
-	PublicInbox::DS::add_timer($intvl, \&poll_fetch_fork,
-					[$self, $intvl, $urls]);
+	add_timer($intvl, \&poll_fetch_fork, $self, $intvl, $urls);
 }
 
 sub watch_imap_init ($$) {
@@ -1013,8 +1011,7 @@ sub watch { # main entry point
 	watch_nntp_init($self, $poll) if $self->{nntp};
 	while (my ($intvl, $urls) = each %$poll) {
 		# poll all URLs for a given interval sequentially
-		PublicInbox::DS::add_timer(0, \&poll_fetch_fork,
-						[$self, $intvl, $urls]);
+		add_timer(0, \&poll_fetch_fork, $self, $intvl, $urls);
 	}
 	watch_fs_init($self) if $self->{mdre};
 	PublicInbox::DS->SetPostLoopCallback(sub { !$self->quit_done });

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

* [PATCH 07/13] lei: start_pager: drop COLUMNS default
  2021-02-08  9:05 [PATCH 00/13] lei approxidate, startup fix, --alert Eric Wong
                   ` (5 preceding siblings ...)
  2021-02-08  9:05 ` [PATCH 06/13] ds: improve add_timer usability Eric Wong
@ 2021-02-08  9:05 ` Eric Wong
  2021-02-08  9:05 ` [PATCH 08/13] lei: avoid racing on unlink + bind + listen Eric Wong
                   ` (5 subsequent siblings)
  12 siblings, 0 replies; 15+ messages in thread
From: Eric Wong @ 2021-02-08  9:05 UTC (permalink / raw)
  To: meta

It shouldn't be needed since none of our subcommands will care
or attempt to format output.  Once "lei show" is implemented,
we'll run "git show" directly on the result.
---
 lib/PublicInbox/LEI.pm | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/lib/PublicInbox/LEI.pm b/lib/PublicInbox/LEI.pm
index 7b2a3e6f..2f370f52 100644
--- a/lib/PublicInbox/LEI.pm
+++ b/lib/PublicInbox/LEI.pm
@@ -792,8 +792,7 @@ sub start_pager {
 	chomp(my $pager = <$fh> // '');
 	close($fh) or warn "`git var PAGER' error: \$?=$?";
 	return if $pager eq 'cat' || $pager eq '';
-	# TODO TIOCGWINSZ
-	my $new_env = { LESS => 'FRX', LV => '-c', COLUMNS => 80 };
+	my $new_env = { LESS => 'FRX', LV => '-c' };
 	$new_env->{MORE} = 'FRX' if $^O eq 'freebsd';
 	pipe(my ($r, $wpager)) or return warn "pipe: $!";
 	my $rdr = { 0 => $r, 1 => $self->{1}, 2 => $self->{2} };

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

* [PATCH 08/13] lei: avoid racing on unlink + bind + listen
  2021-02-08  9:05 [PATCH 00/13] lei approxidate, startup fix, --alert Eric Wong
                   ` (6 preceding siblings ...)
  2021-02-08  9:05 ` [PATCH 07/13] lei: start_pager: drop COLUMNS default Eric Wong
@ 2021-02-08  9:05 ` Eric Wong
  2021-02-08  9:05 ` [PATCH 09/13] lei: drop BSD::Resource usage Eric Wong
                   ` (4 subsequent siblings)
  12 siblings, 0 replies; 15+ messages in thread
From: Eric Wong @ 2021-02-08  9:05 UTC (permalink / raw)
  To: meta

When multiple lei(1) processes are starting in parallel without
lei-daemon already running, it's possible for them to trample
each others' socket path trying to start lei-daemon.  Lock
errors.log before unlink/bind/listen.  We'll add an extra
connect(2) attempt to check if the starter lost the race.

Without this change, a stress script like the following could
easily cause problems:

	lei q -o ~/tmp/a foo ... &
	lei q -o ~/tmp/b bar ... &
	lei q -o ~/tmp/c quux ... &
	lei q -o ~/tmp/d baz ... &
---
 lib/PublicInbox/LEI.pm | 37 +++++++++++++++++++++++--------------
 1 file changed, 23 insertions(+), 14 deletions(-)

diff --git a/lib/PublicInbox/LEI.pm b/lib/PublicInbox/LEI.pm
index 2f370f52..cddb94e9 100644
--- a/lib/PublicInbox/LEI.pm
+++ b/lib/PublicInbox/LEI.pm
@@ -22,7 +22,7 @@ use PublicInbox::Syscall qw(SFD_NONBLOCK EPOLLIN EPOLLET);
 use PublicInbox::Sigfd;
 use PublicInbox::DS qw(now dwaitpid);
 use PublicInbox::Spawn qw(spawn popen_rd);
-use PublicInbox::OnDestroy;
+use PublicInbox::Lock;
 use Time::HiRes qw(stat); # ctime comparisons for config cache
 use File::Path qw(mkpath);
 use File::Spec;
@@ -828,17 +828,19 @@ sub accept_dispatch { # Listener {post_accept} callback
 	vec(my $rvec = '', fileno($sock), 1) = 1;
 	select($rvec, undef, undef, 60) or
 		return send($sock, 'timed out waiting to recv FDs', MSG_EOR);
-	my @fds = $recv_cmd->($sock, my $buf, 4096 * 33); # >MAX_ARG_STRLEN
+	# (4096 * 33) >MAX_ARG_STRLEN
+	my @fds = $recv_cmd->($sock, my $buf, 4096 * 33) or return; # EOF
 	if (scalar(@fds) == 4) {
 		for my $i (0..3) {
 			my $fd = shift(@fds);
 			open($self->{$i}, '+<&=', $fd) and next;
 			send($sock, "open(+<&=$fd) (FD=$i): $!", MSG_EOR);
 		}
-	} else {
-		my $msg = "recv_cmd failed: $!";
-		warn $msg;
+	} elsif (!defined($fds[0])) {
+		warn(my $msg = "recv_cmd failed: $!");
 		return send($sock, $msg, MSG_EOR);
+	} else {
+		return;
 	}
 	$self->{2}->autoflush(1); # keep stdout buffered until x_it|DESTROY
 	# $ENV_STR = join('', map { "\0$_=$ENV{$_}" } keys %ENV);
@@ -923,9 +925,19 @@ sub dump_and_clear_log {
 # lei(1) calls this when it can't connect
 sub lazy_start {
 	my ($path, $errno, $narg) = @_;
-	if ($errno == ECONNREFUSED) {
-		unlink($path) or die "unlink($path): $!";
-	} elsif ($errno != ENOENT) {
+	local ($errors_log, $listener);
+	($errors_log) = ($path =~ m!\A(.+?/)[^/]+\z!);
+	$errors_log .= 'errors.log';
+	my $addr = pack_sockaddr_un($path);
+	my $lk = bless { lock_path => $errors_log }, 'PublicInbox::Lock';
+	$lk->lock_acquire;
+	socket($listener, AF_UNIX, SOCK_SEQPACKET, 0) or die "socket: $!";
+	if ($errno == ECONNREFUSED || $errno == ENOENT) {
+		return if connect($listener, $addr); # another process won
+		if ($errno == ECONNREFUSED && -S $path) {
+			unlink($path) or die "unlink($path): $!";
+		}
+	} else {
 		$! = $errno; # allow interpolation to stringify in die
 		die "connect($path): $!";
 	}
@@ -935,10 +947,10 @@ sub lazy_start {
 		BSD::Resource::setrlimit($NOFILE, $h, $h) if $s < $h;
 	}
 	umask(077) // die("umask(077): $!");
-	local $listener;
-	socket($listener, AF_UNIX, SOCK_SEQPACKET, 0) or die "socket: $!";
-	bind($listener, pack_sockaddr_un($path)) or die "bind($path): $!";
+	bind($listener, $addr) or die "bind($path): $!";
 	listen($listener, 1024) or die "listen: $!";
+	$lk->lock_release;
+	undef $lk;
 	my @st = stat($path) or die "stat($path): $!";
 	my $dev_ino_expect = pack('dd', $st[0], $st[1]); # dev+ino
 	local $oldset = PublicInbox::DS::block_signals();
@@ -956,9 +968,6 @@ sub lazy_start {
 	require PublicInbox::Listener;
 	require PublicInbox::EOFpipe;
 	(-p STDOUT) or die "E: stdout must be a pipe\n";
-	local $errors_log;
-	($errors_log) = ($path =~ m!\A(.+?/)[^/]+\z!);
-	$errors_log .= 'errors.log';
 	open(STDIN, '+>>', $errors_log) or die "open($errors_log): $!";
 	STDIN->autoflush(1);
 	dump_and_clear_log("from previous daemon process:\n");

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

* [PATCH 09/13] lei: drop BSD::Resource usage
  2021-02-08  9:05 [PATCH 00/13] lei approxidate, startup fix, --alert Eric Wong
                   ` (7 preceding siblings ...)
  2021-02-08  9:05 ` [PATCH 08/13] lei: avoid racing on unlink + bind + listen Eric Wong
@ 2021-02-08  9:05 ` Eric Wong
  2021-02-08  9:05 ` [PATCH 10/13] git: implement date_parse method Eric Wong
                   ` (3 subsequent siblings)
  12 siblings, 0 replies; 15+ messages in thread
From: Eric Wong @ 2021-02-08  9:05 UTC (permalink / raw)
  To: meta

It's no longer necessary with the changes to stop doing
FD passing in our backend.

cf. commits 5180ed0a1cd65139 and 7d440bf3667b8ef5
    ("lei q: eliminate $not_done temporary git dir hack")
    ("lei q: reorder internals to reduce FD passing")
---
 lib/PublicInbox/LEI.pm | 5 -----
 script/lei             | 8 --------
 2 files changed, 13 deletions(-)

diff --git a/lib/PublicInbox/LEI.pm b/lib/PublicInbox/LEI.pm
index cddb94e9..e2a945a4 100644
--- a/lib/PublicInbox/LEI.pm
+++ b/lib/PublicInbox/LEI.pm
@@ -941,11 +941,6 @@ sub lazy_start {
 		$! = $errno; # allow interpolation to stringify in die
 		die "connect($path): $!";
 	}
-	if (eval { require BSD::Resource }) {
-		my $NOFILE = BSD::Resource::RLIMIT_NOFILE();
-		my ($s, $h) = BSD::Resource::getrlimit($NOFILE);
-		BSD::Resource::setrlimit($NOFILE, $h, $h) if $s < $h;
-	}
 	umask(077) // die("umask(077): $!");
 	bind($listener, $addr) or die "bind($path): $!";
 	listen($listener, 1024) or die "listen: $!";
diff --git a/script/lei b/script/lei
index 0b0e2976..cb605e2e 100755
--- a/script/lei
+++ b/script/lei
@@ -82,14 +82,6 @@ Falling back to (slow) one-shot mode
 	while (my ($k, $v) = each %ENV) { $buf .= "\0$k=$v" }
 	$buf .= "\0\0";
 	my $n = $send_cmd->($sock, [0, 1, 2, fileno($dh)], $buf, MSG_EOR);
-	if (!$n && $!{ETOOMANYREFS} && eval { require BSD::Resource }) {
-		my $NOFILE = BSD::Resource::RLIMIT_NOFILE();
-		my ($s, $h) = BSD::Resource::getrlimit($NOFILE);
-		if ($s < $h && BSD::Resource::setrlimit($NOFILE, $h, $h)) {
-			$n = $send_cmd->($sock, [0, 1, 2, fileno($dh)],
-					$buf, MSG_EOR);
-		}
-	}
 	if (!$n) {
 		die "sendmsg: $! (check RLIMIT_NOFILE)\n" if $!{ETOOMANYREFS};
 		die "sendmsg: $!\n";

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

* [PATCH 10/13] git: implement date_parse method
  2021-02-08  9:05 [PATCH 00/13] lei approxidate, startup fix, --alert Eric Wong
                   ` (8 preceding siblings ...)
  2021-02-08  9:05 ` [PATCH 09/13] lei: drop BSD::Resource usage Eric Wong
@ 2021-02-08  9:05 ` Eric Wong
  2021-02-08  9:05 ` [PATCH 11/13] lei q: use git approxidate with d:, dt: and rt: ranges Eric Wong
                   ` (2 subsequent siblings)
  12 siblings, 0 replies; 15+ messages in thread
From: Eric Wong @ 2021-02-08  9:05 UTC (permalink / raw)
  To: meta

Users are expected to be familiar with git's "approxidate"
functionality for parsing dates, so we'll expose that
in our UIs.  Xapian itself has limited date parsing functionality
and I can't expect users to learn it.

This takes around 4-5ms on my aging workstation, so it'll
probably be made acceptable for the WWW UI, even.

libgit2 has a git__date_parse function which I expect to have
less overhead, but it's only for internal use at the moment.
---
 lib/PublicInbox/Git.pm |  8 ++++++--
 t/git.t                | 15 +++++++++++++--
 2 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/lib/PublicInbox/Git.pm b/lib/PublicInbox/Git.pm
index c6c1c802..9207962b 100644
--- a/lib/PublicInbox/Git.pm
+++ b/lib/PublicInbox/Git.pm
@@ -362,8 +362,7 @@ sub popen {
 
 # same args as popen above
 sub qx {
-	my $self = shift;
-	my $fh = $self->popen(@_);
+	my $fh = popen(@_);
 	if (wantarray) {
 		local $/ = "\n";
 		my @ret = <$fh>;
@@ -377,6 +376,11 @@ sub qx {
 	}
 }
 
+sub date_parse {
+	my $d = $_[0]->qx('rev-parse', "--since=$_[1]");
+	substr($d, length('--max-age='), -1)
+}
+
 # check_async and cat_async may trigger the other, so ensure they're
 # both completely done by using this:
 sub async_wait_all ($) {
diff --git a/t/git.t b/t/git.t
index 0c85e492..7b950d88 100644
--- a/t/git.t
+++ b/t/git.t
@@ -1,12 +1,11 @@
 # Copyright (C) 2015-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 PublicInbox::TestCommon;
 my ($dir, $for_destroy) = tmpdir();
-use PublicInbox::Spawn qw(popen_rd);
 use PublicInbox::Import;
+use POSIX qw(strftime);
 
 use_ok 'PublicInbox::Git';
 
@@ -19,6 +18,18 @@ use_ok 'PublicInbox::Git';
 	xsys([qw(git fast-import --quiet)], { GIT_DIR => $dir }, $rdr);
 	is($?, 0, 'fast-import succeeded');
 }
+{
+	my $git = PublicInbox::Git->new($dir);
+	my $s = $git->date_parse('1970-01-01T00:00:00Z');
+	is($s, 0, 'parsed epoch');
+	local $ENV{TZ} = 'UTC';
+	$s = $git->date_parse('1993-10-02 01:02:09');
+	is(strftime('%Y-%m-%dT%H:%M:%SZ', gmtime($s)), '1993-10-02T01:02:09Z',
+		'round trips');
+	$s = $git->date_parse('1993-10-02');
+	is(strftime('%Y-%m-%d', gmtime($s)), '1993-10-02',
+		'round trips date-only');
+}
 
 {
 	my $gcf = PublicInbox::Git->new($dir);

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

* [PATCH 11/13] lei q: use git approxidate with d:, dt: and rt: ranges
  2021-02-08  9:05 [PATCH 00/13] lei approxidate, startup fix, --alert Eric Wong
                   ` (9 preceding siblings ...)
  2021-02-08  9:05 ` [PATCH 10/13] git: implement date_parse method Eric Wong
@ 2021-02-08  9:05 ` Eric Wong
  2021-02-10  9:59   ` [PATCH] search: fix argv handling of quoted phrases Eric Wong
  2021-02-08  9:05 ` [PATCH 12/13] search: use one git-rev-parse process for all dates Eric Wong
  2021-02-08  9:05 ` [PATCH 13/13] spawnpp: raise exception on E2BIG errors Eric Wong
  12 siblings, 1 reply; 15+ messages in thread
From: Eric Wong @ 2021-02-08  9:05 UTC (permalink / raw)
  To: meta

Instead of having --(sent|received)-(before|after)=s
command-line switches, we'll just try to make sense of argv so
it's usable within parenthesized statements and such.

Given the negligible performance penalty with Inline::C
process spawning, we'll probably wire this up to the
WWW interface, too.

"d:" is for mairix compatibility.  I don't know if "dt:" and
"rt:" will be too useful, but they exist because of IMAP
(and JMAP).
---
 lib/PublicInbox/LeiQuery.pm | 12 +++----
 lib/PublicInbox/Search.pm   | 67 +++++++++++++++++++++++++++++++++++++
 t/search.t                  | 44 ++++++++++++++++++++++++
 3 files changed, 115 insertions(+), 8 deletions(-)

diff --git a/lib/PublicInbox/LeiQuery.pm b/lib/PublicInbox/LeiQuery.pm
index 9a6fa718..d637b1ae 100644
--- a/lib/PublicInbox/LeiQuery.pm
+++ b/lib/PublicInbox/LeiQuery.pm
@@ -34,9 +34,10 @@ sub lei_q {
 	my @only = @{$opt->{only} // []};
 	# --local is enabled by default unless --only is used
 	# we'll allow "--only $LOCATION --local"
+	my $sto = $self->_lei_store(1);
+	my $lse = $sto->search;
 	if ($opt->{'local'} //= scalar(@only) ? 0 : 1) {
-		my $sto = $self->_lei_store(1);
-		$lxs->prepare_external($sto->search);
+		$lxs->prepare_external($lse);
 	}
 	if (@only) {
 		for my $loc (@only) {
@@ -107,12 +108,7 @@ no query allowed on command-line with --stdin
 		PublicInbox::InputPipe::consume($self->{0}, \&qstr_add, $self);
 		return;
 	}
-	# Consider spaces in argv to be for phrase search in Xapian.
-	# In other words, the users should need only care about
-	# normal shell quotes and not have to learn Xapian quoting.
-	$mset_opt{qstr} = join(' ', map {;
-		/\s/ ? (s/\A(\w+:)// ? qq{$1"$_"} : qq{"$_"}) : $_
-	} @argv);
+	$mset_opt{qstr} = $lse->query_argv_to_string($lse->git, \@argv);
 	$lxs->do_query($self);
 }
 
diff --git a/lib/PublicInbox/Search.pm b/lib/PublicInbox/Search.pm
index dbae3bc5..f42d70e3 100644
--- a/lib/PublicInbox/Search.pm
+++ b/lib/PublicInbox/Search.pm
@@ -8,6 +8,7 @@ use strict;
 use parent qw(Exporter);
 our @EXPORT_OK = qw(retry_reopen int_val get_pct xap_terms);
 use List::Util qw(max);
+use POSIX qw(strftime);
 
 # values for searching, changing the numeric value breaks
 # compatibility with old indices (so don't change them it)
@@ -259,6 +260,72 @@ sub reopen {
 	$self; # make chaining easier
 }
 
+# Convert git "approxidate" ranges to something usable with our
+# Xapian indices.  At the moment, Xapian only offers a C++-only API
+# and neither the SWIG nor XS bindings allow us to use custom code
+# to parse dates (and libgit2 doesn't expose git__date_parse, either,
+# so we're running git-rev-parse(1)).
+sub date_range {
+	my ($git, $pfx, $range) = @_;
+	# are we inside a parenthesized statement?
+	my $end = $range =~ s/([\)\s]*)\z// ? $1 : '';
+	my @r = split(/\.\./, $range, 2);
+
+	# expand "d:20101002" => "d:20101002..20101003" and like
+	# n.b. git doesn't do YYYYMMDD w/o '-', it needs YYYY-MM-DD
+	if ($pfx eq 'd') {
+		if (!defined($r[1])) {
+			$r[0] =~ s/\A([0-9]{4})([0-9]{2})([0-9]{2})\z/$1-$2-$3/;
+			$r[0] = $git->date_parse($r[0]);
+			$r[1] = $r[0] + 86400;
+			for my $x (@r) {
+				$x = strftime('%Y%m%d', gmtime($x));
+			}
+		} else {
+			for my $x (@r) {
+				next if $x eq '' || $x =~ /\A[0-9]{8}\z/;
+				$x = strftime('%Y%m%d',
+						gmtime($git->date_parse($x)));
+			}
+		}
+	} elsif ($pfx eq 'dt') {
+		if (!defined($r[1])) { # git needs gaps and not /\d{14}/
+			$r[0] =~ s/\A([0-9]{4})([0-9]{2})([0-9]{2})
+					([0-9]{2})([0-9]{2})([0-9]{2})\z
+				/$1-$2-$3 $4:$5:$6/x;
+			$r[0] = $git->date_parse($r[0]);
+			$r[1] = $r[0] + 86400;
+			for my $x (@r) {
+				$x = strftime('%Y%m%d%H%M%S', gmtime($x));
+			}
+		} else {
+			for my $x (@r) {
+				next if $x eq '' || $x =~ /\A[0-9]{14}\z/;
+				$x = strftime('%Y%m%d%H%M%S',
+						gmtime($git->date_parse($x)));
+			}
+		}
+	} else { # "rt", let git interpret "YYYY", deal with Y10K later :P
+		for my $x (@r) {
+			next if $x eq '' || $x =~ /\A[0-9]{5,}\z/;
+			$x = $git->date_parse($x);
+		}
+		$r[1] //= $r[0] + 86400;
+	}
+	"$pfx:".join('..', @r).$end;
+}
+
+sub query_argv_to_string {
+	my (undef, $git, $argv) = @_;
+	join(' ', map {;
+		if (s!\b(d|rt|dt):(.+)\z!date_range($git, $1, $2)!sge) {
+			$_;
+		} else {
+			/\s/ ? (s/\A(\w+:)// ? qq{$1"$_"} : qq{"$_}) : $_
+		}
+	} @$argv);
+}
+
 # read-only
 sub mset {
 	my ($self, $query_string, $opts) = @_;
diff --git a/t/search.t b/t/search.t
index b2958c00..56c7db1c 100644
--- a/t/search.t
+++ b/t/search.t
@@ -9,6 +9,7 @@ require PublicInbox::SearchIdx;
 require PublicInbox::Inbox;
 require PublicInbox::InboxWritable;
 use PublicInbox::Eml;
+use POSIX qw(strftime);
 my ($tmpdir, $for_destroy) = tmpdir();
 my $git_dir = "$tmpdir/a.git";
 my $ibx = PublicInbox::Inbox->new({ inboxdir => $git_dir });
@@ -534,4 +535,47 @@ $ibx->with_umask(sub {
 		'Subject search reaches inside message/rfc822');
 });
 
+SKIP: {
+	local $ENV{TZ} = 'UTC';
+	my $now = strftime('%H:%M:%S', gmtime(time));
+	if ($now =~ /\A23:(?:59|60)/ || $now =~ /\A00:00:0[01]\z/) {
+		skip 'too close to midnight, time is tricky', 6;
+	}
+	my ($s, $g) = ($ibx->search, $ibx->git);
+	my $q = $s->query_argv_to_string($g, [qw(d:20101002 blah)]);
+	is($q, 'd:20101002..20101003 blah', 'YYYYMMDD expanded to range');
+	$q = $s->query_argv_to_string($g, [qw(d:2010-10-02)]);
+	is($q, 'd:20101002..20101003', 'YYYY-MM-DD expanded to range');
+	$q = $s->query_argv_to_string($g, [qw(rt:2010-10-02.. yy)]);
+	$q =~ /\Art:(\d+)\.\. yy/ or fail("rt: expansion failed: $q");
+	is(strftime('%Y-%m-%d', gmtime($1//0)), '2010-10-02', 'rt: beg expand');
+	$q = $s->query_argv_to_string($g, [qw(rt:..2010-10-02 zz)]);
+	$q =~ /\Art:\.\.(\d+) zz/ or fail("rt: expansion failed: $q");
+	is(strftime('%Y-%m-%d', gmtime($1//0)), '2010-10-02', 'rt: end expand');
+	$q = $s->query_argv_to_string($g, [qw(something dt:2010-10-02..)]);
+	like($q, qr/\Asomething dt:20101002\d{6}\.\./, 'dt: expansion');
+	$q = $s->query_argv_to_string($g, [qw(x d:yesterday.. y)]);
+	is($q, strftime('x d:%Y%m%d.. y', gmtime(time - 86400)),
+		'"yesterday" handled');
+	$q = $s->query_argv_to_string($g, [qw(x dt:20101002054123)]);
+	is($q, 'x dt:20101002054123..20101003054123', 'single dt: expanded');
+	$q = $s->query_argv_to_string($g, [qw(x dt:2010-10-02T05:41:23Z)]);
+	is($q, 'x dt:20101002054123..20101003054123', 'ISO8601 dt: expanded');
+	$q = $s->query_argv_to_string($g, [qw(rt:1970..1971)]);
+	$q =~ /\Art:(\d+)\.\.(\d+)\z/ or fail "YYYY rt: expansion: $q";
+	my ($beg, $end) = ($1, $2);
+	is(strftime('%Y', gmtime($beg)), 1970, 'rt: starts at 1970');
+	is(strftime('%Y', gmtime($end)), 1971, 'rt: ends at 1971');
+	$q = $s->query_argv_to_string($g, [qw(rt:1970-01-01)]);
+	$q =~ /\Art:(\d+)\.\.(\d+)\z/ or fail "YYYY-MM-DD rt: expansion: $q";
+	($beg, $end) = ($1, $2);
+	is(strftime('%Y-%m-%d', gmtime($beg)), '1970-01-01',
+			'rt: date-only w/o range');
+	is(strftime('%Y-%m-%d', gmtime($end)), '1970-01-02',
+			'rt: date-only auto-end');
+	$q = $s->query_argv_to_string($g, [qw{OR (rt:1993-10-02)}]);
+	like($q, qr/\AOR \(rt:749\d{6}\.\.749\d{6}\)\z/,
+		'trailing parentheses preserved');
+}
+
 done_testing();

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

* [PATCH 12/13] search: use one git-rev-parse process for all dates
  2021-02-08  9:05 [PATCH 00/13] lei approxidate, startup fix, --alert Eric Wong
                   ` (10 preceding siblings ...)
  2021-02-08  9:05 ` [PATCH 11/13] lei q: use git approxidate with d:, dt: and rt: ranges Eric Wong
@ 2021-02-08  9:05 ` Eric Wong
  2021-02-08  9:05 ` [PATCH 13/13] spawnpp: raise exception on E2BIG errors Eric Wong
  12 siblings, 0 replies; 15+ messages in thread
From: Eric Wong @ 2021-02-08  9:05 UTC (permalink / raw)
  To: meta

This is necessary to avoid slowdowns with pathological cases
with many dates in the query, since each rev-parse invocation
takes ~5ms.

This is immeasurably slower with one open-ended range, but
already faster with any closed range featuring two dates which
require parsing via git.
---
 lib/PublicInbox/Git.pm    |  6 ++--
 lib/PublicInbox/Search.pm | 63 +++++++++++++++++++++++++--------------
 t/git.t                   | 16 +++++-----
 3 files changed, 54 insertions(+), 31 deletions(-)

diff --git a/lib/PublicInbox/Git.pm b/lib/PublicInbox/Git.pm
index 9207962b..ac7ff267 100644
--- a/lib/PublicInbox/Git.pm
+++ b/lib/PublicInbox/Git.pm
@@ -377,8 +377,10 @@ sub qx {
 }
 
 sub date_parse {
-	my $d = $_[0]->qx('rev-parse', "--since=$_[1]");
-	substr($d, length('--max-age='), -1)
+	my $self = shift;
+	map {
+		substr($_, length('--max-age='), -1)
+	} $self->qx('rev-parse', map { "--since=$_" } @_);
 }
 
 # check_async and cat_async may trigger the other, so ensure they're
diff --git a/lib/PublicInbox/Search.pm b/lib/PublicInbox/Search.pm
index f42d70e3..aa737d63 100644
--- a/lib/PublicInbox/Search.pm
+++ b/lib/PublicInbox/Search.pm
@@ -265,8 +265,10 @@ sub reopen {
 # and neither the SWIG nor XS bindings allow us to use custom code
 # to parse dates (and libgit2 doesn't expose git__date_parse, either,
 # so we're running git-rev-parse(1)).
-sub date_range {
-	my ($git, $pfx, $range) = @_;
+# This replaces things we need to send to $git->date_parse with
+# "\0".$strftime_format.['+'|$idx]."\0" placeholders
+sub date_parse_prepare {
+	my ($to_parse, $pfx, $range) = @_;
 	# are we inside a parenthesized statement?
 	my $end = $range =~ s/([\)\s]*)\z// ? $1 : '';
 	my @r = split(/\.\./, $range, 2);
@@ -275,55 +277,72 @@ sub date_range {
 	# n.b. git doesn't do YYYYMMDD w/o '-', it needs YYYY-MM-DD
 	if ($pfx eq 'd') {
 		if (!defined($r[1])) {
-			$r[0] =~ s/\A([0-9]{4})([0-9]{2})([0-9]{2})\z/$1-$2-$3/;
-			$r[0] = $git->date_parse($r[0]);
-			$r[1] = $r[0] + 86400;
-			for my $x (@r) {
-				$x = strftime('%Y%m%d', gmtime($x));
+			if ($r[0] =~ /\A([0-9]{4})([0-9]{2})([0-9]{2})\z/) {
+				push @$to_parse, "$1-$2-$3";
+				# we could've handled as-is, but we need
+				# to parse anyways for "d+" below
+			} else {
+				push @$to_parse, $r[0];
 			}
+			$r[0] = "\0%Y%m%d$#$to_parse\0";
+			$r[1] = "\0%Y%m%d+\0";
 		} else {
 			for my $x (@r) {
 				next if $x eq '' || $x =~ /\A[0-9]{8}\z/;
-				$x = strftime('%Y%m%d',
-						gmtime($git->date_parse($x)));
+				push @$to_parse, $x;
+				$x = "\0%Y%m%d$#$to_parse\0";
 			}
 		}
 	} elsif ($pfx eq 'dt') {
 		if (!defined($r[1])) { # git needs gaps and not /\d{14}/
-			$r[0] =~ s/\A([0-9]{4})([0-9]{2})([0-9]{2})
-					([0-9]{2})([0-9]{2})([0-9]{2})\z
-				/$1-$2-$3 $4:$5:$6/x;
-			$r[0] = $git->date_parse($r[0]);
-			$r[1] = $r[0] + 86400;
-			for my $x (@r) {
-				$x = strftime('%Y%m%d%H%M%S', gmtime($x));
+			if ($r[0] =~ /\A([0-9]{4})([0-9]{2})([0-9]{2})
+					([0-9]{2})([0-9]{2})([0-9]{2})\z/x) {
+				push @$to_parse, "$1-$2-$3 $4:$5:$6";
+			} else {
+				push @$to_parse, $r[0];
 			}
+			$r[0] = "\0%Y%m%d%H%M%S$#$to_parse\0";
+			$r[1] = "\0%Y%m%d%H%M%S+\0";
 		} else {
 			for my $x (@r) {
 				next if $x eq '' || $x =~ /\A[0-9]{14}\z/;
-				$x = strftime('%Y%m%d%H%M%S',
-						gmtime($git->date_parse($x)));
+				push @$to_parse, $x;
+				$x = "\0%Y%m%d%H%M%S$#$to_parse\0";
 			}
 		}
 	} else { # "rt", let git interpret "YYYY", deal with Y10K later :P
 		for my $x (@r) {
 			next if $x eq '' || $x =~ /\A[0-9]{5,}\z/;
-			$x = $git->date_parse($x);
+			push @$to_parse, $x;
+			$x = "\0%s$#$to_parse\0";
 		}
-		$r[1] //= $r[0] + 86400;
+		$r[1] //= "\0%s+\0";
 	}
 	"$pfx:".join('..', @r).$end;
 }
 
+# n.b. argv never has NUL, though we'll need to filter it out
+# if this $argv isn't from a command execution
 sub query_argv_to_string {
 	my (undef, $git, $argv) = @_;
-	join(' ', map {;
-		if (s!\b(d|rt|dt):(.+)\z!date_range($git, $1, $2)!sge) {
+	my $to_parse;
+	my $tmp = join(' ', map {;
+		if (s!\b(d|rt|dt):([[:print:]]+)\z!date_parse_prepare(
+						$to_parse //= [], $1, $2)!sge) {
 			$_;
 		} else {
 			/\s/ ? (s/\A(\w+:)// ? qq{$1"$_"} : qq{"$_}) : $_
 		}
 	} @$argv);
+	# git-rev-parse can handle any number of args up to system
+	# limits (around (4096*32) bytes on Linux).
+	if ($to_parse) {
+		my @r = $git->date_parse(@$to_parse);
+		my $i;
+		$tmp =~ s/\0(%[%YmdHMSs]+)([0-9\+]+)\0/strftime($1,
+			gmtime($2 eq '+' ? ($r[$i]+86400) : $r[$i=$2+0]))/sge;
+	}
+	$tmp
 }
 
 # read-only
diff --git a/t/git.t b/t/git.t
index 7b950d88..4a45bbaf 100644
--- a/t/git.t
+++ b/t/git.t
@@ -20,14 +20,16 @@ use_ok 'PublicInbox::Git';
 }
 {
 	my $git = PublicInbox::Git->new($dir);
-	my $s = $git->date_parse('1970-01-01T00:00:00Z');
-	is($s, 0, 'parsed epoch');
+	my @s = $git->date_parse('1970-01-01T00:00:00Z');
+	is($s[0], 0, 'parsed epoch');
 	local $ENV{TZ} = 'UTC';
-	$s = $git->date_parse('1993-10-02 01:02:09');
-	is(strftime('%Y-%m-%dT%H:%M:%SZ', gmtime($s)), '1993-10-02T01:02:09Z',
-		'round trips');
-	$s = $git->date_parse('1993-10-02');
-	is(strftime('%Y-%m-%d', gmtime($s)), '1993-10-02',
+	@s = $git->date_parse('1993-10-02 01:02:09', '2010-10-02 01:03:04');
+	is(strftime('%Y-%m-%dT%H:%M:%SZ', gmtime($s[0])),
+		'1993-10-02T01:02:09Z', 'round trips');
+	is(strftime('%Y-%m-%dT%H:%M:%SZ', gmtime($s[1])),
+		'2010-10-02T01:03:04Z', '2nd arg round trips');
+	@s = $git->date_parse('1993-10-02');
+	is(strftime('%Y-%m-%d', gmtime($s[0])), '1993-10-02',
 		'round trips date-only');
 }
 

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

* [PATCH 13/13] spawnpp: raise exception on E2BIG errors
  2021-02-08  9:05 [PATCH 00/13] lei approxidate, startup fix, --alert Eric Wong
                   ` (11 preceding siblings ...)
  2021-02-08  9:05 ` [PATCH 12/13] search: use one git-rev-parse process for all dates Eric Wong
@ 2021-02-08  9:05 ` Eric Wong
  12 siblings, 0 replies; 15+ messages in thread
From: Eric Wong @ 2021-02-08  9:05 UTC (permalink / raw)
  To: meta

This matches the Inline::C version, and lets us test
argv overflow with $search->query_argv_to_string;
---
 lib/PublicInbox/SpawnPP.pm | 23 +++++++++++++++++++----
 t/search.t                 |  7 +++++++
 2 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/lib/PublicInbox/SpawnPP.pm b/lib/PublicInbox/SpawnPP.pm
index 2c5edef6..6d7e2c34 100644
--- a/lib/PublicInbox/SpawnPP.pm
+++ b/lib/PublicInbox/SpawnPP.pm
@@ -16,13 +16,19 @@ sub pi_fork_exec ($$$$$$$) {
 	$set->fillset or die "fillset failed: $!";
 	sigprocmask(SIG_SETMASK, $set, $old) or die "can't block signals: $!";
 	my $syserr;
+	pipe(my ($r, $w));
 	my $pid = fork;
 	unless (defined $pid) { # compat with Inline::C version
 		$syserr = $!;
 		$pid = -1;
 	}
 	if ($pid == 0) {
-		$SIG{__DIE__} = sub { warn @_; _exit 1 };
+		close $r;
+		$SIG{__DIE__} = sub {
+			warn(@_);
+			syswrite($w, my $num = $! + 0);
+			_exit(1);
+		};
 		for my $child_fd (0..$#$redir) {
 			my $parent_fd = $redir->[$child_fd];
 			next if $parent_fd == $child_fd;
@@ -32,7 +38,9 @@ sub pi_fork_exec ($$$$$$$) {
 		if ($pgid >= 0 && !defined(setpgid(0, $pgid))) {
 			die "setpgid(0, $pgid): $!";
 		}
-		$SIG{$_} = 'DEFAULT' for keys %SIG;
+		for (keys %SIG) {
+			$SIG{$_} = 'DEFAULT' if substr($_, 0, 1) ne '_';
+		}
 		if ($cd ne '') {
 			chdir $cd or die "chdir $cd: $!";
 		}
@@ -49,11 +57,18 @@ sub pi_fork_exec ($$$$$$$) {
 		} else {
 			%ENV = map { split(/=/, $_, 2) } @$env;
 		}
-		exec @$cmd;
+		undef $r;
+		exec { $f } @$cmd;
 		die "exec @$cmd failed: $!";
 	}
+	close $w;
 	sigprocmask(SIG_SETMASK, $old) or die "can't unblock signals: $!";
-	$! = $syserr;
+	if (my $cerrnum = do { local $/, <$r> }) {
+		$pid = -1;
+		$! = $cerrnum;
+	} else {
+		$! = $syserr;
+	}
 	$pid;
 }
 
diff --git a/t/search.t b/t/search.t
index 56c7db1c..36a8fb30 100644
--- a/t/search.t
+++ b/t/search.t
@@ -576,6 +576,13 @@ SKIP: {
 	$q = $s->query_argv_to_string($g, [qw{OR (rt:1993-10-02)}]);
 	like($q, qr/\AOR \(rt:749\d{6}\.\.749\d{6}\)\z/,
 		'trailing parentheses preserved');
+	$ENV{TEST_EXPENSIVE} or
+		skip 'TEST_EXPENSIVE not set for argv overflow check', 1;
+	my @w;
+	local $SIG{__WARN__} = sub { push @w, @_ }; # for pure Perl version
+	my @fail = map { 'd:1993-10-02..2010-10-02' } (1..(4096 * 32));
+	eval { $s->query_argv_to_string($g, \@fail) };
+	ok($@, 'exception raised');
 }
 
 done_testing();

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

* [PATCH] search: fix argv handling of quoted phrases
  2021-02-08  9:05 ` [PATCH 11/13] lei q: use git approxidate with d:, dt: and rt: ranges Eric Wong
@ 2021-02-10  9:59   ` Eric Wong
  0 siblings, 0 replies; 15+ messages in thread
From: Eric Wong @ 2021-02-10  9:59 UTC (permalink / raw)
  To: meta

This fixes both an old bug in "lei q" argv handling and one
recent regression introduced with the change to use approxidate.

Field prefixes are also handled correctly inside parenthesized
statements when the field follows "(" without a separation
character.

Fixes: fbb7ccabbf54a405 ("lei q: use git approxidate with d:, dt: and rt: ranges")
---
 lib/PublicInbox/Search.pm |  4 +++-
 t/search.t                | 11 +++++++++--
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/lib/PublicInbox/Search.pm b/lib/PublicInbox/Search.pm
index 76a270bc..b3fd532d 100644
--- a/lib/PublicInbox/Search.pm
+++ b/lib/PublicInbox/Search.pm
@@ -330,8 +330,10 @@ sub query_argv_to_string {
 		if (s!\b(d|rt|dt):([[:print:]]+)\z!date_parse_prepare(
 						$to_parse //= [], $1, $2)!sge) {
 			$_;
+		} elsif (/\s/) {
+			s/(.*?)\b(\w+:)// ? qq{$1$2"$_"} : qq{"$_"};
 		} else {
-			/\s/ ? (s/\A(\w+:)// ? qq{$1"$_"} : qq{"$_}) : $_
+			$_
 		}
 	} @$argv);
 	# git-rev-parse can handle any number of args up to system
diff --git a/t/search.t b/t/search.t
index 36a8fb30..bcfe91f5 100644
--- a/t/search.t
+++ b/t/search.t
@@ -536,13 +536,20 @@ $ibx->with_umask(sub {
 });
 
 SKIP: {
+	my ($s, $g) = ($ibx->search, $ibx->git);
+	my $q = $s->query_argv_to_string($g, ["quoted phrase"]);
+	is($q, q["quoted phrase"], 'quoted phrase');
+	$q = $s->query_argv_to_string($g, ['s:pa ce']);
+	is($q, q[s:"pa ce"], 'space with prefix');
+	$q = $s->query_argv_to_string($g, ["\(s:pa ce", "AND", "foo\)"]);
+	is($q, q[(s:"pa ce" AND foo)], 'space AND foo');
+
 	local $ENV{TZ} = 'UTC';
 	my $now = strftime('%H:%M:%S', gmtime(time));
 	if ($now =~ /\A23:(?:59|60)/ || $now =~ /\A00:00:0[01]\z/) {
 		skip 'too close to midnight, time is tricky', 6;
 	}
-	my ($s, $g) = ($ibx->search, $ibx->git);
-	my $q = $s->query_argv_to_string($g, [qw(d:20101002 blah)]);
+	$q = $s->query_argv_to_string($g, [qw(d:20101002 blah)]);
 	is($q, 'd:20101002..20101003 blah', 'YYYYMMDD expanded to range');
 	$q = $s->query_argv_to_string($g, [qw(d:2010-10-02)]);
 	is($q, 'd:20101002..20101003', 'YYYY-MM-DD expanded to range');

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

end of thread, other threads:[~2021-02-10  9:59 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-08  9:05 [PATCH 00/13] lei approxidate, startup fix, --alert Eric Wong
2021-02-08  9:05 ` [PATCHv2 01/13] lei q: improve remote mboxrd UX + MUA Eric Wong
2021-02-08  9:05 ` [PATCH 02/13] lei_xsearch: quiet Eml warnings from remote mboxrds Eric Wong
2021-02-08  9:05 ` [PATCH 03/13] lei q: SIGWINCH process group with the terminal Eric Wong
2021-02-08  9:05 ` [PATCH 04/13] lei q: support --alert=CMD for early MUA users Eric Wong
2021-02-08  9:05 ` [PATCH 05/13] tests: favor IPv6 Eric Wong
2021-02-08  9:05 ` [PATCH 06/13] ds: improve add_timer usability Eric Wong
2021-02-08  9:05 ` [PATCH 07/13] lei: start_pager: drop COLUMNS default Eric Wong
2021-02-08  9:05 ` [PATCH 08/13] lei: avoid racing on unlink + bind + listen Eric Wong
2021-02-08  9:05 ` [PATCH 09/13] lei: drop BSD::Resource usage Eric Wong
2021-02-08  9:05 ` [PATCH 10/13] git: implement date_parse method Eric Wong
2021-02-08  9:05 ` [PATCH 11/13] lei q: use git approxidate with d:, dt: and rt: ranges Eric Wong
2021-02-10  9:59   ` [PATCH] search: fix argv handling of quoted phrases Eric Wong
2021-02-08  9:05 ` [PATCH 12/13] search: use one git-rev-parse process for all dates Eric Wong
2021-02-08  9:05 ` [PATCH 13/13] spawnpp: raise exception on E2BIG errors 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).