Since CodeSearchIdx doesn't deal with inboxes, it makes sense
to split it out from inbox-specific code and start moving
towards using OnDestroy to restore the umask at the end of
scope and reducing extra functions.
---
MANIFEST | 1 +
lib/PublicInbox/CodeSearchIdx.pm | 28 ++++---------
lib/PublicInbox/ExtSearchIdx.pm | 3 +-
lib/PublicInbox/InboxWritable.pm | 70 +-------------------------------
lib/PublicInbox/SearchIdx.pm | 6 ++-
lib/PublicInbox/Umask.pm | 70 ++++++++++++++++++++++++++++++++
t/search.t | 7 ++--
7 files changed, 91 insertions(+), 94 deletions(-)
create mode 100644 lib/PublicInbox/Umask.pm
diff --git a/MANIFEST b/MANIFEST
index a0e64c6a..c338f0f4 100644
--- a/MANIFEST
+++ b/MANIFEST
@@ -340,6 +340,7 @@ lib/PublicInbox/TestCommon.pm
lib/PublicInbox/Tmpfile.pm
lib/PublicInbox/URIimap.pm
lib/PublicInbox/URInntps.pm
+lib/PublicInbox/Umask.pm
lib/PublicInbox/Unsubscribe.pm
lib/PublicInbox/UserContent.pm
lib/PublicInbox/V2Writable.pm
diff --git a/lib/PublicInbox/CodeSearchIdx.pm b/lib/PublicInbox/CodeSearchIdx.pm
index 3a3fc03e..78032c00 100644
--- a/lib/PublicInbox/CodeSearchIdx.pm
+++ b/lib/PublicInbox/CodeSearchIdx.pm
@@ -770,28 +770,23 @@ sub init_tmp_git_dir ($) {
sub prep_umask ($) {
my ($self) = @_;
- my $um;
- my $cur = umask;
if ($self->{-cidx_internal}) { # respect core.sharedRepository
@{$self->{git_dirs}} == 1 or die 'BUG: only for GIT_DIR';
- # yuck, FIXME move umask handling out of inbox-specific stuff
- require PublicInbox::InboxWritable;
- my $git = PublicInbox::Git->new($self->{git_dirs}->[0]);
- chomp($um = $git->qx('config', 'core.sharedRepository') // '');
- $um = PublicInbox::InboxWritable::_git_config_perm(undef, $um);
- $um = PublicInbox::InboxWritable::_umask_for($um);
- umask == $um or progress($self, 'umask from git: ',
- sprintf('0%03o', $um));
+ local $self->{git} =
+ PublicInbox::Git->new($self->{git_dirs}->[0]);
+ $self->with_umask;
} elsif (-d $self->{cidx_dir}) { # respect existing perms
my @st = stat(_);
- $um = (~$st[2] & 0777);
+ my $um = (~$st[2] & 0777);
+ $self->{umask} = $um; # for SearchIdx->with_umask
umask == $um or progress($self, 'using umask from ',
$self->{cidx_dir}, ': ',
sprintf('0%03o', $um));
- }
- defined($um) ?
- PublicInbox::OnDestroy->new(\&CORE::umask, umask($um)) :
+ PublicInbox::OnDestroy->new($$, \&CORE::umask, umask($um));
+ } else {
+ $self->{umask} = umask; # for SearchIdx->with_umask
undef;
+ }
}
sub start_prune ($) {
@@ -896,9 +891,4 @@ sub shard_done_wait { # awaitpid cb via ipc_worker_reap
PublicInbox::DS::enqueue_reap() if !shards_active(); # once more for PLC
}
-sub with_umask { # TODO get rid of this treewide and rely on OnDestroy
- my ($self, $cb, @arg) = @_;
- $cb->(@arg);
-}
-
1;
diff --git a/lib/PublicInbox/ExtSearchIdx.pm b/lib/PublicInbox/ExtSearchIdx.pm
index 5445b156..6f3711ba 100644
--- a/lib/PublicInbox/ExtSearchIdx.pm
+++ b/lib/PublicInbox/ExtSearchIdx.pm
@@ -16,7 +16,7 @@
package PublicInbox::ExtSearchIdx;
use strict;
use v5.10.1;
-use parent qw(PublicInbox::ExtSearch PublicInbox::Lock);
+use parent qw(PublicInbox::ExtSearch PublicInbox::Umask PublicInbox::Lock);
use Carp qw(croak carp);
use Scalar::Util qw(blessed);
use Sys::Hostname qw(hostname);
@@ -1397,7 +1397,6 @@ sub eidx_watch { # public-inbox-extindex --watch main loop
no warnings 'once';
*done = \&PublicInbox::V2Writable::done;
-*with_umask = \&PublicInbox::InboxWritable::with_umask;
*parallel_init = \&PublicInbox::V2Writable::parallel_init;
*nproc_shards = \&PublicInbox::V2Writable::nproc_shards;
*sync_prepare = \&PublicInbox::V2Writable::sync_prepare;
diff --git a/lib/PublicInbox/InboxWritable.pm b/lib/PublicInbox/InboxWritable.pm
index 022e2a69..65952aa2 100644
--- a/lib/PublicInbox/InboxWritable.pm
+++ b/lib/PublicInbox/InboxWritable.pm
@@ -1,25 +1,17 @@
-# Copyright (C) 2018-2021 all contributors <meta@public-inbox.org>
+# Copyright (C) all contributors <meta@public-inbox.org>
# License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
# Extends read-only Inbox for writing
package PublicInbox::InboxWritable;
use strict;
use v5.10.1;
-use parent qw(PublicInbox::Inbox Exporter);
+use parent qw(PublicInbox::Inbox PublicInbox::Umask Exporter);
use PublicInbox::Import;
use PublicInbox::Filter::Base qw(REJECT);
use Errno qw(ENOENT);
our @EXPORT_OK = qw(eml_from_path);
use Fcntl qw(O_RDONLY O_NONBLOCK);
-use constant {
- PERM_UMASK => 0,
- OLD_PERM_GROUP => 1,
- OLD_PERM_EVERYBODY => 2,
- PERM_GROUP => 0660,
- PERM_EVERYBODY => 0664,
-};
-
sub new {
my ($class, $ibx, $creat_opt) = @_;
return $ibx if ref($ibx) eq $class;
@@ -176,64 +168,6 @@ sub import_mbox {
$im->done;
}
-sub _read_git_config_perm {
- my ($self) = @_;
- chomp(my $perm = $self->git->qx('config', 'core.sharedRepository'));
- $perm;
-}
-
-sub _git_config_perm {
- my $self = shift;
- my $perm = scalar @_ ? $_[0] : _read_git_config_perm($self);
- return PERM_UMASK if (!defined($perm) || $perm eq '');
- return PERM_UMASK if ($perm eq 'umask');
- return PERM_GROUP if ($perm eq 'group');
- if ($perm =~ /\A(?:all|world|everybody)\z/) {
- return PERM_EVERYBODY;
- }
- return PERM_GROUP if ($perm =~ /\A(?:true|yes|on|1)\z/);
- return PERM_UMASK if ($perm =~ /\A(?:false|no|off|0)\z/);
-
- my $i = oct($perm);
- return PERM_UMASK if ($i == PERM_UMASK);
- return PERM_GROUP if ($i == OLD_PERM_GROUP);
- return PERM_EVERYBODY if ($i == OLD_PERM_EVERYBODY);
-
- if (($i & 0600) != 0600) {
- die "core.sharedRepository mode invalid: ".
- sprintf('%.3o', $i) . "\nOwner must have permissions\n";
- }
- ($i & 0666);
-}
-
-sub _umask_for {
- my ($perm) = @_; # _git_config_perm return value
- my $rv = $perm;
- return umask if $rv == 0;
-
- # set +x bit if +r or +w were set
- $rv |= 0100 if ($rv & 0600);
- $rv |= 0010 if ($rv & 0060);
- $rv |= 0001 if ($rv & 0006);
- (~$rv & 0777);
-}
-
-sub with_umask {
- my ($self, $cb, @arg) = @_;
- my $old = umask($self->{umask} //= umask_prepare($self));
- my $rv = eval { $cb->(@arg) };
- my $err = $@;
- umask $old;
- die $err if $err;
- $rv;
-}
-
-sub umask_prepare {
- my ($self) = @_;
- my $perm = _git_config_perm($self);
- _umask_for($perm);
-}
-
sub cleanup ($) {
delete @{$_[0]}{qw(over mm git search)};
}
diff --git a/lib/PublicInbox/SearchIdx.pm b/lib/PublicInbox/SearchIdx.pm
index 699af432..69ab30e6 100644
--- a/lib/PublicInbox/SearchIdx.pm
+++ b/lib/PublicInbox/SearchIdx.pm
@@ -9,7 +9,8 @@
package PublicInbox::SearchIdx;
use strict;
use v5.10.1;
-use parent qw(PublicInbox::Search PublicInbox::Lock Exporter);
+use parent qw(PublicInbox::Search PublicInbox::Lock PublicInbox::Umask
+ Exporter);
use PublicInbox::Eml;
use PublicInbox::Search qw(xap_terms);
use PublicInbox::InboxWritable;
@@ -821,7 +822,8 @@ sub unindex_both { # git->cat_async callback
sub with_umask {
my $self = shift;
- ($self->{ibx} // $self->{eidx})->with_umask(@_);
+ my $owner = $self->{ibx} // $self->{eidx};
+ $owner ? $owner->with_umask(@_) : $self->SUPER::with_umask(@_)
}
# called by public-inbox-index
diff --git a/lib/PublicInbox/Umask.pm b/lib/PublicInbox/Umask.pm
new file mode 100644
index 00000000..00772ce5
--- /dev/null
+++ b/lib/PublicInbox/Umask.pm
@@ -0,0 +1,70 @@
+# Copyright (C) all contributors <meta@public-inbox.org>
+# License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
+
+# base class to ensures Xapian||SQLite files respect core.sharedRepository
+# of git repos
+package PublicInbox::Umask;
+use v5.12;
+use PublicInbox::OnDestroy;
+
+use constant {
+ PERM_UMASK => 0,
+ OLD_PERM_GROUP => 1,
+ OLD_PERM_EVERYBODY => 2,
+ PERM_GROUP => 0660,
+ PERM_EVERYBODY => 0664,
+};
+
+sub _read_git_config_perm {
+ my ($self) = @_;
+ chomp(my $perm = $self->git->qx('config', 'core.sharedRepository'));
+ $perm;
+}
+
+sub _git_config_perm {
+ my $self = shift;
+ my $perm = scalar @_ ? $_[0] : _read_git_config_perm($self);
+ $perm //= '';
+ return PERM_UMASK if $perm eq '' || $perm eq 'umask';
+ return PERM_GROUP if $perm eq 'group';
+ return PERM_EVERYBODY if $perm =~ /\A(?:all|world|everybody)\z/;
+ return PERM_GROUP if ($perm =~ /\A(?:true|yes|on|1)\z/);
+ return PERM_UMASK if ($perm =~ /\A(?:false|no|off|0)\z/);
+
+ my $i = oct($perm);
+ return PERM_UMASK if $i == PERM_UMASK;
+ return PERM_GROUP if $i == OLD_PERM_GROUP;
+ return PERM_EVERYBODY if $i == OLD_PERM_EVERYBODY;
+
+ if (($i & 0600) != 0600) {
+ die "core.sharedRepository mode invalid: ".
+ sprintf('%.3o', $i) . "\nOwner must have permissions\n";
+ }
+ ($i & 0666);
+}
+
+sub _umask_for {
+ my ($perm) = @_; # _git_config_perm return value
+ my $rv = $perm;
+ return umask if $rv == 0;
+
+ # set +x bit if +r or +w were set
+ $rv |= 0100 if ($rv & 0600);
+ $rv |= 0010 if ($rv & 0060);
+ $rv |= 0001 if ($rv & 0006);
+ (~$rv & 0777);
+}
+
+sub with_umask {
+ my ($self, $cb, @arg) = @_;
+ my $old = umask($self->{umask} //= umask_prepare($self));
+ my $restore = PublicInbox::OnDestroy->new($$, \&CORE::umask, $old);
+ $cb ? $cb->(@arg) : $restore;
+}
+
+sub umask_prepare {
+ my ($self) = @_;
+ _umask_for(_git_config_perm($self));
+}
+
+1;
diff --git a/t/search.t b/t/search.t
index cf639a6d..ea5a2a5a 100644
--- a/t/search.t
+++ b/t/search.t
@@ -41,8 +41,9 @@ sub oct_is ($$$) {
{
# git repository perms
+ use_ok 'PublicInbox::Umask';
oct_is($ibx->_git_config_perm(),
- &PublicInbox::InboxWritable::PERM_GROUP,
+ PublicInbox::Umask::PERM_GROUP(),
'undefined permission is group');
my @t = (
[ '0644', 0022, '644 => umask(0022)' ],
@@ -54,8 +55,8 @@ sub oct_is ($$$) {
);
for (@t) {
my ($perm, $exp, $msg) = @$_;
- my $got = PublicInbox::InboxWritable::_umask_for(
- PublicInbox::InboxWritable->_git_config_perm($perm));
+ my $got = PublicInbox::Umask::_umask_for(
+ PublicInbox::Umask->_git_config_perm($perm));
oct_is($got, $exp, $msg);
}
}
