Failing test cases

[Carl Worth]

On Tue, 04 May 2010 11:21:27 -0700, Carl Worth <cworth at cworth.org> wrote:
> I installed Xapian 1.0.20 (from the libxapian-dev package in sid) and am
> now getting this failure.
> 
> I'm investigating more closely now, but I thought I'd at least share
> that much information.

Here are my conclusions based on some additional investigation:

What's happening here is that notmuch is creating a Xapian TermIterator,
iterating over some of the results, and then modifying the database such
that the terms being iterated over are affected. Specifically, in this
test case, notmuch is inserting a new term before the term being pointed
at by the iterator.

With Xapian 1.0.18 incrementing the iterator from this state causes it
to point to the next term. However, with Xapian 1.0.20 this same
increment causes the iterator to point at the same term again.

I was able to put a little workaround into notmuch-new.c that avoids the
bug (see below). But this isn't entirely satisfactory for several
reasons:

1. We need to know what the intended semantics of the Xapian iterators
   really are. If this behavior is just a bug in Xapian, then that
   should be fixed.

   But if the iterators are to have explicitly undefined behavior in
   cases like this, then notmuch is going to need to carefully copy
   contents out of Xapian iterators when creating notmuch iterators (at
   some performance cost).

2. Working around the bug in notmuch-new.c doesn't fix the notmuch
   library itself, (so that python bindings, etc. will remain
   broken). So we really do need a lower-level fix for this.

To properly do the lower-level fix for (2), we'll need to know the
answer to (1). Olly, do you have an answer on the intended semantics of
Xapian iterators?

-Carl

diff --git a/notmuch-new.c b/notmuch-new.c
index 8818728..928bb94 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -380,7 +380,11 @@ add_files_recursive (notmuch_database_t *notmuch,
 	if (notmuch_filenames_valid (db_files) &&
 	    strcmp (notmuch_filenames_get (db_files), entry->d_name) == 0)
 	{
-	    notmuch_filenames_move_to_next (db_files);
+	    while (notmuch_filenames_valid (db_files) &&
+		   strcmp (notmuch_filenames_get (db_files), entry->d_name) <= 0)
+	    {
+		notmuch_filenames_move_to_next (db_files);
+	    }
 	    continue;
 	}
 


-- 
carl.d.worth at intel.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://notmuchmail.org/pipermail/notmuch/attachments/20100504/ec0fba7b/attachment.pgp>