user/dev discussion of public-inbox itself
 help / color / mirror / code / Atom feed
* [PATCH] lei: fix git-credential handling
@ 2021-04-02  9:42 Eric Wong
  2021-04-05  4:17 ` Kyle Meyer
  0 siblings, 1 reply; 5+ messages in thread
From: Eric Wong @ 2021-04-02  9:42 UTC (permalink / raw)
  To: meta

I completely forgot about git-credential prompting when
making lei background the client process for MUA.

Now it backgrounds itself only for the MUA when no FDs are
passed, since the MUA is the final command run.  Otherwise, it
relies on FD passing as before.

Fixes: c790a75439f3a1db ("script/lei: background ourselves on MUA/pager exec")
---
 lib/PublicInbox/LEI.pm |  8 +++++++-
 script/lei             | 46 ++++++++++++++++++++++++++++--------------
 2 files changed, 38 insertions(+), 16 deletions(-)

diff --git a/lib/PublicInbox/LEI.pm b/lib/PublicInbox/LEI.pm
index 69d48bd1..f9361c68 100644
--- a/lib/PublicInbox/LEI.pm
+++ b/lib/PublicInbox/LEI.pm
@@ -839,7 +839,13 @@ sub start_mua {
 	if (my $sock = $self->{sock}) { # lei(1) client process runs it
 		send($sock, exec_buf(\@cmd, {}), MSG_EOR);
 	} elsif ($self->{oneshot}) {
-		$self->{"pid.$self.$$"}->{spawn(\@cmd)} = \@cmd;
+		my $pid = fork // die "fork: $!";
+		if ($pid > 0) { # original process
+			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));
diff --git a/script/lei b/script/lei
index bea8dcde..78a7dab9 100755
--- a/script/lei
+++ b/script/lei
@@ -14,31 +14,45 @@ my $send_cmd = PublicInbox::CmdIPC4->can('send_cmd4') // do {
 	PublicInbox::Spawn->can('send_cmd4');
 };
 
-my @orig_pid;
+my %pids;
+my $sigchld = sub {
+	my $flags = scalar(@_) ? POSIX::WNOHANG() : 0;
+	for my $pid (keys %pids) {
+		delete($pids{$pid}) if waitpid($pid, $flags) == $pid;
+	}
+};
+my @parent;
 my $exec_cmd = sub {
 	my ($fds, $argc, @argv) = @_;
-	die "BUG: already exec-ed\n" if @orig_pid;
-	@orig_pid = ($$);
-	require POSIX; # WNOHANG
+	my $parent = $$;
+	require POSIX;
 	my @old = (*STDIN{IO}, *STDOUT{IO}, *STDERR{IO});
 	my @rdr;
 	for my $fd (@$fds) {
-		open(my $tmpfh, '+<&=', $fd) or die "open +<&=$fd: $!";
-		push @rdr, shift(@old), $tmpfh;
+		open(my $newfh, '+<&=', $fd) or die "open +<&=$fd: $!";
+		push @rdr, shift(@old), $newfh;
 	}
+	my $do_exec = sub {
+		my %env = map { split(/=/, $_, 2) } splice(@argv, $argc);
+		@ENV{keys %env} = values %env;
+		exec(@argv);
+		warn "exec: @argv: $!\n";
+		POSIX::_exit(1);
+	};
+	$SIG{CHLD} = $sigchld;
 	my $pid = fork // die "fork: $!";
 	if ($pid == 0) {
+		while (my ($io, $newfh) = splice(@rdr, 0, 2)) {
+			open $io, '+<&', $newfh or die "open +<&=: $!";
+		}
+		$do_exec->() if scalar(@$fds); # git-credential, pager
+
+		# parent backgrounds on MUA
 		POSIX::setsid() > 0 or die "setsid: $!";
+		@parent = ($parent);
 		return; # continue $recv_cmd in background
 	}
-	my %env = map { split(/=/, $_, 2) } splice(@argv, $argc);
-	while (my ($old_io, $tmpfh) = splice(@rdr, 0, 2)) {
-		open $old_io, '+<&', $tmpfh or die "open +<&=: $!";
-	}
-	@ENV{keys %env} = values %env;
-	exec(@argv);
-	warn "exec: @argv: $!\n";
-	POSIX::_exit(1);
+	$do_exec->() if !scalar(@$fds); # MUA reuses all FDs
 };
 
 if ($send_cmd && eval {
@@ -95,16 +109,18 @@ Falling back to (slow) one-shot mode
 		if ($buf =~ /\Aexec (.+)\z/) {
 			$exec_cmd->(\@fds, split(/\0/, $1));
 		} elsif ($buf eq '-WINCH') {
-			kill($buf, @orig_pid); # for MUA
+			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;

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

* Re: [PATCH] lei: fix git-credential handling
  2021-04-02  9:42 [PATCH] lei: fix git-credential handling Eric Wong
@ 2021-04-05  4:17 ` Kyle Meyer
  2021-04-05  8:36   ` [PATCH] script/lei: waitpid for git-credential and pager Eric Wong
  0 siblings, 1 reply; 5+ messages in thread
From: Kyle Meyer @ 2021-04-05  4:17 UTC (permalink / raw)
  To: Eric Wong; +Cc: meta

Eric Wong writes:

> I completely forgot about git-credential prompting when
> making lei background the client process for MUA.
>
> Now it backgrounds itself only for the MUA when no FDs are
> passed, since the MUA is the final command run.  Otherwise, it
> relies on FD passing as before.

This seems to break the pager functionality.  For example, if I do

  lei q -I https://public-inbox.org/meta/ s:blob

I end up in a misbehaving pager (/bin/less, on my system).  Navigation
keys like 'j' and 'k' are instead passed through, and enter returns to
the prompt.

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

* [PATCH] script/lei: waitpid for git-credential and pager
  2021-04-05  4:17 ` Kyle Meyer
@ 2021-04-05  8:36   ` Eric Wong
  2021-04-05  8:59     ` [RFC] script/lei: don't setsid on MUA spawn Eric Wong
  2021-04-05 21:26     ` [PATCH] script/lei: waitpid for git-credential and pager Kyle Meyer
  0 siblings, 2 replies; 5+ messages in thread
From: Eric Wong @ 2021-04-05  8:36 UTC (permalink / raw)
  To: Kyle Meyer; +Cc: meta

Kyle Meyer <kyle@kyleam.com> wrote:
> This seems to break the pager functionality.  For example, if I do
> 
>   lei q -I https://public-inbox.org/meta/ s:blob
> 
> I end up in a misbehaving pager (/bin/less, on my system).  Navigation
> keys like 'j' and 'k' are instead passed through, and enter returns to
> the prompt.

Odd, I'm not getting the exact same behavior, but I noticed
it's not getting reaped.  Does this fix things for you?

-----8<-----
Subject: [PATCH] script/lei: waitpid for git-credential and pager

We need to ensure we reap things we spawn.
---
 script/lei | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/script/lei b/script/lei
index 78a7dab9..76217ab9 100755
--- a/script/lei
+++ b/script/lei
@@ -52,7 +52,11 @@ my $exec_cmd = sub {
 		@parent = ($parent);
 		return; # continue $recv_cmd in background
 	}
-	$do_exec->() if !scalar(@$fds); # MUA reuses all FDs
+	if (scalar(@$fds)) {
+		$pids{$pid} = undef;
+	} else {
+		$do_exec->(); # MUA reuses all FDs
+	}
 };
 
 if ($send_cmd && eval {

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

* [RFC] script/lei: don't setsid on MUA spawn
  2021-04-05  8:36   ` [PATCH] script/lei: waitpid for git-credential and pager Eric Wong
@ 2021-04-05  8:59     ` Eric Wong
  2021-04-05 21:26     ` [PATCH] script/lei: waitpid for git-credential and pager Kyle Meyer
  1 sibling, 0 replies; 5+ messages in thread
From: Eric Wong @ 2021-04-05  8:59 UTC (permalink / raw)
  To: Kyle Meyer; +Cc: meta

Btw, the setsid call doesn't seem necessary, either, the MUA
seems to work fine without it...

------8<------
Subject: [PATCH] script/lei: don't setsid on MUA spawn

---
 script/lei | 1 -
 1 file changed, 1 deletion(-)

diff --git a/script/lei b/script/lei
index 76217ab9..cd535afb 100755
--- a/script/lei
+++ b/script/lei
@@ -48,7 +48,6 @@ my $exec_cmd = sub {
 		$do_exec->() if scalar(@$fds); # git-credential, pager
 
 		# parent backgrounds on MUA
-		POSIX::setsid() > 0 or die "setsid: $!";
 		@parent = ($parent);
 		return; # continue $recv_cmd in background
 	}

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

* Re: [PATCH] script/lei: waitpid for git-credential and pager
  2021-04-05  8:36   ` [PATCH] script/lei: waitpid for git-credential and pager Eric Wong
  2021-04-05  8:59     ` [RFC] script/lei: don't setsid on MUA spawn Eric Wong
@ 2021-04-05 21:26     ` Kyle Meyer
  1 sibling, 0 replies; 5+ messages in thread
From: Kyle Meyer @ 2021-04-05 21:26 UTC (permalink / raw)
  To: Eric Wong; +Cc: meta

Eric Wong writes:

> Odd, I'm not getting the exact same behavior, but I noticed
> it's not getting reaped.  Does this fix things for you?

That seems to do the trick.  Thank you!

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

end of thread, other threads:[~2021-04-05 21:27 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-02  9:42 [PATCH] lei: fix git-credential handling Eric Wong
2021-04-05  4:17 ` Kyle Meyer
2021-04-05  8:36   ` [PATCH] script/lei: waitpid for git-credential and pager Eric Wong
2021-04-05  8:59     ` [RFC] script/lei: don't setsid on MUA spawn Eric Wong
2021-04-05 21:26     ` [PATCH] script/lei: waitpid for git-credential and pager Kyle Meyer

user/dev discussion of public-inbox itself

This inbox may be cloned and mirrored by anyone:

	git clone --mirror http://public-inbox.org/meta
	git clone --mirror http://czquwvybam4bgbro.onion/meta
	git clone --mirror http://hjrcffqmbrq6wope.onion/meta
	git clone --mirror http://ou63pmih66umazou.onion/meta

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V1 meta meta/ http://public-inbox.org/meta \
		meta@public-inbox.org
	public-inbox-index meta

Example config snippet for mirrors.
Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.mail.public-inbox.meta
	nntp://ou63pmih66umazou.onion/inbox.comp.mail.public-inbox.meta
	nntp://czquwvybam4bgbro.onion/inbox.comp.mail.public-inbox.meta
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.mail.public-inbox.meta
	nntp://news.gmane.io/gmane.mail.public-inbox.general
 note: .onion URLs require Tor: https://www.torproject.org/

code repositories for project(s) associated with this inbox:

	https://80x24.org/public-inbox.git

AGPL code for this site: git clone https://public-inbox.org/public-inbox.git