Re: [PATCH 0/2] A bug in the exclude code

Subject: Re: [PATCH 0/2] A bug in the exclude code

Date: Mon, 12 Mar 2012 21:07:49 +0000

To: Jameson Graef Rollins, notmuch@notmuchmail.org

Cc:

From: Mark Walters


On Mon, 12 Mar 2012 13:03:12 -0700, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> On Mon, 12 Mar 2012 11:31:52 +0000, Mark Walters <markwalters1009@gmail.com> wrote:
> > There is a bug in the exclude code (found by jrollins in the
> > --with-excluded series) but also present in master.  None of the
> > current tests were finding it so the first patch adds two tests.
> 
> Hey, Mark.  Thanks so much for looking in to this.  It look like this
> patch fixes the issue for me too!  Very excited to see this all coming
> together.
> 
> > The bug (and test failure) do not appear in all configuations: on my
> > main test machine (an oldish debian testing 32bit userspace with a
> > 64bit kernel and xapian 1.2.7) all tests pass. On my laptop (a recent
> > debian testing 64bit userspace and xapian 1.2.8) one of the new tests
> > fails.
> 
> It was failing for me too, but looking closer at the test I actually
> found a bug: you accidentally used the old style --no-exclude option,
> instead of the new --with-excludes.  When you fix the call all the tests
> pass fine.

That's great.

> > The second patch fixes the behaviour for me but I don't see why it
> > should make a difference: searches for A and not B should give the
> > same results as A and not (A and B). It could be a bug in xapian, it
> > could be that I am not allowed to reuse queries as I do (is query1 =
> > query1 and query2 allowed?) or it could be some memory use bug on my
> > part.
> 
> I can't explain it either, but there's certainly a lot about xapian that
> I don't understand.  Maybe one of the xapian gurus will have some ideas
> (Olly?  Austin?).
> 
> Anyway, thanks again for pushing on all of this, Mark.
> 
> jamie.
> 
> PS. Not a big deal, but it would have been nice for this patch set to
> have been sent in-reply-to the original series it fixes, just to keep
> everything together.

Just to emphasise the bug is already present in current master (just
better hidden because of the defaults). Hence this pair of patches
(unlike the first one I sent privately) are to current master rather
than to the exclude the series (though they apply there to to modulo the
minor change you mention).

Best wishes

Mark


Thread: