]> Sergey Matveev's repositories - public-inbox.git/commitdiff
lei: require Socket::MsgHdr or Inline::C, drop oneshot
authorEric Wong <e@80x24.org>
Wed, 26 May 2021 18:08:57 +0000 (18:08 +0000)
committerEric Wong <e@80x24.org>
Wed, 26 May 2021 19:33:30 +0000 (19:33 +0000)
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
lib/PublicInbox/LEI.pm
lib/PublicInbox/LeiEditSearch.pm
lib/PublicInbox/LeiStore.pm
lib/PublicInbox/LeiSucks.pm
lib/PublicInbox/LeiUp.pm
lib/PublicInbox/TestCommon.pm
script/lei
t/lei-daemon.t

index 2d81817c8d7cff1c8f996bdd0312ba09c2f498c2..b29780d67c54ce1405977e7660e3c8aca2390a1d 100644 (file)
@@ -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 {
index c8d2f315b72b02da5dbed1f205fdab074649e24d..6ff249d09ca7d1b2bb3db41dcfcc8eaecdb7590b 100644 (file)
@@ -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
index 30ac65bdc18c7d402abb410f203d27cddc25df15..13713d24570906dfc6d2b88720b3bf2a1188abd8 100644 (file)
@@ -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;
index a7a0ebef097d41f68f1adfe0b0de4b656d1b702b..af5edbc24732817eb3b2dcfd51b4d9da2dacc17d 100644 (file)
@@ -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;
 }
index 2ce64d629167294e61256a8f9f02ec66e81bd877..a71158f36f03c97c9b3cdc8a0c0a96e8cbdd8c70 100644 (file)
@@ -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");
index 4399c4fba7641a0e4df7933415e207bda203376d..9069232b4b616e99a50027b40d56bf4db51fe4f6 100644 (file)
@@ -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);
        }
index 460c9da01ad41596fadf8bb77ff700fee1a32d79..83dcf650d9c915e5357725b6bb23fe8f5e259756 100644 (file)
@@ -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>;
index bec6b00125fb4270d9c37152dce79ebe6972dcef..4d752fd8afdfd027789ea6dc86bbc235bcdd7161 100755 (executable)
@@ -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);
index 84e2791d6fcc5a0839dd882659cbf9b241776016..a7c4b7992a378e83aadf7548fe49dd8cde6fd9a2 100644 (file)
@@ -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;