[PATCH 01/16] git: decouple cat_async_retry from POSIX pipe semantics

[PATCH 01/16] git: decouple cat_async_retry from POSIX pipe semantics

[Eric Wong]

While pipes guarantee writes of <= 512 bytes to be atomic,
Unix stream sockets (or TCP sockets) have no such guarantees.
Removing the pipe assumption will make it possible for us to
switch to bidirectional Unix stream sockets and save FDs with
`git cat-file' processes as we have with Gcf2Client.  The
performance benefit of larger pipe buffers over stream sockets
isn't irrelevant when interacting with git as it is with
SearchIdx shards.
---
 lib/PublicInbox/Git.pm | 20 ++++++++------------
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/lib/PublicInbox/Git.pm b/lib/PublicInbox/Git.pm
index eb88aa48..8ac40d2b 100644
--- a/lib/PublicInbox/Git.pm
+++ b/lib/PublicInbox/Git.pm
@@ -223,25 +223,21 @@ sub my_readline ($$) {
 }
 
 sub cat_async_retry ($$) {
-	my ($self, $inflight) = @_;
+	my ($self, $old_inflight) = @_;
 
 	# {inflight} may be non-existent, but if it isn't we delete it
 	# here to prevent cleanup() from waiting:
 	delete $self->{inflight};
 	cleanup($self);
+	batch_prepare($self, my $new_inflight = []);
 
-	batch_prepare($self, $inflight);
-	my $buf = '';
-	for (my $i = 0; $i < @$inflight; $i += 3) {
-		$buf .= "$inflight->[$i]\n";
+	while (my ($oid, $cb, $arg) = splice(@$old_inflight, 0, 3)) {
+		write_all($self, $self->{out}, $oid."\n",
+				\&cat_async_step, $new_inflight);
+		$oid = \$oid if !@$new_inflight; # to indicate oid retried
+		push @$new_inflight, $oid, $cb, $arg;
 	}
-	$self->{out}->blocking(1); # brand new pipe, should never block
-	print { $self->{out} } $buf or $self->fail("write error: $!");
-	$self->{out}->blocking(0);
-	my $req = shift @$inflight;
-	unshift(@$inflight, \$req); # \$ref to indicate retried
-
-	cat_async_step($self, $inflight); # take one step
+	cat_async_step($self, $new_inflight); # take one step
 }
 
 # returns true if prefetch is successful

[PATCH 02/16] git: use Unix stream sockets for `cat-file --batch-*'

[Eric Wong]

The benefit of 1MB potential pipe buffer size (on Linux) doesn't
seem noticeable when reading from git (unlike when writing to v2
shards), so Unix stream sockets seem fine, here.

This allows us to simplify our process management by using the
same socket FD for reads and writes and enables us to use our
ProcessPipe class for reaping (as we can do with Gcf2Client).

Gcf2Client no longer relies on PublicInbox::DS for write
buffering, and instead just waits for requests to complete
once the number of inflight requests hits the MAX_INFLIGHT
threshold as we do with PublicInbox::Git.

We reuse the existing MAX_INFLIGHT limit (18) that was
determined by the minimum allowed PIPE_BUF (512).  (AFAIK) Unix
stream sockets have no analogy to PIPE_BUF, but all *BSDs and
Linux I've checked have default SO_RCVBUF and SO_SNDBUF values
larger than the previously-required PIPE_BUF size of 512 bytes.
---
 lib/PublicInbox/Gcf2Client.pm  |  57 ++------
 lib/PublicInbox/Git.pm         | 254 +++++++++++++++++----------------
 lib/PublicInbox/GitAsyncCat.pm |  98 +------------
 lib/PublicInbox/LeiToMail.pm   |   2 +-
 lib/PublicInbox/ViewVCS.pm     |   2 +-
 5 files changed, 151 insertions(+), 262 deletions(-)

diff --git a/lib/PublicInbox/Gcf2Client.pm b/lib/PublicInbox/Gcf2Client.pm
index a49e2aad..8f442787 100644
--- a/lib/PublicInbox/Gcf2Client.pm
+++ b/lib/PublicInbox/Gcf2Client.pm
@@ -3,14 +3,15 @@
 
 # connects public-inbox processes to PublicInbox::Gcf2::loop()
 package PublicInbox::Gcf2Client;
-use strict;
+use v5.12;
 use parent qw(PublicInbox::DS);
 use PublicInbox::Git;
 use PublicInbox::Gcf2; # fails if Inline::C or libgit2-dev isn't available
 use PublicInbox::Spawn qw(spawn);
 use Socket qw(AF_UNIX SOCK_STREAM);
 use PublicInbox::Syscall qw(EPOLLIN EPOLLET);
-use PublicInbox::DS qw(awaitpid);
+use PublicInbox::ProcessPipe;
+
 # fields:
 #	sock => socket to Gcf2::loop
 # The rest of these fields are compatible with what PublicInbox::Git
@@ -21,66 +22,36 @@ use PublicInbox::DS qw(awaitpid);
 #	inflight => array (see PublicInbox::Git)
 #	rbuf => scalarref, may be non-existent or empty
 sub new  {
-	my ($rdr) = @_;
+	my ($opt) = @_;
 	my $self = bless {}, __PACKAGE__;
 	# ensure the child process has the same @INC we do:
 	my $env = { PERL5LIB => join(':', @INC) };
 	my ($s1, $s2);
 	socketpair($s1, $s2, AF_UNIX, SOCK_STREAM, 0) or die "socketpair $!";
-	$rdr //= {};
-	$rdr->{0} = $rdr->{1} = $s2;
-	my $cmd = [$^X, qw[-MPublicInbox::Gcf2 -e PublicInbox::Gcf2::loop]];
-	$self->{'pid.owner'} = $$;
-	awaitpid($self->{pid} = spawn($cmd, $env, $rdr), undef);
 	$s1->blocking(0);
+	$opt->{0} = $opt->{1} = $s2;
+	my $cmd = [$^X, qw[-MPublicInbox::Gcf2 -e PublicInbox::Gcf2::loop]];
+	my $pid = spawn($cmd, $env, $opt);
+	my $sock = PublicInbox::ProcessPipe->maybe_new($pid, $s1);
 	$self->{inflight} = [];
-	$self->{in} = $s1;
-	$self->SUPER::new($s1, EPOLLIN|EPOLLET);
-}
-
-sub fail {
-	my $self = shift;
-	$self->close; # PublicInbox::DS::close
-	PublicInbox::Git::fail($self, @_);
+	$self->{epwatch} = \undef; # for Git->cleanup
+	$self->SUPER::new($sock, EPOLLIN|EPOLLET);
 }
 
 sub gcf2_async ($$$;$) {
 	my ($self, $req, $cb, $arg) = @_;
 	my $inflight = $self->{inflight} or return $self->close;
-
-	# {wbuf} is rare, I hope:
-	cat_async_step($self, $inflight) if $self->{wbuf};
-
-	$self->fail("gcf2c write: $!") if !$self->write($req) && !$self->{sock};
+	PublicInbox::Git::write_all($self, $$req, \&cat_async_step, $inflight);
 	push @$inflight, $req, $cb, $arg;
 }
 
 # ensure PublicInbox::Git::cat_async_step never calls cat_async_retry
 sub alternates_changed {}
 
-# DS::event_loop will call this
-sub event_step {
-	my ($self) = @_;
-	$self->flush_write;
-	$self->close if !$self->{in} || !$self->{sock}; # process died
-	my $inflight = $self->{inflight};
-	if ($inflight && @$inflight) {
-		cat_async_step($self, $inflight);
-		return $self->close unless $self->{in}; # process died
-
-		# ok, more to do, requeue for fairness
-		$self->requeue if @$inflight || exists($self->{rbuf});
-	}
-}
-
-sub DESTROY {
-	my ($self) = @_;
-	delete $self->{sock}; # if outside event_loop
-	PublicInbox::Git::DESTROY($self);
-}
-
 no warnings 'once';
 
-*cat_async_step = \&PublicInbox::Git::cat_async_step;
+*cat_async_step = \&PublicInbox::Git::cat_async_step; # for event_step
+*event_step = \&PublicInbox::Git::event_step;
+*DESTROY = \&PublicInbox::Git::DESTROY;
 
 1;
diff --git a/lib/PublicInbox/Git.pm b/lib/PublicInbox/Git.pm
index 8ac40d2b..3062f293 100644
--- a/lib/PublicInbox/Git.pm
+++ b/lib/PublicInbox/Git.pm
@@ -9,23 +9,23 @@
 package PublicInbox::Git;
 use strict;
 use v5.10.1;
-use parent qw(Exporter);
+use parent qw(Exporter PublicInbox::DS);
 use POSIX ();
-use IO::Handle; # ->autoflush
+use IO::Handle; # ->blocking
+use Socket qw(AF_UNIX SOCK_STREAM);
+use PublicInbox::Syscall qw(EPOLLIN EPOLLET);
 use Errno qw(EINTR EAGAIN);
 use File::Glob qw(bsd_glob GLOB_NOSORT);
 use File::Spec ();
 use Time::HiRes qw(stat);
-use PublicInbox::Spawn qw(popen_rd which);
+use PublicInbox::Spawn qw(spawn popen_rd which);
 use PublicInbox::Tmpfile;
 use IO::Poll qw(POLLIN);
 use Carp qw(croak carp);
 use PublicInbox::SHA ();
-use PublicInbox::DS qw(awaitpid);
 our %HEXLEN2SHA = (40 => 1, 64 => 256);
 our %OFMT2HEXLEN = (sha1 => 40, sha256 => 64);
 our @EXPORT_OK = qw(git_unquote git_quote %HEXLEN2SHA %OFMT2HEXLEN);
-our $PIPE_BUFSIZ = 65536; # Linux default
 our $in_cleanup;
 our $RDTIMEO = 60_000; # milliseconds
 our $async_warn; # true in read-only daemons
@@ -34,11 +34,8 @@ our $async_warn; # true in read-only daemons
 my @MODIFIED_DATE = qw[for-each-ref --sort=-committerdate
 			--format=%(committerdate:raw) --count=1];
 
-# 512: POSIX PIPE_BUF minimum (see pipe(7))
-# 65: SHA-256 hex size + "\n" in preparation for git using non-SHA1
-# 3: @$inflight is flattened [ $OID, $cb, $arg ]
 use constant {
-	MAX_INFLIGHT => int(512 / (65 + length('contents '))) * 3,
+	MAX_INFLIGHT => 18, # arbitrary, formerly based on PIPE_BUF
 	BATCH_CMD_VER => v2.36.0, # git 2.36+
 };
 
@@ -65,7 +62,7 @@ sub check_git_exe () {
 	if ($st ne $EXE_ST) {
 		my $rd = popen_rd([ $GIT_EXE, '--version' ]);
 		my $v = readline($rd);
-		close($rd) or die "$GIT_EXE --version: $?";
+		CORE::close($rd) or die "$GIT_EXE --version: $?";
 		$v =~ /\b([0-9]+(?:\.[0-9]+){2})/ or die
 			"$GIT_EXE --version output: $v # unparseable";
 		$GIT_VER = eval("v$1") // die "BUG: bad vstring: $1 ($v)";
@@ -144,17 +141,16 @@ sub last_check_err {
 	$buf;
 }
 
-sub _bidi_pipe {
-	my ($self, $batch, $in, $out, $pid, $err) = @_;
-	if (defined $self->{$pid}) {
-		Carp::cluck("BUG: self->{$pid} exists unexpectedly");
-		return;
-	}
-	pipe(my ($out_r, $out_w)) or $self->fail("pipe failed: $!");
-	my $rdr = { 0 => $out_r, pgid => 0 };
+sub _sock_cmd {
+	my ($self, $batch, $err_c) = @_;
+	$self->{sock} and Carp::confess('BUG: {sock} exists');
+	my ($s1, $s2);
+	socketpair($s1, $s2, AF_UNIX, SOCK_STREAM, 0) or die "socketpair $!";
+	$s1->blocking(0);
+	my $opt = { pgid => 0, 0 => $s2, 1 => $s2 };
 	my $gd = $self->{git_dir};
 	if ($gd =~ s!/([^/]+/[^/]+)\z!/!) {
-		$rdr->{-C} = $gd;
+		$opt->{-C} = $gd;
 		$gd = $1;
 	}
 
@@ -163,23 +159,13 @@ sub _bidi_pipe {
 	my $abbr = $GIT_VER lt v2.31.0 ? 40 : 'no';
 	my @cmd = ($GIT_EXE, "--git-dir=$gd", '-c', "core.abbrev=$abbr",
 			'cat-file', "--$batch");
-	if ($err) {
+	if ($err_c) {
 		my $id = "git.$self->{git_dir}.$batch.err";
-		$self->{$err} = $rdr->{2} = tmpfile($id, undef, 1) or
+		$self->{err_c} = $opt->{2} = tmpfile($id, undef, 1) or
 						$self->fail("tmpfile($id): $!");
 	}
-	# see lib/PublicInbox/ProcessPipe.pm for why we don't use that here
-	my ($in_r, $p) = popen_rd(\@cmd, undef, $rdr);
-	awaitpid($self->{$pid} = $p, undef);
-	$self->{"$pid.owner"} = $$;
-	$out_w->autoflush(1);
-	if ($^O eq 'linux') { # 1031: F_SETPIPE_SZ
-		fcntl($out_w, 1031, 4096);
-		fcntl($in_r, 1031, 4096) if $batch eq 'batch-check';
-	}
-	$out_w->blocking(0);
-	$self->{$out} = $out_w;
-	$self->{$in} = $in_r;
+	my $pid = spawn(\@cmd, undef, $opt);
+	$self->{sock} = PublicInbox::ProcessPipe->maybe_new($pid, $s1);
 }
 
 sub poll_in ($) { IO::Poll::_poll($RDTIMEO, fileno($_[0]), my $ev = POLLIN) }
@@ -189,7 +175,7 @@ sub my_read ($$$) {
 	my $left = $len - length($$rbuf);
 	my $r;
 	while ($left > 0) {
-		$r = sysread($fh, $$rbuf, $PIPE_BUFSIZ, length($$rbuf));
+		$r = sysread($fh, $$rbuf, $left, length($$rbuf));
 		if ($r) {
 			$left -= $r;
 		} elsif (defined($r)) { # EOF
@@ -210,8 +196,7 @@ sub my_readline ($$) {
 		if ((my $n = index($$rbuf, "\n")) >= 0) {
 			return substr($$rbuf, 0, $n + 1, '');
 		}
-		my $r = sysread($fh, $$rbuf, $PIPE_BUFSIZ, length($$rbuf))
-								and next;
+		my $r = sysread($fh, $$rbuf, 65536, length($$rbuf)) and next;
 
 		# return whatever's left on EOF
 		return substr($$rbuf, 0, length($$rbuf)+1, '') if defined($r);
@@ -229,11 +214,10 @@ sub cat_async_retry ($$) {
 	# here to prevent cleanup() from waiting:
 	delete $self->{inflight};
 	cleanup($self);
-	batch_prepare($self, my $new_inflight = []);
+	my $new_inflight = batch_prepare($self);
 
 	while (my ($oid, $cb, $arg) = splice(@$old_inflight, 0, 3)) {
-		write_all($self, $self->{out}, $oid."\n",
-				\&cat_async_step, $new_inflight);
+		write_all($self, $oid."\n", \&cat_async_step, $new_inflight);
 		$oid = \$oid if !@$new_inflight; # to indicate oid retried
 		push @$new_inflight, $oid, $cb, $arg;
 	}
@@ -246,7 +230,7 @@ sub async_prefetch {
 	my $inflight = $self->{inflight} or return;
 	return if @$inflight;
 	substr($oid, 0, 0) = 'contents ' if $self->{-bc};
-	write_all($self, $self->{out}, "$oid\n", \&cat_async_step, $inflight);
+	write_all($self, "$oid\n", \&cat_async_step, $inflight);
 	push(@$inflight, $oid, $cb, $arg);
 }
 
@@ -256,14 +240,14 @@ sub cat_async_step ($$) {
 	my ($req, $cb, $arg) = @$inflight[0, 1, 2];
 	my $rbuf = delete($self->{rbuf}) // \(my $new = '');
 	my ($bref, $oid, $type, $size);
-	my $head = my_readline($self->{in}, $rbuf);
+	my $head = my_readline($self->{sock}, $rbuf);
 	my $cmd = ref($req) ? $$req : $req;
 	# ->fail may be called via Gcf2Client.pm
 	my $info = $self->{-bc} && substr($cmd, 0, 5) eq 'info ';
 	if ($head =~ /^([0-9a-f]{40,}) (\S+) ([0-9]+)$/) {
 		($oid, $type, $size) = ($1, $2, $3 + 0);
 		unless ($info) { # --batch-command
-			$bref = my_read($self->{in}, $rbuf, $size + 1) or
+			$bref = my_read($self->{sock}, $rbuf, $size + 1) or
 				$self->fail(defined($bref) ?
 						'read EOF' : "read: $!");
 			chop($$bref) eq "\n" or
@@ -302,16 +286,16 @@ sub cat_async_wait ($) {
 	}
 }
 
-sub batch_prepare ($$) {
-	my ($self, $inflight) = @_;
+sub batch_prepare ($) {
+	my ($self) = @_;
 	check_git_exe();
 	if ($GIT_VER ge BATCH_CMD_VER) {
-		_bidi_pipe($self, qw(batch-command in out pid err_c));
 		$self->{-bc} = 1;
+		_sock_cmd($self, 'batch-command', 1);
 	} else {
-		_bidi_pipe($self, qw(batch in out pid));
+		_sock_cmd($self, 'batch');
 	}
-	$self->{inflight} = $inflight;
+	$self->{inflight} = [];
 }
 
 sub _cat_file_cb {
@@ -328,52 +312,59 @@ sub cat_file {
 }
 
 sub check_async_step ($$) {
-	my ($self, $inflight_c) = @_;
-	die 'BUG: inflight empty or odd' if scalar(@$inflight_c) < 3;
-	my ($req, $cb, $arg) = @$inflight_c[0, 1, 2];
-	my $rbuf = delete($self->{rbuf_c}) // \(my $new = '');
-	chomp(my $line = my_readline($self->{in_c}, $rbuf));
+	my ($ck, $inflight) = @_;
+	die 'BUG: inflight empty or odd' if scalar(@$inflight) < 3;
+	my ($req, $cb, $arg) = @$inflight[0, 1, 2];
+	my $rbuf = delete($ck->{rbuf}) // \(my $new = '');
+	chomp(my $line = my_readline($ck->{sock}, $rbuf));
 	my ($hex, $type, $size) = split(/ /, $line);
 
 	# git <2.21 would show `dangling' (2.21+ shows `ambiguous')
 	# https://public-inbox.org/git/20190118033845.s2vlrb3wd3m2jfzu@dcvr/T/
 	if ($hex eq 'dangling') {
-		my $ret = my_read($self->{in_c}, $rbuf, $type + 1);
-		$self->fail(defined($ret) ? 'read EOF' : "read: $!") if !$ret;
+		my $ret = my_read($ck->{sock}, $rbuf, $type + 1);
+		$ck->fail(defined($ret) ? 'read EOF' : "read: $!") if !$ret;
 	}
-	$self->{rbuf_c} = $rbuf if $$rbuf ne '';
-	splice(@$inflight_c, 0, 3); # don't retry $cb on ->fail
+	$ck->{rbuf} = $rbuf if $$rbuf ne '';
+	splice(@$inflight, 0, 3); # don't retry $cb on ->fail
 	eval { $cb->(undef, $hex, $type, $size, $arg) };
-	async_err($self, $req, $hex, $@, 'check') if $@;
+	async_err($ck, $req, $hex, $@, 'check') if $@;
 }
 
 sub check_async_wait ($) {
 	my ($self) = @_;
 	return cat_async_wait($self) if $self->{-bc};
-	my $inflight_c = $self->{inflight_c} or return;
-	check_async_step($self, $inflight_c) while (scalar(@$inflight_c));
+	my $ck = $self->{ck} or return;
+	my $inflight = $ck->{inflight} or return;
+	check_async_step($ck, $inflight) while (scalar(@$inflight));
+}
+
+# git <2.36
+sub ck {
+	$_[0]->{ck} //= bless { git_dir => $_[0]->{git_dir} },
+				'PublicInbox::GitCheck';
 }
 
 sub check_async_begin ($) {
 	my ($self) = @_;
-	die 'BUG: already in async check' if $self->{inflight_c};
 	cleanup($self) if alternates_changed($self);
 	check_git_exe();
 	if ($GIT_VER ge BATCH_CMD_VER) {
-		_bidi_pipe($self, qw(batch-command in out pid err_c));
 		$self->{-bc} = 1;
-		$self->{inflight} = [];
+		_sock_cmd($self, 'batch-command', 1);
 	} else {
-		_bidi_pipe($self, qw(batch-check in_c out_c pid_c err_c));
-		$self->{inflight_c} = [];
+		_sock_cmd($self = ck($self), 'batch-check', 1);
 	}
+	$self->{inflight} = [];
 }
 
 sub write_all {
-	my ($self, $out, $buf, $read_step, $inflight) = @_;
+	my ($self, $buf, $read_step, $inflight) = @_;
+	$self->{sock} // Carp::confess 'BUG: no {sock}';
+	Carp::confess('BUG: not an array') if ref($inflight) ne 'ARRAY';
 	$read_step->($self, $inflight) while @$inflight >= MAX_INFLIGHT;
 	do {
-		my $w = syswrite($out, $buf);
+		my $w = syswrite($self->{sock}, $buf);
 		if (defined $w) {
 			return if $w == length($buf);
 			substr($buf, 0, $w, ''); # sv_chop
@@ -386,16 +377,17 @@ sub write_all {
 
 sub check_async ($$$$) {
 	my ($self, $oid, $cb, $arg) = @_;
-	my $inflight = $self->{-bc} ?
-			($self->{inflight} // cat_async_begin($self)) :
-			($self->{inflight_c} // check_async_begin($self));
-	if ($self->{-bc}) {
+	my $inflight;
+	if ($self->{-bc}) { # likely as time goes on
+batch_command:
+		$inflight = $self->{inflight} // cat_async_begin($self);
 		substr($oid, 0, 0) = 'info ';
-		write_all($self, $self->{out}, "$oid\n",
-				\&cat_async_step, $inflight);
-	} else {
-		write_all($self, $self->{out_c}, "$oid\n",
-				\&check_async_step, $inflight);
+		write_all($self, "$oid\n", \&cat_async_step, $inflight);
+	} else { # accounts for git upgrades while we're running:
+		my $ck = $self->{ck}; # undef OK, maybe set in check_async_begin
+		$inflight = $ck->{inflight} // check_async_begin($self);
+		goto batch_command if $self->{-bc};
+		write_all($self->{ck}, "$oid\n", \&check_async_step, $inflight);
 	}
 	push(@$inflight, $oid, $cb, $arg);
 }
@@ -418,39 +410,9 @@ sub check {
 	($hex, $type, $size);
 }
 
-sub _destroy {
-	my ($self, $pid, @rest) = @_; # rest = rbuf, in, out, err
-	my ($p) = delete @$self{($pid, @rest)};
-
-	# GitAsyncCat::event_step may delete {$pid}
-	awaitpid($p) if defined($p) && $$ == $self->{"$pid.owner"};
-}
-
-sub async_abort ($) {
-	my ($self) = @_;
-	while (scalar(@{$self->{inflight_c} // []}) ||
-			scalar(@{$self->{inflight} // []})) {
-		for my $c ('', '_c') {
-			my $q = $self->{"inflight$c"} or next;
-			while (@$q) {
-				my ($req, $cb, $arg) = splice(@$q, 0, 3);
-				$req = $$req if ref($req);
-				$self->{-bc} and
-					$req =~ s/\A(?:contents|info) //;
-				$req =~ s/ .*//; # drop git_dir for Gcf2Client
-				eval { $cb->(undef, $req, undef, undef, $arg) };
-				warn "E: (in abort) $req: $@" if $@;
-			}
-			delete $self->{"inflight$c"};
-			delete $self->{"rbuf$c"};
-		}
-	}
-	cleanup($self);
-}
-
-sub fail { # may be augmented in subclasses
+sub fail {
 	my ($self, $msg) = @_;
-	async_abort($self);
+	$self->close;
 	croak(ref($self) . ' ' . ($self->{git_dir} // '') . ": $msg");
 }
 
@@ -475,12 +437,12 @@ sub qx {
 	my $fh = popen(@_);
 	if (wantarray) {
 		my @ret = <$fh>;
-		close $fh; # caller should check $?
+		CORE::close $fh; # caller should check $?
 		@ret;
 	} else {
 		local $/;
 		my $ret = <$fh>;
-		close $fh; # caller should check $?
+		CORE::close $fh; # caller should check $?
 		$ret;
 	}
 }
@@ -492,12 +454,16 @@ sub date_parse {
 	} $self->qx('rev-parse', map { "--since=$_" } @_);
 }
 
+sub _active ($) {
+	scalar(@{$_[0]->{inflight} // []}) ||
+		($_[0]->{ck} && scalar(@{$_[0]->{ck}->{inflight} // []}))
+}
+
 # check_async and cat_async may trigger the other, so ensure they're
 # both completely done by using this:
 sub async_wait_all ($) {
 	my ($self) = @_;
-	while (scalar(@{$self->{inflight_c} // []}) ||
-			scalar(@{$self->{inflight} // []})) {
+	while (_active($self)) {
 		check_async_wait($self);
 		cat_async_wait($self);
 	}
@@ -506,14 +472,10 @@ sub async_wait_all ($) {
 # returns true if there are pending "git cat-file" processes
 sub cleanup {
 	my ($self, $lazy) = @_;
-	return 1 if $lazy && (scalar(@{$self->{inflight_c} // []}) ||
-				scalar(@{$self->{inflight} // []}));
+	return 1 if $lazy && _active($self);
 	local $in_cleanup = 1;
-	delete @$self{qw(async_cat async_chk)};
 	async_wait_all($self);
-	delete @$self{qw(inflight inflight_c -bc)};
-	_destroy($self, qw(pid rbuf in out err_c));
-	_destroy($self, qw(pid_c rbuf_c in_c out_c err_c));
+	$_->close for ($self, (delete($self->{ck}) // ()));
 	undef;
 }
 
@@ -530,7 +492,7 @@ sub packed_bytes {
 	$n
 }
 
-sub DESTROY { cleanup(@_) }
+sub DESTROY { cleanup($_[0]) }
 
 sub local_nick ($) {
 	# don't show full FS path, basename should be OK:
@@ -571,14 +533,14 @@ sub cat_async_begin {
 	my ($self) = @_;
 	cleanup($self) if $self->alternates_changed;
 	die 'BUG: already in async' if $self->{inflight};
-	batch_prepare($self, []);
+	batch_prepare($self);
 }
 
 sub cat_async ($$$;$) {
 	my ($self, $oid, $cb, $arg) = @_;
 	my $inflight = $self->{inflight} // cat_async_begin($self);
 	substr($oid, 0, 0) = 'contents ' if $self->{-bc};
-	write_all($self, $self->{out}, $oid."\n", \&cat_async_step, $inflight);
+	write_all($self, $oid."\n", \&cat_async_step, $inflight);
 	push(@$inflight, $oid, $cb, $arg);
 }
 
@@ -648,7 +610,7 @@ sub manifest_entry {
 	}
 	my $dig = PublicInbox::SHA->new(1);
 	while (read($sr, $buf, 65536)) { $dig->add($buf) }
-	close $sr or return; # empty, uninitialized git repo
+	CORE::close $sr or return; # empty, uninitialized git repo
 	$ent->{fingerprint} = $dig->hexdigest;
 	$ent->{modified} = modified(undef, $mod);
 	chomp($buf = <$own> // '');
@@ -664,8 +626,10 @@ sub cleanup_if_unlinked {
 	# Linux-specific /proc/$PID/maps access
 	# TODO: support this inside git.git
 	my $ret = 0;
-	for my $fld (qw(pid pid_c)) {
-		my $pid = $self->{$fld} // next;
+	for my $obj ($self, ($self->{ck} // ())) {
+		my $sock = $obj->{sock} // next;
+		my PublicInbox::ProcessPipe $pp = tied *$sock; # ProcessPipe
+		my $pid = $pp->{pid} // next;
 		open my $fh, '<', "/proc/$pid/maps" or return cleanup($self, 1);
 		while (<$fh>) {
 			# n.b. we do not restart for unlinked multi-pack-index
@@ -679,6 +643,50 @@ sub cleanup_if_unlinked {
 	$ret;
 }
 
+sub event_step {
+	my ($self) = @_;
+	$self->close if !$self->{sock}; # process died while requeued
+	my $inflight = $self->{inflight};
+	if ($inflight && @$inflight) {
+		$self->cat_async_step($inflight);
+		return $self->close unless $self->{sock};
+		# more to do? requeue for fairness:
+		$self->requeue if @$inflight || exists($self->{rbuf});
+	}
+}
+
+# idempotently registers with DS epoll/kqueue/select/poll
+sub watch_async ($) {
+	$_[0]->{epwatch} //= do {
+		$_[0]->SUPER::new($_[0]->{sock}, EPOLLIN|EPOLLET);
+		\undef;
+	}
+}
+
+sub close {
+	my ($self) = @_;
+	if (my $q = $self->{inflight}) { # abort inflight requests
+		while (@$q) {
+			my ($req, $cb, $arg) = splice(@$q, 0, 3);
+			$req = $$req if ref($req);
+			$self->{-bc} and $req =~ s/\A(?:contents|info) //;
+			$req =~ s/ .*//; # drop git_dir for Gcf2Client
+			eval { $cb->(undef, $req, undef, undef, $arg) };
+			warn "E: (in abort) $req: $@" if $@;
+		}
+	}
+	delete @$self{qw(-bc err_c inflight rbuf)};
+	delete($self->{epwatch}) ? $self->SUPER::close : delete($self->{sock});
+}
+
+package PublicInbox::GitCheck; # only for git <2.36
+use v5.12;
+our @ISA = qw(PublicInbox::Git);
+no warnings 'once';
+
+# for event_step
+*cat_async_step = \&PublicInbox::Git::check_async_step;
+
 1;
 __END__
 =pod
diff --git a/lib/PublicInbox/GitAsyncCat.pm b/lib/PublicInbox/GitAsyncCat.pm
index 671654b5..71ee1147 100644
--- a/lib/PublicInbox/GitAsyncCat.pm
+++ b/lib/PublicInbox/GitAsyncCat.pm
@@ -1,70 +1,12 @@
 # Copyright (C) all contributors <meta@public-inbox.org>
 # License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
-#
-# internal class used by PublicInbox::Git + PublicInbox::DS
-# This parses the output pipe of "git cat-file --batch"
 package PublicInbox::GitAsyncCat;
 use v5.12;
-use parent qw(PublicInbox::DS Exporter);
-use PublicInbox::DS qw(awaitpid);
-use POSIX qw(WNOHANG);
-use PublicInbox::Syscall qw(EPOLLIN EPOLLET);
+use parent qw(Exporter);
 our @EXPORT = qw(ibx_async_cat ibx_async_prefetch async_check);
-use PublicInbox::Git ();
 
 our $GCF2C; # singleton PublicInbox::Gcf2Client
 
-# close w/o aborting another git process
-sub vanish {
-	delete $_[0]->{git};
-	$_[0]->close;
-}
-
-sub close {
-	my ($self) = @_;
-	if (my $git = delete $self->{git}) {
-		$git->async_abort;
-	}
-	$self->SUPER::close; # PublicInbox::DS::close
-}
-
-sub aclose {
-	my (undef, $self, $f) = @_; # ignore PID ($_[0])
-	if (my $g = $self->{git}) {
-		return vanish($self) if ($g->{$f} // 0) != ($self->{sock} // 1);
-	}
-	$self->close;
-}
-
-sub event_step {
-	my ($self) = @_;
-	my $git = $self->{git} or return;
-	return vanish($self) if ($git->{in} // 0) != ($self->{sock} // 1);
-	my $inflight = $git->{inflight};
-	if ($inflight && @$inflight) {
-		$git->cat_async_step($inflight);
-
-		# child death?
-		if (($git->{in} // 0) != ($self->{sock} // 1)) {
-			vanish($self);
-		} elsif (@$inflight || exists $git->{rbuf}) {
-			# ok, more to do, requeue for fairness
-			$self->requeue;
-		}
-	}
-}
-
-sub watch_cat {
-	my ($git) = @_;
-	$git->{async_cat} //= do {
-		my $self = bless { git => $git }, __PACKAGE__;
-		$git->{in}->blocking(0);
-		$self->SUPER::new($git->{in}, EPOLLIN|EPOLLET);
-		awaitpid($git->{pid}, \&aclose, $self, 'in');
-		\undef; # this is a true ref()
-	};
-}
-
 sub ibx_async_cat ($$$$) {
 	my ($ibx, $oid, $cb, $arg) = @_;
 	my $git = $ibx->{git} // $ibx->git;
@@ -80,7 +22,7 @@ sub ibx_async_cat ($$$$) {
 		\undef;
 	} else { # read-only end of git-cat-file pipe
 		$git->cat_async($oid, $cb, $arg);
-		watch_cat($git);
+		$git->watch_async;
 	}
 }
 
@@ -88,14 +30,7 @@ sub async_check ($$$$) {
 	my ($ibx, $oidish, $cb, $arg) = @_; # $ibx may be $ctx
 	my $git = $ibx->{git} // $ibx->git;
 	$git->check_async($oidish, $cb, $arg);
-	return watch_cat($git) if $git->{-bc}; # --batch-command
-	$git->{async_chk} //= do {
-		my $self = bless { git => $git }, 'PublicInbox::GitAsyncCheck';
-		$git->{in_c}->blocking(0);
-		$self->SUPER::new($git->{in_c}, EPOLLIN|EPOLLET);
-		awaitpid($git->{pid_c}, \&aclose, $self, 'in_c');
-		\undef; # this is a true ref()
-	};
+	($git->{ck} // $git)->watch_async;
 }
 
 # this is safe to call inside $cb, but not guaranteed to enqueue
@@ -109,35 +44,10 @@ sub ibx_async_prefetch {
 			$oid .= " $git->{git_dir}\n";
 			return $GCF2C->gcf2_async(\$oid, $cb, $arg); # true
 		}
-	} elsif ($git->{async_cat}) {
+	} elsif ($git->{epwatch}) {
 		return $git->async_prefetch($oid, $cb, $arg);
 	}
 	undef;
 }
 
 1;
-package PublicInbox::GitAsyncCheck;
-use v5.12;
-our @ISA = qw(PublicInbox::GitAsyncCat);
-use POSIX qw(WNOHANG);
-use PublicInbox::Syscall qw(EPOLLIN EPOLLET);
-
-sub event_step {
-	my ($self) = @_;
-	my $git = $self->{git} or return;
-	return $self->vanish if ($git->{in_c} // 0) != ($self->{sock} // 1);
-	my $inflight = $git->{inflight_c};
-	if ($inflight && @$inflight) {
-		$git->check_async_step($inflight);
-
-		# child death?
-		if (($git->{in_c} // 0) != ($self->{sock} // 1)) {
-			$self->vanish;
-		} elsif (@$inflight || exists $git->{rbuf_c}) {
-			# ok, more to do, requeue for fairness
-			$self->requeue;
-		}
-	}
-}
-
-1;
diff --git a/lib/PublicInbox/LeiToMail.pm b/lib/PublicInbox/LeiToMail.pm
index 2dddf00b..98d0ac19 100644
--- a/lib/PublicInbox/LeiToMail.pm
+++ b/lib/PublicInbox/LeiToMail.pm
@@ -133,7 +133,7 @@ sub eml2mboxcl2 {
 sub git_to_mail { # git->cat_async callback
 	my ($bref, $oid, $type, $size, $smsg) = @_;
 	my $self = delete $smsg->{l2m} // die "BUG: no l2m";
-	$type // return; # called by git->async_abort
+	$type // return; # called by PublicInbox::Git::close
 	eval {
 		if ($type eq 'missing' &&
 			  ($bref = $self->{-lms_rw}->local_blob($oid, 1))) {
diff --git a/lib/PublicInbox/ViewVCS.pm b/lib/PublicInbox/ViewVCS.pm
index 5529ed5b..f80fb4cb 100644
--- a/lib/PublicInbox/ViewVCS.pm
+++ b/lib/PublicInbox/ViewVCS.pm
@@ -133,7 +133,7 @@ sub do_cat_async {
 	# favor git(1) over Gcf2 (libgit2) for SHA-256 support
 	$ctx->{git}->cat_async($_, $cb, $ctx) for @req;
 	if ($ctx->{env}->{'pi-httpd.async'}) {
-		PublicInbox::GitAsyncCat::watch_cat($ctx->{git});
+		$ctx->{git}->watch_async;
 	} else { # synchronous, generic PSGI
 		$ctx->{git}->cat_async_wait;
 	}

[PATCH 03/16] git+gcf2client: switch to level-triggered wakeups

[Eric Wong]

Instead of using ->requeue to emulate level-triggered wakeups in
userspace, just use level-triggered wakeups in the kernel to
save some user time at the expense of system (kernel) time.  Of
course, the ready list implementation in the kernel via C is
faster than a Perl one on our end.

We must still use requeue if we've got buffered data, however.

Followup-to: 1181a7e6a853 (listener: switch to level-triggered epoll)
---
 lib/PublicInbox/Gcf2Client.pm | 4 ++--
 lib/PublicInbox/Git.pm        | 7 ++++---
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/lib/PublicInbox/Gcf2Client.pm b/lib/PublicInbox/Gcf2Client.pm
index 8f442787..8ac44a5e 100644
--- a/lib/PublicInbox/Gcf2Client.pm
+++ b/lib/PublicInbox/Gcf2Client.pm
@@ -9,7 +9,7 @@ use PublicInbox::Git;
 use PublicInbox::Gcf2; # fails if Inline::C or libgit2-dev isn't available
 use PublicInbox::Spawn qw(spawn);
 use Socket qw(AF_UNIX SOCK_STREAM);
-use PublicInbox::Syscall qw(EPOLLIN EPOLLET);
+use PublicInbox::Syscall qw(EPOLLIN);
 use PublicInbox::ProcessPipe;
 
 # fields:
@@ -35,7 +35,7 @@ sub new  {
 	my $sock = PublicInbox::ProcessPipe->maybe_new($pid, $s1);
 	$self->{inflight} = [];
 	$self->{epwatch} = \undef; # for Git->cleanup
-	$self->SUPER::new($sock, EPOLLIN|EPOLLET);
+	$self->SUPER::new($sock, EPOLLIN);
 }
 
 sub gcf2_async ($$$;$) {
diff --git a/lib/PublicInbox/Git.pm b/lib/PublicInbox/Git.pm
index 3062f293..5003be53 100644
--- a/lib/PublicInbox/Git.pm
+++ b/lib/PublicInbox/Git.pm
@@ -650,15 +650,16 @@ sub event_step {
 	if ($inflight && @$inflight) {
 		$self->cat_async_step($inflight);
 		return $self->close unless $self->{sock};
-		# more to do? requeue for fairness:
-		$self->requeue if @$inflight || exists($self->{rbuf});
+		# don't loop here to keep things fair, but we must requeue
+		# if there's already-read data in rbuf
+		$self->requeue if exists($self->{rbuf});
 	}
 }
 
 # idempotently registers with DS epoll/kqueue/select/poll
 sub watch_async ($) {
 	$_[0]->{epwatch} //= do {
-		$_[0]->SUPER::new($_[0]->{sock}, EPOLLIN|EPOLLET);
+		$_[0]->SUPER::new($_[0]->{sock}, EPOLLIN);
 		\undef;
 	}
 }

[PATCH 04/16] gcf2:

[Eric Wong]

---
 lib/PublicInbox/Gcf2Client.pm  |  8 ++++----
 lib/PublicInbox/GitAsyncCat.pm |  4 ++--
 t/gcf2_client.t                | 32 ++++++++++++++++----------------
 3 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/lib/PublicInbox/Gcf2Client.pm b/lib/PublicInbox/Gcf2Client.pm
index 8ac44a5e..8e313c84 100644
--- a/lib/PublicInbox/Gcf2Client.pm
+++ b/lib/PublicInbox/Gcf2Client.pm
@@ -11,6 +11,7 @@ use PublicInbox::Spawn qw(spawn);
 use Socket qw(AF_UNIX SOCK_STREAM);
 use PublicInbox::Syscall qw(EPOLLIN);
 use PublicInbox::ProcessPipe;
+use autodie qw(socketpair);
 
 # fields:
 #	sock => socket to Gcf2::loop
@@ -26,8 +27,7 @@ sub new  {
 	my $self = bless {}, __PACKAGE__;
 	# ensure the child process has the same @INC we do:
 	my $env = { PERL5LIB => join(':', @INC) };
-	my ($s1, $s2);
-	socketpair($s1, $s2, AF_UNIX, SOCK_STREAM, 0) or die "socketpair $!";
+	socketpair(my $s1, my $s2, AF_UNIX, SOCK_STREAM, 0);
 	$s1->blocking(0);
 	$opt->{0} = $opt->{1} = $s2;
 	my $cmd = [$^X, qw[-MPublicInbox::Gcf2 -e PublicInbox::Gcf2::loop]];
@@ -41,8 +41,8 @@ sub new  {
 sub gcf2_async ($$$;$) {
 	my ($self, $req, $cb, $arg) = @_;
 	my $inflight = $self->{inflight} or return $self->close;
-	PublicInbox::Git::write_all($self, $$req, \&cat_async_step, $inflight);
-	push @$inflight, $req, $cb, $arg;
+	PublicInbox::Git::write_all($self, $req, \&cat_async_step, $inflight);
+	push @$inflight, \$req, $cb, $arg; # ref prevents Git.pm retries
 }
 
 # ensure PublicInbox::Git::cat_async_step never calls cat_async_retry
diff --git a/lib/PublicInbox/GitAsyncCat.pm b/lib/PublicInbox/GitAsyncCat.pm
index 71ee1147..f8b2a9fc 100644
--- a/lib/PublicInbox/GitAsyncCat.pm
+++ b/lib/PublicInbox/GitAsyncCat.pm
@@ -18,7 +18,7 @@ sub ibx_async_cat ($$$$) {
 		require PublicInbox::Gcf2Client;
 		PublicInbox::Gcf2Client::new();
 	} // 0)) { # 0: do not retry if libgit2 or Inline::C are missing
-		$GCF2C->gcf2_async(\"$oid $git->{git_dir}\n", $cb, $arg);
+		$GCF2C->gcf2_async("$oid $git->{git_dir}\n", $cb, $arg);
 		\undef;
 	} else { # read-only end of git-cat-file pipe
 		$git->cat_async($oid, $cb, $arg);
@@ -42,7 +42,7 @@ sub ibx_async_prefetch {
 	if (!defined($ibx->{topdir}) && $GCF2C) {
 		if (!@{$GCF2C->{inflight} // []}) {
 			$oid .= " $git->{git_dir}\n";
-			return $GCF2C->gcf2_async(\$oid, $cb, $arg); # true
+			return $GCF2C->gcf2_async($oid, $cb, $arg); # true
 		}
 	} elsif ($git->{epwatch}) {
 		return $git->async_prefetch($oid, $cb, $arg);
diff --git a/t/gcf2_client.t b/t/gcf2_client.t
index 6d059cad..33ee2c91 100644
--- a/t/gcf2_client.t
+++ b/t/gcf2_client.t
@@ -1,10 +1,10 @@
 #!perl -w
-# Copyright (C) 2020-2021 all contributors <meta@public-inbox.org>
+# Copyright (C) all contributors <meta@public-inbox.org>
 # License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
-use strict;
+use v5.12;
 use PublicInbox::TestCommon;
-use Test::More;
 use Cwd qw(getcwd);
+use autodie qw(open close);
 use PublicInbox::Import;
 use PublicInbox::DS;
 
@@ -17,7 +17,7 @@ PublicInbox::Import::init_bare($git_a);
 PublicInbox::Import::init_bare($git_b);
 my $fi_data = './t/git.fast-import-data';
 my $rdr = {};
-open $rdr->{0}, '<', $fi_data or BAIL_OUT $!;
+open $rdr->{0}, '<', $fi_data;
 xsys([qw(git fast-import --quiet)], { GIT_DIR => $git_a }, $rdr);
 is($?, 0, 'fast-import succeeded');
 
@@ -26,9 +26,9 @@ my $called = 0;
 my $err_f = "$tmpdir/err";
 {
 	PublicInbox::DS->Reset;
-	open my $err, '>>', $err_f or BAIL_OUT $!;
+	open my $err, '>>', $err_f;
 	my $gcf2c = PublicInbox::Gcf2Client::new({ 2 => $err });
-	$gcf2c->gcf2_async(\"$tree $git_a\n", sub {
+	$gcf2c->gcf2_async("$tree $git_a\n", sub {
 		my ($bref, $oid, $type, $size, $arg) = @_;
 		is($oid, $tree, 'got expected OID');
 		is($size, 30, 'got expected length');
@@ -39,12 +39,12 @@ my $err_f = "$tmpdir/err";
 	}, 'hi');
 	$gcf2c->cat_async_step($gcf2c->{inflight});
 
-	open $err, '<', $err_f or BAIL_OUT $!;
+	open $err, '<', $err_f;
 	my $estr = do { local $/; <$err> };
 	is($estr, '', 'nothing in stderr');
 
 	my $trunc = substr($tree, 0, 39);
-	$gcf2c->gcf2_async(\"$trunc $git_a\n", sub {
+	$gcf2c->gcf2_async("$trunc $git_a\n", sub {
 		my ($bref, $oid, $type, $size, $arg) = @_;
 		is(undef, $bref, 'missing bref is undef');
 		is($oid, $trunc, 'truncated OID printed');
@@ -55,30 +55,30 @@ my $err_f = "$tmpdir/err";
 	}, 'bye');
 	$gcf2c->cat_async_step($gcf2c->{inflight});
 
-	open $err, '<', $err_f or BAIL_OUT $!;
+	open $err, '<', $err_f;
 	$estr = do { local $/; <$err> };
 	like($estr, qr/retrying/, 'warned about retry');
 
 	# try failed alternates lookup
 	PublicInbox::DS->Reset;
-	open $err, '>', $err_f or BAIL_OUT $!;
+	open $err, '>', $err_f;
 	$gcf2c = PublicInbox::Gcf2Client::new({ 2 => $err });
-	$gcf2c->gcf2_async(\"$tree $git_b\n", sub {
+	$gcf2c->gcf2_async("$tree $git_b\n", sub {
 		my ($bref, $oid, $type, $size, $arg) = @_;
 		is(undef, $bref, 'missing bref from alt is undef');
 		$called++;
 	});
 	$gcf2c->cat_async_step($gcf2c->{inflight});
-	open $err, '<', $err_f or BAIL_OUT $!;
+	open $err, '<', $err_f;
 	$estr = do { local $/; <$err> };
 	like($estr, qr/retrying/, 'warned about retry before alt update');
 
 	# now try successful alternates lookup
-	open my $alt, '>>', "$git_b/objects/info/alternates" or BAIL_OUT $!;
-	print $alt "$git_a/objects\n" or BAIL_OUT $!;
-	close $alt or BAIL_OUT;
+	open my $alt, '>>', "$git_b/objects/info/alternates";
+	print $alt "$git_a/objects\n";
+	close $alt;
 	my $expect = xqx(['git', "--git-dir=$git_a", qw(cat-file tree), $tree]);
-	$gcf2c->gcf2_async(\"$tree $git_a\n", sub {
+	$gcf2c->gcf2_async("$tree $git_a\n", sub {
 		my ($bref, $oid, $type, $size, $arg) = @_;
 		is($oid, $tree, 'oid match on alternates retry');
 		is($$bref, $expect, 'tree content matched');

[PATCH 05/16] t/git: show git_version in diag output

[Eric Wong]

This is useful to ensure we're testing properly with git <= 2.35
to ensure we don't break --batch-check support for those users.
---
 t/git.t | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/t/git.t b/t/git.t
index bde6d35b..b7df6186 100644
--- a/t/git.t
+++ b/t/git.t
@@ -1,8 +1,7 @@
 #!perl -w
 # Copyright (C) all contributors <meta@public-inbox.org>
 # License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
-use strict;
-use v5.10.1;
+use v5.12;
 use PublicInbox::TestCommon;
 my ($dir, $for_destroy) = tmpdir();
 use PublicInbox::Import;
@@ -205,4 +204,5 @@ is(git_quote($s = "hello\nworld"), '"hello\\nworld"', 'quoted LF');
 is(git_quote($s = "hello\x06world"), '"hello\\006world"', 'quoted \\x06');
 is(git_unquote($s = '"hello\\006world"'), "hello\x06world", 'unquoted \\x06');
 
-done_testing();
+diag 'git_version='.sprintf('%vd', PublicInbox::Git::git_version());
+done_testing;

[PATCH 06/16] process_pipe: do not run `close' perlop unless requested

[Eric Wong]

If a user is relying on reference counts to invalidate FDs
(as we do in many places), rely on them instead of explicit
`close'.  This forces us to do a better job of managing refs
and avoiding redundant fields which make our code more fragile.
---
 lib/PublicInbox/ProcessPipe.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/PublicInbox/ProcessPipe.pm b/lib/PublicInbox/ProcessPipe.pm
index 16971801..ba2c1ecb 100644
--- a/lib/PublicInbox/ProcessPipe.pm
+++ b/lib/PublicInbox/ProcessPipe.pm
@@ -58,7 +58,7 @@ sub FILENO { fileno($_[0]->{fh}) }
 sub _close ($;$) {
 	my ($self, $wait) = @_;
 	my ($fh, $pid) = delete(@$self{qw(fh pid)});
-	my $ret = defined($fh) ? close($fh) : '';
+	my $ret = (defined($fh) && $wait) ? close($fh) : ($fh = '');
 	return $ret unless defined($pid) && $self->{ppid} == $$;
 	if ($wait) { # caller cares about the exit status:
 		# synchronous wait via defined(wantarray) on awaitpid:

[PATCH 07/16] git: improve error reporting

[Eric Wong]

We can use autodie for socketpair to handle errors for us,
but we need Time::HiRes::stat so we must write the error message
ourselves if stat-ing the git executable fails.
---
 lib/PublicInbox/Git.pm | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/PublicInbox/Git.pm b/lib/PublicInbox/Git.pm
index 5003be53..1dbd10b7 100644
--- a/lib/PublicInbox/Git.pm
+++ b/lib/PublicInbox/Git.pm
@@ -10,6 +10,7 @@ package PublicInbox::Git;
 use strict;
 use v5.10.1;
 use parent qw(Exporter PublicInbox::DS);
+use autodie qw(socketpair);
 use POSIX ();
 use IO::Handle; # ->blocking
 use Socket qw(AF_UNIX SOCK_STREAM);
@@ -57,7 +58,7 @@ my ($GIT_EXE, $GIT_VER);
 
 sub check_git_exe () {
 	$GIT_EXE = which('git') // die "git not found in $ENV{PATH}";
-	my @st = stat($GIT_EXE) or die "stat: $!";
+	my @st = stat($GIT_EXE) or die "stat($GIT_EXE): $!";
 	my $st = pack('dd', $st[10], $st[7]);
 	if ($st ne $EXE_ST) {
 		my $rd = popen_rd([ $GIT_EXE, '--version' ]);
@@ -144,8 +145,7 @@ sub last_check_err {
 sub _sock_cmd {
 	my ($self, $batch, $err_c) = @_;
 	$self->{sock} and Carp::confess('BUG: {sock} exists');
-	my ($s1, $s2);
-	socketpair($s1, $s2, AF_UNIX, SOCK_STREAM, 0) or die "socketpair $!";
+	socketpair(my $s1, my $s2, AF_UNIX, SOCK_STREAM, 0);
 	$s1->blocking(0);
 	my $opt = { pgid => 0, 0 => $s2, 1 => $s2 };
 	my $gd = $self->{git_dir};

[PATCH 08/16] git: packed_bytes: account for TOUTTOC between glob and stat

[Eric Wong]

There's not much we can do about this aside from just ignoring
errors and considering un-stat-able files as zero-sized.
There's no syscalls which expose FUSE3 `readdirplus' type
functionality to userspace.
---
 lib/PublicInbox/Git.pm | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/lib/PublicInbox/Git.pm b/lib/PublicInbox/Git.pm
index 1dbd10b7..0fd621e1 100644
--- a/lib/PublicInbox/Git.pm
+++ b/lib/PublicInbox/Git.pm
@@ -486,9 +486,7 @@ sub packed_bytes {
 	my ($self) = @_;
 	my $n = 0;
 	my $pack_dir = git_path($self, 'objects/pack');
-	foreach my $p (bsd_glob("$pack_dir/*.pack", GLOB_NOSORT)) {
-		$n += -s $p;
-	}
+	$n += (-s $_ // 0) for (bsd_glob("$pack_dir/*.pack", GLOB_NOSORT));
 	$n
 }
 

[PATCH 09/16] gcf2client: warnings

[Eric Wong]

---
 lib/PublicInbox/Gcf2Client.pm | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/PublicInbox/Gcf2Client.pm b/lib/PublicInbox/Gcf2Client.pm
index 8e313c84..4a0348b4 100644
--- a/lib/PublicInbox/Gcf2Client.pm
+++ b/lib/PublicInbox/Gcf2Client.pm
@@ -30,7 +30,8 @@ sub new  {
 	socketpair(my $s1, my $s2, AF_UNIX, SOCK_STREAM, 0);
 	$s1->blocking(0);
 	$opt->{0} = $opt->{1} = $s2;
-	my $cmd = [$^X, qw[-MPublicInbox::Gcf2 -e PublicInbox::Gcf2::loop]];
+	my $cmd = [$^X, $^W ? ('-w') : (),
+			qw[-MPublicInbox::Gcf2 -e PublicInbox::Gcf2::loop]];
 	my $pid = spawn($cmd, $env, $opt);
 	my $sock = PublicInbox::ProcessPipe->maybe_new($pid, $s1);
 	$self->{inflight} = [];

[PATCH 10/16] lei rediff: order-file support

[Eric Wong]

---
 lib/PublicInbox/LEI.pm       | 6 +++---
 lib/PublicInbox/LeiRediff.pm | 1 +
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/lib/PublicInbox/LEI.pm b/lib/PublicInbox/LEI.pm
index beb0f897..48c5644b 100644
--- a/lib/PublicInbox/LEI.pm
+++ b/lib/PublicInbox/LEI.pm
@@ -159,7 +159,7 @@ our @diff_opt = qw(unified|U=i output-indicator-new=s output-indicator-old=s
 	rename-empty! check ws-error-highlight=s full-index binary
 	abbrev:i break-rewrites|B:s find-renames|M:s find-copies:s
 	find-copies-harder irreversible-delete|D l=i diff-filter=s
-	S=s G=s find-object=s pickaxe-all pickaxe-regex O=s R
+	S=s G=s find-object=s pickaxe-all pickaxe-regex R
 	relative:s text|a ignore-cr-at-eol ignore-space-at-eol
 	ignore-space-change|b ignore-all-space|w ignore-blank-lines
 	inter-hunk-context=i function-context|W exit-code ext-diff
@@ -198,8 +198,8 @@ our %CMD = ( # sorted in order of importance/use:
 'rediff' => [ '--stdin|LOCATION...',
 		'regenerate a diff with different options',
 	'stdin|', # /|\z/ must be first for lone dash
-	qw(git-dir=s@ cwd! verbose|v+ color:s no-color drq:1 dequote-only:1),
-	@diff_opt, @lxs_opt, @net_opt, @c_opt ],
+	qw(git-dir=s@ cwd! verbose|v+ color:s no-color drq:1 dequote-only:1
+	order-file=s), @diff_opt, @lxs_opt, @net_opt, @c_opt ],
 
 'mail-diff' => [ '--stdin|LOCATION...', 'diff the contents of emails',
 	'stdin|', # /|\z/ must be first for lone dash
diff --git a/lib/PublicInbox/LeiRediff.pm b/lib/PublicInbox/LeiRediff.pm
index efd24d17..6cc6131b 100644
--- a/lib/PublicInbox/LeiRediff.pm
+++ b/lib/PublicInbox/LeiRediff.pm
@@ -82,6 +82,7 @@ sub _lei_diff_prepare ($$) {
 			push @$cmd, $c ? "-$c" : "--$o";
 		}
 	}
+	push(@$cmd, "-O$opt->{'order-file'}") if $opt->{'order-file'};
 }
 
 sub diff_ctxq ($$) {

[PATCH 11/16] lei: correct exit signal

[Eric Wong]

---
 lib/PublicInbox/LEI.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/PublicInbox/LEI.pm b/lib/PublicInbox/LEI.pm
index 48c5644b..fe5ed0f3 100644
--- a/lib/PublicInbox/LEI.pm
+++ b/lib/PublicInbox/LEI.pm
@@ -1310,7 +1310,7 @@ sub lazy_start {
 	local $quit = do {
 		my (undef, $eof_p) = PublicInbox::PktOp->pair;
 		sub {
-			$exit_code //= shift;
+			$exit_code //= eval { POSIX->can("SIG$_[0]")->() } // 1;
 			eval 'PublicInbox::LeiNoteEvent::flush_task()';
 			my $lis = $pil or exit($exit_code);
 			# closing eof_p triggers \&noop wakeup

[PATCH 12/16] lei mail-diff: remove correct temporary directory

[Eric Wong]

---
 lib/PublicInbox/LeiMailDiff.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/PublicInbox/LeiMailDiff.pm b/lib/PublicInbox/LeiMailDiff.pm
index 5e2e4b0b..84070b29 100644
--- a/lib/PublicInbox/LeiMailDiff.pm
+++ b/lib/PublicInbox/LeiMailDiff.pm
@@ -21,7 +21,7 @@ sub diff_a ($$) {
 	my $rdr = { -C => "$self->{tmp}" };
 	@$rdr{1, 2} = @$lei{1, 2};
 	run_wait($cmd, $lei->{env}, $rdr) and $lei->child_error($?);
-	File::Path::remove_tree($self->{curdir});
+	File::Path::remove_tree($dir);
 }
 
 sub input_eml_cb { # used by PublicInbox::LeiInput::input_fh

[PATCH 13/16] lei rediff: perl -e instead of -E

[Eric Wong]

---
 lib/PublicInbox/LeiRediff.pm | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/PublicInbox/LeiRediff.pm b/lib/PublicInbox/LeiRediff.pm
index 6cc6131b..a886931c 100644
--- a/lib/PublicInbox/LeiRediff.pm
+++ b/lib/PublicInbox/LeiRediff.pm
@@ -140,7 +140,7 @@ EOM
 
 sub wait_requote { # OnDestroy callback
 	my ($lei, $pid, $old_1) = @_;
-	$lei->{1} = $old_1; # closes stdin of `perl -pE 's/^/> /'`
+	$lei->{1} = $old_1; # closes stdin of `perl -pe 's/^/> /'`
 	waitpid($pid, 0) == $pid or die "BUG(?) waitpid: \$!=$! \$?=$?";
 	$lei->child_error($?) if $?;
 }
@@ -150,7 +150,7 @@ sub requote ($$) {
 	my $old_1 = $lei->{1};
 	my $opt = { 1 => $old_1, 2 => $lei->{2} };
 	# $^X (perl) is overkill, but maybe there's a weird system w/o sed
-	my ($w, $pid) = popen_wr([$^X, '-pE', "s/^/$pfx/"], $lei->{env}, $opt);
+	my ($w, $pid) = popen_wr([$^X, '-pe', "s/^/$pfx/"], $lei->{env}, $opt);
 	$w->autoflush(1);
 	binmode $w, ':utf8'; # incompatible with ProcessPipe due to syswrite
 	$lei->{1} = $w;

[PATCH 14/16] lei_store: unlink early

[Eric Wong]

---
 lib/PublicInbox/LeiStore.pm | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/lib/PublicInbox/LeiStore.pm b/lib/PublicInbox/LeiStore.pm
index 727de066..e97931dd 100644
--- a/lib/PublicInbox/LeiStore.pm
+++ b/lib/PublicInbox/LeiStore.pm
@@ -27,7 +27,8 @@ use PublicInbox::MDA;
 use PublicInbox::Spawn qw(spawn);
 use PublicInbox::MdirReader;
 use PublicInbox::LeiToMail;
-use File::Temp ();
+use File::Temp qw(tmpnam);
+use Fcntl qw(SEEK_CUR);
 use POSIX ();
 use IO::Handle (); # ->autoflush
 use Sys::Syslog qw(syslog openlog);
@@ -110,6 +111,7 @@ sub search {
 # follows the stderr file
 sub _tail_err {
 	my ($self) = @_;
+	seek($self->{-tmp_err} // return, 0, SEEK_CUR);
 	print { $self->{-err_wr} } readline($self->{-tmp_err});
 }
 
@@ -566,11 +568,10 @@ sub xchg_stderr {
 	_tail_err($self) if $self->{-err_wr};
 	my $dir = $self->{priv_eidx}->{topdir};
 	return unless -e $dir;
-	my $old = delete $self->{-tmp_err};
-	my $pfx = POSIX::strftime('%Y%m%d%H%M%S', gmtime(time));
-	my $err = File::Temp->new(TEMPLATE => "$pfx.$$.err-XXXX",
-				SUFFIX => '.err', DIR => $dir);
-	open STDERR, '>>', $err->filename or die "dup2: $!";
+	delete $self->{-tmp_err};
+	my ($err, $name) = tmpnam();
+	open STDERR, '>>', $name or die "dup2: $!";
+	unlink($name);
 	STDERR->autoflush(1); # shared with shard subprocesses
 	$self->{-tmp_err} = $err; # separate file description for RO access
 	undef;

[PATCH 15/16] overidx: fix version comparison

[Eric Wong]

---
 lib/PublicInbox/OverIdx.pm | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/lib/PublicInbox/OverIdx.pm b/lib/PublicInbox/OverIdx.pm
index 6cc86d5d..231c626e 100644
--- a/lib/PublicInbox/OverIdx.pm
+++ b/lib/PublicInbox/OverIdx.pm
@@ -671,13 +671,14 @@ sub vivify_xvmd {
 }
 
 sub fork_ok {
-	return 1 if $DBD::SQLite::sqlite_version >= 3008003;
+	state $ver = eval("v$DBD::SQLite::sqlite_version");
+	return 1 if $ver ge v3.8.3;
 	my ($opt) = @_;
 	my @j = split(/,/, $opt->{jobs} // '');
 	state $warned;
-	grep { $_ > 1 } @j and $warned //= warn('DBD::SQLite version is ',
-		 $DBD::SQLite::sqlite_version,
-		", need >= 3008003 (3.8.3) for --jobs > 1\n");
+	grep { $_ > 1 } @j and $warned //= warn(<<EOM);
+DBD::SQLite version is v$DBD::SQLite::sqlite_version, need >= v3.8.3 for --jobs > 1
+EOM
 	$opt->{jobs} = '1,1';
 	undef;
 }

[PATCH 16/16] enable warnings globally

[Eric Wong]

---
 script/lei      |  4 ++--
 t/edit.t        | 29 +++++++++++++++--------------
 t/extsearch.t   |  2 +-
 t/lei-q-save.t  |  4 ++--
 t/mbox_reader.t |  2 +-
 t/spawn.t       |  4 ++--
 6 files changed, 23 insertions(+), 22 deletions(-)

diff --git a/script/lei b/script/lei
index a77ea880..1d90be0a 100755
--- a/script/lei
+++ b/script/lei
@@ -92,8 +92,8 @@ my $addr = pack_sockaddr_un($path);
 socket($sock, AF_UNIX, SOCK_SEQPACKET, 0) or die "socket: $!";
 unless (connect($sock, $addr)) { # start the daemon if not started
 	local $ENV{PERL5LIB} = join(':', @INC);
-	open(my $daemon, '-|', $^X, qw[-MPublicInbox::LEI
-		-E PublicInbox::LEI::lazy_start(@ARGV)],
+	open(my $daemon, '-|', $^X, $^W ? ('-w') : (),
+		qw[-MPublicInbox::LEI -e PublicInbox::LEI::lazy_start(@ARGV)],
 		$path, $! + 0, $narg) or die "popen: $!";
 	while (<$daemon>) { warn $_ } # EOF when STDERR is redirected
 	close($daemon) or warn <<"";
diff --git a/t/edit.t b/t/edit.t
index e6e0f9cf..1621df3b 100644
--- a/t/edit.t
+++ b/t/edit.t
@@ -1,5 +1,5 @@
 #!perl -w
-# Copyright (C) 2019-2021 all contributors <meta@public-inbox.org>
+# Copyright (C) all contributors <meta@public-inbox.org>
 # License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
 # edit frontend behavior test (t/replace.t for backend)
 use strict;
@@ -24,10 +24,11 @@ local $ENV{PI_CONFIG} = $cfgfile;
 my ($in, $out, $err, $cmd, $cur, $t);
 my $git = PublicInbox::Git->new("$ibx->{inboxdir}/git/0.git");
 my $opt = { 0 => \$in, 1 => \$out, 2 => \$err };
+my $ipe = "$^X -w -i -p -e";
 
 $t = '-F FILE'; {
 	$in = $out = $err = '';
-	local $ENV{MAIL_EDITOR} = "$^X -i -p -e 's/boolean prefix/bool pfx/'";
+	local $ENV{MAIL_EDITOR} = "$ipe 's/boolean prefix/bool pfx/'";
 	$cmd = [ '-edit', "-F$file", $inboxdir ];
 	ok(run_script($cmd, undef, $opt), "$t edit OK");
 	$cur = PublicInbox::Eml->new($ibx->msg_by_mid($mid));
@@ -37,7 +38,7 @@ $t = '-F FILE'; {
 
 $t = '-m MESSAGE_ID'; {
 	$in = $out = $err = '';
-	local $ENV{MAIL_EDITOR} = "$^X -i -p -e 's/bool pfx/boolean prefix/'";
+	local $ENV{MAIL_EDITOR} = "$ipe 's/bool pfx/boolean prefix/'";
 	$cmd = [ '-edit', "-m$mid", $inboxdir ];
 	ok(run_script($cmd, undef, $opt), "$t edit OK");
 	$cur = PublicInbox::Eml->new($ibx->msg_by_mid($mid));
@@ -48,7 +49,7 @@ $t = '-m MESSAGE_ID'; {
 $t = 'no-op -m MESSAGE_ID'; {
 	$in = $out = $err = '';
 	my $before = $git->qx(qw(rev-parse HEAD));
-	local $ENV{MAIL_EDITOR} = "$^X -i -p -e 's/bool pfx/boolean prefix/'";
+	local $ENV{MAIL_EDITOR} = "$ipe 's/bool pfx/boolean prefix/'";
 	$cmd = [ '-edit', "-m$mid", $inboxdir ];
 	ok(run_script($cmd, undef, $opt), "$t succeeds");
 	my $prev = $cur;
@@ -64,7 +65,7 @@ $t = 'no-op -m MESSAGE_ID'; {
 $t = 'no-op -m MESSAGE_ID w/Status: header'; { # because mutt does it
 	$in = $out = $err = '';
 	my $before = $git->qx(qw(rev-parse HEAD));
-	local $ENV{MAIL_EDITOR} = "$^X -i -p -e 's/^Subject:.*/Status: RO\\n\$&/'";
+	local $ENV{MAIL_EDITOR} = "$ipe 's/^Subject:.*/Status: RO\\n\$&/'";
 	$cmd = [ '-edit', "-m$mid", $inboxdir ];
 	ok(run_script($cmd, undef, $opt), "$t succeeds");
 	my $prev = $cur;
@@ -80,7 +81,7 @@ $t = 'no-op -m MESSAGE_ID w/Status: header'; { # because mutt does it
 
 $t = '-m MESSAGE_ID can change Received: headers'; {
 	$in = $out = $err = '';
-	local $ENV{MAIL_EDITOR} = "$^X -i -p -e 's/^Subject:.*/Received: x\\n\$&/'";
+	local $ENV{MAIL_EDITOR} = "$ipe 's/^Subject:.*/Received: x\\n\$&/'";
 	$cmd = [ '-edit', "-m$mid", $inboxdir ];
 	ok(run_script($cmd, undef, $opt), "$t succeeds");
 	$cur = PublicInbox::Eml->new($ibx->msg_by_mid($mid));
@@ -91,7 +92,7 @@ $t = '-m MESSAGE_ID can change Received: headers'; {
 
 $t = '-m miss'; {
 	$in = $out = $err = '';
-	local $ENV{MAIL_EDITOR} = "$^X -i -p -e 's/boolean/FAIL/'";
+	local $ENV{MAIL_EDITOR} = "$ipe 's/boolean/FAIL/'";
 	$cmd = [ '-edit', "-m$mid-miss", $inboxdir ];
 	ok(!run_script($cmd, undef, $opt), "$t fails on invalid MID");
 	like($err, qr/No message found/, "$t shows error");
@@ -99,7 +100,7 @@ $t = '-m miss'; {
 
 $t = 'non-interactive editor failure'; {
 	$in = $out = $err = '';
-	local $ENV{MAIL_EDITOR} = "$^X -i -p -e 'END { exit 1 }'";
+	local $ENV{MAIL_EDITOR} = "$ipe 'END { exit 1 }'";
 	$cmd = [ '-edit', "-m$mid", $inboxdir ];
 	ok(!run_script($cmd, undef, $opt), "$t detected");
 	like($err, qr/END \{ exit 1 \}' failed:/, "$t shows error");
@@ -109,7 +110,7 @@ $t = 'mailEditor set in config'; {
 	$in = $out = $err = '';
 	my $rc = xsys(qw(git config), "--file=$cfgfile",
 			'publicinbox.maileditor',
-			"$^X -i -p -e 's/boolean prefix/bool pfx/'");
+			"$ipe 's/boolean prefix/bool pfx/'");
 	is($rc, 0, 'set publicinbox.mailEditor');
 	local $ENV{MAIL_EDITOR};
 	delete $ENV{MAIL_EDITOR};
@@ -123,20 +124,20 @@ $t = 'mailEditor set in config'; {
 
 $t = '--raw and mbox escaping'; {
 	$in = $out = $err = '';
-	local $ENV{MAIL_EDITOR} = "$^X -i -p -e 's/^\$/\\nFrom not mbox\\n/'";
+	local $ENV{MAIL_EDITOR} = "$ipe 's/^\$/\\nFrom not mbox\\n/'";
 	$cmd = [ '-edit', "-m$mid", '--raw', $inboxdir ];
 	ok(run_script($cmd, undef, $opt), "$t succeeds");
 	$cur = PublicInbox::Eml->new($ibx->msg_by_mid($mid));
 	like($cur->body, qr/^From not mbox/sm, 'put "From " line into body');
 
-	local $ENV{MAIL_EDITOR} = "$^X -i -p -e 's/^>From not/\$& an/'";
+	local $ENV{MAIL_EDITOR} = "$ipe 's/^>From not/\$& an/'";
 	$cmd = [ '-edit', "-m$mid", $inboxdir ];
 	ok(run_script($cmd, undef, $opt), "$t succeeds with mbox escaping");
 	$cur = PublicInbox::Eml->new($ibx->msg_by_mid($mid));
 	like($cur->body, qr/^From not an mbox/sm,
 		'changed "From " line unescaped');
 
-	local $ENV{MAIL_EDITOR} = "$^X -i -p -e 's/^From not an mbox\\n//s'";
+	local $ENV{MAIL_EDITOR} = "$ipe 's/^From not an mbox\\n//s'";
 	$cmd = [ '-edit', "-m$mid", '--raw', $inboxdir ];
 	ok(run_script($cmd, undef, $opt), "$t succeeds again");
 	$cur = PublicInbox::Eml->new($ibx->msg_by_mid($mid));
@@ -154,7 +155,7 @@ $t = 'reuse Message-ID'; {
 
 $t = 'edit ambiguous Message-ID with -m'; {
 	$in = $out = $err = '';
-	local $ENV{MAIL_EDITOR} = "$^X -i -p -e 's/bool pfx/boolean prefix/'";
+	local $ENV{MAIL_EDITOR} = "$ipe 's/bool pfx/boolean prefix/'";
 	$cmd = [ '-edit', "-m$mid", $inboxdir ];
 	ok(!run_script($cmd, undef, $opt), "$t fails w/o --force");
 	like($err, qr/Multiple messages with different content found matching/,
@@ -164,7 +165,7 @@ $t = 'edit ambiguous Message-ID with -m'; {
 
 $t .= ' and --force'; {
 	$in = $out = $err = '';
-	local $ENV{MAIL_EDITOR} = "$^X -i -p -e 's/^Subject:.*/Subject:x/i'";
+	local $ENV{MAIL_EDITOR} = "$ipe 's/^Subject:.*/Subject:x/i'";
 	$cmd = [ '-edit', "-m$mid", '--force', $inboxdir ];
 	ok(run_script($cmd, undef, $opt), "$t succeeds");
 	like($err, qr/Will edit all of them/, "$t notes all will be edited");
diff --git a/t/extsearch.t b/t/extsearch.t
index 19eaf3b5..2995cf95 100644
--- a/t/extsearch.t
+++ b/t/extsearch.t
@@ -151,7 +151,7 @@ if ('inbox edited') {
 	my ($in, $out, $err);
 	$in = $out = $err = '';
 	my $opt = { 0 => \$in, 1 => \$out, 2 => \$err };
-	my $env = { MAIL_EDITOR => "$^X -i -p -e 's/test message/BEST MSG/'" };
+	my $env = { MAIL_EDITOR => "$^X -w -i -p -e 's/test message/BEST MSG/'" };
 	my $cmd = [ qw(-edit -Ft/utf8.eml), "$home/v2test" ];
 	ok(run_script($cmd, $env, $opt), '-edit');
 	ok(run_script([qw(-extindex --all), "$home/extindex"], undef, $opt),
diff --git a/t/lei-q-save.t b/t/lei-q-save.t
index d09c8397..1d9d5a51 100644
--- a/t/lei-q-save.t
+++ b/t/lei-q-save.t
@@ -227,7 +227,7 @@ test_lei(sub {
 	my @lss = glob("$home/" .
 		'.local/share/lei/saved-searches/*/lei.saved-search');
 	my $out = xqx([qw(git config -f), $lss[0], 'lei.q.output']);
-	xsys($^X, qw(-i -p -e), "s/\\[/\\0/", $lss[0])
+	xsys($^X, qw(-w -i -p -e), "s/\\[/\\0/", $lss[0])
 		and xbail "-ipe $lss[0]: $?";
 	lei_ok qw(ls-search);
 	like($lei_err, qr/bad config line.*?\Q$lss[0]\E/,
@@ -235,7 +235,7 @@ test_lei(sub {
 	lei_ok qw(up --all), \'up works with bad config';
 	like($lei_err, qr/bad config line.*?\Q$lss[0]\E/,
 		'git config parse error shown w/ lei up');
-	xsys($^X, qw(-i -p -e), "s/\\0/\\[/", $lss[0])
+	xsys($^X, qw(-w -i -p -e), "s/\\0/\\[/", $lss[0])
 		and xbail "-ipe $lss[0]: $?";
 	lei_ok qw(ls-search);
 	is($lei_err, '', 'no errors w/ fixed config');
diff --git a/t/mbox_reader.t b/t/mbox_reader.t
index 87e8f397..ad74bb39 100644
--- a/t/mbox_reader.t
+++ b/t/mbox_reader.t
@@ -113,7 +113,7 @@ EOM
 
 SKIP: {
 	use PublicInbox::Spawn qw(popen_rd);
-	my $fh = popen_rd([ $^X, '-E', <<'' ]);
+	my $fh = popen_rd([ $^X, '-w', '-E', <<'' ]);
 say "From x@y Fri Oct  2 00:00:00 1993";
 print "a: b\n\n", "x" x 70000, "\n\n";
 say "From x@y Fri Oct  2 00:00:00 2010";
diff --git a/t/spawn.t b/t/spawn.t
index 9ed3be36..04589437 100644
--- a/t/spawn.t
+++ b/t/spawn.t
@@ -62,7 +62,7 @@ elsif ($pid > 0) {
 }
 EOF
 	my $oldset = PublicInbox::DS::block_signals();
-	my $rd = popen_rd([$^X, '-e', $script]);
+	my $rd = popen_rd([$^X, qw(-w -e), $script]);
 	diag 'waiting for child to reap grandchild...';
 	chomp(my $line = readline($rd));
 	my ($rdy, $pid) = split(/ /, $line);
@@ -185,7 +185,7 @@ SKIP: {
 		require BSD::Resource;
 		defined(BSD::Resource::RLIMIT_CPU())
 	} or skip 'BSD::Resource::RLIMIT_CPU missing', 3;
-	my $cmd = [ $^X, ($^W ? ('-w') : ()), '-e', <<'EOM' ];
+	my $cmd = [ $^X, qw(-w -e), <<'EOM' ];
 use POSIX qw(:signal_h);
 use BSD::Resource qw(times);
 use Time::HiRes qw(time); # gettimeofday