* [PATCH 0/6] cindex fixes, and some spring cleaning
@ 2023-04-07 12:40 7% Eric Wong
2023-04-07 12:40 4% ` [PATCH 3/6] umask: hoist out of InboxWritable Eric Wong
0 siblings, 1 reply; 2+ results
From: Eric Wong @ 2023-04-07 12:40 UTC (permalink / raw)
To: meta
cindex now preserves indexlevel like older mail-based indices
We'll reuse umask code across inboxes, extindex, and now cindex
for code. This also lets us simplify some of our old code by
eliminating some useless functions.
The vstring stuff should make the code more readable to new
contributors.
Eric Wong (6):
cindex: improve progress display
cindex: preserve indexlevel across invocations
umask: hoist out of InboxWritable
umask: rely on the OnDestroy-based call where applicable
searchidx: use vstring to improve readability
switch git version comparisons to vstrings, too
MANIFEST | 1 +
lib/PublicInbox/CodeSearchIdx.pm | 49 ++++++++++++----------
lib/PublicInbox/ExtSearchIdx.pm | 17 +++-----
lib/PublicInbox/Git.pm | 11 +++--
lib/PublicInbox/InboxWritable.pm | 70 +-------------------------------
lib/PublicInbox/LeiMirror.pm | 2 +-
lib/PublicInbox/SearchIdx.pm | 37 ++++++++---------
lib/PublicInbox/TestCommon.pm | 16 +++-----
lib/PublicInbox/Umask.pm | 70 ++++++++++++++++++++++++++++++++
lib/PublicInbox/V2Writable.pm | 23 ++++-------
lib/PublicInbox/Xapcmd.pm | 35 ++++++++--------
script/public-inbox-convert | 9 ++--
t/cindex.t | 45 ++++++++++++++++++++
t/search.t | 7 ++--
14 files changed, 212 insertions(+), 180 deletions(-)
create mode 100644 lib/PublicInbox/Umask.pm
^ permalink raw reply [relevance 7%]
* [PATCH 3/6] umask: hoist out of InboxWritable
2023-04-07 12:40 7% [PATCH 0/6] cindex fixes, and some spring cleaning Eric Wong
@ 2023-04-07 12:40 4% ` Eric Wong
0 siblings, 0 replies; 2+ results
From: Eric Wong @ 2023-04-07 12:40 UTC (permalink / raw)
To: meta
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);
}
}
^ permalink raw reply related [relevance 4%]
Results 1-2 of 2 | reverse | options above
-- pct% links below jump to the message on this page, permalinks otherwise --
2023-04-07 12:40 7% [PATCH 0/6] cindex fixes, and some spring cleaning Eric Wong
2023-04-07 12:40 4% ` [PATCH 3/6] umask: hoist out of InboxWritable Eric Wong
Code repositories for project(s) associated with this public inbox
https://80x24.org/public-inbox.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).