umask: rely on the OnDestroy-based call where applicable

This lets us get rid of some awkwardness around the old API
and single-use subroutines while saving us some LoC.

diff --git a/lib/PublicInbox/ExtSearchIdx.pm b/lib/PublicInbox/ExtSearchIdx.pm
index 6f3711ba656d73343851687fa3fc15bb8f949274..6856ae66dcc8f699105a3eb07f7ff07601ddbb0d 100644 (file)
--- 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 69ab30e6d2516b319e5ca15fa2185800d133d9a2..511dd4d6b66e8ff44935ad8ad99b2a7187d0f639 100644 (file)
--- 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 d3d139410ef52f859825cf8209873400c6d29ac7..856442a9ca4e1773455e39dfb83f75d731f8e25c 100644 (file)
--- 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 106856369c689007470b7ec364796e29079821f6..c87baa7bbe74f7c067e7b442a55b2992c6520459 100644 (file)
--- 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 750adca475310539ad51dc430db6d30ef7d69b3e..96931cbf39312edf0c8a7d6ec1e4460e521b24b2 100755 (executable)
--- 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);