From: Carl Worth <cworth@cworth.org>
To: Olly Betts <olly@survex.com>, notmuch@notmuchmail.org
Subject: Re: Failing test cases
In-Reply-To: <87zl0fmst4.fsf@yoom.home.cworth.org>
References: <20100425001743.GA9293@jdc.jasonjgw.net>
	<20100428002615.GA10022@jdc.jasonjgw.net>
	<87wrvsqti6.fsf@yoom.home.cworth.org>
	<20100428012547.GA680@jdc.jasonjgw.net>
	<slrnhtkc84.pf1.olly@msgid.survex.com>
	<87zl0fmst4.fsf@yoom.home.cworth.org>
User-Agent: Notmuch/0.3-7-g2baa576 (http://notmuchmail.org) Emacs/23.1.1
	(i486-pc-linux-gnu)
Date: Tue, 04 May 2010 12:09:56 -0700
Message-ID: <87wrvjmqkb.fsf@yoom.home.cworth.org>
MIME-Version: 1.0
Content-Type: multipart/signed; boundary="=-=-=";
	micalg=pgp-sha1; protocol="application/pgp-signature"
Precedence: list

--=-=-=
Content-Transfer-Encoding: quoted-printable

On Tue, 04 May 2010 11:21:27 -0700, Carl Worth <cworth@cworth.org> wrote:
> I installed Xapian 1.0.20 (from the libxapian-dev package in sid) and am
> now getting this failure.
>=20
> 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?

=2DCarl

diff --git a/notmuch-new.c b/notmuch-new.c
index 8818728..928bb94 100644
=2D-- 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) =3D=3D 0)
 	{
=2D	    notmuch_filenames_move_to_next (db_files);
+	    while (notmuch_filenames_valid (db_files) &&
+		   strcmp (notmuch_filenames_get (db_files), entry->d_name) <=3D 0)
+	    {
+		notmuch_filenames_move_to_next (db_files);
+	    }
 	    continue;
 	}
=20


=2D-=20
carl.d.worth@intel.com

--=-=-=
Content-Type: application/pgp-signature

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)

iD8DBQFL4HEE6JDdNq8qSWgRAnlcAJ9XL4VqTAbkPVzX8xTDGM3G2X0ZeQCffdux
m3fmvhDiXuJHbO7lUKue1mw=
=X3CT
-----END PGP SIGNATURE-----
--=-=-=--