Drop support for pre-1.4 Xapian, prepare for 1.5.x

Drop support for pre-1.4 Xapian, prepare for 1.5.x

[David Bremner]

The current development release of Xapian drops several deprecated
APIs. This hampers testing of notmuch against Xapian master (i.e. to
see if a problem has been fixed). This series fixes most of the
incompatibilities.

The one thing this series doesn't handle is test/T530-upgrade.sh,
which mostly fails because Xapian 1.5 / master drops support for
Chert. I think that we should probably deprecate support for notmuch
v1 databases. These were replaced by version 2 in 2010. I think more
thought is probably needed for this migration so I left those failing
tests for now.  This isn't quite as scary as it sounds, because we
really only support stable releases of Xapian, so the chert removal
effectively won't happen until 1.6.

[PATCH 1/3] build: require Xapian >= 1.4.0

[David Bremner]

This is necessary before we can support Xapian 1.5, which drops the
old compact API, and replaces ValueRangeProcessor classes with
RangeProcessor classes. It also simplifies maintainability as 3
previously optional features are now part of the baseline Xapian
version.
---
 INSTALL   | 17 +++++-------
 NEWS      |  6 +++++
 configure | 78 +++++++++++++------------------------------------------
 3 files changed, 30 insertions(+), 71 deletions(-)

diff --git a/INSTALL b/INSTALL
index 6e6f4799..87fd65f2 100644
--- a/INSTALL
+++ b/INSTALL
@@ -24,20 +24,15 @@ Notmuch depends on four libraries: Xapian, GMime 2.6,
 Talloc, and zlib which are each described below:
 
 	Xapian
-	------
-	Xapian is the search-engine library underlying Notmuch.
+        ------
+        Xapian is the search-engine library underlying Notmuch.
 
-	It provides all the real machinery of indexing and searching,
-	(including the very nice parsing of the query string).
+        It provides all the real machinery of indexing and searching,
+        (including the very nice parsing of the query string).
 
-	Xapian is available from https://xapian.org
+        Xapian is available from https://xapian.org
 
-	Note: Notmuch will work best with Xapian 1.0.18 (or later) or
-	Xapian 1.1.4 (or later). Previous versions of Xapian (whether
-	1.0 or 1.1) had a performance bug that made notmuch very slow
-	when modifying tags. This would cause distracting pauses when
-	reading mail while notmuch would wait for Xapian when removing
-	the "inbox" and "unread" tags from messages in a thread.
+        Notmuch needs a Xapian version at least 1.4.0.
 
 	GMime
 	-----
diff --git a/NEWS b/NEWS
index 0a4ab4bb..9980ed9a 100644
--- a/NEWS
+++ b/NEWS
@@ -13,6 +13,12 @@ Emacs
 Support for GNU Emacs older than 25.1 is deprecated with this release,
 and may be removed in a future release.
 
+Dependencies
+------------
+
+As of this release, support for versions of Xapian before 1.4.0 is
+removed. It was deprecated in release 0.27.
+
 Notmuch 0.28.3 (2019-03-05)
 ===========================
 
diff --git a/configure b/configure
index 5e7e5aa9..edd12488 100755
--- a/configure
+++ b/configure
@@ -401,75 +401,33 @@ else
     have_pkg_config=0
 fi
 
-printf "Checking for Xapian development files... "
+XAPIAN_MINVER=1.4
+
+printf "Checking for Xapian development files (>= $XAPIAN_MINVER)..."
 have_xapian=0
-for xapian_config in ${XAPIAN_CONFIG} xapian-config xapian-config-1.3; do
-    if ${xapian_config} --version > /dev/null 2>&1; then
-	xapian_version=$(${xapian_config} --version | sed -e 's/.* //')
-	printf "Yes (%s).\n" ${xapian_version}
-	have_xapian=1
-	xapian_cxxflags=$(${xapian_config} --cxxflags)
-	xapian_ldflags=$(${xapian_config} --libs)
-	break
-    fi
-done
+if pkg-config --atleast-version=${XAPIAN_MINVER} xapian-core; then
+   for xapian_config in ${XAPIAN_CONFIG} xapian-config; do
+       if ${xapian_config} --version > /dev/null 2>&1; then
+	   xapian_version=$(${xapian_config} --version | sed -e 's/.* //')
+	   printf "Yes (%s).\n" ${xapian_version}
+	   have_xapian=1
+           have_xapian_compact=1
+           have_xapian_field_processor=1
+           have_xapian_db_retry_lock=1
+	   xapian_cxxflags=$(${xapian_config} --cxxflags)
+	   xapian_ldflags=$(${xapian_config} --libs)
+	   break
+       fi
+   done
+fi
 if [ ${have_xapian} = "0" ]; then
     printf "No.\n"
     errors=$((errors + 1))
 fi
 
-have_xapian_compact=0
-have_xapian_field_processor=0
 if [ ${have_xapian} = "1" ]; then
-    printf "Checking for Xapian compaction support... "
-    cat>_compact.cc<<EOF
-#include <xapian.h>
-class TestCompactor : public Xapian::Compactor { };
-EOF
-    if ${CXX} ${CXXFLAGS_for_sh} ${xapian_cxxflags} -c _compact.cc -o _compact.o > /dev/null 2>&1
-    then
-	have_xapian_compact=1
-	printf "Yes.\n"
-    else
-	printf "No.\n"
-	errors=$((errors + 1))
-    fi
-
-    rm -f _compact.o _compact.cc
-
-    printf "Checking for Xapian FieldProcessor API... "
-    cat>_field_processor.cc<<EOF
-#include <xapian.h>
-class TitleFieldProcessor : public Xapian::FieldProcessor { };
-EOF
-    if ${CXX} ${CXXFLAGS_for_sh} ${xapian_cxxflags} -c _field_processor.cc -o _field_processor.o > /dev/null 2>&1
-    then
-	have_xapian_field_processor=1
-	printf "Yes.\n"
-    else
-	printf "No. (optional)\n"
-    fi
-
-    rm -f _field_processor.o _field_processor.cc
 
     default_xapian_backend=""
-    # DB_RETRY_LOCK is only supported on Xapian > 1.3.2
-    have_xapian_db_retry_lock=0
-    if [ $WITH_RETRY_LOCK = "1" ]; then
-	printf "Checking for Xapian lock retry support... "
-	cat>_retry.cc<<EOF
-#include <xapian.h>
-int flag = Xapian::DB_RETRY_LOCK;
-EOF
-	if ${CXX} ${CXXFLAGS_for_sh} ${xapian_cxxflags} -c _retry.cc -o _retry.o > /dev/null 2>&1
-	then
-	    have_xapian_db_retry_lock=1
-	    printf "Yes.\n"
-	else
-	    printf "No. (optional)\n"
-	fi
-	rm -f _retry.o _retry.cc
-    fi
 
     printf "Testing default Xapian backend... "
     cat >_default_backend.cc <<EOF
-- 
2.20.1

[PATCH 2/3] lib: migrate to post Xapian 1.3.4 compact support

[David Bremner]

The old API was deprecated in Xapian 1.3.4 and (will be) removed in 1.5.0
---
 lib/database.cc | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/lib/database.cc b/lib/database.cc
index 09ab9cb0..bad6df5f 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -1197,11 +1197,7 @@ notmuch_database_compact (const char *path,
 
     try {
 	NotmuchCompactor compactor (status_cb, closure);
-
-	compactor.set_renumber (false);
-	compactor.add_source (xapian_path);
-	compactor.set_destdir (compact_xapian_path);
-	compactor.compact ();
+	notmuch->xapian_db->compact (compact_xapian_path, Xapian::DBCOMPACT_NO_RENUMBER, 0, compactor);
     } catch (const Xapian::Error &error) {
 	_notmuch_database_log (notmuch, "Error while compacting: %s\n", error.get_msg().c_str());
 	ret = NOTMUCH_STATUS_XAPIAN_EXCEPTION;
-- 
2.20.1

[PATCH 3/3] lib: migrate from Xapian ValueRangeProcessor to RangeProcessor

[David Bremner]

This will be mandatory as of Xapian 1.5.  The API is also more
consistent with the FieldProcessor API, which helps code re-use a bit.

Note that this switches to using the built-in Xapian support for
prefixes on ranges (i.e. deleted code at beginning of
ParseTimeRangeProcessor::operator(), added prefix to constructor).

Another side effect of the migration is that we are generating smaller
queries, using one OP_VALUE_RANGE instead of an AND of two OP_VALUE_*
queries.
---
 lib/database-private.h |  6 ++--
 lib/database.cc        | 23 +++++----------
 lib/parse-time-vrp.cc  | 64 ++++++++++++++++++++----------------------
 lib/parse-time-vrp.h   | 17 +++++------
 4 files changed, 50 insertions(+), 60 deletions(-)

diff --git a/lib/database-private.h b/lib/database-private.h
index a499b259..c3389a7a 100644
--- a/lib/database-private.h
+++ b/lib/database-private.h
@@ -212,9 +212,9 @@ struct _notmuch_database {
     unsigned long view;
     Xapian::QueryParser *query_parser;
     Xapian::TermGenerator *term_gen;
-    Xapian::ValueRangeProcessor *value_range_processor;
-    Xapian::ValueRangeProcessor *date_range_processor;
-    Xapian::ValueRangeProcessor *last_mod_range_processor;
+    Xapian::RangeProcessor *value_range_processor;
+    Xapian::RangeProcessor *date_range_processor;
+    Xapian::RangeProcessor *last_mod_range_processor;
 };
 
 /* Prior to database version 3, features were implied by the database
diff --git a/lib/database.cc b/lib/database.cc
index bad6df5f..f207890f 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -315,7 +315,6 @@ _setup_query_field_default (const prefix_t *prefix, notmuch_database_t *notmuch)
 	notmuch->query_parser->add_boolean_prefix (prefix->name, prefix->prefix);
 }
 
-#if HAVE_XAPIAN_FIELD_PROCESSOR
 static void
 _setup_query_field (const prefix_t *prefix, notmuch_database_t *notmuch)
 {
@@ -323,7 +322,7 @@ _setup_query_field (const prefix_t *prefix, notmuch_database_t *notmuch)
 	Xapian::FieldProcessor *fp;
 
 	if (STRNCMP_LITERAL (prefix->name, "date") == 0)
-	    fp = (new DateFieldProcessor())->release ();
+	    fp = (new DateFieldProcessor(NOTMUCH_VALUE_TIMESTAMP))->release ();
 	else if (STRNCMP_LITERAL(prefix->name, "query") == 0)
 	    fp = (new QueryFieldProcessor (*notmuch->query_parser, notmuch))->release ();
 	else if (STRNCMP_LITERAL(prefix->name, "thread") == 0)
@@ -338,13 +337,6 @@ _setup_query_field (const prefix_t *prefix, notmuch_database_t *notmuch)
 	_setup_query_field_default (prefix, notmuch);
     }
 }
-#else
-static inline void
-_setup_query_field (const prefix_t *prefix, notmuch_database_t *notmuch)
-{
-    _setup_query_field_default (prefix, notmuch);
-}
-#endif
 
 const char *
 _find_prefix (const char *name)
@@ -954,17 +946,16 @@ notmuch_database_open_verbose (const char *path,
 	notmuch->query_parser = new Xapian::QueryParser;
 	notmuch->term_gen = new Xapian::TermGenerator;
 	notmuch->term_gen->set_stemmer (Xapian::Stem ("english"));
-	notmuch->value_range_processor = new Xapian::NumberValueRangeProcessor (NOTMUCH_VALUE_TIMESTAMP);
-	notmuch->date_range_processor = new ParseTimeValueRangeProcessor (NOTMUCH_VALUE_TIMESTAMP);
-	notmuch->last_mod_range_processor = new Xapian::NumberValueRangeProcessor (NOTMUCH_VALUE_LAST_MOD, "lastmod:");
-
+	notmuch->value_range_processor = new Xapian::NumberRangeProcessor (NOTMUCH_VALUE_TIMESTAMP);
+	notmuch->date_range_processor = new ParseTimeRangeProcessor (NOTMUCH_VALUE_TIMESTAMP, "date:");
+	notmuch->last_mod_range_processor = new Xapian::NumberRangeProcessor (NOTMUCH_VALUE_LAST_MOD, "lastmod:");
 	notmuch->query_parser->set_default_op (Xapian::Query::OP_AND);
 	notmuch->query_parser->set_database (*notmuch->xapian_db);
 	notmuch->query_parser->set_stemmer (Xapian::Stem ("english"));
 	notmuch->query_parser->set_stemming_strategy (Xapian::QueryParser::STEM_SOME);
-	notmuch->query_parser->add_valuerangeprocessor (notmuch->value_range_processor);
-	notmuch->query_parser->add_valuerangeprocessor (notmuch->date_range_processor);
-	notmuch->query_parser->add_valuerangeprocessor (notmuch->last_mod_range_processor);
+	notmuch->query_parser->add_rangeprocessor (notmuch->value_range_processor);
+	notmuch->query_parser->add_rangeprocessor (notmuch->date_range_processor);
+	notmuch->query_parser->add_rangeprocessor (notmuch->last_mod_range_processor);
 
 	for (i = 0; i < ARRAY_SIZE (prefix_table); i++) {
 	    const prefix_t *prefix = &prefix_table[i];
diff --git a/lib/parse-time-vrp.cc b/lib/parse-time-vrp.cc
index dd691494..a64e9461 100644
--- a/lib/parse-time-vrp.cc
+++ b/lib/parse-time-vrp.cc
@@ -24,48 +24,49 @@
 #include "parse-time-vrp.h"
 #include "parse-time-string.h"
 
-#define PREFIX "date:"
+static Xapian::Query _make_query (Xapian::valueno slot, time_t from, time_t to) {
+    if (to < 0) {
+	return Xapian::Query (Xapian::Query::OP_VALUE_GE, slot,
+			      Xapian::sortable_serialise ((double) from));
+    } else if (from < 0) {
+	return Xapian::Query (Xapian::Query::OP_VALUE_LE, slot,
+			      Xapian::sortable_serialise ((double) to));
+    } else {
+	return Xapian::Query (Xapian::Query::OP_VALUE_RANGE, slot,
+			      Xapian::sortable_serialise ((double) from),
+			      Xapian::sortable_serialise ((double) to));
+    }
+}
 
-/* See *ValueRangeProcessor in xapian-core/api/valuerangeproc.cc */
-Xapian::valueno
-ParseTimeValueRangeProcessor::operator() (std::string &begin, std::string &end)
+Xapian::Query
+ParseTimeRangeProcessor::operator() (const std::string &begin, const std::string &end)
 {
-    time_t t, now;
-    std::string b;
-
-    /* Require date: prefix in start of the range... */
-    if (STRNCMP_LITERAL (begin.c_str (), PREFIX))
-	return Xapian::BAD_VALUENO;
-
-    /* ...and remove it. */
-    begin.erase (0, sizeof (PREFIX) - 1);
-    b = begin;
+    time_t from = -1, to = -1, now = -1;
+    std::string str;
 
     /* Use the same 'now' for begin and end. */
     if (time (&now) == (time_t) -1)
-	return Xapian::BAD_VALUENO;
+	throw Xapian::QueryParserError ("unable to get current time");
 
-    if (!begin.empty ()) {
-	if (parse_time_string (begin.c_str (), &t, &now, PARSE_TIME_ROUND_DOWN))
-	    return Xapian::BAD_VALUENO;
+    if (!begin.empty ())
+	if (parse_time_string (begin.c_str (), &from, &now, PARSE_TIME_ROUND_DOWN))
+	    throw Xapian::QueryParserError ("Didn't understand date specification '" + begin + "'");
 
-	begin.assign (Xapian::sortable_serialise ((double) t));
-    }
 
     if (!end.empty ()) {
-	if (end == "!" && ! b.empty ())
-	    end = b;
-
-	if (parse_time_string (end.c_str (), &t, &now, PARSE_TIME_ROUND_UP_INCLUSIVE))
-	    return Xapian::BAD_VALUENO;
-
-	end.assign (Xapian::sortable_serialise ((double) t));
+	if (end == "!" && ! begin.empty ())
+	    str = begin;
+	else
+	    str = end;
+
+	if (parse_time_string (str.c_str (), &to, &now, PARSE_TIME_ROUND_UP_INCLUSIVE))
+	    if (parse_time_string (str.c_str (), &to, &now, PARSE_TIME_ROUND_UP_INCLUSIVE))
+		throw Xapian::QueryParserError ("Didn't understand date specification '" + str + "'");
     }
 
-    return valno;
+    return _make_query (slot, from, to);
 }
 
-#if HAVE_XAPIAN_FIELD_PROCESSOR
 /* XXX TODO: is throwing an exception the right thing to do here? */
 Xapian::Query DateFieldProcessor::operator()(const std::string & str) {
     time_t from, to, now;
@@ -80,8 +81,5 @@ Xapian::Query DateFieldProcessor::operator()(const std::string & str) {
     if (parse_time_string (str.c_str (), &to, &now, PARSE_TIME_ROUND_UP_INCLUSIVE))
 	throw Xapian::QueryParserError ("Didn't understand date specification '" + str + "'");
 
-    return Xapian::Query(Xapian::Query::OP_AND,
-			 Xapian::Query(Xapian::Query::OP_VALUE_GE, 0, Xapian::sortable_serialise ((double) from)),
-			 Xapian::Query(Xapian::Query::OP_VALUE_LE, 0, Xapian::sortable_serialise ((double) to)));
+    return _make_query (slot, from, to);
 }
-#endif
diff --git a/lib/parse-time-vrp.h b/lib/parse-time-vrp.h
index c024dba2..f495e716 100644
--- a/lib/parse-time-vrp.h
+++ b/lib/parse-time-vrp.h
@@ -26,20 +26,21 @@
 #include <xapian.h>
 
 /* see *ValueRangeProcessor in xapian-core/include/xapian/queryparser.h */
-class ParseTimeValueRangeProcessor : public Xapian::ValueRangeProcessor {
-protected:
-    Xapian::valueno valno;
+class ParseTimeRangeProcessor : public Xapian::RangeProcessor {
 
 public:
-    ParseTimeValueRangeProcessor (Xapian::valueno slot_)
-	: valno(slot_) { }
+    ParseTimeRangeProcessor (Xapian::valueno slot_, const std::string prefix_)
+	:  Xapian::RangeProcessor(slot_, prefix_, 0) { }
 
-    Xapian::valueno operator() (std::string &begin, std::string &end);
+    Xapian::Query operator() (const std::string &begin, const std::string &end);
 };
 
-#if HAVE_XAPIAN_FIELD_PROCESSOR
 class DateFieldProcessor : public Xapian::FieldProcessor {
+private:
+    Xapian::valueno slot;
+public:
+    DateFieldProcessor(Xapian::valueno slot_) : slot(slot_) { };
     Xapian::Query operator()(const std::string & str);
 };
-#endif
+
 #endif /* NOTMUCH_PARSE_TIME_VRP_H */
-- 
2.20.1

Re: Drop support for pre-1.4 Xapian, prepare for 1.5.x

[David Bremner]

David Bremner <david@tethera.net> writes:

> The current development release of Xapian drops several deprecated
> APIs. This hampers testing of notmuch against Xapian master (i.e. to
> see if a problem has been fixed). This series fixes most of the
> incompatibilities.
>
> The one thing this series doesn't handle is test/T530-upgrade.sh,
> which mostly fails because Xapian 1.5 / master drops support for
> Chert. I think that we should probably deprecate support for notmuch
> v1 databases. These were replaced by version 2 in 2010. I think more
> thought is probably needed for this migration so I left those failing
> tests for now.  This isn't quite as scary as it sounds, because we
> really only support stable releases of Xapian, so the chert removal
> effectively won't happen until 1.6.
>

I forgot, there's one other failing test with xapian 1.5

T650-regexp-query: Testing regular expression searches
 FAIL   null from: search
	--- T650-regexp-query.7.EXPECTED	2019-04-15 11:03:10.128070898 +0000
	+++ T650-regexp-query.7.OUTPUT	2019-04-15 11:03:10.128070898 +0000
	@@ -1 +0,0 @@
	-thread:XXX   2001-01-05 [1/1] -; empty from (inbox unread)

Re: Drop support for pre-1.4 Xapian, prepare for 1.5.x

[David Bremner]

David Bremner <david@tethera.net> writes:

> David Bremner <david@tethera.net> writes:
>
>> The current development release of Xapian drops several deprecated
>> APIs. This hampers testing of notmuch against Xapian master (i.e. to
>> see if a problem has been fixed). This series fixes most of the
>> incompatibilities.
>>
>> The one thing this series doesn't handle is test/T530-upgrade.sh,
>> which mostly fails because Xapian 1.5 / master drops support for
>> Chert. I think that we should probably deprecate support for notmuch
>> v1 databases. These were replaced by version 2 in 2010. I think more
>> thought is probably needed for this migration so I left those failing
>> tests for now.  This isn't quite as scary as it sounds, because we
>> really only support stable releases of Xapian, so the chert removal
>> effectively won't happen until 1.6.
>>
>
> I forgot, there's one other failing test with xapian 1.5
>
> T650-regexp-query: Testing regular expression searches
>  FAIL   null from: search
> 	--- T650-regexp-query.7.EXPECTED	2019-04-15 11:03:10.128070898 +0000
> 	+++ T650-regexp-query.7.OUTPUT	2019-04-15 11:03:10.128070898 +0000
> 	@@ -1 +0,0 @@
> 	-thread:XXX   2001-01-05 [1/1] -; empty from (inbox unread)

I'm continuing to investigate this. The following program finds one
result (in tmp.T650-regexp-query) in 1.4 and 0 results in Xapian
master. I'd give about 50% odds that I'm just doing something silly.

#include <xapian.h>
#include <iostream>

int main(int argc, char **argv) {

    Xapian::Database db("mail/.notmuch/xapian");
    Xapian::Query query(Xapian::Query::OP_AND_NOT,
			Xapian::Query("Tmail"),
			Xapian::Query(Xapian::Query::OP_WILDCARD, "XFROM", 0,
				     Xapian::Query::WILDCARD_LIMIT_ERROR, Xapian::Query::OP_SYNONYM));

    Xapian::Enquire enquire(db);

    enquire.set_query(query);
    
    Xapian::MSet mset = enquire.get_mset(0,1000000);

    std::cout << "mset " << mset.get_description() << std::endl;
    
    for (auto i = mset.begin(); i != mset.end(); ++i) {
    	std::cout << (*i) << std::endl;
    }
}

Re: Drop support for pre-1.4 Xapian, prepare for 1.5.x

[David Bremner]

David Bremner <david@tethera.net> writes:

> David Bremner <david@tethera.net> writes:
>
>>
>> I forgot, there's one other failing test with xapian 1.5
>>
>> T650-regexp-query: Testing regular expression searches
>>  FAIL   null from: search
>> 	--- T650-regexp-query.7.EXPECTED	2019-04-15 11:03:10.128070898 +0000
>> 	+++ T650-regexp-query.7.OUTPUT	2019-04-15 11:03:10.128070898 +0000
>> 	@@ -1 +0,0 @@
>> 	-thread:XXX   2001-01-05 [1/1] -; empty from (inbox unread)
>
> I'm continuing to investigate this. The following program finds one
> result (in tmp.T650-regexp-query) in 1.4 and 0 results in Xapian
> master. I'd give about 50% odds that I'm just doing something silly.
>

This turned out to be a Xapian bug, fixed in commit f513d191f9e6.

Thanks to Olly Betts for the quick fix.

d