[PATCH 4/6] umask: rely on the OnDestroy-based call where applicable

[Eric Wong <e@80x24.org>]

This lets us get rid of some awkwardness around the old API
and single-use subroutines while saving us some LoC.
---
 lib/PublicInbox/ExtSearchIdx.pm | 14 +++++--------
 lib/PublicInbox/SearchIdx.pm    | 19 ++++++------------
 lib/PublicInbox/V2Writable.pm   | 23 +++++++++-------------
 lib/PublicInbox/Xapcmd.pm       | 35 +++++++++++++++------------------
 script/public-inbox-convert     |  9 ++++-----
 5 files changed, 40 insertions(+), 60 deletions(-)

diff --git a/lib/PublicInbox/ExtSearchIdx.pm b/lib/PublicInbox/ExtSearchIdx.pm
index 6f3711ba..6856ae66 100644
--- a/lib/PublicInbox/ExtSearchIdx.pm
+++ b/lib/PublicInbox/ExtSearchIdx.pm
@@ -1179,12 +1179,6 @@ sub update_last_commit { # overrides V2Writable
 	$self->{oidx}->eidx_meta($meta_key, $latest_cmt);
 }
 
-sub _idx_init { # with_umask callback
-	my ($self, $opt) = @_;
-	PublicInbox::V2Writable::_idx_init($self, $opt); # acquires ei.lock
-	$self->{midx} = PublicInbox::MiscIdx->new($self);
-}
-
 sub symlink_packs ($$) {
 	my ($ibx, $pd) = @_;
 	my $ret = 0;
@@ -1270,15 +1264,17 @@ sub idx_init { # similar to V2Writable
 	}
 	($has_new || $prune_nr || $new ne '') and
 		$self->{mg}->write_alternates($mode, $alt, $new);
-	$git_midx and $self->with_umask(sub {
+	my $restore = $self->with_umask;
+	if ($git_midx) {
 		my @cmd = ('multi-pack-index');
 		push @cmd, '--no-progress' if ($opt->{quiet}//0) > 1;
 		my $lk = $self->lock_for_scope;
 		system('git', "--git-dir=$ALL", @cmd, 'write');
 		# ignore errors, fairly new command, may not exist
-	});
+	}
 	$self->parallel_init($self->{indexlevel});
-	$self->with_umask(\&_idx_init, $self, $opt);
+	PublicInbox::V2Writable::_idx_init($self, $opt); # acquires ei.lock
+	$self->{midx} = PublicInbox::MiscIdx->new($self);
 	$self->{oidx}->begin_lazy;
 	$self->{oidx}->eidx_prep;
 	$self->{midx}->create_xdb if $new ne '';
diff --git a/lib/PublicInbox/SearchIdx.pm b/lib/PublicInbox/SearchIdx.pm
index 69ab30e6..511dd4d6 100644
--- a/lib/PublicInbox/SearchIdx.pm
+++ b/lib/PublicInbox/SearchIdx.pm
@@ -1110,8 +1110,10 @@ sub DESTROY {
 	$_[0]->{lockfh} = undef;
 }
 
-sub _begin_txn {
+sub begin_txn_lazy {
 	my ($self) = @_;
+	return if $self->{txn};
+	my $restore = $self->with_umask;
 	my $xdb = $self->{xdb} || idx_acquire($self);
 	$self->{oidx}->begin_lazy if $self->{oidx};
 	$xdb->begin_transaction if $xdb;
@@ -1119,11 +1121,6 @@ sub _begin_txn {
 	$xdb;
 }
 
-sub begin_txn_lazy {
-	my ($self) = @_;
-	$self->with_umask(\&_begin_txn, $self) if !$self->{txn};
-}
-
 # store 'indexlevel=medium' in v2 shard=0 and v1 (only one shard)
 # This metadata is read by InboxWritable->detect_indexlevel:
 sub set_metadata_once {
@@ -1147,8 +1144,10 @@ sub set_metadata_once {
 	}
 }
 
-sub _commit_txn {
+sub commit_txn_lazy {
 	my ($self) = @_;
+	return unless delete($self->{txn});
+	my $restore = $self->with_umask;
 	if (my $eidx = $self->{eidx}) {
 		$eidx->git->async_wait_all;
 		$eidx->{transact_bytes} = 0;
@@ -1160,12 +1159,6 @@ sub _commit_txn {
 	$self->{oidx}->commit_lazy if $self->{oidx};
 }
 
-sub commit_txn_lazy {
-	my ($self) = @_;
-	delete($self->{txn}) and
-		$self->with_umask(\&_commit_txn, $self);
-}
-
 sub eidx_shard_new {
 	my ($class, $eidx, $shard) = @_;
 	my $self = bless {
diff --git a/lib/PublicInbox/V2Writable.pm b/lib/PublicInbox/V2Writable.pm
index d3d13941..856442a9 100644
--- a/lib/PublicInbox/V2Writable.pm
+++ b/lib/PublicInbox/V2Writable.pm
@@ -89,13 +89,6 @@ sub init_inbox {
 	$self->done;
 }
 
-# returns undef on duplicate or spam
-# mimics Import::add and wraps it for v2
-sub add {
-	my ($self, $eml, $check_cb) = @_;
-	$self->{ibx}->with_umask(\&_add, $self, $eml, $check_cb);
-}
-
 sub idx_shard ($$) {
 	my ($self, $num) = @_;
 	$self->{idx_shards}->[$num % scalar(@{$self->{idx_shards}})];
@@ -113,8 +106,11 @@ sub do_idx ($$$) {
 	$n >= $self->{batch_bytes};
 }
 
-sub _add {
+# returns undef on duplicate or spam
+# mimics Import::add and wraps it for v2
+sub add {
 	my ($self, $mime, $check_cb) = @_;
+	my $restore = $self->{ibx}->with_umask;
 
 	# spam check:
 	if ($check_cb) {
@@ -391,17 +387,16 @@ sub rewrite_internal ($$;$$$) {
 # (retval[2]) is not part of the stable API shared with Import->remove
 sub remove {
 	my ($self, $eml, $cmt_msg) = @_;
-	my $r = $self->{ibx}->with_umask(\&rewrite_internal,
-						$self, $eml, $cmt_msg);
+	my $restore = $self->{ibx}->with_umask;
+	my $r = rewrite_internal($self, $eml, $cmt_msg);
 	defined($r) && defined($r->[0]) ? @$r: undef;
 }
 
 sub _replace ($$;$$) {
 	my ($self, $old_eml, $new_eml, $sref) = @_;
-	my $arg = [ $self, $old_eml, undef, $new_eml, $sref ];
-	my $rewritten = $self->{ibx}->with_umask(\&rewrite_internal,
-			$self, $old_eml, undef, $new_eml, $sref) or return;
-
+	my $restore = $self->{ibx}->with_umask;
+	my $rewritten = rewrite_internal($self, $old_eml, undef,
+					$new_eml, $sref) or return;
 	my $rewrites = $rewritten->{rewrites};
 	# ->done is called if there are rewrites since we gc+prune from git
 	$self->idx_init if @$rewrites;
diff --git a/lib/PublicInbox/Xapcmd.pm b/lib/PublicInbox/Xapcmd.pm
index 10685636..c87baa7b 100644
--- a/lib/PublicInbox/Xapcmd.pm
+++ b/lib/PublicInbox/Xapcmd.pm
@@ -256,24 +256,6 @@ sub prepare_run {
 
 sub check_compact () { runnable_or_die($XAPIAN_COMPACT) }
 
-sub _run { # with_umask callback
-	my ($ibx, $cb, $opt) = @_;
-	my $im = $ibx->can('importer') ? $ibx->importer(0) : undef;
-	($im // $ibx)->lock_acquire;
-	my ($tmp, $queue) = prepare_run($ibx, $opt);
-
-	# fine-grained locking if we prepare for reindex
-	if (!$opt->{-coarse_lock}) {
-		prepare_reindex($ibx, $opt);
-		($im // $ibx)->lock_release;
-	}
-
-	$ibx->cleanup if $ibx->can('cleanup');
-	process_queue($queue, $cb, $opt);
-	($im // $ibx)->lock_acquire if !$opt->{-coarse_lock};
-	commit_changes($ibx, $im, $tmp, $opt);
-}
-
 sub run {
 	my ($ibx, $task, $opt) = @_; # task = 'cpdb' or 'compact'
 	my $cb = \&$task;
@@ -296,7 +278,22 @@ sub run {
 
 	local @SIG{keys %SIG} = values %SIG;
 	setup_signals();
-	$ibx->with_umask(\&_run, $ibx, $cb, $opt);
+	my $restore = $ibx->with_umask;
+
+	my $im = $ibx->can('importer') ? $ibx->importer(0) : undef;
+	($im // $ibx)->lock_acquire;
+	my ($tmp, $queue) = prepare_run($ibx, $opt);
+
+	# fine-grained locking if we prepare for reindex
+	if (!$opt->{-coarse_lock}) {
+		prepare_reindex($ibx, $opt);
+		($im // $ibx)->lock_release;
+	}
+
+	$ibx->cleanup if $ibx->can('cleanup');
+	process_queue($queue, $cb, $opt);
+	($im // $ibx)->lock_acquire if !$opt->{-coarse_lock};
+	commit_changes($ibx, $im, $tmp, $opt);
 }
 
 sub cpdb_retryable ($$) {
diff --git a/script/public-inbox-convert b/script/public-inbox-convert
index 750adca4..96931cbf 100755
--- a/script/public-inbox-convert
+++ b/script/public-inbox-convert
@@ -89,7 +89,8 @@ sub link_or_copy ($$) {
 	File::Copy::cp($src, $dst) or die "cp $src, $dst failed: $!\n";
 }
 
-$old->with_umask(sub {
+{
+	my $restore = $old->with_umask;
 	my $old_cfg = "$old->{inboxdir}/config";
 	local $ENV{GIT_CONFIG} = $old_cfg;
 	my $new_cfg = "$new->{inboxdir}/all.git/config";
@@ -110,12 +111,10 @@ $old->with_umask(sub {
 	my $desc = "$old->{inboxdir}/description";
 	link_or_copy($desc, "$new->{inboxdir}/description") if -e $desc;
 	my $clone = "$old->{inboxdir}/cloneurl";
-	if (-e $clone) {
-		warn <<"";
+	warn <<"" if -e $clone;
 $clone may not be valid after migrating to v2, not copying
 
-	}
-});
+}
 my $state = '';
 my $head = $old->{ref_head} || 'HEAD';
 my ($rd, $pid) = $old->git->popen(qw(fast-export --use-done-feature), $head);