Opened 14 years ago
Closed 21 months ago
#523 closed defect (fixed)
Migrate from XS Search::Xapian to SWIG version
Reported by: | Olly Betts | Owned by: | Olly Betts |
---|---|---|---|
Priority: | normal | Milestone: | 1.4.23 |
Component: | Search::Xapian | Version: | git master |
Severity: | normal | Keywords: | |
Cc: | Blocked By: | ||
Blocking: | Operating System: | All |
Description (last modified by )
This ticket is to track the issues remaining to solve before the SWIG Perl bindings are a complete replacement for the XS ones.
~~"$it"
doesn't work in SWIG, while in XS it dereferences (for TermIterator and ValueIterator at least). I did some digging last year and it seemed there was some problematic interaction with eq/ne which I didn't get to the bottom of.~"Stringify" is also missing for subclasses of Error (which in the XS version I'm using in 1.2.4 to give backwards compatibility for cases which were still throwing string exceptions in 1.2.3).~~XS version has simple anon-sub wrappers for MatchDecider and ExpandDecider. SWIG doesn't support directors for Perl in current releases (though there's work on a branch). But perhaps we can support these simple wrappers without needing directors.~~The SWIG-generated version is fussier about what sort of scalars you pass - for example, if an integer is expected, passing a string containing an integer or floating point number doesn't work, at least in some cases.~ Now documented at least.
One option is to deprecate the stringify stuff and make the switch at the start of the next release series.
Change History (22)
comment:1 by , 14 years ago
Milestone: | 1.2.x → 1.3.0 |
---|
comment:2 by , 14 years ago
It would be nice to wrap iterators in a more natural way for Perl - this article gives a good approach which would probably work:
comment:3 by , 14 years ago
Description: | modified (diff) |
---|
comment:5 by , 10 years ago
Milestone: | 1.3.x → 1.3.5 |
---|
comment:6 by , 9 years ago
Milestone: | 1.3.5 → 1.3.x |
---|---|
Version: | SVN trunk → git master |
I talked through these issues at a Wellington PM meeting a while ago and have notes from that, but I think this is going to take a while to work through properly.
I'm pondering if we should release 1.4.0 with the Perl bindings clearly labelled as "still experimental", and then resolve this in early 1.4.x.
Moving to 1.3.x for now.
comment:7 by , 9 years ago
SWIG now supports directors for Perl, though taking a code ref (which would typically be an anonymous subroutine) would be a nicer Perl API for the simple functors.
comment:9 by , 8 years ago
Milestone: | 1.4.1 → 1.4.3 |
---|
Search::Xapian 1.2.24.0 will be compatible with xapian-core 1.4.x so this is a little less urgent.
comment:13 by , 6 years ago
Xapian::PostingIterator lacks "->inc" sub from Search::Xapian::PostingIterator; so I need to rely on "++" which I find confusing (overloading confuses me :x)
Also, I could not find Xapian::ENQ_ASCENDING, since I rely on Search::Xapian::ENQ_ASCENDING.
comment:14 by , 5 years ago
Xapian::PostingIterator
lacks "->inc" sub from Search::Xapian::PostingIterator; so I need to rely on "++" which I find confusing (overloading confuses me :x)
Thanks for reporting. I've pushed fde36e74d72b36b19dea6eb83b70ee1311423c40 to fix that, and will backport for 1.4.16.
comment:15 by , 5 years ago
I could not find
Xapian::ENQ_ASCENDING
, since I rely onSearch::Xapian::ENQ_ASCENDING
.
This is wrapped as $Xapian::Enquire::ASCENDING
(which better matches the C++ API).
But we try to provide compatibility with Search::Xapian
so I've pushed d7776b1206fb6fd33a715a38b87678e14ecadd3f to add aliases for these.
comment:16 by , 5 years ago
I debugged what happens when stringify overloads are added back in.
The problem is that SWIG internally adds objects to a hash using the object as the hash key. Perl stringifies the hash key to get a string to hash, and that doesn't work out well if the stringify overloads aren't a unique identifier for the C++ object (which the Search::Xapian
overloads aren't).
It'd be better if SWIG didn't rely on stringifying objects like this, but I'm not sure the Search::Xapian
stringify overloads are a good idea either.
comment:17 by , 5 years ago
Description: | modified (diff) |
---|---|
Milestone: | 1.4.6 → 1.5.0 |
Status: | new → assigned |
The convert to integer overload on PositionIterator
also seems to cause problems, though I haven't tried to dig into why.
I think these magic implicit method calls on conversions to string and integer are too confusing, and it's better not to support them. This is incompatible with Search::Xapian
, but we can document this and it's easy to write code which works with both versions - just make the method call explicit.
comment:18 by , 5 years ago
We should also document a good way to load whichever of Search::Xapian
or Xapian
is available, rather than everybody having to invent their own way. Perhaps:
BEGIN { eval { require Xapian; Xapian->import(':all'); }; if ($@) { require Search::Xapian; Search::Xapian->import(':all'); } }
comment:19 by , 5 years ago
I've improved the documentation of the newer bindings and added a compatibility section which shows the require
code to load either, and at least document the increased fussiness about string vs integer scalars.
I think the main thing left is wrapping MatchDecider
and ExpandDecider
to call Perl functions.
comment:20 by , 5 years ago
Description: | modified (diff) |
---|
comment:21 by , 5 years ago
Description: | modified (diff) |
---|
I've added support for passing a Perl sub for simple functor classes in d5d26eb54bfb68e1674b8f0e314de29e81e16b1d.
SWIG now supports directors for Perl so we should be able to support subclassing, but accepting a Perl sub as an alternative still seems useful for simple cases (it'd be nice to support for C++ too).
I think the coding work for this ticket is now done (unless further incompatibilities are found), but we should make sure the documentation reflects the updated status of the SWIG-generated Perl bindings.
comment:22 by , 21 months ago
Milestone: | 1.5.0 → 1.4.23 |
---|---|
Resolution: | → fixed |
Status: | assigned → closed |
[b2c5a064bb28088f024ac1526c429e5e0876dee3] updated some wording about the status of these bindings.
I'm working through putting the docs for these on the website too, though probably that won't be completed until the next 1.4.x release as the docs there are built from release tarballs.
I'll also update the getting started guide.
Doing this for the next release series seems to make most sense. Now we've made a release branch for 1.2.x, I've removed the XS Search::Xapian from trunk.