[PATCH] lib: Simplify close and codify aborting atomic section

2014-09-24 Thread David Bremner
"W. Trevor King" writes: > On Wed, Sep 24, 2014 at 09:25:20PM +0200, David Bremner wrote: >> I think the fact that you have to close the notmuch database (when >> not using begin/end atomic) to get a commit is surprising for many >> people, so it would be nice to make that clearer somehow. > >

[PATCH] lib: Simplify close and codify aborting atomic section

2014-09-24 Thread David Bremner
"W. Trevor King" writes: > > Sorry, I didn't phrase that very well. The notmuch docs (as of this > patch) explain that we don't commit if we're in an atomic block. The > Xapian docs also say that, *and* they say that if we're not in atomic > block the close *does* try to commit. I think

[PATCH] lib: Simplify close and codify aborting atomic section

2014-09-24 Thread David Bremner
"W. Trevor King" writes: > Ah, I thought the implicit flush/commit was just in our code. Since > it's also in the underlying Xapian close, then this patch looks pretty > good to me. I'd mention Xapian's explicit close in the notmuch. h > message. Xapain's docs say [1]: > > For a

[PATCH] lib: Simplify close and codify aborting atomic section

2014-09-24 Thread W. Trevor King
On Wed, Sep 24, 2014 at 10:13:31PM +0200, David Bremner wrote: > W. Trevor King writes: > I think it would be better to write our own, not because of licensing > issues, but because the user of the notmuch API won't know what a xapian > commit is. Between version control and databases, I feel

[PATCH] lib: Simplify close and codify aborting atomic section

2014-09-24 Thread W. Trevor King
On Wed, Sep 24, 2014 at 09:25:20PM +0200, David Bremner wrote: > I think the fact that you have to close the notmuch database (when > not using begin/end atomic) to get a commit is surprising for many > people, so it would be nice to make that clearer somehow. It looks like Xapian is GPLv2+, so

[PATCH] lib: Simplify close and codify aborting atomic section

2014-09-24 Thread W. Trevor King
On Wed, Sep 24, 2014 at 08:09:27PM +0200, David Bremner wrote: > W. Trevor King writes: > > Ah, I thought the implicit flush/commit was just in our code. > > Since it's also in the underlying Xapian close, then this patch > > looks pretty good to me. I'd mention Xapian's explicit close in > > the

Re: [PATCH] lib: Simplify close and codify aborting atomic section

2014-09-24 Thread David Bremner
W. Trevor King wk...@tremily.us writes: Ah, I thought the implicit flush/commit was just in our code. Since it's also in the underlying Xapian close, then this patch looks pretty good to me. I'd mention Xapian's explicit close in the notmuch. h message. Xapain's docs say [1]: For a

Re: [PATCH] lib: Simplify close and codify aborting atomic section

2014-09-24 Thread David Bremner
W. Trevor King wk...@tremily.us writes: Sorry, I didn't phrase that very well. The notmuch docs (as of this patch) explain that we don't commit if we're in an atomic block. The Xapian docs also say that, *and* they say that if we're not in atomic block the close *does* try to commit. I

Re: [PATCH] lib: Simplify close and codify aborting atomic section

2014-09-24 Thread W. Trevor King
On Wed, Sep 24, 2014 at 09:25:20PM +0200, David Bremner wrote: I think the fact that you have to close the notmuch database (when not using begin/end atomic) to get a commit is surprising for many people, so it would be nice to make that clearer somehow. It looks like Xapian is GPLv2+, so we

Re: [PATCH] lib: Simplify close and codify aborting atomic section

2014-09-24 Thread David Bremner
W. Trevor King wk...@tremily.us writes: On Wed, Sep 24, 2014 at 09:25:20PM +0200, David Bremner wrote: I think the fact that you have to close the notmuch database (when not using begin/end atomic) to get a commit is surprising for many people, so it would be nice to make that clearer

Re: [PATCH] lib: Simplify close and codify aborting atomic section

2014-09-24 Thread W. Trevor King
On Wed, Sep 24, 2014 at 10:13:31PM +0200, David Bremner wrote: W. Trevor King writes: I think it would be better to write our own, not because of licensing issues, but because the user of the notmuch API won't know what a xapian commit is. Between version control and databases, I feel like

[PATCH] lib: Simplify close and codify aborting atomic section

2014-09-22 Thread Austin Clements
Quoth W. Trevor King on Sep 22 at 9:59 am: > On Mon, Sep 22, 2014 at 11:43:35AM -0400, Austin Clements wrote: > > This patch simplifies notmuch_database_close to just call > > Database::close. This works for both read-only and read/write > > databases, takes care of committing changes, unifies

[PATCH] lib: Simplify close and codify aborting atomic section

2014-09-22 Thread W. Trevor King
On Mon, Sep 22, 2014 at 06:50:50PM +, Austin Clements wrote: > Quoth W. Trevor King on Sep 22 at 9:59 am: > > On Mon, Sep 22, 2014 at 11:43:35AM -0400, Austin Clements wrote: > > > This patch simplifies notmuch_database_close to just call > > > Database::close. This works for both read-only

[PATCH] lib: Simplify close and codify aborting atomic section

2014-09-22 Thread Austin Clements
In Xapian, closing a database implicitly aborts any outstanding transaction and commits changes. For historical reasons, notmuch_database_close had grown to almost, but not quite duplicate this behavior. Before closing the database, it would explicitly (and unnecessarily) commit it. However, if

[PATCH] lib: Simplify close and codify aborting atomic section

2014-09-22 Thread W. Trevor King
On Mon, Sep 22, 2014 at 11:43:35AM -0400, Austin Clements wrote: > This patch simplifies notmuch_database_close to just call > Database::close. This works for both read-only and read/write > databases, takes care of committing changes, unifies the exception > handling path, and codifies aborting

[PATCH] lib: Simplify close and codify aborting atomic section

2014-09-22 Thread Austin Clements
In Xapian, closing a database implicitly aborts any outstanding transaction and commits changes. For historical reasons, notmuch_database_close had grown to almost, but not quite duplicate this behavior. Before closing the database, it would explicitly (and unnecessarily) commit it. However, if

Re: [PATCH] lib: Simplify close and codify aborting atomic section

2014-09-22 Thread W. Trevor King
On Mon, Sep 22, 2014 at 11:43:35AM -0400, Austin Clements wrote: This patch simplifies notmuch_database_close to just call Database::close. This works for both read-only and read/write databases, takes care of committing changes, unifies the exception handling path, and codifies aborting

Re: [PATCH] lib: Simplify close and codify aborting atomic section

2014-09-22 Thread Austin Clements
Quoth W. Trevor King on Sep 22 at 9:59 am: On Mon, Sep 22, 2014 at 11:43:35AM -0400, Austin Clements wrote: This patch simplifies notmuch_database_close to just call Database::close. This works for both read-only and read/write databases, takes care of committing changes, unifies the

Re: [PATCH] lib: Simplify close and codify aborting atomic section

2014-09-22 Thread W. Trevor King
On Mon, Sep 22, 2014 at 06:50:50PM +, Austin Clements wrote: Quoth W. Trevor King on Sep 22 at 9:59 am: On Mon, Sep 22, 2014 at 11:43:35AM -0400, Austin Clements wrote: This patch simplifies notmuch_database_close to just call Database::close. This works for both read-only and