[PATCH 0/3] lei_auth: document and simplify

[PATCH 0/3] lei_auth: document and simplify

[Eric Wong]

Some clarity ahead of planned IMAP improvements for "lei up"
and keyword propagation from lei/store => IMAP folders.

Eric Wong (3):
  lei_auth: diagram for current behavior
  lei_auth: remove net_merge_done1 step
  lei_auth: simplify users

 lib/PublicInbox/LeiAuth.pm          | 36 +++++++++++++++++++----------
 lib/PublicInbox/LeiExportKw.pm      |  3 ---
 lib/PublicInbox/LeiImport.pm        |  2 --
 lib/PublicInbox/LeiIndex.pm         |  2 --
 lib/PublicInbox/LeiLsMailSource.pm  |  1 -
 lib/PublicInbox/LeiPruneMailSync.pm |  1 -
 lib/PublicInbox/LeiRediff.pm        |  1 -
 lib/PublicInbox/LeiRm.pm            |  1 -
 lib/PublicInbox/LeiTag.pm           |  1 -
 lib/PublicInbox/LeiToMail.pm        |  2 --
 10 files changed, 24 insertions(+), 26 deletions(-)

[PATCH 1/3] lei_auth: diagram for current behavior

[Eric Wong]

Before making potentially major changes, lets clarify readers'
understanding of how LeiAuth currently works.
---
 lib/PublicInbox/LeiAuth.pm | 31 ++++++++++++++++++++++++++++---
 1 file changed, 28 insertions(+), 3 deletions(-)

diff --git a/lib/PublicInbox/LeiAuth.pm b/lib/PublicInbox/LeiAuth.pm
index bbd713f0..3155d79b 100644
--- a/lib/PublicInbox/LeiAuth.pm
+++ b/lib/PublicInbox/LeiAuth.pm
@@ -3,18 +3,42 @@
 
 # Authentication worker for anything that needs auth for read/write IMAP
 # (eventually for read-only NNTP access)
+#
+# timelines
+# lei-daemon              |  LeiAuth worker #0      | other WQ workers
+# ----------------------------------------------------------
+# spawns all workers ---->[ workers all start and run ipc_atfork_child ]
+#                         | do_auth_atfork          | wq_worker_loop sleep
+#                         | # reads .netrc          |
+#                         | # queries git-credential|
+#                         | send net_merge_continue |
+#                         |         |               |
+#                         |         v               |
+# recv net_merge_continue <---------/               |
+#            |            |                         |
+#            v            |                         |
+# broadcast net_merge_all [ all workers (including LeiAuth worker #0) ]
+#                         [ LeiAuth worker #0 becomes just another WQ worker ]
+#                         |
+#                         | each worker sends net_merge_done1 to lei-daemon
+#                         |              |  | ... |
+#                         |              v  v     v
+# recv net_merge_done1 <--<-------<------/--/--<--/
+#
+# call net_merge_all_done ->-> do per-class defined actions
 package PublicInbox::LeiAuth;
 use strict;
 use v5.10.1;
 
 sub do_auth_atfork { # used by IPC WQ workers
 	my ($self, $wq) = @_;
-	return if $wq->{-wq_worker_nr} != 0;
+	return if $wq->{-wq_worker_nr} != 0; # only first worker calls this
 	my $lei = $wq->{lei};
 	my $net = $lei->{net};
-	eval {
+	eval { # fill auth info (may prompt user or read netrc)
 		my $mics = $net->imap_common_init($lei);
 		my $nn = $net->nntp_common_init($lei);
+		# broadcast successful auth info to lei-daemon:
 		$lei->{pkt_op_p}->pkt_do('net_merge_continue', $net) or
 				die "pkt_do net_merge_continue: $!";
 		$net->{mics_cached} = $mics if $mics;
@@ -33,6 +57,7 @@ sub net_merge_all { # called in wq worker via wq_broadcast
 	my ($wq, $net_new) = @_;
 	my $net = $wq->{lei}->{net};
 	%$net = (%$net, %$net_new);
+	# notify daemon we're ready
 	$wq->{lei}->{pkt_op_p}->pkt_do('net_merge_done1') or
 		die "pkt_op_do net_merge_done1: $!";
 }
@@ -40,7 +65,7 @@ sub net_merge_all { # called in wq worker via wq_broadcast
 # called by top-level lei-daemon when first worker is done with auth
 sub net_merge_continue {
 	my ($wq, $net_new) = @_;
-	$wq->wq_broadcast('net_merge_all', $net_new);
+	$wq->wq_broadcast('net_merge_all', $net_new); # pass to current workers
 }
 
 sub op_merge { # prepares PktOp->pair ops

[PATCH 2/3] lei_auth: remove net_merge_done1 step

[Eric Wong]

It turns out this step is unnecessary, since SOCK_SEQPACKET
ordering is guaranteed and we know wq_broadcast calls will
always be handled sequentially.
---
 lib/PublicInbox/LeiAuth.pm | 18 ++----------------
 1 file changed, 2 insertions(+), 16 deletions(-)

diff --git a/lib/PublicInbox/LeiAuth.pm b/lib/PublicInbox/LeiAuth.pm
index 3155d79b..73c0be59 100644
--- a/lib/PublicInbox/LeiAuth.pm
+++ b/lib/PublicInbox/LeiAuth.pm
@@ -20,12 +20,7 @@
 # broadcast net_merge_all [ all workers (including LeiAuth worker #0) ]
 #                         [ LeiAuth worker #0 becomes just another WQ worker ]
 #                         |
-#                         | each worker sends net_merge_done1 to lei-daemon
-#                         |              |  | ... |
-#                         |              v  v     v
-# recv net_merge_done1 <--<-------<------/--/--<--/
-#
-# call net_merge_all_done ->-> do per-class defined actions
+# call net_merge_all_done ->-> do per-WQ-class defined actions
 package PublicInbox::LeiAuth;
 use strict;
 use v5.10.1;
@@ -47,31 +42,22 @@ sub do_auth_atfork { # used by IPC WQ workers
 	$lei->fail($@) if $@;
 }
 
-sub net_merge_done1 { # bump merge-count in top-level lei-daemon
-	my ($wq) = @_;
-	return if ++$wq->{nr_net_merge_done} != $wq->{-wq_nr_workers};
-	$wq->net_merge_all_done; # defined per wq-class (e.g. LeiImport)
-}
-
 sub net_merge_all { # called in wq worker via wq_broadcast
 	my ($wq, $net_new) = @_;
 	my $net = $wq->{lei}->{net};
 	%$net = (%$net, %$net_new);
-	# notify daemon we're ready
-	$wq->{lei}->{pkt_op_p}->pkt_do('net_merge_done1') or
-		die "pkt_op_do net_merge_done1: $!";
 }
 
 # called by top-level lei-daemon when first worker is done with auth
 sub net_merge_continue {
 	my ($wq, $net_new) = @_;
 	$wq->wq_broadcast('net_merge_all', $net_new); # pass to current workers
+	$wq->net_merge_all_done; # defined per-WQ
 }
 
 sub op_merge { # prepares PktOp->pair ops
 	my ($self, $ops, $wq) = @_;
 	$ops->{net_merge_continue} = [ \&net_merge_continue, $wq ];
-	$ops->{net_merge_done1} = [ \&net_merge_done1, $wq ];
 }
 
 sub new { bless \(my $x), __PACKAGE__ }

[PATCH 3/3] lei_auth: simplify users

[Eric Wong]

There's no need to alias net_merge_all in each WQ class
which uses LeiAuth, `$obj->$sub' works even when `$sub'
is a fully-qualified subroutine name with `::' in it.
perlobj(1) documents it under "Method Call Variations".
---
 lib/PublicInbox/LeiAuth.pm          | 3 ++-
 lib/PublicInbox/LeiExportKw.pm      | 3 ---
 lib/PublicInbox/LeiImport.pm        | 2 --
 lib/PublicInbox/LeiIndex.pm         | 2 --
 lib/PublicInbox/LeiLsMailSource.pm  | 1 -
 lib/PublicInbox/LeiPruneMailSync.pm | 1 -
 lib/PublicInbox/LeiRediff.pm        | 1 -
 lib/PublicInbox/LeiRm.pm            | 1 -
 lib/PublicInbox/LeiTag.pm           | 1 -
 lib/PublicInbox/LeiToMail.pm        | 2 --
 10 files changed, 2 insertions(+), 15 deletions(-)

diff --git a/lib/PublicInbox/LeiAuth.pm b/lib/PublicInbox/LeiAuth.pm
index 73c0be59..465a2758 100644
--- a/lib/PublicInbox/LeiAuth.pm
+++ b/lib/PublicInbox/LeiAuth.pm
@@ -49,9 +49,10 @@ sub net_merge_all { # called in wq worker via wq_broadcast
 }
 
 # called by top-level lei-daemon when first worker is done with auth
+# passes updated net auth info to current workers
 sub net_merge_continue {
 	my ($wq, $net_new) = @_;
-	$wq->wq_broadcast('net_merge_all', $net_new); # pass to current workers
+	$wq->wq_broadcast('PublicInbox::LeiAuth::net_merge_all', $net_new);
 	$wq->net_merge_all_done; # defined per-WQ
 }
 
diff --git a/lib/PublicInbox/LeiExportKw.pm b/lib/PublicInbox/LeiExportKw.pm
index 78c6c6f9..d37f3768 100644
--- a/lib/PublicInbox/LeiExportKw.pm
+++ b/lib/PublicInbox/LeiExportKw.pm
@@ -145,7 +145,4 @@ no warnings 'once';
 *ipc_atfork_child = \&PublicInbox::LeiInput::input_only_atfork_child;
 *net_merge_all_done = \&PublicInbox::LeiInput::input_only_net_merge_all_done;
 
-# the following works even when LeiAuth is lazy-loaded
-*net_merge_all = \&PublicInbox::LeiAuth::net_merge_all;
-
 1;
diff --git a/lib/PublicInbox/LeiImport.pm b/lib/PublicInbox/LeiImport.pm
index 7580e37e..7c563bd8 100644
--- a/lib/PublicInbox/LeiImport.pm
+++ b/lib/PublicInbox/LeiImport.pm
@@ -133,6 +133,4 @@ no warnings 'once';
 *ipc_atfork_child = \&PublicInbox::LeiInput::input_only_atfork_child;
 *net_merge_all_done = \&PublicInbox::LeiInput::input_only_net_merge_all_done;
 
-# the following works even when LeiAuth is lazy-loaded
-*net_merge_all = \&PublicInbox::LeiAuth::net_merge_all;
 1;
diff --git a/lib/PublicInbox/LeiIndex.pm b/lib/PublicInbox/LeiIndex.pm
index ef3e4d0b..1b327a2c 100644
--- a/lib/PublicInbox/LeiIndex.pm
+++ b/lib/PublicInbox/LeiIndex.pm
@@ -43,6 +43,4 @@ for my $m (qw(pmdir_cb input_net_cb)) {
 *ipc_atfork_child = \&PublicInbox::LeiInput::input_only_atfork_child;
 *net_merge_all_done = \&PublicInbox::LeiInput::input_only_net_merge_all_done;
 
-# the following works even when LeiAuth is lazy-loaded
-*net_merge_all = \&PublicInbox::LeiAuth::net_merge_all;
 1;
diff --git a/lib/PublicInbox/LeiLsMailSource.pm b/lib/PublicInbox/LeiLsMailSource.pm
index 2d8913ac..71e253d9 100644
--- a/lib/PublicInbox/LeiLsMailSource.pm
+++ b/lib/PublicInbox/LeiLsMailSource.pm
@@ -114,6 +114,5 @@ sub _complete_ls_mail_source {
 no warnings 'once';
 *ipc_atfork_child = \&PublicInbox::LeiInput::input_only_atfork_child;
 *net_merge_all_done = \&PublicInbox::LeiInput::input_only_net_merge_all_done;
-*net_merge_all = \&PublicInbox::LeiAuth::net_merge_all;
 
 1;
diff --git a/lib/PublicInbox/LeiPruneMailSync.pm b/lib/PublicInbox/LeiPruneMailSync.pm
index 98239a13..1a277122 100644
--- a/lib/PublicInbox/LeiPruneMailSync.pm
+++ b/lib/PublicInbox/LeiPruneMailSync.pm
@@ -91,7 +91,6 @@ EOM
 no warnings 'once';
 *_complete_prune_mail_sync = \&PublicInbox::LeiExportKw::_complete_export_kw;
 *ipc_atfork_child = \&PublicInbox::LeiInput::input_only_atfork_child;
-*net_merge_all = \&PublicInbox::LeiAuth::net_merge_all;
 *net_merge_all_done = \&PublicInbox::LeiInput::input_only_net_merge_all_done;
 
 1;
diff --git a/lib/PublicInbox/LeiRediff.pm b/lib/PublicInbox/LeiRediff.pm
index 60286b06..ea9b2a64 100644
--- a/lib/PublicInbox/LeiRediff.pm
+++ b/lib/PublicInbox/LeiRediff.pm
@@ -255,5 +255,4 @@ sub ipc_atfork_child {
 
 no warnings 'once';
 *net_merge_all_done = \&PublicInbox::LeiInput::input_only_net_merge_all_done;
-*net_merge_all = \&PublicInbox::LeiAuth::net_merge_all;
 1;
diff --git a/lib/PublicInbox/LeiRm.pm b/lib/PublicInbox/LeiRm.pm
index 578e9811..778fa1de 100644
--- a/lib/PublicInbox/LeiRm.pm
+++ b/lib/PublicInbox/LeiRm.pm
@@ -44,6 +44,5 @@ sub lei_rm {
 no warnings 'once';
 *ipc_atfork_child = \&PublicInbox::LeiInput::input_only_atfork_child;
 *net_merge_all_done = \&PublicInbox::LeiInput::input_only_net_merge_all_done;
-*net_merge_all = \&PublicInbox::LeiAuth::net_merge_all;
 
 1;
diff --git a/lib/PublicInbox/LeiTag.pm b/lib/PublicInbox/LeiTag.pm
index 463fb921..44d77b88 100644
--- a/lib/PublicInbox/LeiTag.pm
+++ b/lib/PublicInbox/LeiTag.pm
@@ -118,7 +118,6 @@ sub _complete_tag {
 }
 
 no warnings 'once'; # the following works even when LeiAuth is lazy-loaded
-*net_merge_all = \&PublicInbox::LeiAuth::net_merge_all;
 *net_merge_all_done = \&PublicInbox::LeiInput::input_only_net_merge_all_done;
 
 1;
diff --git a/lib/PublicInbox/LeiToMail.pm b/lib/PublicInbox/LeiToMail.pm
index 1221d3c7..2c7a92de 100644
--- a/lib/PublicInbox/LeiToMail.pm
+++ b/lib/PublicInbox/LeiToMail.pm
@@ -783,6 +783,4 @@ sub net_merge_all_done {
 	$self->wq_close(1);
 }
 
-no warnings 'once'; # the following works even when LeiAuth is lazy-loaded
-*net_merge_all = \&PublicInbox::LeiAuth::net_merge_all;
 1;