* [PATCH 1/2] xap_helper: drop qp_extra_done flag and conditions
2025-03-06 20:34 [PATCH 0/2] Perl XapHelper fix + cleanup Eric Wong
@ 2025-03-06 20:34 ` Eric Wong
2025-03-06 20:34 ` [PATCH 2/2] XapHelper.pm: fix QP_FLAGS initialization Eric Wong
1 sibling, 0 replies; 3+ messages in thread
From: Eric Wong @ 2025-03-06 20:34 UTC (permalink / raw)
To: meta
As with -d (directories), the -Q (extra query prefixes) flag is
already part of the cache key so there's no need to initialize
it lazily.
While we're at it, drop a `map' op from the cache key generation
for the Perl implementation.
---
lib/PublicInbox/XapHelper.pm | 12 ++++----
lib/PublicInbox/xap_helper.h | 54 +++++++++++++++++-------------------
2 files changed, 30 insertions(+), 36 deletions(-)
diff --git a/lib/PublicInbox/XapHelper.pm b/lib/PublicInbox/XapHelper.pm
index e98553c8..8f3e7e84 100644
--- a/lib/PublicInbox/XapHelper.pm
+++ b/lib/PublicInbox/XapHelper.pm
@@ -186,16 +186,15 @@ sub cmd_mset { # to be used by WWW + IMAP
}
}
-sub srch_init_extra ($) {
- my ($req) = @_;
- my $qp = $req->{srch}->{qp};
+sub srch_init_extra ($$) {
+ my ($srch, $req) = @_;
+ my $qp = $srch->{qp};
for (@{$req->{Q}}) {
my ($upfx, $m, $xpfx) = split /([:=])/;
$xpfx // die "E: bad -Q $_";
$m = $m eq '=' ? 'add_boolean_prefix' : 'add_prefix';
$qp->$m($upfx, $xpfx);
}
- $req->{srch}->{qp_extra_done} = 1;
}
sub dispatch {
@@ -205,7 +204,7 @@ sub dispatch {
or return;
my $dirs = delete $req->{d} or die 'no -d args';
my $key = "-d\0".join("\0-d\0", @$dirs);
- $key .= "\0".join("\0", map { ('-Q', $_) } @{$req->{Q}}) if $req->{Q};
+ $key .= "\0-Q\0".join("\0-Q\0", @{$req->{Q}}) if $req->{Q};
my $new;
$req->{srch} = $SRCH{$key} // do {
$new = { qp_flags => $QP_FLAGS };
@@ -240,11 +239,10 @@ sub dispatch {
bless $new, $req->{c} ? 'PublicInbox::CodeSearch' :
'PublicInbox::Search';
$new->qparse_new;
+ srch_init_extra $new, $req;
$SRCH{$key} = $new;
};
$req->{srch}->{xdb}->reopen unless $new;
- $req->{Q} && !$req->{srch}->{qp_extra_done} and
- srch_init_extra $req;
my $timeo = $req->{K};
alarm($timeo) if $timeo;
$fn->($req, @argv);
diff --git a/lib/PublicInbox/xap_helper.h b/lib/PublicInbox/xap_helper.h
index f6da52fb..fd52a70d 100644
--- a/lib/PublicInbox/xap_helper.h
+++ b/lib/PublicInbox/xap_helper.h
@@ -120,7 +120,6 @@ static void *xreallocarray(void *ptr, size_t nmemb, size_t size)
struct srch {
int ckey_len; // int for comparisons
unsigned qp_flags;
- bool qp_extra_done;
Xapian::Database *db;
Xapian::QueryParser *qp;
unsigned char ckey[]; // $shard_path0\0$shard_path1\0...
@@ -594,6 +593,30 @@ static void srch_cache_renew(struct srch *keep)
#include "xh_date.h" // GitDateRangeProcessor + GitDateFieldProcessor
#include "xh_thread_fp.h" // ThreadFieldProcessor
+// setup query parser for altid and arbitrary headers
+static void srch_init_extra(struct srch *srch, struct req *req)
+{
+ const char *XPFX;
+ for (int i = 0; i < req->qpfxc; i++) {
+ size_t len = strlen(req->qpfxv[i]);
+ char *c = (char *)memchr(req->qpfxv[i], '=', len);
+
+ if (c) { // it's boolean "gmane=XGMANE"
+ XPFX = c + 1;
+ *c = 0;
+ srch->qp->add_boolean_prefix(req->qpfxv[i], XPFX);
+ continue;
+ }
+ // maybe it's a non-boolean prefix "blob:XBLOBID"
+ c = (char *)memchr(req->qpfxv[i], ':', len);
+ if (!c)
+ errx(EXIT_FAILURE, "bad -Q %s", req->qpfxv[i]);
+ XPFX = c + 1;
+ *c = 0;
+ srch->qp->add_prefix(req->qpfxv[i], XPFX);
+ }
+}
+
static void srch_init(struct req *req)
{
int i;
@@ -659,31 +682,7 @@ static void srch_init(struct req *req)
srch->qp->add_boolean_prefix("L", "L");
}
}
-}
-
-// setup query parser for altid and arbitrary headers
-static void srch_init_extra(struct req *req)
-{
- const char *XPFX;
- for (int i = 0; i < req->qpfxc; i++) {
- size_t len = strlen(req->qpfxv[i]);
- char *c = (char *)memchr(req->qpfxv[i], '=', len);
-
- if (c) { // it's boolean "gmane=XGMANE"
- XPFX = c + 1;
- *c = 0;
- req->srch->qp->add_boolean_prefix(req->qpfxv[i], XPFX);
- continue;
- }
- // maybe it's a non-boolean prefix "blob:XBLOBID"
- c = (char *)memchr(req->qpfxv[i], ':', len);
- if (!c)
- errx(EXIT_FAILURE, "bad -Q %s", req->qpfxv[i]);
- XPFX = c + 1;
- *c = 0;
- req->srch->qp->add_prefix(req->qpfxv[i], XPFX);
- }
- req->srch->qp_extra_done = true;
+ srch_init_extra(req->srch, req);
}
#define OPT_U(ch, var, fn, max) do { \
@@ -764,7 +763,6 @@ static void dispatch(struct req *req)
ERR_CLOSE(kfp, EXIT_FAILURE); // may ENOMEM, sets kbuf.srch
kbuf.srch->db = NULL;
kbuf.srch->qp = NULL;
- kbuf.srch->qp_extra_done = false;
kbuf.srch->ckey_len = size - offsetof(struct srch, ckey);
if (kbuf.srch->ckey_len <= 0 || !req->dirc)
ABORT("no -d args (or too many)");
@@ -780,8 +778,6 @@ static void dispatch(struct req *req)
srch_free(kbuf.srch);
req->srch->db->reopen();
}
- if (req->qpfxc && !req->srch->qp_extra_done)
- srch_init_extra(req);
if (req->timeout_sec)
alarm(req->timeout_sec > UINT_MAX ?
UINT_MAX : (unsigned)req->timeout_sec);
^ permalink raw reply related [flat|nested] 3+ messages in thread* [PATCH 2/2] XapHelper.pm: fix QP_FLAGS initialization
2025-03-06 20:34 [PATCH 0/2] Perl XapHelper fix + cleanup Eric Wong
2025-03-06 20:34 ` [PATCH 1/2] xap_helper: drop qp_extra_done flag and conditions Eric Wong
@ 2025-03-06 20:34 ` Eric Wong
1 sibling, 0 replies; 3+ messages in thread
From: Eric Wong @ 2025-03-06 20:34 UTC (permalink / raw)
To: meta
We can't use $PublicInbox::Search::QP_FLAGS until after
load_xapian is called. Failure to set the correct query parser
flags was causing failures in
`PI_NO_CXX=1 TEST_DAEMON_XH=-X0 prove -bw t/imapd.t'
since phrase parsing was broken with the Perl bindings
XapHelper implementation.
Now, the `check-xh0' and `check-xh1' targets both pass with
PI_NO_CXX=1 set to disable the C++ xap_helper.
Fixes: fa6a7919 (xap_helper: enable FLAG_PURE_NOT in external process, 2025-02-23)
---
lib/PublicInbox/XapHelper.pm | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/lib/PublicInbox/XapHelper.pm b/lib/PublicInbox/XapHelper.pm
index 8f3e7e84..54e2c174 100644
--- a/lib/PublicInbox/XapHelper.pm
+++ b/lib/PublicInbox/XapHelper.pm
@@ -20,7 +20,7 @@ use Carp qw(croak);
my $X = \%PublicInbox::Search::X;
our (%SRCH, %WORKERS, $nworker, $workerset, $in, $SHARD_NFD, $MY_FD_MAX);
our $stderr = \*STDERR;
-my $QP_FLAGS = $PublicInbox::Search::QP_FLAGS;
+my $QP_FLAGS;
sub cmd_test_inspect {
my ($req) = @_;
@@ -338,6 +338,8 @@ sub start (@) {
local (%SRCH, %WORKERS, $SHARD_NFD, $MY_FD_MAX);
PublicInbox::Search::load_xapian();
+ $QP_FLAGS = $PublicInbox::Search::QP_FLAGS |
+ PublicInbox::Search::FLAG_PURE_NOT();
$GLP->getoptionsfromarray(\@argv, my $opt = { j => 1 }, 'j=i') or
die 'bad args';
local $workerset = POSIX::SigSet->new;
@@ -349,7 +351,6 @@ sub start (@) {
die "E: unable to get RLIMIT_NOFILE: $!";
warn "W: RLIMIT_NOFILE=$MY_FD_MAX too low\n" if $MY_FD_MAX < 72;
$MY_FD_MAX -= 64;
- $QP_FLAGS |= PublicInbox::Search::FLAG_PURE_NOT();
local $nworker = $opt->{j};
return recv_loop() if $nworker == 0;
^ permalink raw reply related [flat|nested] 3+ messages in thread