user/dev discussion of public-inbox itself
 help / color / mirror / code / Atom feed
* [PATCH 0/3] lei import fix, other fixes
@ 2021-03-21  9:50 Eric Wong
  2021-03-21  9:50 ` [PATCH 1/3] lei import: vivify external-only messages Eric Wong
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Eric Wong @ 2021-03-21  9:50 UTC (permalink / raw)
  To: meta

Things are finally coming together...

Eric Wong (3):
  lei import: vivify external-only messages
  lei q: fix warning on remote imports
  lei: fix some warnings in tests

 lib/PublicInbox/ContentHash.pm | 15 ++++++++---
 lib/PublicInbox/Import.pm      | 14 ++++++++++-
 lib/PublicInbox/LEI.pm         |  8 +++---
 lib/PublicInbox/LeiDedupe.pm   |  9 ++-----
 lib/PublicInbox/LeiExternal.pm |  2 +-
 lib/PublicInbox/LeiImport.pm   | 22 ++++++++++------
 lib/PublicInbox/LeiP2q.pm      |  2 +-
 lib/PublicInbox/LeiSearch.pm   |  5 +++-
 lib/PublicInbox/LeiStore.pm    | 46 +++++++++++++++++++++++++++++-----
 lib/PublicInbox/LeiXSearch.pm  |  6 ++++-
 lib/PublicInbox/MboxLock.pm    |  8 +++---
 lib/PublicInbox/Over.pm        |  2 +-
 lib/PublicInbox/SearchIdx.pm   | 12 +++++++--
 lib/PublicInbox/TestCommon.pm  |  9 +++++++
 t/lei-q-kw.t                   | 44 ++++++++++++++++++++++++++++++++
 t/lei-q-remote-import.t        |  3 ++-
 16 files changed, 167 insertions(+), 40 deletions(-)

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 1/3] lei import: vivify external-only messages
  2021-03-21  9:50 [PATCH 0/3] lei import fix, other fixes Eric Wong
@ 2021-03-21  9:50 ` Eric Wong
  2021-03-21  9:50 ` [PATCH 2/3] lei q: fix warning on remote imports Eric Wong
  2021-03-21  9:50 ` [PATCH 3/3] lei: fix some warnings in tests Eric Wong
  2 siblings, 0 replies; 4+ messages in thread
From: Eric Wong @ 2021-03-21  9:50 UTC (permalink / raw)
  To: meta

Keyword storage for external-only messages was preventing
messages from being explicitly imported.  Teach lei_store
to vivify keyword-only entries into fully-indexed messages
on import.
---
 lib/PublicInbox/Import.pm    | 14 ++++++++++-
 lib/PublicInbox/LeiImport.pm | 22 +++++++++++------
 lib/PublicInbox/LeiSearch.pm |  5 +++-
 lib/PublicInbox/LeiStore.pm  | 46 +++++++++++++++++++++++++++++++-----
 lib/PublicInbox/Over.pm      |  2 +-
 lib/PublicInbox/SearchIdx.pm | 12 ++++++++--
 t/lei-q-kw.t                 | 44 ++++++++++++++++++++++++++++++++++
 7 files changed, 127 insertions(+), 18 deletions(-)

diff --git a/lib/PublicInbox/Import.pm b/lib/PublicInbox/Import.pm
index b8fa5c21..34738279 100644
--- a/lib/PublicInbox/Import.pm
+++ b/lib/PublicInbox/Import.pm
@@ -413,7 +413,19 @@ sub add {
 		$smsg->{blob} = $self->get_mark(":$blob");
 		$smsg->set_bytes($raw_email, $n);
 		if (my $oidx = delete $smsg->{-oidx}) { # used by LeiStore
-			return if $oidx->blob_exists($smsg->{blob});
+			my @docids = $oidx->blob_exists($smsg->{blob});
+			my @vivify_xvmd;
+			for my $id (@docids) {
+				if (my $cur = $oidx->get_art($id)) {
+					# already imported if bytes > 0
+					return if $cur->{bytes} > 0;
+					push @vivify_xvmd, $id;
+				} else {
+					warn "W: $smsg->{blob} ",
+						"#$id gone (bug?)\n";
+				}
+			}
+			$smsg->{-vivify_xvmd} = \@vivify_xvmd;
 		}
 	}
 	my $ref = $self->{ref};
diff --git a/lib/PublicInbox/LeiImport.pm b/lib/PublicInbox/LeiImport.pm
index 137c22fc..ae24a1fa 100644
--- a/lib/PublicInbox/LeiImport.pm
+++ b/lib/PublicInbox/LeiImport.pm
@@ -10,9 +10,14 @@ use PublicInbox::Eml;
 use PublicInbox::PktOp qw(pkt_do);
 
 sub _import_eml { # MboxReader callback
-	my ($eml, $sto, $set_kw) = @_;
-	$sto->ipc_do('set_eml', $eml, $set_kw ?
-		{ kw => PublicInbox::MboxReader::mbox_keywords($eml) } : ());
+	my ($eml, $lei, $mbox_keywords) = @_;
+	my $vmd;
+	if ($mbox_keywords) {
+		my $kw = $mbox_keywords->($eml);
+		$vmd = { kw => $kw } if scalar(@$kw);
+	}
+	my $xoids = $lei->{ale}->xoids_for($eml);
+	$lei->{sto}->ipc_do('set_eml', $eml, $vmd, $xoids);
 }
 
 sub import_done_wait { # dwaitpid callback
@@ -41,6 +46,7 @@ sub net_merge_complete { # callback used by LeiAuth
 sub import_start {
 	my ($lei) = @_;
 	my $self = $lei->{imp};
+	$lei->ale;
 	my $j = $lei->{opt}->{jobs} // scalar(@{$self->{inputs}}) || 1;
 	if (my $net = $lei->{net}) {
 		# $j = $net->net_concurrency($j); TODO
@@ -130,7 +136,8 @@ sub ipc_atfork_child {
 
 sub _import_fh {
 	my ($lei, $fh, $input, $ifmt) = @_;
-	my $set_kw = $lei->{opt}->{kw};
+	my $kw = $lei->{opt}->{kw} ?
+		PublicInbox::MboxReader->can('mbox_keywords') : undef;
 	eval {
 		if ($ifmt eq 'eml') {
 			my $buf = do { local $/; <$fh> } //
@@ -138,11 +145,11 @@ sub _import_fh {
 error reading $input: $!
 
 			my $eml = PublicInbox::Eml->new(\$buf);
-			_import_eml($eml, $lei->{sto}, $set_kw);
+			_import_eml($eml, $lei, $kw);
 		} else { # some mbox (->can already checked in call);
 			my $cb = PublicInbox::MboxReader->can($ifmt) //
 				die "BUG: bad fmt=$ifmt";
-			$cb->(undef, $fh, \&_import_eml, $lei->{sto}, $set_kw);
+			$cb->(undef, $fh, \&_import_eml, $lei, $kw);
 		}
 	};
 	$lei->child_error(1 << 8, "$input: $@") if $@;
@@ -193,7 +200,8 @@ EOM
 sub import_stdin {
 	my ($self) = @_;
 	my $lei = $self->{lei};
-	_import_fh($lei, delete $self->{0}, '<stdin>', $lei->{opt}->{'in-format'});
+	my $in = delete $self->{0};
+	_import_fh($lei, $in, '<stdin>', $lei->{opt}->{'in-format'});
 }
 
 no warnings 'once'; # the following works even when LeiAuth is lazy-loaded
diff --git a/lib/PublicInbox/LeiSearch.pm b/lib/PublicInbox/LeiSearch.pm
index 360a37e5..bbb00661 100644
--- a/lib/PublicInbox/LeiSearch.pm
+++ b/lib/PublicInbox/LeiSearch.pm
@@ -63,7 +63,10 @@ sub _cmp_1st { # git->cat_async callback
 	}
 }
 
-sub xoids_for { # returns { OID => docid } mapping for $eml matches
+# returns { OID => num } mapping for $eml matches
+# The `num' hash value only makes sense from LeiSearch itself
+# and is nonsense from the PublicInbox::LeiALE subclass
+sub xoids_for {
 	my ($self, $eml, $min) = @_;
 	my ($chash, $mids) = content_key($eml);
 	my @overs = ($self->over // $self->overs_all);
diff --git a/lib/PublicInbox/LeiStore.pm b/lib/PublicInbox/LeiStore.pm
index c66d3dc2..b390b318 100644
--- a/lib/PublicInbox/LeiStore.pm
+++ b/lib/PublicInbox/LeiStore.pm
@@ -161,7 +161,7 @@ sub remove_eml_vmd {
 }
 
 sub add_eml {
-	my ($self, $eml, $vmd) = @_;
+	my ($self, $eml, $vmd, $xoids) = @_;
 	my $im = $self->importer; # may create new epoch
 	my $eidx = eidx_init($self); # writes ALL.git/objects/info/alternates
 	my $oidx = $eidx->{oidx}; # PublicInbox::Import::add checks this
@@ -169,7 +169,40 @@ sub add_eml {
 	$im->add($eml, undef, $smsg) or return; # duplicate returns undef
 
 	local $self->{current_info} = $smsg->{blob};
-	if (my @docids = _docids_for($self, $eml)) {
+	my $vivify_xvmd = delete($smsg->{-vivify_xvmd}) // []; # exact matches
+	if ($xoids) { # fuzzy matches from externals in ale->xoids_for
+		delete $xoids->{$smsg->{blob}}; # added later
+		if (scalar keys %$xoids) {
+			my %docids = map { $_ => 1 } @$vivify_xvmd;
+			for my $oid (keys %$xoids) {
+				my @id = $oidx->blob_exists($oid);
+				@docids{@id} = @id;
+			}
+			@$vivify_xvmd = sort { $a <=> $b } keys(%docids);
+		}
+	}
+	if (@$vivify_xvmd) {
+		$xoids //= {};
+		$xoids->{$smsg->{blob}} = 1;
+		for my $docid (@$vivify_xvmd) {
+			my $cur = $oidx->get_art($docid);
+			my $idx = $eidx->idx_shard($docid);
+			if (!$cur || $cur->{bytes} == 0) { # really vivifying
+				$smsg->{num} = $docid;
+				$oidx->add_overview($eml, $smsg);
+				$smsg->{-merge_vmd} = 1;
+				$idx->index_eml($eml, $smsg);
+			} else { # lse fuzzy hit off ale
+				$idx->ipc_do('add_eidx_info', $docid, '.', $eml);
+			}
+			for my $oid (keys %$xoids) {
+				$oidx->add_xref3($docid, -1, $oid, '.');
+			}
+			$idx->ipc_do('add_vmd', $docid, $vmd) if $vmd;
+		}
+		$vivify_xvmd;
+	} elsif (my @docids = _docids_for($self, $eml)) {
+		# fuzzy match from within lei/store
 		for my $docid (@docids) {
 			my $idx = $eidx->idx_shard($docid);
 			$oidx->add_xref3($docid, -1, $smsg->{blob}, '.');
@@ -178,20 +211,21 @@ sub add_eml {
 			$idx->ipc_do('add_vmd', $docid, $vmd) if $vmd;
 		}
 		\@docids;
-	} else {
+	} else { # totally new message
 		$smsg->{num} = $oidx->adj_counter('eidx_docid', '+');
 		$oidx->add_overview($eml, $smsg);
 		$oidx->add_xref3($smsg->{num}, -1, $smsg->{blob}, '.');
 		my $idx = $eidx->idx_shard($smsg->{num});
 		$idx->index_eml($eml, $smsg);
-		$idx->ipc_do('add_vmd', $smsg->{num}, $vmd ) if $vmd;
+		$idx->ipc_do('add_vmd', $smsg->{num}, $vmd) if $vmd;
 		$smsg;
 	}
 }
 
 sub set_eml {
-	my ($self, $eml, $vmd) = @_;
-	add_eml($self, $eml, $vmd) // set_eml_vmd($self, $eml, $vmd);
+	my ($self, $eml, $vmd, $xoids) = @_;
+	add_eml($self, $eml, $vmd, $xoids) //
+		set_eml_vmd($self, $eml, $vmd);
 }
 
 # set or update keywords for external message, called via ipc_do
diff --git a/lib/PublicInbox/Over.pm b/lib/PublicInbox/Over.pm
index 587e0516..0e191c47 100644
--- a/lib/PublicInbox/Over.pm
+++ b/lib/PublicInbox/Over.pm
@@ -353,7 +353,7 @@ sub blob_exists {
 	my ($self, $oidhex) = @_;
 	if (wantarray) {
 		my $sth = $self->dbh->prepare_cached(<<'', undef, 1);
-SELECT docid FROM xref3 WHERE oidbin = ?
+SELECT docid FROM xref3 WHERE oidbin = ? ORDER BY docid ASC
 
 		$sth->bind_param(1, pack('H*', $oidhex), SQL_BLOB);
 		$sth->execute;
diff --git a/lib/PublicInbox/SearchIdx.pm b/lib/PublicInbox/SearchIdx.pm
index 3237aadc..3f933121 100644
--- a/lib/PublicInbox/SearchIdx.pm
+++ b/lib/PublicInbox/SearchIdx.pm
@@ -11,6 +11,7 @@ use strict;
 use v5.10.1;
 use parent qw(PublicInbox::Search PublicInbox::Lock Exporter);
 use PublicInbox::Eml;
+use PublicInbox::Search qw(xap_terms);
 use PublicInbox::InboxWritable;
 use PublicInbox::MID qw(mids_for_index mids);
 use PublicInbox::MsgIter;
@@ -34,6 +35,7 @@ use constant DEBUG => !!$ENV{DEBUG};
 my $xapianlevels = qr/\A(?:full|medium)\z/;
 my $hex = '[a-f0-9]';
 my $OID = $hex .'{40,}';
+my @VMD_MAP = (kw => 'K', label => 'L');
 our $INDEXLEVELS = qr/\A(?:full|medium|basic)\z/;
 
 sub new {
@@ -428,7 +430,15 @@ sub eml2doc ($$$;$) {
 sub add_xapian ($$$$) {
 	my ($self, $eml, $smsg, $mids) = @_;
 	begin_txn_lazy($self);
+	my $merge_vmd = delete $smsg->{-merge_vmd};
 	my $doc = eml2doc($self, $eml, $smsg, $mids);
+	if (my $old = $merge_vmd ? _get_doc($self, $smsg->{num}) : undef) {
+		my @x = @VMD_MAP;
+		while (my ($field, $pfx) = splice(@x, 0, 2)) {
+			my $vals = xap_terms($pfx, $old);
+			$doc->add_boolean_term($pfx.$_) for keys %$vals;
+		}
+	}
 	$self->{xdb}->replace_document($smsg->{num}, $doc);
 }
 
@@ -531,8 +541,6 @@ sub remove_eidx_info {
 	$self->{xdb}->replace_document($docid, $doc);
 }
 
-my @VMD_MAP = (kw => 'K', label => 'L');
-
 sub set_vmd {
 	my ($self, $docid, $vmd) = @_;
 	begin_txn_lazy($self);
diff --git a/t/lei-q-kw.t b/t/lei-q-kw.t
index b5e22e9b..4db27363 100644
--- a/t/lei-q-kw.t
+++ b/t/lei-q-kw.t
@@ -161,5 +161,49 @@ like($s, qr/^Status: O\nX-Status: AF\n/ms,
 lei_ok(qw(q --pretty), "m:$m", @inc);
 like($lei_out, qr/^  "kw": \["answered", "flagged"\],\n/sm,
 	'--pretty JSON output shows kw: on one line');
+
+# ensure import on previously external-only message works
+lei_ok('q', "m:$m");
+is_deeply(json_utf8->decode($lei_out), [ undef ],
+	'to-be-imported message non-existent');
+lei_ok(qw(import -F eml t/x-unknown-alpine.eml));
+is($lei_err, '', 'no errors importing previous external-only message');
+lei_ok('q', "m:$m");
+$res = json_utf8->decode($lei_out);
+is($res->[1], undef, 'got one result');
+is_deeply($res->[0]->{kw}, [ qw(answered flagged) ], 'kw preserved on exact');
+
+# ensure fuzzy match import works, too
+$m = 'multipart@example.com';
+$o = "$ENV{HOME}/fuzz";
+lei_ok('q', '-o', $o, "m:$m", @inc);
+@fn = glob("$o/cur/*");
+scalar(@fn) == 1 or BAIL_OUT "wrote multiple or zero files: ".explain(\@fn);
+rename($fn[0], "$fn[0]S") or BAIL_OUT "rename $!";
+lei_ok('q', '-o', $o, "m:$m");
+is_deeply([glob("$o/cur/*")], [], 'clobbered output results');
+my $eml = eml_load('t/plack-2-txt-bodies.eml');
+$eml->header_set('List-Id', '<list.example.com>');
+my $in = $eml->as_string;
+lei_ok([qw(import -F eml --stdin)], undef, { 0 => \$in, %$lei_opt });
+is($lei_err, '', 'no errors from import');
+lei_ok(qw(q -f mboxrd), "m:$m");
+open $fh, '<', \$lei_out or BAIL_OUT $!;
+my @res;
+PublicInbox::MboxReader->mboxrd($fh, sub { push @res, shift });
+is($res[0]->header('Status'), 'RO', 'seen kw set');
+$res[0]->header_set('Status');
+is_deeply(\@res, [ $eml ], 'imported message matches w/ List-Id');
+
+$eml->header_set('List-Id', '<another.example.com>');
+$in = $eml->as_string;
+lei_ok([qw(import -F eml --stdin)], undef, { 0 => \$in, %$lei_opt });
+is($lei_err, '', 'no errors from 2nd import');
+lei_ok(qw(q -f mboxrd), "m:$m", 'l:another.example.com');
+my @another;
+open $fh, '<', \$lei_out or BAIL_OUT $!;
+PublicInbox::MboxReader->mboxrd($fh, sub { push @another, shift });
+is($another[0]->header('Status'), 'RO', 'seen kw set');
+
 }); # test_lei
 done_testing;

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 2/3] lei q: fix warning on remote imports
  2021-03-21  9:50 [PATCH 0/3] lei import fix, other fixes Eric Wong
  2021-03-21  9:50 ` [PATCH 1/3] lei import: vivify external-only messages Eric Wong
@ 2021-03-21  9:50 ` Eric Wong
  2021-03-21  9:50 ` [PATCH 3/3] lei: fix some warnings in tests Eric Wong
  2 siblings, 0 replies; 4+ messages in thread
From: Eric Wong @ 2021-03-21  9:50 UTC (permalink / raw)
  To: meta

This will let us tie keywords from remote externals
to those which only exist in local externals.
---
 lib/PublicInbox/ContentHash.pm | 15 ++++++++++++---
 lib/PublicInbox/LeiDedupe.pm   |  9 ++-------
 lib/PublicInbox/LeiXSearch.pm  |  6 +++++-
 t/lei-q-remote-import.t        |  3 ++-
 4 files changed, 21 insertions(+), 12 deletions(-)

diff --git a/lib/PublicInbox/ContentHash.pm b/lib/PublicInbox/ContentHash.pm
index 4dbe7b50..112b1ea6 100644
--- a/lib/PublicInbox/ContentHash.pm
+++ b/lib/PublicInbox/ContentHash.pm
@@ -8,9 +8,9 @@
 # See L<public-inbox-v2-format(5)> manpage for more details.
 package PublicInbox::ContentHash;
 use strict;
-use warnings;
-use base qw/Exporter/;
-our @EXPORT_OK = qw/content_hash content_digest/;
+use v5.10.1;
+use parent qw(Exporter);
+our @EXPORT_OK = qw(content_hash content_digest git_sha);
 use PublicInbox::MID qw(mids references);
 use PublicInbox::MsgIter;
 
@@ -94,4 +94,13 @@ sub content_hash ($) {
 	content_digest($_[0])->digest;
 }
 
+sub git_sha ($$) {
+	my ($n, $eml) = @_;
+	my $dig = Digest::SHA->new($n);
+	my $buf = $eml->as_string;
+	$dig->add('blob '.length($buf)."\0");
+	$dig->add($buf);
+	$dig;
+}
+
 1;
diff --git a/lib/PublicInbox/LeiDedupe.pm b/lib/PublicInbox/LeiDedupe.pm
index 5fec9384..a62b3a7c 100644
--- a/lib/PublicInbox/LeiDedupe.pm
+++ b/lib/PublicInbox/LeiDedupe.pm
@@ -3,7 +3,7 @@
 package PublicInbox::LeiDedupe;
 use strict;
 use v5.10.1;
-use PublicInbox::ContentHash qw(content_hash);
+use PublicInbox::ContentHash qw(content_hash git_sha);
 use Digest::SHA ();
 
 # n.b. mutt sets most of these headers not sure about Bytes
@@ -18,12 +18,7 @@ sub _regen_oid ($) {
 		push @stash, [ $k, \@v ];
 		$eml->header_set($k); # restore below
 	}
-	my $dig = Digest::SHA->new(1); # XXX SHA256 later
-	my $buf = $eml->as_string;
-	$dig->add('blob '.length($buf)."\0");
-	$dig->add($buf);
-	undef $buf;
-
+	my $dig = git_sha(1, $eml);
 	for my $kv (@stash) { # restore stashed headers
 		my ($k, @v) = @$kv;
 		$eml->header_set($k, @v);
diff --git a/lib/PublicInbox/LeiXSearch.pm b/lib/PublicInbox/LeiXSearch.pm
index 17171a7f..b6aaf3e1 100644
--- a/lib/PublicInbox/LeiXSearch.pm
+++ b/lib/PublicInbox/LeiXSearch.pm
@@ -18,6 +18,7 @@ use PublicInbox::MID qw(mids);
 use PublicInbox::Smsg;
 use PublicInbox::Eml;
 use Fcntl qw(SEEK_SET F_SETFL O_APPEND O_RDWR);
+use PublicInbox::ContentHash qw(git_sha);
 
 sub new {
 	my ($class) = @_;
@@ -207,10 +208,13 @@ sub query_mset { # non-parallel for non-"--threads" users
 
 sub each_remote_eml { # callback for MboxReader->mboxrd
 	my ($eml, $self, $lei, $each_smsg) = @_;
-	if ($self->{import_sto} && !$lei->{ale}->xoids_for($eml, 1)) {
+	my $xoids = $lei->{ale}->xoids_for($eml, 1);
+	if ($self->{import_sto} && !$xoids) {
 		$self->{import_sto}->ipc_do('add_eml', $eml);
 	}
 	my $smsg = bless {}, 'PublicInbox::Smsg';
+	$smsg->{blob} = $xoids ? (keys(%$xoids))[0]
+				: git_sha(1, $eml)->hexdigest;
 	$smsg->populate($eml);
 	$smsg->parse_references($eml, mids($eml));
 	$smsg->{$_} //= '' for qw(from to cc ds subject references mid);
diff --git a/t/lei-q-remote-import.t b/t/lei-q-remote-import.t
index 25e461ac..93828a24 100644
--- a/t/lei-q-remote-import.t
+++ b/t/lei-q-remote-import.t
@@ -65,8 +65,9 @@ test_lei({ tmpdir => $tmpdir }, sub {
 		$im->add(eml_load('t/utf8.eml')) or BAIL_OUT '->add';
 	};
 	lei_ok(qw(add-external -q), $ibx->{inboxdir});
-	lei_ok(qw(q -o), "mboxrd:$o", '--only', $url,
+	lei_ok(qw(q -q -o), "mboxrd:$o", '--only', $url,
 		'm:testmessage@example.com');
+	is($lei_err, '', 'no warnings or errors');
 	ok(-s $o, 'got result from remote external');
 	my $exp = eml_load('t/utf8.eml');
 	is_deeply($slurp_emls->($o), [$exp], 'got expected result');

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 3/3] lei: fix some warnings in tests
  2021-03-21  9:50 [PATCH 0/3] lei import fix, other fixes Eric Wong
  2021-03-21  9:50 ` [PATCH 1/3] lei import: vivify external-only messages Eric Wong
  2021-03-21  9:50 ` [PATCH 2/3] lei q: fix warning on remote imports Eric Wong
@ 2021-03-21  9:50 ` Eric Wong
  2 siblings, 0 replies; 4+ messages in thread
From: Eric Wong @ 2021-03-21  9:50 UTC (permalink / raw)
  To: meta

And then test the contents of $lei_err to ensure it doesn't
happen again.

We'll also make MboxLock emit nicer warnings without the line
number, since the line number is irrelevant to the user fixing
an mbox lock contention problem.

Finally, we'll also allow showing loud warnings via
TEST_LEI_ERR_LOUD=1
---
 lib/PublicInbox/LEI.pm         | 8 ++++----
 lib/PublicInbox/LeiExternal.pm | 2 +-
 lib/PublicInbox/LeiP2q.pm      | 2 +-
 lib/PublicInbox/MboxLock.pm    | 8 ++++----
 lib/PublicInbox/TestCommon.pm  | 9 +++++++++
 5 files changed, 19 insertions(+), 10 deletions(-)

diff --git a/lib/PublicInbox/LEI.pm b/lib/PublicInbox/LEI.pm
index 72a0e52c..bf97a680 100644
--- a/lib/PublicInbox/LEI.pm
+++ b/lib/PublicInbox/LEI.pm
@@ -757,7 +757,7 @@ sub lei__complete {
 	my ($proto, undef, @spec) = @$info;
 	my $cur = pop @argv;
 	my $re = defined($cur) ? qr/\A\Q$cur\E/ : qr/./;
-	if (substr($cur // '-', 0, 1) eq '-') { # --switches
+	if (substr(my $_cur = $cur // '-', 0, 1) eq '-') { # --switches
 		# gross special case since the only git-config options
 		# Consider moving to a table if we need more special cases
 		# we use Getopt::Long for are the ones we reject, so these
@@ -781,7 +781,7 @@ sub lei__complete {
 			}
 			map {
 				my $x = length > 1 ? "--$_" : "-$_";
-				$x eq $cur ? () : $x;
+				$x eq $_cur ? () : $x;
 			} grep(!/_/, split(/\|/, $_, -1)) # help|h
 		} grep { $OPTDESC{"$_\t$cmd"} || $OPTDESC{$_} } @spec);
 	} elsif ($cmd eq 'config' && !@argv && !$CONFIG_KEYS{$cur}) {
@@ -796,13 +796,13 @@ sub lei__complete {
 			my @v = ref($v) ? split(/\|/, $v->[0]) : ();
 			# get rid of ALL CAPS placeholder (e.g "OUT")
 			# (TODO: completion for external paths)
-			shift(@v) if uc($v[0]) eq $v[0];
+			shift(@v) if scalar(@v) && uc($v[0]) eq $v[0];
 			@v;
 		} grep(/\A(?:[\w-]+\|)*$opt\b.*?(?:\t$cmd)?\z/, keys %OPTDESC);
 	}
 	$cmd =~ tr/-/_/;
 	if (my $sub = $self->can("_complete_$cmd")) {
-		puts $self, $sub->($self, @argv, $cur);
+		puts $self, $sub->($self, @argv, $cur ? ($cur) : ());
 	}
 	# TODO: URLs, pathnames, OIDs, MIDs, etc...  See optparse() for
 	# proto parsing.
diff --git a/lib/PublicInbox/LeiExternal.pm b/lib/PublicInbox/LeiExternal.pm
index b5dd85e1..f4e24c2a 100644
--- a/lib/PublicInbox/LeiExternal.pm
+++ b/lib/PublicInbox/LeiExternal.pm
@@ -222,7 +222,7 @@ sub _complete_url_common ($) {
 	# Maybe there's a better way to go about this in
 	# contrib/completion/lei-completion.bash
 	my $re = '';
-	my $cur = pop @$argv;
+	my $cur = pop(@$argv) // '';
 	if (@$argv) {
 		my @x = @$argv;
 		if ($cur eq ':' && @x) {
diff --git a/lib/PublicInbox/LeiP2q.pm b/lib/PublicInbox/LeiP2q.pm
index e7ddc852..c5718603 100644
--- a/lib/PublicInbox/LeiP2q.pm
+++ b/lib/PublicInbox/LeiP2q.pm
@@ -144,7 +144,7 @@ sub do_p2q { # via wq_do
 			my $end = ($pfx =~ s/([0-9\*]+)\z//) ? $1 : '';
 			my $x = delete($lei->{qterms}->{$pfx}) or next;
 			my $star = $end =~ tr/*//d ? '*' : '';
-			my $min_len = ($end // 0) + 0;
+			my $min_len = ($end || 0) + 0;
 
 			# no wildcards for bool_pfx_external
 			$star = '' if $pfx =~ /\A(dfpre|dfpost|mid)\z/;
diff --git a/lib/PublicInbox/MboxLock.pm b/lib/PublicInbox/MboxLock.pm
index 4e2a2d9a..bea0e325 100644
--- a/lib/PublicInbox/MboxLock.pm
+++ b/lib/PublicInbox/MboxLock.pm
@@ -43,13 +43,13 @@ EOF
 		}
 		select(undef, undef, undef, $self->{delay});
 	} while (now < $end);
-	croak "fcntl lock $self->{f}: $!";
+	die "fcntl lock timeout $self->{f}: $!\n";
 }
 
 sub acq_dotlock {
 	my ($self) = @_;
 	my $dot_lock = "$self->{f}.lock";
-	my ($pfx, $base) = ($self->{f} =~ m!(\A.*?/)([^/]+)\z!);
+	my ($pfx, $base) = ($self->{f} =~ m!(\A.*?/)?([^/]+)\z!);
 	$pfx //= '';
 	my $pid = $$;
 	my $end = now + $self->{timeout};
@@ -68,7 +68,7 @@ sub acq_dotlock {
 			croak "open $tmp (for $dot_lock): $!" if !$!{EXIST};
 		}
 	} while (now < $end);
-	croak "dotlock $dot_lock";
+	die "dotlock timeout $dot_lock\n";
 }
 
 sub acq_flock {
@@ -80,7 +80,7 @@ sub acq_flock {
 		return if flock($self->{fh}, $op);
 		select(undef, undef, undef, $self->{delay});
 	} while (now < $end);
-	croak "flock $self->{f}: $!";
+	die "flock timeout $self->{f}: $!\n";
 }
 
 sub acq {
diff --git a/lib/PublicInbox/TestCommon.pm b/lib/PublicInbox/TestCommon.pm
index 0d15514e..e67e94ea 100644
--- a/lib/PublicInbox/TestCommon.pm
+++ b/lib/PublicInbox/TestCommon.pm
@@ -457,6 +457,15 @@ sub lei (@) {
 	my $res = run_script(['lei', @$cmd], $env, $xopt // $lei_opt);
 	$err_skip and
 		$lei_err = join('', grep(!/$err_skip/, split(/^/m, $lei_err)));
+	if ($lei_err ne '') {
+		if ($lei_err =~ /Use of uninitialized/ ||
+			$lei_err =~ m!\bArgument .*? isn't numeric in !) {
+			fail "lei_err=$lei_err";
+		} else {
+			state $loud = $ENV{TEST_LEI_ERR_LOUD};
+			diag "lei_err=$lei_err" if $loud;
+		}
+	}
 	$res;
 };
 

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-03-21  9:50 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-21  9:50 [PATCH 0/3] lei import fix, other fixes Eric Wong
2021-03-21  9:50 ` [PATCH 1/3] lei import: vivify external-only messages Eric Wong
2021-03-21  9:50 ` [PATCH 2/3] lei q: fix warning on remote imports Eric Wong
2021-03-21  9:50 ` [PATCH 3/3] lei: fix some warnings in tests Eric Wong

user/dev discussion of public-inbox itself

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://public-inbox.org/meta
	git clone --mirror http://czquwvybam4bgbro.onion/meta
	git clone --mirror http://hjrcffqmbrq6wope.onion/meta
	git clone --mirror http://ou63pmih66umazou.onion/meta

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V1 meta meta/ https://public-inbox.org/meta \
		meta@public-inbox.org
	public-inbox-index meta

Example config snippet for mirrors.
Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.mail.public-inbox.meta
	nntp://ou63pmih66umazou.onion/inbox.comp.mail.public-inbox.meta
	nntp://czquwvybam4bgbro.onion/inbox.comp.mail.public-inbox.meta
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.mail.public-inbox.meta
	nntp://news.gmane.io/gmane.mail.public-inbox.general
 note: .onion URLs require Tor: https://www.torproject.org/

code repositories for project(s) associated with this inbox:

	https://80x24.org/public-inbox.git

AGPL code for this site: git clone https://public-inbox.org/public-inbox.git