* [PATCH 1/5] lei: style fix for $oldset declaration
2021-02-26 9:41 [PATCH 0/5] lei mbox locking Eric Wong
@ 2021-02-26 9:41 ` Eric Wong
2021-02-26 9:41 ` [PATCH 2/5] lei q: support mbox locking by default Eric Wong
` (3 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Eric Wong @ 2021-02-26 9:41 UTC (permalink / raw)
To: meta
We want /^sub oldset/ to match to keep editors and
things like ctags happy.
---
lib/PublicInbox/LEI.pm | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lib/PublicInbox/LEI.pm b/lib/PublicInbox/LEI.pm
index 8825fa43..5cdaabc6 100644
--- a/lib/PublicInbox/LEI.pm
+++ b/lib/PublicInbox/LEI.pm
@@ -27,7 +27,7 @@ use Time::HiRes qw(stat); # ctime comparisons for config cache
use File::Path qw(mkpath);
use File::Spec;
our $quit = \&CORE::exit;
-our ($current_lei, $errors_log, $listener);
+our ($current_lei, $errors_log, $listener, $oldset);
my ($recv_cmd, $send_cmd);
my $GLP = Getopt::Long::Parser->new;
$GLP->configure(qw(gnu_getopt no_ignore_case auto_abbrev));
@@ -976,7 +976,7 @@ sub event_step_init {
sub noop {}
-our $oldset; sub oldset { $oldset }
+sub oldset { $oldset }
sub dump_and_clear_log {
if (defined($errors_log) && -s STDIN && seek(STDIN, 0, SEEK_SET)) {
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 2/5] lei q: support mbox locking by default
2021-02-26 9:41 [PATCH 0/5] lei mbox locking Eric Wong
2021-02-26 9:41 ` [PATCH 1/5] lei: style fix for $oldset declaration Eric Wong
@ 2021-02-26 9:41 ` Eric Wong
2021-02-26 9:41 ` [PATCH 3/5] lei import|convert: support mbox locking on reads Eric Wong
` (2 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Eric Wong @ 2021-02-26 9:41 UTC (permalink / raw)
To: meta
While this diverges from from mairix(1) behavior, it's the safer
option. We'll follow Debian policy by supporting fcntl and
dotlocks by default (in that order). Users who do not want
locking can use "--lock=none"
This will be used in a read-only capacity for watching
mailboxes for keyword updates via inotify or EVFILT_VNODE.
---
MANIFEST | 2 +
lib/PublicInbox/LEI.pm | 2 +-
lib/PublicInbox/LeiToMail.pm | 16 +++--
lib/PublicInbox/LeiXSearch.pm | 1 +
lib/PublicInbox/MboxLock.pm | 121 ++++++++++++++++++++++++++++++++++
t/lei-q-remote-import.t | 12 ++++
t/mbox_lock.t | 90 +++++++++++++++++++++++++
7 files changed, 239 insertions(+), 5 deletions(-)
create mode 100644 lib/PublicInbox/MboxLock.pm
create mode 100644 t/mbox_lock.t
diff --git a/MANIFEST b/MANIFEST
index 9cf33d48..11ec5c01 100644
--- a/MANIFEST
+++ b/MANIFEST
@@ -201,6 +201,7 @@ lib/PublicInbox/MIME.pm
lib/PublicInbox/ManifestJsGz.pm
lib/PublicInbox/Mbox.pm
lib/PublicInbox/MboxGz.pm
+lib/PublicInbox/MboxLock.pm
lib/PublicInbox/MboxReader.pm
lib/PublicInbox/MdirReader.pm
lib/PublicInbox/MiscIdx.pm
@@ -383,6 +384,7 @@ t/lei_to_mail.t
t/lei_xsearch.t
t/linkify.t
t/main-bin/spamc
+t/mbox_lock.t
t/mbox_reader.t
t/mda-mime.eml
t/mda.t
diff --git a/lib/PublicInbox/LEI.pm b/lib/PublicInbox/LEI.pm
index 5cdaabc6..b5bdda21 100644
--- a/lib/PublicInbox/LEI.pm
+++ b/lib/PublicInbox/LEI.pm
@@ -112,7 +112,7 @@ our %CMD = ( # sorted in order of importance/use:
save-as=s output|mfolder|o=s format|f=s dedupe|d=s threads|t+ augment|a
sort|s=s reverse|r offset=i remote! local! external! pretty
include|I=s@ exclude=s@ only=s@ jobs|j=s globoff|g stdin|
- import-remote!
+ import-remote! lock=s@
alert=s@ mua=s no-torsocks torsocks=s verbose|v+ quiet|q C=s@),
PublicInbox::LeiQuery::curl_opt(), opt_dash('limit|n=i', '[0-9]+') ],
diff --git a/lib/PublicInbox/LeiToMail.pm b/lib/PublicInbox/LeiToMail.pm
index 630da67c..de640657 100644
--- a/lib/PublicInbox/LeiToMail.pm
+++ b/lib/PublicInbox/LeiToMail.pm
@@ -463,11 +463,19 @@ sub _pre_augment_mbox {
my ($self, $lei) = @_;
my $dst = $lei->{ovv}->{dst};
if ($dst ne '/dev/stdout') {
- my $mode = -p $dst ? '>' : '+>>';
- if (-f _ && !$lei->{opt}->{augment} and !unlink($dst)) {
- $! == ENOENT or die "unlink($dst): $!";
+ my $out;
+ if (-p $dst) {
+ open $out, '>', $dst or die "open($dst): $!";
+ } elsif (-f _ || !-e _) {
+ require PublicInbox::MboxLock;
+ my $m = $lei->{opt}->{'lock'} //
+ PublicInbox::MboxLock->defaults;
+ $self->{mbl} = PublicInbox::MboxLock->acq($dst, 1, $m);
+ $out = $self->{mbl}->{fh};
+ if (!$lei->{opt}->{augment} and !truncate($out, 0)) {
+ die "truncate($dst): $!";
+ }
}
- open my $out, $mode, $dst or die "open($dst): $!";
$lei->{old_1} = $lei->{1}; # keep for spawning MUA
$lei->{1} = $out;
}
diff --git a/lib/PublicInbox/LeiXSearch.pm b/lib/PublicInbox/LeiXSearch.pm
index eb015978..7ec696f4 100644
--- a/lib/PublicInbox/LeiXSearch.pm
+++ b/lib/PublicInbox/LeiXSearch.pm
@@ -338,6 +338,7 @@ Error closing $lei->{ovv}->{dst}: $!
$l2m->poke_dst;
$lei->poke_mua;
} else { # mbox users
+ delete $l2m->{mbl}; # drop dotlock
$lei->start_mua;
}
}
diff --git a/lib/PublicInbox/MboxLock.pm b/lib/PublicInbox/MboxLock.pm
new file mode 100644
index 00000000..4e2a2d9a
--- /dev/null
+++ b/lib/PublicInbox/MboxLock.pm
@@ -0,0 +1,121 @@
+# Copyright (C) 2021 all contributors <meta@public-inbox.org>
+# License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
+
+# Various mbox locking methods
+package PublicInbox::MboxLock;
+use strict;
+use v5.10.1;
+use PublicInbox::OnDestroy;
+use Fcntl qw(:flock F_SETLK F_SETLKW F_RDLCK F_WRLCK
+ O_CREAT O_EXCL O_WRONLY SEEK_SET);
+use Carp qw(croak);
+use PublicInbox::DS qw(now); # ugh...
+
+our $TMPL = do {
+ if ($^O eq 'linux') { \'s @32' }
+ elsif ($^O =~ /bsd/) { \'@20 s @256' } # n.b. @32 may be enough...
+ else { eval { require File::FcntlLock; 1 } }
+};
+
+# This order matches Debian policy on Linux systems.
+# See policy/ch-customized-programs.rst in
+# https://salsa.debian.org/dbnpolicy/policy.git
+sub defaults { [ qw(fcntl dotlock) ] }
+
+sub acq_fcntl {
+ my ($self) = @_;
+ my $op = $self->{nb} ? F_SETLK : F_SETLKW;
+ my $t = $self->{rw} ? F_WRLCK : F_RDLCK;
+ my $end = now + $self->{timeout};
+ $TMPL or die <<EOF;
+"struct flock" layout not available on $^O, install File::FcntlLock?
+EOF
+ do {
+ if (ref $TMPL) {
+ return if fcntl($self->{fh}, $op, pack($$TMPL, $t));
+ } else {
+ my $fl = File::FcntlLock->new;
+ $fl->l_type($t);
+ $fl->l_whence(SEEK_SET);
+ $fl->l_start(0);
+ $fl->l_len(0);
+ return if $fl->lock($self->{fh}, $op);
+ }
+ select(undef, undef, undef, $self->{delay});
+ } while (now < $end);
+ croak "fcntl lock $self->{f}: $!";
+}
+
+sub acq_dotlock {
+ my ($self) = @_;
+ my $dot_lock = "$self->{f}.lock";
+ my ($pfx, $base) = ($self->{f} =~ m!(\A.*?/)([^/]+)\z!);
+ $pfx //= '';
+ my $pid = $$;
+ my $end = now + $self->{timeout};
+ do {
+ my $tmp = "$pfx.$base-".sprintf('%x,%x,%x',
+ rand(0xffffffff), $pid, time);
+ if (sysopen(my $fh, $tmp, O_CREAT|O_EXCL|O_WRONLY)) {
+ if (link($tmp, $dot_lock)) {
+ unlink($tmp) or die "unlink($tmp): $!";
+ $self->{".lock$pid"} = $dot_lock;
+ return;
+ }
+ unlink($tmp) or die "unlink($tmp): $!";
+ select(undef, undef, undef, $self->{delay});
+ } else {
+ croak "open $tmp (for $dot_lock): $!" if !$!{EXIST};
+ }
+ } while (now < $end);
+ croak "dotlock $dot_lock";
+}
+
+sub acq_flock {
+ my ($self) = @_;
+ my $op = $self->{rw} ? LOCK_EX : LOCK_SH;
+ $op |= LOCK_NB if $self->{nb};
+ my $end = now + $self->{timeout};
+ do {
+ return if flock($self->{fh}, $op);
+ select(undef, undef, undef, $self->{delay});
+ } while (now < $end);
+ croak "flock $self->{f}: $!";
+}
+
+sub acq {
+ my ($cls, $f, $rw, $methods) = @_;
+ my $fh;
+ unless (open $fh, $rw ? '+>>' : '<', $f) {
+ croak "open($f): $!" if $rw || !$!{ENOENT};
+ }
+ my $self = bless { f => $f, fh => $fh, rw => $rw }, $cls;
+ my $m = "@$methods";
+ if ($m ne 'none') {
+ my @m = map {
+ if (/\A(timeout|delay)=([0-9\.]+)s?\z/) {
+ $self->{$1} = $2 + 0;
+ ();
+ } else {
+ $cls->can("acq_$_") // $_
+ }
+ } split(/[, ]/, $m);
+ my @bad = grep { !ref } @m;
+ croak "Unsupported lock methods: @bad\n" if @bad;
+ croak "No lock methods supplied with $m\n" if !@m;
+ $self->{nb} = $#m || defined($self->{timeout});
+ $self->{delay} //= 0.1;
+ $self->{timeout} //= 5;
+ $_->($self) for @m;
+ }
+ $self;
+}
+
+sub DESTROY {
+ my ($self) = @_;
+ if (my $f = $self->{".lock$$"}) {
+ unlink($f) or die "unlink($f): $! (lock stolen?)";
+ }
+}
+
+1;
diff --git a/t/lei-q-remote-import.t b/t/lei-q-remote-import.t
index f73524cf..4088b6ad 100644
--- a/t/lei-q-remote-import.t
+++ b/t/lei-q-remote-import.t
@@ -46,5 +46,17 @@ test_lei({ tmpdir => $tmpdir }, sub {
unlink $o or BAIL_OUT $!;
lei_ok(@cmd);
ok(-f $o && !-s _, '--no-import-remote did not memoize');
+
+ open my $fh, '>', "$o.lock";
+ $cmd[-1] = 'm:qp@example.com';
+ unlink $o or BAIL_OUT $!;
+ lei_ok(@cmd, '--lock=none');
+ ok(-f $o && -s _, '--lock=none respected');
+ unlink $o or BAIL_OUT $!;
+ ok(!lei(@cmd, '--lock=dotlock,timeout=0.000001'), 'dotlock fails');
+ ok(-f $o && !-s _, 'nothing output on lock failure');
+ unlink "$o.lock" or BAIL_OUT $!;
+ lei_ok(@cmd, '--lock=dotlock,timeout=0.000001',
+ \'succeeds after lock removal');
});
done_testing;
diff --git a/t/mbox_lock.t b/t/mbox_lock.t
new file mode 100644
index 00000000..3dc3b449
--- /dev/null
+++ b/t/mbox_lock.t
@@ -0,0 +1,90 @@
+#!perl -w
+# Copyright (C) 2021 all contributors <meta@public-inbox.org>
+# License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
+use strict; use v5.10.1; use PublicInbox::TestCommon;
+use POSIX qw(_exit);
+use PublicInbox::DS qw(now);
+use Errno qw(EAGAIN);
+use_ok 'PublicInbox::MboxLock';
+my ($tmpdir, $for_destroy) = tmpdir();
+my $f = "$tmpdir/f";
+my $mbl = PublicInbox::MboxLock->acq($f, 1, ['dotlock']);
+ok(-f "$f.lock", 'dotlock created');
+undef $mbl;
+ok(!-f "$f.lock", 'dotlock gone');
+$mbl = PublicInbox::MboxLock->acq($f, 1, ['none']);
+ok(!-f "$f.lock", 'no dotlock with none');
+undef $mbl;
+
+eval {
+ PublicInbox::MboxLock->acq($f, 1, ['bogus']);
+ fail "should not succeed with `bogus'";
+};
+ok($@, "fails on `bogus' lock method");
+eval {
+ PublicInbox::MboxLock->acq($f, 1, ['timeout=1']);
+ fail "should not succeed with only timeout";
+};
+ok($@, "fails with only `timeout=' and no lock method");
+
+my $defaults = PublicInbox::MboxLock->defaults;
+is(ref($defaults), 'ARRAY', 'default lock methods');
+my $test_rw_lock = sub {
+ my ($func) = @_;
+ my $m = ["$func,timeout=0.000001"];
+ for my $i (1..2) {
+ pipe(my ($r, $w)) or BAIL_OUT "pipe: $!";
+ my $t0 = now;
+ my $pid = fork // BAIL_OUT "fork $!";
+ if ($pid == 0) {
+ eval { PublicInbox::MboxLock->acq($f, 1, $m) };
+ my $err = $@;
+ syswrite $w, "E: $err";
+ _exit($err ? 0 : 1);
+ }
+ undef $w;
+ waitpid($pid, 0);
+ is($?, 0, "$func r/w lock behaved as expected #$i");
+ my $d = now - $t0;
+ ok($d < 1, "$func r/w timeout #$i") or diag "elapsed=$d";
+ my $err = do { local $/; <$r> };
+ $! = EAGAIN;
+ my $msg = "$!";
+ like($err, qr/\Q$msg\E/, "got EAGAIN in child #$i");
+ }
+};
+
+my $test_ro_lock = sub {
+ my ($func) = @_;
+ for my $i (1..2) {
+ my $t0 = now;
+ my $pid = fork // BAIL_OUT "fork $!";
+ if ($pid == 0) {
+ eval { PublicInbox::MboxLock->acq($f, 0, [ $func ]) };
+ _exit($@ ? 1 : 0);
+ }
+ waitpid($pid, 0);
+ is($?, 0, "$func ro lock behaved as expected #$i");
+ my $d = now - $t0;
+ ok($d < 1, "$func timeout respected #$i") or diag "elapsed=$d";
+ }
+};
+
+SKIP: {
+ grep(/fcntl/, @$defaults) or skip 'File::FcntlLock not available', 1;
+ my $top = PublicInbox::MboxLock->acq($f, 1, $defaults);
+ ok($top, 'fcntl lock acquired');
+ $test_rw_lock->('fcntl');
+ undef $top;
+ $top = PublicInbox::MboxLock->acq($f, 0, $defaults);
+ ok($top, 'fcntl read lock acquired');
+ $test_ro_lock->('fcntl');
+}
+$mbl = PublicInbox::MboxLock->acq($f, 1, ['flock']);
+ok($mbl, 'flock acquired');
+$test_rw_lock->('flock');
+undef $mbl;
+$mbl = PublicInbox::MboxLock->acq($f, 0, ['flock']);
+$test_ro_lock->('flock');
+
+done_testing;
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 3/5] lei import|convert: support mbox locking on reads
2021-02-26 9:41 [PATCH 0/5] lei mbox locking Eric Wong
2021-02-26 9:41 ` [PATCH 1/5] lei: style fix for $oldset declaration Eric Wong
2021-02-26 9:41 ` [PATCH 2/5] lei q: support mbox locking by default Eric Wong
@ 2021-02-26 9:41 ` Eric Wong
2021-02-26 21:03 ` [SQUASH 6/5] require MboxLock even for .eml files Eric Wong
2021-02-26 9:41 ` [PATCH 4/5] t/lei_store: rename $lst to $sto Eric Wong
2021-02-26 9:41 ` [PATCH 5/5] lei_xsearch: more detail about ->xdb call chain Eric Wong
4 siblings, 1 reply; 7+ messages in thread
From: Eric Wong @ 2021-02-26 9:41 UTC (permalink / raw)
To: meta
In case somebody is writing non-atomically, ensure we
take read locks when opening mbox files for reading.
---
lib/PublicInbox/LEI.pm | 13 +++++++++----
lib/PublicInbox/LeiConvert.pm | 9 ++++++---
lib/PublicInbox/LeiImport.pm | 13 +++++++------
3 files changed, 22 insertions(+), 13 deletions(-)
diff --git a/lib/PublicInbox/LEI.pm b/lib/PublicInbox/LEI.pm
index b5bdda21..e133b357 100644
--- a/lib/PublicInbox/LEI.pm
+++ b/lib/PublicInbox/LEI.pm
@@ -172,12 +172,12 @@ our %CMD = ( # sorted in order of importance/use:
'import' => [ 'LOCATION...|--stdin',
'one-time import/update from URL or filesystem',
qw(stdin| offset=i recursive|r exclude=s include|I=s
- in-format|F=s kw|keywords|flags! C=s@),
+ lock=s@ in-format|F=s kw|keywords|flags! C=s@),
],
'convert' => [ 'LOCATION...|--stdin',
'one-time conversion from URL or filesystem to another format',
qw(stdin| in-format|F=s out-format|f=s output|mfolder|o=s quiet|q
- kw|keywords|flags! C=s@),
+ lock=s@ kw|keywords|flags! C=s@),
],
'config' => [ '[...]', sub {
'git-config(1) wrapper for '._config_path($_[0]);
@@ -218,6 +218,9 @@ my %OPTDESC = (
'help|h' => 'show this built-in help',
'C=s@' => [ 'DIR', 'chdir to specify to directory' ],
'quiet|q' => 'be quiet',
+'lock=s@' => [ 'METHOD|dotlock|fcntl|flock|none',
+ 'mbox(5) locking method(s) to use (default: fcntl,dotlock)' ],
+
'globoff|g' => "do not match locations using '*?' wildcards ".
"and\xa0'[]'\x{a0}ranges",
'verbose|v+' => 'be more verbose',
@@ -410,8 +413,10 @@ sub check_input_format ($;$) {
return 1 if $fmt eq 'eml';
# XXX: should this handle {gz,bz2,xz}? that's currently in LeiToMail
require PublicInbox::MboxReader;
- PublicInbox::MboxReader->can($fmt) ||
- fail($self, "--$opt_key=$fmt unrecognized");
+ PublicInbox::MboxReader->can($fmt) or
+ return fail($self, "--$opt_key=$fmt unrecognized");
+ require PublicInbox::MboxLock if $files;
+ 1;
}
sub out ($;@) {
diff --git a/lib/PublicInbox/LeiConvert.pm b/lib/PublicInbox/LeiConvert.pm
index 45d42c9c..4c0bbd88 100644
--- a/lib/PublicInbox/LeiConvert.pm
+++ b/lib/PublicInbox/LeiConvert.pm
@@ -62,9 +62,11 @@ sub do_convert { # via wq_do
$ifmt = lc $1;
}
if (-f $input) {
- open my $fh, '<', $input or
- return $lei->fail("open $input: $!");
- convert_fh($self, $ifmt, $fh, $input);
+ my $m = $lei->{opt}->{'lock'} //
+ ($ifmt eq 'eml' ? ['none'] :
+ PublicInbox::MboxLock->defaults);
+ my $mbl = PublicInbox::MboxLock->acq($input, 0, $m);
+ convert_fh($self, $ifmt, $mbl->{fh}, $input);
} elsif (-d _) {
PublicInbox::MdirReader::maildir_each_eml($input,
\&mdir_cb, $self);
@@ -109,6 +111,7 @@ sub call { # the main "lei convert" method
}
if (-f $input_path) {
+ require PublicInbox::MboxLock;
require PublicInbox::MboxReader;
PublicInbox::MboxReader->can($ifmt) or return
$lei->fail("$ifmt not supported");
diff --git a/lib/PublicInbox/LeiImport.pm b/lib/PublicInbox/LeiImport.pm
index 7f247b64..c2c98030 100644
--- a/lib/PublicInbox/LeiImport.pm
+++ b/lib/PublicInbox/LeiImport.pm
@@ -80,10 +80,11 @@ sub call { # the main "lei import" method
my $ifmt = lc $1;
if (($fmt // $ifmt) ne $ifmt) {
return $lei->fail(<<"");
---format=$fmt and `$ifmt:' conflict
+--in-format=$fmt and `$ifmt:' conflict
}
if (-f $input_path) {
+ require PublicInbox::MboxLock;
require PublicInbox::MboxReader;
PublicInbox::MboxReader->can($ifmt) or return
$lei->fail("$ifmt not supported");
@@ -142,7 +143,7 @@ error reading $input: $!
$cb->(undef, $fh, \&_import_eml, $lei->{sto}, $set_kw);
}
};
- $lei->child_error(1 << 8, "<stdin>: $@") if $@;
+ $lei->child_error(1 << 8, "$input: $@") if $@;
}
sub _import_maildir { # maildir_each_file cb
@@ -171,10 +172,10 @@ sub import_path_url {
$ifmt = lc $1;
}
if (-f $input) {
- open my $fh, '<', $input or return $lei->child_error(1 << 8, <<"");
-unable to open $input: $!
-
- _import_fh($lei, $fh, $input, $ifmt);
+ my $m = $lei->{opt}->{'lock'} // ($ifmt eq 'eml' ? ['none'] :
+ PublicInbox::MboxLock->defaults);
+ my $mbl = PublicInbox::MboxLock->acq($input, 0, $m);
+ _import_fh($lei, $mbl->{fh}, $input, $ifmt);
} elsif (-d _ && (-d "$input/cur" || -d "$input/new")) {
return $lei->fail(<<EOM) if $ifmt && $ifmt ne 'maildir';
$input appears to a be a maildir, not $ifmt
^ permalink raw reply related [flat|nested] 7+ messages in thread* [SQUASH 6/5] require MboxLock even for .eml files
2021-02-26 9:41 ` [PATCH 3/5] lei import|convert: support mbox locking on reads Eric Wong
@ 2021-02-26 21:03 ` Eric Wong
0 siblings, 0 replies; 7+ messages in thread
From: Eric Wong @ 2021-02-26 21:03 UTC (permalink / raw)
To: meta
We rely on MboxLock->acq to open .eml files, at the moment
(which may not be a great idea, but it's the most natural from
the current callers perspective).
diff --git a/lib/PublicInbox/LEI.pm b/lib/PublicInbox/LEI.pm
index e133b357..0da24499 100644
--- a/lib/PublicInbox/LEI.pm
+++ b/lib/PublicInbox/LEI.pm
@@ -410,12 +410,12 @@ sub check_input_format ($;$) {
my $err = $files ? "regular file(s):\n@$files" : '--stdin';
return fail($self, "--$opt_key unset for $err");
}
+ require PublicInbox::MboxLock if $files;
return 1 if $fmt eq 'eml';
# XXX: should this handle {gz,bz2,xz}? that's currently in LeiToMail
require PublicInbox::MboxReader;
PublicInbox::MboxReader->can($fmt) or
return fail($self, "--$opt_key=$fmt unrecognized");
- require PublicInbox::MboxLock if $files;
1;
}
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 4/5] t/lei_store: rename $lst to $sto
2021-02-26 9:41 [PATCH 0/5] lei mbox locking Eric Wong
` (2 preceding siblings ...)
2021-02-26 9:41 ` [PATCH 3/5] lei import|convert: support mbox locking on reads Eric Wong
@ 2021-02-26 9:41 ` Eric Wong
2021-02-26 9:41 ` [PATCH 5/5] lei_xsearch: more detail about ->xdb call chain Eric Wong
4 siblings, 0 replies; 7+ messages in thread
From: Eric Wong @ 2021-02-26 9:41 UTC (permalink / raw)
To: meta
`$sto' is prevalent throughout the rest of the lei code,
and `$lst' seems like an abbreviation for "list".
I don't like the noise from commits like this, but I hope the
long-term payoff being less confusing to new developers is worth
it...
---
t/lei_store.t | 102 +++++++++++++++++++++++++-------------------------
1 file changed, 51 insertions(+), 51 deletions(-)
diff --git a/t/lei_store.t b/t/lei_store.t
index c9360f8f..e93fe779 100644
--- a/t/lei_store.t
+++ b/t/lei_store.t
@@ -11,32 +11,32 @@ require_ok 'PublicInbox::LeiStore';
require_ok 'PublicInbox::ExtSearch';
my ($home, $for_destroy) = tmpdir();
my $opt = { 1 => \(my $out = ''), 2 => \(my $err = '') };
-my $store_dir = "$home/lst";
+my $store_dir = "$home/sto";
local $ENV{GIT_COMMITTER_EMAIL} = 'lei@example.com';
local $ENV{GIT_COMMITTER_NAME} = 'lei user';
-my $lst = PublicInbox::LeiStore->new($store_dir, { creat => 1 });
-ok($lst, '->new');
-my $smsg = $lst->add_eml(eml_load('t/data/0001.patch'));
+my $sto = PublicInbox::LeiStore->new($store_dir, { creat => 1 });
+ok($sto, '->new');
+my $smsg = $sto->add_eml(eml_load('t/data/0001.patch'));
like($smsg->{blob}, qr/\A[0-9a-f]+\z/, 'add returned OID');
my $eml = eml_load('t/data/0001.patch');
-is($lst->add_eml($eml), undef, 'idempotent');
-$lst->done;
-is_deeply([$lst->mbox_keywords($eml)], [], 'no keywords');
+is($sto->add_eml($eml), undef, 'idempotent');
+$sto->done;
+is_deeply([$sto->mbox_keywords($eml)], [], 'no keywords');
$eml->header_set('Status', 'RO');
-is_deeply([$lst->mbox_keywords($eml)], ['seen'], 'seen extracted');
+is_deeply([$sto->mbox_keywords($eml)], ['seen'], 'seen extracted');
$eml->header_set('X-Status', 'A');
-is_deeply([$lst->mbox_keywords($eml)], [qw(answered seen)],
+is_deeply([$sto->mbox_keywords($eml)], [qw(answered seen)],
'seen+answered extracted');
$eml->header_set($_) for qw(Status X-Status);
-is_deeply([$lst->maildir_keywords('/foo:2,')], [], 'Maildir no keywords');
-is_deeply([$lst->maildir_keywords('/foo:2,S')], ['seen'], 'Maildir seen');
-is_deeply([$lst->maildir_keywords('/foo:2,RS')], ['answered', 'seen'],
+is_deeply([$sto->maildir_keywords('/foo:2,')], [], 'Maildir no keywords');
+is_deeply([$sto->maildir_keywords('/foo:2,S')], ['seen'], 'Maildir seen');
+is_deeply([$sto->maildir_keywords('/foo:2,RS')], ['answered', 'seen'],
'Maildir answered + seen');
-is_deeply([$lst->maildir_keywords('/foo:2,RSZ')], ['answered', 'seen'],
+is_deeply([$sto->maildir_keywords('/foo:2,RSZ')], ['answered', 'seen'],
'Maildir answered + seen w/o Z');
{
- my $es = $lst->search;
+ my $es = $sto->search;
my $msgs = $es->over->query_xover(0, 1000);
is(scalar(@$msgs), 1, 'one message');
is($msgs->[0]->{blob}, $smsg->{blob}, 'blob matches');
@@ -49,82 +49,82 @@ is_deeply([$lst->maildir_keywords('/foo:2,RSZ')], ['answered', 'seen'],
}
for my $parallel (0, 1) {
- $lst->{priv_eidx}->{parallel} = $parallel;
- my $docids = $lst->set_eml_keywords($eml, qw(seen draft));
+ $sto->{priv_eidx}->{parallel} = $parallel;
+ my $docids = $sto->set_eml_keywords($eml, qw(seen draft));
is(scalar @$docids, 1, 'set keywords on one doc');
- $lst->done;
- my @kw = $lst->search->msg_keywords($docids->[0]);
+ $sto->done;
+ my @kw = $sto->search->msg_keywords($docids->[0]);
is_deeply(\@kw, [qw(draft seen)], 'kw matches');
- $docids = $lst->add_eml_keywords($eml, qw(seen draft));
- $lst->done;
+ $docids = $sto->add_eml_keywords($eml, qw(seen draft));
+ $sto->done;
is(scalar @$docids, 1, 'idempotently added keywords to doc');
- @kw = $lst->search->msg_keywords($docids->[0]);
+ @kw = $sto->search->msg_keywords($docids->[0]);
is_deeply(\@kw, [qw(draft seen)], 'kw matches after noop');
- $docids = $lst->remove_eml_keywords($eml, qw(seen draft));
+ $docids = $sto->remove_eml_keywords($eml, qw(seen draft));
is(scalar @$docids, 1, 'removed from one doc');
- $lst->done;
- @kw = $lst->search->msg_keywords($docids->[0]);
+ $sto->done;
+ @kw = $sto->search->msg_keywords($docids->[0]);
is_deeply(\@kw, [], 'kw matches after remove');
- $docids = $lst->remove_eml_keywords($eml, qw(answered));
+ $docids = $sto->remove_eml_keywords($eml, qw(answered));
is(scalar @$docids, 1, 'removed from one doc (idempotently)');
- $lst->done;
- @kw = $lst->search->msg_keywords($docids->[0]);
+ $sto->done;
+ @kw = $sto->search->msg_keywords($docids->[0]);
is_deeply(\@kw, [], 'kw matches after remove (idempotent)');
- $docids = $lst->add_eml_keywords($eml, qw(answered));
+ $docids = $sto->add_eml_keywords($eml, qw(answered));
is(scalar @$docids, 1, 'added to empty doc');
- $lst->done;
- @kw = $lst->search->msg_keywords($docids->[0]);
+ $sto->done;
+ @kw = $sto->search->msg_keywords($docids->[0]);
is_deeply(\@kw, ['answered'], 'kw matches after add');
- $docids = $lst->set_eml_keywords($eml);
+ $docids = $sto->set_eml_keywords($eml);
is(scalar @$docids, 1, 'set to clobber');
- $lst->done;
- @kw = $lst->search->msg_keywords($docids->[0]);
+ $sto->done;
+ @kw = $sto->search->msg_keywords($docids->[0]);
is_deeply(\@kw, [], 'set clobbers all');
my $set = eml_load('t/plack-qp.eml');
$set->header_set('Message-ID', "<set\@$parallel>");
- my $ret = $lst->set_eml($set, 'seen');
+ my $ret = $sto->set_eml($set, 'seen');
is(ref $ret, 'PublicInbox::Smsg', 'initial returns smsg');
- my $ids = $lst->set_eml($set, qw(seen));
+ my $ids = $sto->set_eml($set, qw(seen));
is_deeply($ids, [ $ret->{num} ], 'set_eml idempotent');
- $ids = $lst->set_eml($set, qw(seen answered));
+ $ids = $sto->set_eml($set, qw(seen answered));
is_deeply($ids, [ $ret->{num} ], 'set_eml to change kw');
- $lst->done;
- @kw = $lst->search->msg_keywords($ids->[0]);
+ $sto->done;
+ @kw = $sto->search->msg_keywords($ids->[0]);
is_deeply(\@kw, [qw(answered seen)], 'set changed kw');
}
SKIP: {
require_mods(qw(Storable), 1);
- ok($lst->can('ipc_do'), 'ipc_do works if we have Storable');
+ ok($sto->can('ipc_do'), 'ipc_do works if we have Storable');
$eml->header_set('Message-ID', '<ipc-test@example>');
- my $pid = $lst->ipc_worker_spawn('lei-store');
+ my $pid = $sto->ipc_worker_spawn('lei-store');
ok($pid > 0, 'got a worker');
- my $smsg = $lst->ipc_do('set_eml', $eml, qw(seen));
+ my $smsg = $sto->ipc_do('set_eml', $eml, qw(seen));
is(ref($smsg), 'PublicInbox::Smsg', 'set_eml works over ipc');
- my $ids = $lst->ipc_do('set_eml', $eml, qw(seen));
+ my $ids = $sto->ipc_do('set_eml', $eml, qw(seen));
is_deeply($ids, [ $smsg->{num} ], 'docid returned');
$eml->header_set('Message-ID');
- my $no_mid = $lst->ipc_do('set_eml', $eml, qw(seen));
- my $wait = $lst->ipc_do('done');
- my @kw = $lst->search->msg_keywords($no_mid->{num});
+ my $no_mid = $sto->ipc_do('set_eml', $eml, qw(seen));
+ my $wait = $sto->ipc_do('done');
+ my @kw = $sto->search->msg_keywords($no_mid->{num});
is_deeply(\@kw, [qw(seen)], 'ipc set changed kw');
is(ref($smsg), 'PublicInbox::Smsg', 'no mid works ipc');
- $ids = $lst->ipc_do('set_eml', $eml, qw(seen));
+ $ids = $sto->ipc_do('set_eml', $eml, qw(seen));
is_deeply($ids, [ $no_mid->{num} ], 'docid returned w/o mid w/ ipc');
- $lst->ipc_do('done');
- $lst->ipc_worker_stop;
- $ids = $lst->ipc_do('set_eml', $eml, qw(seen answered));
+ $sto->ipc_do('done');
+ $sto->ipc_worker_stop;
+ $ids = $sto->ipc_do('set_eml', $eml, qw(seen answered));
is_deeply($ids, [ $no_mid->{num} ], 'docid returned w/o mid w/o ipc');
- $wait = $lst->ipc_do('done');
- @kw = $lst->search->msg_keywords($no_mid->{num});
+ $wait = $sto->ipc_do('done');
+ @kw = $sto->search->msg_keywords($no_mid->{num});
is_deeply(\@kw, [qw(answered seen)], 'set changed kw w/o ipc');
}
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 5/5] lei_xsearch: more detail about ->xdb call chain
2021-02-26 9:41 [PATCH 0/5] lei mbox locking Eric Wong
` (3 preceding siblings ...)
2021-02-26 9:41 ` [PATCH 4/5] t/lei_store: rename $lst to $sto Eric Wong
@ 2021-02-26 9:41 ` Eric Wong
4 siblings, 0 replies; 7+ messages in thread
From: Eric Wong @ 2021-02-26 9:41 UTC (permalink / raw)
To: meta
I was just wondering this myself :x
---
lib/PublicInbox/LeiXSearch.pm | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/PublicInbox/LeiXSearch.pm b/lib/PublicInbox/LeiXSearch.pm
index 7ec696f4..9a6457d7 100644
--- a/lib/PublicInbox/LeiXSearch.pm
+++ b/lib/PublicInbox/LeiXSearch.pm
@@ -63,7 +63,7 @@ sub locals { @{$_[0]->{locals} // []} }
sub remotes { @{$_[0]->{remotes} // []} }
-# called by PublicInbox::Search::xdb
+# called by PublicInbox::Search::xdb (usually via ->mset)
sub xdb_shards_flat { @{$_[0]->{shards_flat} // []} }
sub mitem_kw ($$;$) {
^ permalink raw reply related [flat|nested] 7+ messages in thread