From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: X-Spam-Status: No, score=-4.2 required=3.0 tests=ALL_TRUSTED,BAYES_00, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF shortcircuit=no autolearn=ham autolearn_force=no version=3.4.6 Received: from localhost (dcvr.yhbt.net [127.0.0.1]) by dcvr.yhbt.net (Postfix) with ESMTP id 93E221F487 for ; Fri, 7 Apr 2023 12:40:54 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=80x24.org; s=selector1; t=1680871254; bh=eViQopz0L0NhIihbew6WfurQ0d0CH07gP4RcBPC5esI=; h=From:To:Subject:Date:In-Reply-To:References:From; b=YMSGeaFnJ/qpMP3YzMWOtTsPwT5HTNVV+N+iDtFFz9CgPjx8cgg9LD5Kv3oYLy/T5 qGL6ROmEALyqk/cLdd9uL4hH4NJK+GeoxHVXoliQjPxPsEgqR+4cZ0gAzMEdULYZPM QeDcCQ75uOW4CW+LulaDgvR5ugSIGZ4u+cIwsOgo= From: Eric Wong To: meta@public-inbox.org Subject: [PATCH 4/6] umask: rely on the OnDestroy-based call where applicable Date: Fri, 7 Apr 2023 12:40:51 +0000 Message-Id: <20230407124053.2233988-5-e@80x24.org> In-Reply-To: <20230407124053.2233988-1-e@80x24.org> References: <20230407124053.2233988-1-e@80x24.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit List-Id: 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);