[PATCH] lei: require Socket::MsgHdr or Inline::C, drop oneshot

[PATCH] lei: require Socket::MsgHdr or Inline::C, drop oneshot

[Eric Wong]

The cost of supporting separate code paths between oneshot and
daemon isn't worth the trouble; especially if there are more
users to support.  The test suite time nearly doubles with
oneshot, so that's hurting developer productivity.

FD passing is currently required to work efficiently with
remote HTTP(S) queries which return large messages, as seen in
commit 708b182a57373172f5523f3dc297659d58e03b58
("ipc: wq: handle >MAX_ARG_STRLEN && <EMSGSIZE case").

Additionally, upcoming support for IMAP IDLE and inotify-based
monitoring of Maildirs cannot work properly without a background
daemon.
---
 lib/PublicInbox/GitCredential.pm |   2 +-
 lib/PublicInbox/LEI.pm           |  58 +------------
 lib/PublicInbox/LeiEditSearch.pm |  19 ++---
 lib/PublicInbox/LeiStore.pm      |  13 +--
 lib/PublicInbox/LeiSucks.pm      |   3 +-
 lib/PublicInbox/LeiUp.pm         |  26 +++---
 lib/PublicInbox/TestCommon.pm    |  15 +---
 script/lei                       | 137 +++++++++++++++----------------
 t/lei-daemon.t                   |   9 --
 9 files changed, 97 insertions(+), 185 deletions(-)

diff --git a/lib/PublicInbox/GitCredential.pm b/lib/PublicInbox/GitCredential.pm
index 2d81817c..b29780d6 100644
--- a/lib/PublicInbox/GitCredential.pm
+++ b/lib/PublicInbox/GitCredential.pm
@@ -9,7 +9,7 @@ sub run ($$;$) {
 	my ($in_r, $in_w, $out_r);
 	my $cmd = [ qw(git credential), $op ];
 	pipe($in_r, $in_w) or die "pipe: $!";
-	if ($lei && !$lei->{oneshot}) { # we'll die if disconnected:
+	if ($lei) { # we'll die if disconnected:
 		pipe($out_r, my $out_w) or die "pipe: $!";
 		$lei->send_exec_cmd([ $in_r, $out_w ], $cmd, {});
 	} else {
diff --git a/lib/PublicInbox/LEI.pm b/lib/PublicInbox/LEI.pm
index c8d2f315..6ff249d0 100644
--- a/lib/PublicInbox/LEI.pm
+++ b/lib/PublicInbox/LEI.pm
@@ -444,19 +444,6 @@ sub x_it ($$) {
 	dump_and_clear_log();
 	if (my $s = $self->{pkt_op_p} // $self->{sock}) {
 		send($s, "x_it $code", MSG_EOR);
-	} elsif ($self->{oneshot}) {
-		# don't want to end up using $? from child processes
-		_drop_wq($self);
-		# cleanup anything that has tempfiles or open file handles
-		%PATH2CFG = ();
-		delete @$self{qw(ovv dedupe sto cfg)};
-		if (my $signum = ($code & 127)) { # usually SIGPIPE (13)
-			$SIG{PIPE} = 'DEFAULT'; # $SIG{$signum} doesn't work
-			kill $signum, $$;
-			sleep(1) while 1; # wait for signal
-		} else {
-			$quit->($code >> 8);
-		}
 	} # else ignore if client disconnected
 }
 
@@ -921,17 +908,6 @@ sub start_mua {
 		my $io = [];
 		$io->[0] = $self->{1} if $self->{opt}->{stdin} && -t $self->{1};
 		send_exec_cmd($self, $io, \@cmd, {});
-	} elsif ($self->{oneshot}) {
-		my $pid = fork // die "fork: $!";
-		if ($pid > 0) { # original process
-			if ($self->{opt}->{stdin} && -t STDOUT) {
-				open STDIN, '+<&', \*STDOUT or die "dup2: $!";
-			}
-			exec(@cmd);
-			warn "exec @cmd: $!\n";
-			POSIX::_exit(1);
-		}
-		POSIX::setsid() > 0 or die "setsid: $!";
 	}
 	if ($self->{lxs} && $self->{au_done}) { # kick wait_startq
 		syswrite($self->{au_done}, 'q' x ($self->{lxs}->{jobs} // 0));
@@ -952,14 +928,11 @@ sub send_exec_cmd { # tell script/lei to execute a command
 sub poke_mua { # forces terminal MUAs to wake up and hopefully notice new mail
 	my ($self) = @_;
 	my $alerts = $self->{opt}->{alert} // return;
+	my $sock = $self->{sock};
 	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', $$);
-			}
+			send($sock, '-WINCH', MSG_EOR) if $sock;
 		} elsif ($op eq ':bell') {
 			out($self, "\a");
 		} elsif ($op =~ /(?<!\\),/) { # bare ',' (not ',,')
@@ -968,11 +941,7 @@ sub poke_mua { # forces terminal MUAs to wake up and hopefully notice new mail
 			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;
-			}
+			send($sock, exec_buf($cmd, {}), MSG_EOR) if $sock;
 		} else {
 			err($self, "W: unsupported --alert=$op"); # non-fatal
 		}
@@ -1009,9 +978,6 @@ sub start_pager {
 	if ($self->{sock}) { # lei(1) process runs it
 		delete @$new_env{keys %$env}; # only set iff unset
 		send_exec_cmd($self, [ @$rdr{0..2} ], [$pager], $new_env);
-	} elsif ($self->{oneshot}) {
-		my $cmd = [$pager];
-		$self->{"pid.$self.$$"}->{spawn($cmd, $new_env, $rdr)} = $cmd;
 	} else {
 		die 'BUG: start_pager w/o socket';
 	}
@@ -1253,29 +1219,13 @@ sub lazy_start {
 
 sub busy { 1 } # prevent daemon-shutdown if client is connected
 
-# for users w/o Socket::Msghdr installed or Inline::C enabled
-sub oneshot {
-	my ($main_pkg) = @_;
-	my $exit = $main_pkg->can('exit'); # caller may override exit()
-	local $quit = $exit if $exit;
-	local %PATH2CFG;
-	umask(077) // die("umask(077): $!");
-	my $self = bless { oneshot => 1, env => \%ENV }, __PACKAGE__;
-	for (0..2) { open($self->{$_}, '+<&=', $_) or die "open fd=$_: $!" }
-	dispatch($self, @ARGV);
-	x_it($self, $self->{child_error}) if $self->{child_error};
-}
-
 # ensures stdout hits the FS before sock disconnects so a client
 # can immediately reread it
 sub DESTROY {
 	my ($self) = @_;
 	$self->{1}->autoflush(1) if $self->{1};
 	stop_pager($self);
-	my $err = $?;
-	my $oneshot_pids = delete $self->{"pid.$self.$$"} or return;
-	waitpid($_, 0) for keys %$oneshot_pids;
-	$? = $err if $err; # preserve ->fail or ->x_it code
+	# preserve $? for ->fail or ->x_it code
 }
 
 sub wq_done_wait { # dwaitpid callback
diff --git a/lib/PublicInbox/LeiEditSearch.pm b/lib/PublicInbox/LeiEditSearch.pm
index 30ac65bd..13713d24 100644
--- a/lib/PublicInbox/LeiEditSearch.pm
+++ b/lib/PublicInbox/LeiEditSearch.pm
@@ -14,19 +14,12 @@ sub lei_edit_search {
 	my @cmd = (qw(git config --edit -f), $lss->{'-f'});
 	$lei->qerr("# spawning @cmd");
 	$lss->edit_begin($lei);
-	if ($lei->{oneshot}) {
-		require PublicInbox::Spawn;
-		waitpid(PublicInbox::Spawn::spawn(\@cmd), 0);
-		# non-fatal, editor could fail after successful write
-		$lei->child_error($?) if $?;
-		$lss->edit_done($lei);
-	} else { # run in script/lei foreground
-		require PublicInbox::PktOp;
-		my ($op_c, $op_p) = PublicInbox::PktOp->pair;
-		# $op_p will EOF when $EDITOR is done
-		$op_c->{ops} = { '' => [$lss->can('edit_done'), $lss, $lei] };
-		$lei->send_exec_cmd([ @$lei{qw(0 1 2)}, $op_p ], \@cmd, {});
-	}
+	# run in script/lei foreground
+	require PublicInbox::PktOp;
+	my ($op_c, $op_p) = PublicInbox::PktOp->pair;
+	# $op_p will EOF when $EDITOR is done
+	$op_c->{ops} = { '' => [$lss->can('edit_done'), $lss, $lei] };
+	$lei->send_exec_cmd([ @$lei{qw(0 1 2)}, $op_p ], \@cmd, {});
 }
 
 *_complete_edit_search = \&PublicInbox::LeiUp::_complete_up;
diff --git a/lib/PublicInbox/LeiStore.pm b/lib/PublicInbox/LeiStore.pm
index a7a0ebef..af5edbc2 100644
--- a/lib/PublicInbox/LeiStore.pm
+++ b/lib/PublicInbox/LeiStore.pm
@@ -431,20 +431,15 @@ sub write_prepare {
 		my $d = $lei->store_path;
 		$self->ipc_lock_init("$d/ipc.lock");
 		substr($d, -length('/lei/store'), 10, '');
-		my $err_pipe;
-		unless ($lei->{oneshot}) {
-			pipe(my ($r, $w)) or die "pipe: $!";
-			$err_pipe = [ $r, $w ];
-		}
+		pipe(my ($r, $w)) or die "pipe: $!";
+		my $err_pipe = [ $r, $w ];
 		# Mail we import into lei are private, so headers filtered out
 		# by -mda for public mail are not appropriate
 		local @PublicInbox::MDA::BAD_HEADERS = ();
 		$self->ipc_worker_spawn("lei/store $d", $lei->oldset,
 					{ lei => $lei, err_pipe => $err_pipe });
-		if ($err_pipe) {
-			require PublicInbox::LeiStoreErr;
-			PublicInbox::LeiStoreErr->new($err_pipe->[0], $lei);
-		}
+		require PublicInbox::LeiStoreErr;
+		PublicInbox::LeiStoreErr->new($err_pipe->[0], $lei);
 	}
 	$lei->{sto} = $self;
 }
diff --git a/lib/PublicInbox/LeiSucks.pm b/lib/PublicInbox/LeiSucks.pm
index 2ce64d62..a71158f3 100644
--- a/lib/PublicInbox/LeiSucks.pm
+++ b/lib/PublicInbox/LeiSucks.pm
@@ -23,8 +23,7 @@ sub lei_sucks {
 	}
 	eval { require PublicInbox };
 	my $pi_ver = eval('$PublicInbox::VERSION') // '(???)';
-	my $daemon = $lei->{oneshot} ? 'oneshot' : 'daemon';
-	my @out = ("lei $pi_ver mode=$daemon\n",
+	my @out = ("lei $pi_ver\n",
 		"perl $Config{version} / $os $rel / $mac ".
 		"ptrsize=$Config{ptrsize}\n");
 	chomp(my $gv = `git --version` || "git missing");
diff --git a/lib/PublicInbox/LeiUp.pm b/lib/PublicInbox/LeiUp.pm
index 4399c4fb..9069232b 100644
--- a/lib/PublicInbox/LeiUp.pm
+++ b/lib/PublicInbox/LeiUp.pm
@@ -76,22 +76,18 @@ sub lei_up {
 		my @all = PublicInbox::LeiSavedSearch::list($lei);
 		my @local = grep(!m!\Aimaps?://!i, @all);
 		$lei->_lei_store->write_prepare($lei); # share early
-		if ($lei->{oneshot}) { # synchronous
-			up1_redispatch($lei, $_) for @local;
-		} else {
-			# daemon mode, re-dispatch into our event loop w/o
-			# creating an extra fork-level
-			require PublicInbox::DS;
-			require PublicInbox::PktOp;
-			my ($op_c, $op_p) = PublicInbox::PktOp->pair;
-			for my $o (@local) {
-				PublicInbox::DS::requeue(sub {
-					up1_redispatch($lei, $o, $op_p);
-				});
-			}
-			$lei->event_step_init;
-			$op_c->{ops} = { '' => [$lei->can('dclose'), $lei] };
+		# daemon mode, re-dispatch into our event loop w/o
+		# creating an extra fork-level
+		require PublicInbox::DS;
+		require PublicInbox::PktOp;
+		my ($op_c, $op_p) = PublicInbox::PktOp->pair;
+		for my $o (@local) {
+			PublicInbox::DS::requeue(sub {
+				up1_redispatch($lei, $o, $op_p);
+			});
 		}
+		$lei->event_step_init;
+		$op_c->{ops} = { '' => [$lei->can('dclose'), $lei] };
 	} else {
 		up1($lei, $out);
 	}
diff --git a/lib/PublicInbox/TestCommon.pm b/lib/PublicInbox/TestCommon.pm
index 460c9da0..83dcf650 100644
--- a/lib/PublicInbox/TestCommon.pm
+++ b/lib/PublicInbox/TestCommon.pm
@@ -544,7 +544,8 @@ EOM
 	($tmpdir, $for_destroy) = tmpdir unless $tmpdir;
 	state $persist_xrd = $ENV{TEST_LEI_DAEMON_PERSIST_DIR};
 	SKIP: {
-		skip 'TEST_LEI_ONESHOT set', 1 if $ENV{TEST_LEI_ONESHOT};
+		$ENV{TEST_LEI_ONESHOT} and
+			xbail 'TEST_LEI_ONESHOT no longer supported';
 		my $home = "$tmpdir/lei-daemon";
 		mkdir($home, 0700) or BAIL_OUT "mkdir: $!";
 		local $ENV{HOME} = $home;
@@ -568,22 +569,12 @@ EOM
 			lei_ok(qw(daemon-kill), \"daemon-kill after $t");
 		}
 	}; # SKIP for lei_daemon
-	unless ($test_opt->{daemon_only}) {
-		$ENV{TEST_LEI_DAEMON_ONLY} and
-			skip 'TEST_LEI_DAEMON_ONLY set', 1;
-		require_ok 'PublicInbox::LEI';
-		my $home = "$tmpdir/lei-oneshot";
-		mkdir($home, 0700) or BAIL_OUT "mkdir: $!";
-		local $ENV{HOME} = $home;
-		local $ENV{XDG_RUNTIME_DIR} = '/dev/null';
-		$cb->();
-	}
 	if ($daemon_pid) {
 		for (0..10) {
 			kill(0, $daemon_pid) or last;
 			tick;
 		}
-		ok(!kill(0, $daemon_pid), "$t daemon stopped after oneshot");
+		ok(!kill(0, $daemon_pid), "$t daemon stopped");
 		my $f = "$daemon_xrd/lei/errors.log";
 		open my $fh, '<', $f or BAIL_OUT "$f: $!";
 		my @l = <$fh>;
diff --git a/script/lei b/script/lei
index bec6b001..4d752fd8 100755
--- a/script/lei
+++ b/script/lei
@@ -9,10 +9,18 @@ my $narg = 5;
 my $sock;
 my $recv_cmd = PublicInbox::CmdIPC4->can('recv_cmd4');
 my $send_cmd = PublicInbox::CmdIPC4->can('send_cmd4') // do {
+	my $inline_dir = $ENV{PERL_INLINE_DIRECTORY} //= (
+			$ENV{XDG_CACHE_HOME} //
+			( ($ENV{HOME} // '/nonexistent').'/.cache' )
+			).'/public-inbox/inline-c';
+	if (!-d $inline_dir) {
+		require File::Path;
+		File::Path::make_path($inline_dir);
+	}
 	require PublicInbox::Spawn; # takes ~50ms even if built *sigh*
 	$recv_cmd = PublicInbox::Spawn->can('recv_cmd4');
 	PublicInbox::Spawn->can('send_cmd4');
-};
+} // die 'please install Inline::C or Socket::MsgHdr';
 
 my %pids;
 my $sigchld = sub {
@@ -66,80 +74,69 @@ my $exec_cmd = sub {
 	}
 };
 
-if ($send_cmd && eval {
-	my $path = do {
-		my $runtime_dir = ($ENV{XDG_RUNTIME_DIR} // '') . '/lei';
-		die \0 if $runtime_dir eq '/dev/null/lei'; # oneshot forced
-		if ($runtime_dir eq '/lei') {
-			require File::Spec;
-			$runtime_dir = File::Spec->tmpdir."/lei-$<";
-		}
-		unless (-d $runtime_dir) {
-			require File::Path;
-			File::Path::mkpath($runtime_dir, 0, 0700);
-		}
-		"$runtime_dir/$narg.seq.sock";
-	};
-	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)],
-			$path, $! + 0, $narg) or die "popen: $!";
-		while (<$daemon>) { warn $_ } # EOF when STDERR is redirected
-		close($daemon) or warn <<"";
+my $runtime_dir = ($ENV{XDG_RUNTIME_DIR} // '') . '/lei';
+if ($runtime_dir eq '/lei') {
+	require File::Spec;
+	$runtime_dir = File::Spec->tmpdir."/lei-$<";
+}
+unless (-d $runtime_dir) {
+	require File::Path;
+	File::Path::make_path($runtime_dir, { mode => 0700 });
+}
+my $path = "$runtime_dir/$narg.seq.sock";
+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)],
+		$path, $! + 0, $narg) or die "popen: $!";
+	while (<$daemon>) { warn $_ } # EOF when STDERR is redirected
+	close($daemon) or warn <<"";
 lei-daemon could not start, exited with \$?=$?
 
-		# try connecting again anyways, unlink+bind may be racy
-		connect($sock, $addr) or die <<"";
+	# try connecting again anyways, unlink+bind may be racy
+	connect($sock, $addr) or die <<"";
 connect($path): $! (after attempted daemon start)
 Falling back to (slow) one-shot mode
 
+}
+# (Socket::MsgHdr|Inline::C), $sock are all available:
+open my $dh, '<', '.' or die "open(.) $!";
+my $buf = join("\0", scalar(@ARGV), @ARGV);
+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) {
+	die "sendmsg: $! (check RLIMIT_NOFILE)\n" if $!{ETOOMANYREFS};
+	die "sendmsg: $!\n";
+}
+my $x_it_code = 0;
+while (1) {
+	my (@fds) = $recv_cmd->($sock, my $buf, 4096 * 33);
+	if (scalar(@fds) == 1 && !defined($fds[0])) {
+		next if $!{EINTR};
+		last if $!{ECONNRESET};
+		die "recvmsg: $!";
 	}
-	# (Socket::MsgHdr|Inline::C), $sock are all available:
-	open my $dh, '<', '.' or die "open(.) $!";
-	my $buf = join("\0", scalar(@ARGV), @ARGV);
-	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) {
-		die "sendmsg: $! (check RLIMIT_NOFILE)\n" if $!{ETOOMANYREFS};
-		die "sendmsg: $!\n";
-	}
-	1;
-}) { # connected and request sent to lei-daemon, wait for responses or EOF
-	my $x_it_code = 0;
-	while (1) {
-		my (@fds) = $recv_cmd->($sock, my $buf, 4096 * 33);
-		if (scalar(@fds) == 1 && !defined($fds[0])) {
-			next if $!{EINTR};
-			last if $!{ECONNRESET};
-			die "recvmsg: $!";
-		}
-		last if $buf eq '';
-		if ($buf =~ /\Aexec (.+)\z/) {
-			$exec_cmd->(\@fds, split(/\0/, $1));
-		} elsif ($buf eq '-WINCH') {
-			kill($buf, @parent); # 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;
-		} else {
-			$sigchld->();
-			die $buf;
-		}
-	}
-	$sigchld->();
-	if (my $sig = ($x_it_code & 127)) {
-		kill $sig, $$;
-		sleep(1) while 1;
+	last if $buf eq '';
+	if ($buf =~ /\Aexec (.+)\z/) {
+		$exec_cmd->(\@fds, split(/\0/, $1));
+	} elsif ($buf eq '-WINCH') {
+		kill($buf, @parent); # 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;
+	} else {
+		$sigchld->();
+		die $buf;
 	}
-	exit($x_it_code >> 8);
-} else { # for systems lacking Socket::MsgHdr or Inline::C
-	warn $@ if $@ && !ref($@);
-	require PublicInbox::LEI;
-	PublicInbox::LEI::oneshot(__PACKAGE__);
 }
+$sigchld->();
+if (my $sig = ($x_it_code & 127)) {
+	kill $sig, $$;
+	sleep(1) while 1;
+}
+exit($x_it_code >> 8);
diff --git a/t/lei-daemon.t b/t/lei-daemon.t
index 84e2791d..a7c4b799 100644
--- a/t/lei-daemon.t
+++ b/t/lei-daemon.t
@@ -73,15 +73,6 @@ test_lei({ daemon_only => 1 }, sub {
 	lei_ok('daemon-pid');
 	chomp $lei_out;
 	is($lei_out, $new_pid, 'PID unchanged after -0/-CHLD');
-
-	SKIP: { # socket inaccessible
-		skip "cannot test connect EPERM as root", 3 if $> == 0;
-		chmod 0000, $sock or BAIL_OUT "chmod 0000: $!";
-		lei_ok('help', \'connect fail, one-shot fallback works');
-		like($lei_err, qr/\bconnect\(/, 'connect error noted');
-		like($lei_out, qr/^usage: /, 'help output works');
-		chmod 0700, $sock or BAIL_OUT "chmod 0700: $!";
-	}
 	unlink $sock or BAIL_OUT "unlink($sock) $!";
 	for (0..100) {
 		kill('CHLD', $new_pid) or last;