[PATCH 3/7] go: Allow notmuch objects to be garbage collected
Le 19/10/2012 06:55, Ethan Glasser-Camp a ?crit : > Adrien Bustany writes: > >> This makes notmuch appropriately free the underlying notmuch C objects >> when garbage collecting their Go wrappers. To make sure we don't break >> the underlying links between objects (for example, a notmuch_messages_t >> being GC'ed before a notmuch_message_t belonging to it), we add for each >> wraper struct a pointer to the owner object (Go objects with a reference >> pointing to them don't get garbage collected). > > Hi Adrien! This whole series is marked moreinfo, but I don't think > that's just. It looks like there were some unresolved issues about > reference tracking and garbage collection, and some suggestions to use > the C values of enums instead of regenerating them with iota, but > there's definitely valid code that I assume would be useful if anyone > ever wanted to write in Go ;). Are you figuring to clean this series up? > > This comment should s/wraper/wrapper/. > > Ethan > Hello Ethan, thanks for the heads up, I still have this on my table, and yes there is additional work to do for the patches to be really clean. I can't give an estimate for now, let's hope sooner than later :/ Cheers Adrien
[PATCH v2 0/2] Add flush/reopen methods to notmuch_database_t
Le 20/10/2012 18:49, Ethan Glasser-Camp a ?crit : > Jani Nikula writes: > >> On Wed, 17 Oct 2012, Adrien Bustany wrote: >>> The code of the patches in unchanged, but the formatting issues are now >>> hopefully fixed. >> >> Hi Adrien, please check at what version flush and reopen have been >> introduced to xapian. If they are new-ish (I don't know, didn't have the >> time to check), please add appropriate #ifdefs. [1] lays the groundwork >> for this. We'll also need to decide what is the minimum xapian version >> required in general, i.e. features earlier than that don't need >> conditional compilation. > > Hi! The new versions of these patches are still pretty trivial and they > still look OK to me, but based on Jani's prompting I decided to look up > the methods. Seems that flush() is a very old (pre-1.1.0, 2009-04) name > for commit(), which is the preferred name these days. You should > probably therefore rename the function notmuch_database_commit, and have > it call the WritableDatabase::commit() method. > > reopen() is a very very old method, seems like it has been around since > 2004. > > So I think Adrien is safe from having to do version checks, but we > should probably use commit() instead of flush(). > > Ethan > Thanks for checking that! Sorry for the late answer, I had a hard time keeping on top of things lately...
Re: [PATCH 3/7] go: Allow notmuch objects to be garbage collected
Le 19/10/2012 06:55, Ethan Glasser-Camp a écrit : Adrien Bustany writes: This makes notmuch appropriately free the underlying notmuch C objects when garbage collecting their Go wrappers. To make sure we don't break the underlying links between objects (for example, a notmuch_messages_t being GC'ed before a notmuch_message_t belonging to it), we add for each wraper struct a pointer to the owner object (Go objects with a reference pointing to them don't get garbage collected). Hi Adrien! This whole series is marked moreinfo, but I don't think that's just. It looks like there were some unresolved issues about reference tracking and garbage collection, and some suggestions to use the C values of enums instead of regenerating them with iota, but there's definitely valid code that I assume would be useful if anyone ever wanted to write in Go ;). Are you figuring to clean this series up? This comment should s/wraper/wrapper/. Ethan Hello Ethan, thanks for the heads up, I still have this on my table, and yes there is additional work to do for the patches to be really clean. I can't give an estimate for now, let's hope sooner than later :/ Cheers Adrien ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH v2 0/2] Add flush/reopen methods to notmuch_database_t
Le 20/10/2012 18:49, Ethan Glasser-Camp a écrit : Jani Nikula writes: On Wed, 17 Oct 2012, Adrien Bustany wrote: The code of the patches in unchanged, but the formatting issues are now hopefully fixed. Hi Adrien, please check at what version flush and reopen have been introduced to xapian. If they are new-ish (I don't know, didn't have the time to check), please add appropriate #ifdefs. [1] lays the groundwork for this. We'll also need to decide what is the minimum xapian version required in general, i.e. features earlier than that don't need conditional compilation. Hi! The new versions of these patches are still pretty trivial and they still look OK to me, but based on Jani's prompting I decided to look up the methods. Seems that flush() is a very old (pre-1.1.0, 2009-04) name for commit(), which is the preferred name these days. You should probably therefore rename the function notmuch_database_commit, and have it call the WritableDatabase::commit() method. reopen() is a very very old method, seems like it has been around since 2004. So I think Adrien is safe from having to do version checks, but we should probably use commit() instead of flush(). Ethan Thanks for checking that! Sorry for the late answer, I had a hard time keeping on top of things lately... ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 2/2] Add notmuch_database_reopen method
Calling notmuch_database_reopen is needed to refresh the database contents when the database on disk was modified by another notmuch_database_t instance, for example in a different thread. --- lib/database.cc | 17 + lib/notmuch.h | 8 2 files changed, 25 insertions(+) diff --git a/lib/database.cc b/lib/database.cc index 1b680ab..f27d57f 100644 --- a/lib/database.cc +++ b/lib/database.cc @@ -763,6 +763,23 @@ notmuch_database_flush(notmuch_database_t *notmuch) return status; } +notmuch_status_t +notmuch_database_reopen(notmuch_database_t *notmuch) +{ +notmuch_status_t status = NOTMUCH_STATUS_SUCCESS; + +try { + if (notmuch->xapian_db != NULL) + (notmuch->xapian_db)->reopen (); +} catch (const Xapian::Error &error) { + fprintf(stderr, "A Xapian exception occured reopening the database: %s\n", + error.get_msg().c_str()); + status = NOTMUCH_STATUS_XAPIAN_EXCEPTION; +} + +return status; +} + void notmuch_database_close (notmuch_database_t *notmuch) { diff --git a/lib/notmuch.h b/lib/notmuch.h index aef5c56..51d6a9a 100644 --- a/lib/notmuch.h +++ b/lib/notmuch.h @@ -205,6 +205,14 @@ notmuch_database_open (const char *path, notmuch_status_t notmuch_database_flush (notmuch_database_t *database); +/* Refresh the database contents to the latest version. + * + * This is needed only if another instance of notmuch_database_t has + * modified the database contents on disk. + */ +notmuch_status_t +notmuch_database_reopen (notmuch_database_t *database); + /* Close the given notmuch database. * * After notmuch_database_close has been called, calls to other -- 1.7.11.7
[PATCH 1/2] Add notmuch_database_flush method
This method explicitly flushes the pending modifications to disk. It is useful if your program has various threads, each with a read only DB and one writer thread with a read/write DB. In that case, you most likely want the writer to sync the changes to disk so that the readers can see them, without having to close and reopen the database completely. --- lib/database.cc | 18 ++ lib/notmuch.h | 4 2 files changed, 22 insertions(+) diff --git a/lib/database.cc b/lib/database.cc index 761dc1a..1b680ab 100644 --- a/lib/database.cc +++ b/lib/database.cc @@ -745,6 +745,24 @@ notmuch_database_open (const char *path, return status; } +notmuch_status_t +notmuch_database_flush(notmuch_database_t *notmuch) +{ +notmuch_status_t status = NOTMUCH_STATUS_SUCCESS; + +try { + if (notmuch->xapian_db != NULL && + notmuch->mode == NOTMUCH_DATABASE_MODE_READ_WRITE) + (static_cast (notmuch->xapian_db))->flush (); +} catch (const Xapian::Error &error) { + fprintf(stderr, "A Xapian exception occured flushing the database: %s\n", + error.get_msg().c_str()); + status = NOTMUCH_STATUS_XAPIAN_EXCEPTION; +} + +return status; +} + void notmuch_database_close (notmuch_database_t *notmuch) { diff --git a/lib/notmuch.h b/lib/notmuch.h index 3633bed..aef5c56 100644 --- a/lib/notmuch.h +++ b/lib/notmuch.h @@ -201,6 +201,10 @@ notmuch_database_open (const char *path, notmuch_database_mode_t mode, notmuch_database_t **database); +/* Flushes all the pending modifications to the database to disk. */ +notmuch_status_t +notmuch_database_flush (notmuch_database_t *database); + /* Close the given notmuch database. * * After notmuch_database_close has been called, calls to other -- 1.7.11.7
[PATCH v2 0/2] Add flush/reopen methods to notmuch_database_t
The code of the patches in unchanged, but the formatting issues are now hopefully fixed.
[PATCH 1/2] Add notmuch_database_flush method
Le 17/10/2012 18:53, Ethan Glasser-Camp a ?crit : > Adrien Bustany writes: > >> This method explicitly flushes the pending modifications to disk. It is >> useful if your program has various threads, each with a read only DB and >> one writer thread with a read/write DB. In that case, you most likely >> want the writer to sync the changes to disk so that the readers can see >> them, without having to close and reopen the database completely. > > These patches are pretty straightforward. But to conform to notmuch style.. > >> +notmuch_status_t >> +notmuch_database_flush(notmuch_database_t *notmuch) >> +{ >> +notmuch_status_t status = NOTMUCH_STATUS_SUCCESS; > > Indent is 4 spaces. (You have tabs here, which are 8 spaces, according > to devel/STYLE.) > >> +try { >> +if (notmuch->xapian_db != NULL && > > if should be more indented than try. (So when you pull try back to 4 > spaces, leave if at 8 spaces.) Sorry about that... I think I copied the style from notmuch_database_close, but my editor was set to have 4-space tabs, so I got confused. I'll send a fixed version of the patches. > >> +notmuch->mode == NOTMUCH_DATABASE_MODE_READ_WRITE) >> +(static_cast >> (notmuch->xapian_db))->flush (); > > This line is 90 characters, and will remain at 86 once you indent using > the in-house style. I'm not sure if it's worth reformatting. > notmuch_database_close calls flush() using exactly the same 86-character > line. I'd say "don't make it worse", but personally I think breaking > this line might be worse. > Yes, I also think breaking this line will not make things prettier :-/ > Ethan > Thanks for the review! Adrien
[PATCH 2/2] Add notmuch_database_reopen method
Calling notmuch_database_reopen is needed to refresh the database contents when the database on disk was modified by another notmuch_database_t instance, for example in a different thread. --- lib/database.cc | 17 + lib/notmuch.h | 8 2 files changed, 25 insertions(+) diff --git a/lib/database.cc b/lib/database.cc index 1b680ab..f27d57f 100644 --- a/lib/database.cc +++ b/lib/database.cc @@ -763,6 +763,23 @@ notmuch_database_flush(notmuch_database_t *notmuch) return status; } +notmuch_status_t +notmuch_database_reopen(notmuch_database_t *notmuch) +{ +notmuch_status_t status = NOTMUCH_STATUS_SUCCESS; + +try { + if (notmuch->xapian_db != NULL) + (notmuch->xapian_db)->reopen (); +} catch (const Xapian::Error &error) { + fprintf(stderr, "A Xapian exception occured reopening the database: %s\n", + error.get_msg().c_str()); + status = NOTMUCH_STATUS_XAPIAN_EXCEPTION; +} + +return status; +} + void notmuch_database_close (notmuch_database_t *notmuch) { diff --git a/lib/notmuch.h b/lib/notmuch.h index aef5c56..51d6a9a 100644 --- a/lib/notmuch.h +++ b/lib/notmuch.h @@ -205,6 +205,14 @@ notmuch_database_open (const char *path, notmuch_status_t notmuch_database_flush (notmuch_database_t *database); +/* Refresh the database contents to the latest version. + * + * This is needed only if another instance of notmuch_database_t has + * modified the database contents on disk. + */ +notmuch_status_t +notmuch_database_reopen (notmuch_database_t *database); + /* Close the given notmuch database. * * After notmuch_database_close has been called, calls to other -- 1.7.11.7 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 1/2] Add notmuch_database_flush method
This method explicitly flushes the pending modifications to disk. It is useful if your program has various threads, each with a read only DB and one writer thread with a read/write DB. In that case, you most likely want the writer to sync the changes to disk so that the readers can see them, without having to close and reopen the database completely. --- lib/database.cc | 18 ++ lib/notmuch.h | 4 2 files changed, 22 insertions(+) diff --git a/lib/database.cc b/lib/database.cc index 761dc1a..1b680ab 100644 --- a/lib/database.cc +++ b/lib/database.cc @@ -745,6 +745,24 @@ notmuch_database_open (const char *path, return status; } +notmuch_status_t +notmuch_database_flush(notmuch_database_t *notmuch) +{ +notmuch_status_t status = NOTMUCH_STATUS_SUCCESS; + +try { + if (notmuch->xapian_db != NULL && + notmuch->mode == NOTMUCH_DATABASE_MODE_READ_WRITE) + (static_cast (notmuch->xapian_db))->flush (); +} catch (const Xapian::Error &error) { + fprintf(stderr, "A Xapian exception occured flushing the database: %s\n", + error.get_msg().c_str()); + status = NOTMUCH_STATUS_XAPIAN_EXCEPTION; +} + +return status; +} + void notmuch_database_close (notmuch_database_t *notmuch) { diff --git a/lib/notmuch.h b/lib/notmuch.h index 3633bed..aef5c56 100644 --- a/lib/notmuch.h +++ b/lib/notmuch.h @@ -201,6 +201,10 @@ notmuch_database_open (const char *path, notmuch_database_mode_t mode, notmuch_database_t **database); +/* Flushes all the pending modifications to the database to disk. */ +notmuch_status_t +notmuch_database_flush (notmuch_database_t *database); + /* Close the given notmuch database. * * After notmuch_database_close has been called, calls to other -- 1.7.11.7 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v2 0/2] Add flush/reopen methods to notmuch_database_t
The code of the patches in unchanged, but the formatting issues are now hopefully fixed. ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 1/2] Add notmuch_database_flush method
Le 17/10/2012 18:53, Ethan Glasser-Camp a écrit : Adrien Bustany writes: This method explicitly flushes the pending modifications to disk. It is useful if your program has various threads, each with a read only DB and one writer thread with a read/write DB. In that case, you most likely want the writer to sync the changes to disk so that the readers can see them, without having to close and reopen the database completely. These patches are pretty straightforward. But to conform to notmuch style.. +notmuch_status_t +notmuch_database_flush(notmuch_database_t *notmuch) +{ + notmuch_status_t status = NOTMUCH_STATUS_SUCCESS; Indent is 4 spaces. (You have tabs here, which are 8 spaces, according to devel/STYLE.) + try { + if (notmuch->xapian_db != NULL && if should be more indented than try. (So when you pull try back to 4 spaces, leave if at 8 spaces.) Sorry about that... I think I copied the style from notmuch_database_close, but my editor was set to have 4-space tabs, so I got confused. I'll send a fixed version of the patches. + notmuch->mode == NOTMUCH_DATABASE_MODE_READ_WRITE) + (static_cast (notmuch->xapian_db))->flush (); This line is 90 characters, and will remain at 86 once you indent using the in-house style. I'm not sure if it's worth reformatting. notmuch_database_close calls flush() using exactly the same 86-character line. I'd say "don't make it worse", but personally I think breaking this line might be worse. Yes, I also think breaking this line will not make things prettier :-/ Ethan Thanks for the review! Adrien ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 3/7] go: Allow notmuch objects to be garbage collected
Le 20/07/2012 06:23, Austin Clements a ?crit : > Quoth Adrien Bustany on Jul 19 at 9:25 pm: >> Le 18/07/2012 23:40, Austin Clements a ?crit : >>> This is subtle enough that I think it deserves a comment in the source >>> code explaining that tracking the talloc owner reference, combined >>> with the fact that Go finalizers are run in dependency order, ensures >>> that the C objects will always be destroyed from the talloc leaves up. >> >> Definitely, I tend to comment in the commit message and forget about >> the code... >> >>> >>> Just one inline comment below. Otherwise, I think this is all >>> correct. >> >> Agree with the comment, the Database should be the parent. I guess I >> wasn't sure of the talloc parenting. >> >>> >>> Is reproducing the talloc hierarchy in all of the bindings really the >>> right approach? I feel like there has to be a better way (or that the >>> way we use talloc in the library is slightly broken). What if the >>> bindings created an additional talloc reference to each managed >>> object, just to keep the object alive, and used talloc_unlink instead >>> of the destroy functions? >> >> Reproducing the hierarchy is probably error prone, and not that >> simple indeed :/ >> I haven't checked at all the way you suggest, but if we use >> talloc_reference/unlink, we get the same issue no? >> - If we do for each new wrapped object talloc_reference(NULL, >> wrapped_object), the the object will be kept alive until we >> talloc_unlink(NULL, wrapped_object), but what about its parents? For >> example will doing that on a notmuch_message_t keep the >> notmuch_messages_t alive? > > Hmm. This is what I was thinking. You have an interesting point; I > think it's slightly wrong, but it exposes something deeper. I believe > there are two different things going on here: some of the talloc > relationships are for convenience, while some are structural. In the > former case, I'm pretty sure my suggestion will work, but in the > latter case the objects should *never* be freed by the finalizer! > > For example, notmuch_query_search_messages returns a new > notmuch_messages_t with the query as the talloc parent, but that > notmuch_messages_t doesn't depend on the query object; this is just so > you can conveniently delete everything retrieved from the query by > deleting the query. In this case, you can either use parent > references like you did---which will prevent a double-free by forcing > destruction to happen from the leaves up but at the cost of having to > encode these relationships and of extending the parent object > lifetimes beyond what's strictly necessary---or you can use my > suggestion of creating an additional talloc reference. Actually, checking the code of notmuch_query_search_messages, it seems that the notmuch_messages_t (and the notmuch_message_t as well) object *does* depend on the database and the query... So in that case I think we need the "owner" Object reference as I currently have (we want the Messages to keep the Query alive, and the Query keeps the Database alive). That said, you example below looks valid, and it seems I'll need to add a flag to createMessage() (and some others) to disable the SetFinalizer call for certain instances (we probably want to keep it for eg. SearchMessageByFilename). - The candidates I found for adding a tmalloc reference and not a "full" Go reference (therefore preventing to keep the parent alive too long needlessly) are GetAllTags, Thread.GetTags, Messages.CollectTags, and Message.GetTags (those are basically string lists) - The methods for which I should remove the SetFinalizer on the wrapper (as you showed in the example below) while keeping the Go reference are Threads.Get and Messages.Get I would also maybe remove all the Destroy() functions, since they now seem more dangerous than anything else... I tried to write a test using runtime.GC to test the behaviour of the bindings, but for some reasons some cases which are supposed to crash don't, which makes me sceptical about the validity of the test :-/ Cheers Adrien > > However, in your example, the notmuch_message_t's are structurally > related to the notmuch_messages_t from whence they came. They're all > part of one data structure and hence it *never* makes sense for a > caller to delete the notmuch_message_t's. For example, even with the > code in this patch, I think the following could lead to a crash: > > 1. Obtain a Messages object, say ms. > 2. m1 := ms.Get() > 3. m1 = nil > 4. m2 := ms.Get() > 5. m2.whatever() > > If a garbage collection h
Re: [PATCH 3/7] go: Allow notmuch objects to be garbage collected
Le 20/07/2012 06:23, Austin Clements a écrit : Quoth Adrien Bustany on Jul 19 at 9:25 pm: Le 18/07/2012 23:40, Austin Clements a écrit : This is subtle enough that I think it deserves a comment in the source code explaining that tracking the talloc owner reference, combined with the fact that Go finalizers are run in dependency order, ensures that the C objects will always be destroyed from the talloc leaves up. Definitely, I tend to comment in the commit message and forget about the code... Just one inline comment below. Otherwise, I think this is all correct. Agree with the comment, the Database should be the parent. I guess I wasn't sure of the talloc parenting. Is reproducing the talloc hierarchy in all of the bindings really the right approach? I feel like there has to be a better way (or that the way we use talloc in the library is slightly broken). What if the bindings created an additional talloc reference to each managed object, just to keep the object alive, and used talloc_unlink instead of the destroy functions? Reproducing the hierarchy is probably error prone, and not that simple indeed :/ I haven't checked at all the way you suggest, but if we use talloc_reference/unlink, we get the same issue no? - If we do for each new wrapped object talloc_reference(NULL, wrapped_object), the the object will be kept alive until we talloc_unlink(NULL, wrapped_object), but what about its parents? For example will doing that on a notmuch_message_t keep the notmuch_messages_t alive? Hmm. This is what I was thinking. You have an interesting point; I think it's slightly wrong, but it exposes something deeper. I believe there are two different things going on here: some of the talloc relationships are for convenience, while some are structural. In the former case, I'm pretty sure my suggestion will work, but in the latter case the objects should *never* be freed by the finalizer! For example, notmuch_query_search_messages returns a new notmuch_messages_t with the query as the talloc parent, but that notmuch_messages_t doesn't depend on the query object; this is just so you can conveniently delete everything retrieved from the query by deleting the query. In this case, you can either use parent references like you did---which will prevent a double-free by forcing destruction to happen from the leaves up but at the cost of having to encode these relationships and of extending the parent object lifetimes beyond what's strictly necessary---or you can use my suggestion of creating an additional talloc reference. Actually, checking the code of notmuch_query_search_messages, it seems that the notmuch_messages_t (and the notmuch_message_t as well) object *does* depend on the database and the query... So in that case I think we need the "owner" Object reference as I currently have (we want the Messages to keep the Query alive, and the Query keeps the Database alive). That said, you example below looks valid, and it seems I'll need to add a flag to createMessage() (and some others) to disable the SetFinalizer call for certain instances (we probably want to keep it for eg. SearchMessageByFilename). - The candidates I found for adding a tmalloc reference and not a "full" Go reference (therefore preventing to keep the parent alive too long needlessly) are GetAllTags, Thread.GetTags, Messages.CollectTags, and Message.GetTags (those are basically string lists) - The methods for which I should remove the SetFinalizer on the wrapper (as you showed in the example below) while keeping the Go reference are Threads.Get and Messages.Get I would also maybe remove all the Destroy() functions, since they now seem more dangerous than anything else... I tried to write a test using runtime.GC to test the behaviour of the bindings, but for some reasons some cases which are supposed to crash don't, which makes me sceptical about the validity of the test :-/ Cheers Adrien However, in your example, the notmuch_message_t's are structurally related to the notmuch_messages_t from whence they came. They're all part of one data structure and hence it *never* makes sense for a caller to delete the notmuch_message_t's. For example, even with the code in this patch, I think the following could lead to a crash: 1. Obtain a Messages object, say ms. 2. m1 := ms.Get() 3. m1 = nil 4. m2 := ms.Get() 5. m2.whatever() If a garbage collection happens between steps 3 and 4, the Message in m1 will get finalized and destroyed. But step 4 will return the same, now dangling, pointer, leading to a potential crash in step 5. Maybe the answer in the structural case is to include the parent pointer in the Go struct and not set a finalizer on the child? That way, if there's a Go reference to the parent wrapper, it won't go away and the children won't get destroyed (collecting wrappers of children is fine) and if
[PATCH 2/2] Add notmuch_database_reopen method
Calling notmuch_database_reopen is needed to refresh the database contents when the database on disk was modified by another notmuch_database_t instance, for example in a different thread. --- lib/database.cc | 17 + lib/notmuch.h |8 2 files changed, 25 insertions(+), 0 deletions(-) diff --git a/lib/database.cc b/lib/database.cc index 55bcd17..3be5a30 100644 --- a/lib/database.cc +++ b/lib/database.cc @@ -763,6 +763,23 @@ notmuch_database_flush(notmuch_database_t *notmuch) return status; } +notmuch_status_t +notmuch_database_reopen(notmuch_database_t *notmuch) +{ + notmuch_status_t status = NOTMUCH_STATUS_SUCCESS; + + try { + if (notmuch->xapian_db != NULL) + (notmuch->xapian_db)->reopen (); + } catch (const Xapian::Error &error) { + fprintf(stderr, "A Xapian exception occured reopening the database: %s\n", + error.get_msg().c_str()); + status = NOTMUCH_STATUS_XAPIAN_EXCEPTION; + } + + return status; +} + void notmuch_database_close (notmuch_database_t *notmuch) { diff --git a/lib/notmuch.h b/lib/notmuch.h index aef5c56..51d6a9a 100644 --- a/lib/notmuch.h +++ b/lib/notmuch.h @@ -205,6 +205,14 @@ notmuch_database_open (const char *path, notmuch_status_t notmuch_database_flush (notmuch_database_t *database); +/* Refresh the database contents to the latest version. + * + * This is needed only if another instance of notmuch_database_t has + * modified the database contents on disk. + */ +notmuch_status_t +notmuch_database_reopen (notmuch_database_t *database); + /* Close the given notmuch database. * * After notmuch_database_close has been called, calls to other -- 1.7.7.6
[PATCH 1/2] Add notmuch_database_flush method
This method explicitly flushes the pending modifications to disk. It is useful if your program has various threads, each with a read only DB and one writer thread with a read/write DB. In that case, you most likely want the writer to sync the changes to disk so that the readers can see them, without having to close and reopen the database completely. --- lib/database.cc | 18 ++ lib/notmuch.h |4 2 files changed, 22 insertions(+), 0 deletions(-) diff --git a/lib/database.cc b/lib/database.cc index 761dc1a..55bcd17 100644 --- a/lib/database.cc +++ b/lib/database.cc @@ -745,6 +745,24 @@ notmuch_database_open (const char *path, return status; } +notmuch_status_t +notmuch_database_flush(notmuch_database_t *notmuch) +{ + notmuch_status_t status = NOTMUCH_STATUS_SUCCESS; + + try { + if (notmuch->xapian_db != NULL && + notmuch->mode == NOTMUCH_DATABASE_MODE_READ_WRITE) + (static_cast (notmuch->xapian_db))->flush (); + } catch (const Xapian::Error &error) { + fprintf(stderr, "A Xapian exception occured flushing the database: %s\n", + error.get_msg().c_str()); + status = NOTMUCH_STATUS_XAPIAN_EXCEPTION; + } + + return status; +} + void notmuch_database_close (notmuch_database_t *notmuch) { diff --git a/lib/notmuch.h b/lib/notmuch.h index 3633bed..aef5c56 100644 --- a/lib/notmuch.h +++ b/lib/notmuch.h @@ -201,6 +201,10 @@ notmuch_database_open (const char *path, notmuch_database_mode_t mode, notmuch_database_t **database); +/* Flushes all the pending modifications to the database to disk. */ +notmuch_status_t +notmuch_database_flush (notmuch_database_t *database); + /* Close the given notmuch database. * * After notmuch_database_close has been called, calls to other -- 1.7.7.6
[PATCH 0/2] Add flush/reopen methods to notmuch_database_t
Xapian (and therefore Notmuch? I would not mind a confirmation here) is thread safe as long as you don't share DB instances across threads. The database backend supports one writer and several concurrent readers. One possible model for multi threaded apps using Notmuch is to have one thread with a writable notmuch_database_t, and various with read-only notmuch_database_t instances. However, for the readers to see the changes done by the writer, the writer needs to flush the changes to disk, and the readers need to reopen the database. Those two patches add two methods to notmuch_database_t for this purpose (the signaling of the writer to the readers that they need to reopen is left to the application). Adrien Bustany (2): Add notmuch_database_flush method Add notmuch_database_reopen method lib/database.cc | 35 +++ lib/notmuch.h | 12 2 files changed, 47 insertions(+), 0 deletions(-) -- 1.7.7.6
[PATCH 0/7] Various fixes for the Go bindings
Well, thanks for the quick review :) I have updated all the patches to take your fixes into account, there is still the discussion left about the GC integration. Once we get that sorted out, I'll send the updated version. Le 18/07/2012 23:51, Austin Clements a ?crit : > This series looks good to me other than the few things I commented on. > It's nice to see the Go bindings get a bit of love! > > Quoth Adrien Bustany on Jul 18 at 9:34 pm: >> The following patches fix some serious memory management issues with >> the Go bindings, and add some missing functions as well. >> >> Adrien Bustany (7): >>go: Use iota in enum bindings >>go: Add missing MESSAGE_FLAG_EXCLUDED in Flag enum >>go: Allow notmuch objects to be garbage collected >>go: Make Destroy functions safe to call several times >>go: Partially bind notmuch_database_upgrade >>go: Bind notmuch_database_find_message_by_filename >>go: Bind notmuch_thread_t functions >> >> bindings/go/src/notmuch/notmuch.go | 471 >> ++-- >> 1 files changed, 447 insertions(+), 24 deletions(-) >>
[PATCH 3/7] go: Allow notmuch objects to be garbage collected
Le 18/07/2012 23:40, Austin Clements a ?crit : > This is subtle enough that I think it deserves a comment in the source > code explaining that tracking the talloc owner reference, combined > with the fact that Go finalizers are run in dependency order, ensures > that the C objects will always be destroyed from the talloc leaves up. Definitely, I tend to comment in the commit message and forget about the code... > > Just one inline comment below. Otherwise, I think this is all > correct. Agree with the comment, the Database should be the parent. I guess I wasn't sure of the talloc parenting. > > Is reproducing the talloc hierarchy in all of the bindings really the > right approach? I feel like there has to be a better way (or that the > way we use talloc in the library is slightly broken). What if the > bindings created an additional talloc reference to each managed > object, just to keep the object alive, and used talloc_unlink instead > of the destroy functions? Reproducing the hierarchy is probably error prone, and not that simple indeed :/ I haven't checked at all the way you suggest, but if we use talloc_reference/unlink, we get the same issue no? - If we do for each new wrapped object talloc_reference(NULL, wrapped_object), the the object will be kept alive until we talloc_unlink(NULL, wrapped_object), but what about its parents? For example will doing that on a notmuch_message_t keep the notmuch_messages_t alive? - If we do talloc_reference(parent, wrapped), then we reproduce the hierarchy again? Note that I have 0 experience with talloc, so I might as well be getting things wrong here. > > Quoth Adrien Bustany on Jul 18 at 9:34 pm: >> This makes notmuch appropriately free the underlying notmuch C objects >> when garbage collecting their Go wrappers. To make sure we don't break >> the underlying links between objects (for example, a notmuch_messages_t >> being GC'ed before a notmuch_message_t belonging to it), we add for each >> wraper struct a pointer to the owner object (Go objects with a reference >> pointing to them don't get garbage collected). >> --- >> bindings/go/src/notmuch/notmuch.go | 153 >> +++- >> 1 files changed, 134 insertions(+), 19 deletions(-) >> >> diff --git a/bindings/go/src/notmuch/notmuch.go >> b/bindings/go/src/notmuch/notmuch.go >> index 1d77fd2..3f436a0 100644 >> --- a/bindings/go/src/notmuch/notmuch.go >> +++ b/bindings/go/src/notmuch/notmuch.go >> @@ -11,6 +11,7 @@ package notmuch >> #include "notmuch.h" >> */ >> import "C" >> +import "runtime" >> import "unsafe" >> >> // Status codes used for the return values of most functions >> @@ -47,40 +48,152 @@ func (self Status) String() string { >> /* Various opaque data types. For each notmuch__t see the various >>* notmuch_ functions below. */ >> >> +type Object interface {} >> + >> type Database struct { >> db *C.notmuch_database_t >> } >> >> +func createDatabase(db *C.notmuch_database_t) *Database { >> +self := &Database{db: db} >> + >> +runtime.SetFinalizer(self, func(x *Database) { >> +if (x.db != nil) { >> +C.notmuch_database_destroy(x.db) >> +} >> +}) >> + >> +return self >> +} >> + >> type Query struct { >> query *C.notmuch_query_t >> +owner Object >> +} >> + >> +func createQuery(query *C.notmuch_query_t, owner Object) *Query { >> +self := &Query{query: query, owner: owner} >> + >> +runtime.SetFinalizer(self, func(x *Query) { >> +if (x.query != nil) { >> +C.notmuch_query_destroy(x.query) >> +} >> +}) >> + >> +return self >> } >> >> type Threads struct { >> threads *C.notmuch_threads_t >> +owner Object >> +} >> + >> +func createThreads(threads *C.notmuch_threads_t, owner Object) *Threads { >> +self := &Threads{threads: threads, owner: owner} >> + >> +runtime.SetFinalizer(self, func(x *Threads) { >> +if (x.threads != nil) { >> +C.notmuch_threads_destroy(x.threads) >> +} >> +}) >> + >> +return self >> } >> >> type Thread struct { >> thread *C.notmuch_thread_t >> +owner Object >> +} >> + >> +func createThread(thread *C.notmuch_thread_t, owner Object) *Thread { >> +self := &Thread{threa
[PATCH 2/2] Add notmuch_database_reopen method
Calling notmuch_database_reopen is needed to refresh the database contents when the database on disk was modified by another notmuch_database_t instance, for example in a different thread. --- lib/database.cc | 17 + lib/notmuch.h |8 2 files changed, 25 insertions(+), 0 deletions(-) diff --git a/lib/database.cc b/lib/database.cc index 55bcd17..3be5a30 100644 --- a/lib/database.cc +++ b/lib/database.cc @@ -763,6 +763,23 @@ notmuch_database_flush(notmuch_database_t *notmuch) return status; } +notmuch_status_t +notmuch_database_reopen(notmuch_database_t *notmuch) +{ + notmuch_status_t status = NOTMUCH_STATUS_SUCCESS; + + try { + if (notmuch->xapian_db != NULL) + (notmuch->xapian_db)->reopen (); + } catch (const Xapian::Error &error) { + fprintf(stderr, "A Xapian exception occured reopening the database: %s\n", + error.get_msg().c_str()); + status = NOTMUCH_STATUS_XAPIAN_EXCEPTION; + } + + return status; +} + void notmuch_database_close (notmuch_database_t *notmuch) { diff --git a/lib/notmuch.h b/lib/notmuch.h index aef5c56..51d6a9a 100644 --- a/lib/notmuch.h +++ b/lib/notmuch.h @@ -205,6 +205,14 @@ notmuch_database_open (const char *path, notmuch_status_t notmuch_database_flush (notmuch_database_t *database); +/* Refresh the database contents to the latest version. + * + * This is needed only if another instance of notmuch_database_t has + * modified the database contents on disk. + */ +notmuch_status_t +notmuch_database_reopen (notmuch_database_t *database); + /* Close the given notmuch database. * * After notmuch_database_close has been called, calls to other -- 1.7.7.6 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 0/2] Add flush/reopen methods to notmuch_database_t
Xapian (and therefore Notmuch? I would not mind a confirmation here) is thread safe as long as you don't share DB instances across threads. The database backend supports one writer and several concurrent readers. One possible model for multi threaded apps using Notmuch is to have one thread with a writable notmuch_database_t, and various with read-only notmuch_database_t instances. However, for the readers to see the changes done by the writer, the writer needs to flush the changes to disk, and the readers need to reopen the database. Those two patches add two methods to notmuch_database_t for this purpose (the signaling of the writer to the readers that they need to reopen is left to the application). Adrien Bustany (2): Add notmuch_database_flush method Add notmuch_database_reopen method lib/database.cc | 35 +++ lib/notmuch.h | 12 2 files changed, 47 insertions(+), 0 deletions(-) -- 1.7.7.6 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 1/2] Add notmuch_database_flush method
This method explicitly flushes the pending modifications to disk. It is useful if your program has various threads, each with a read only DB and one writer thread with a read/write DB. In that case, you most likely want the writer to sync the changes to disk so that the readers can see them, without having to close and reopen the database completely. --- lib/database.cc | 18 ++ lib/notmuch.h |4 2 files changed, 22 insertions(+), 0 deletions(-) diff --git a/lib/database.cc b/lib/database.cc index 761dc1a..55bcd17 100644 --- a/lib/database.cc +++ b/lib/database.cc @@ -745,6 +745,24 @@ notmuch_database_open (const char *path, return status; } +notmuch_status_t +notmuch_database_flush(notmuch_database_t *notmuch) +{ + notmuch_status_t status = NOTMUCH_STATUS_SUCCESS; + + try { + if (notmuch->xapian_db != NULL && + notmuch->mode == NOTMUCH_DATABASE_MODE_READ_WRITE) + (static_cast (notmuch->xapian_db))->flush (); + } catch (const Xapian::Error &error) { + fprintf(stderr, "A Xapian exception occured flushing the database: %s\n", + error.get_msg().c_str()); + status = NOTMUCH_STATUS_XAPIAN_EXCEPTION; + } + + return status; +} + void notmuch_database_close (notmuch_database_t *notmuch) { diff --git a/lib/notmuch.h b/lib/notmuch.h index 3633bed..aef5c56 100644 --- a/lib/notmuch.h +++ b/lib/notmuch.h @@ -201,6 +201,10 @@ notmuch_database_open (const char *path, notmuch_database_mode_t mode, notmuch_database_t **database); +/* Flushes all the pending modifications to the database to disk. */ +notmuch_status_t +notmuch_database_flush (notmuch_database_t *database); + /* Close the given notmuch database. * * After notmuch_database_close has been called, calls to other -- 1.7.7.6 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 0/7] Various fixes for the Go bindings
Well, thanks for the quick review :) I have updated all the patches to take your fixes into account, there is still the discussion left about the GC integration. Once we get that sorted out, I'll send the updated version. Le 18/07/2012 23:51, Austin Clements a écrit : This series looks good to me other than the few things I commented on. It's nice to see the Go bindings get a bit of love! Quoth Adrien Bustany on Jul 18 at 9:34 pm: The following patches fix some serious memory management issues with the Go bindings, and add some missing functions as well. Adrien Bustany (7): go: Use iota in enum bindings go: Add missing MESSAGE_FLAG_EXCLUDED in Flag enum go: Allow notmuch objects to be garbage collected go: Make Destroy functions safe to call several times go: Partially bind notmuch_database_upgrade go: Bind notmuch_database_find_message_by_filename go: Bind notmuch_thread_t functions bindings/go/src/notmuch/notmuch.go | 471 ++-- 1 files changed, 447 insertions(+), 24 deletions(-) ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 3/7] go: Allow notmuch objects to be garbage collected
Le 18/07/2012 23:40, Austin Clements a écrit : This is subtle enough that I think it deserves a comment in the source code explaining that tracking the talloc owner reference, combined with the fact that Go finalizers are run in dependency order, ensures that the C objects will always be destroyed from the talloc leaves up. Definitely, I tend to comment in the commit message and forget about the code... Just one inline comment below. Otherwise, I think this is all correct. Agree with the comment, the Database should be the parent. I guess I wasn't sure of the talloc parenting. Is reproducing the talloc hierarchy in all of the bindings really the right approach? I feel like there has to be a better way (or that the way we use talloc in the library is slightly broken). What if the bindings created an additional talloc reference to each managed object, just to keep the object alive, and used talloc_unlink instead of the destroy functions? Reproducing the hierarchy is probably error prone, and not that simple indeed :/ I haven't checked at all the way you suggest, but if we use talloc_reference/unlink, we get the same issue no? - If we do for each new wrapped object talloc_reference(NULL, wrapped_object), the the object will be kept alive until we talloc_unlink(NULL, wrapped_object), but what about its parents? For example will doing that on a notmuch_message_t keep the notmuch_messages_t alive? - If we do talloc_reference(parent, wrapped), then we reproduce the hierarchy again? Note that I have 0 experience with talloc, so I might as well be getting things wrong here. Quoth Adrien Bustany on Jul 18 at 9:34 pm: This makes notmuch appropriately free the underlying notmuch C objects when garbage collecting their Go wrappers. To make sure we don't break the underlying links between objects (for example, a notmuch_messages_t being GC'ed before a notmuch_message_t belonging to it), we add for each wraper struct a pointer to the owner object (Go objects with a reference pointing to them don't get garbage collected). --- bindings/go/src/notmuch/notmuch.go | 153 +++- 1 files changed, 134 insertions(+), 19 deletions(-) diff --git a/bindings/go/src/notmuch/notmuch.go b/bindings/go/src/notmuch/notmuch.go index 1d77fd2..3f436a0 100644 --- a/bindings/go/src/notmuch/notmuch.go +++ b/bindings/go/src/notmuch/notmuch.go @@ -11,6 +11,7 @@ package notmuch #include "notmuch.h" */ import "C" +import "runtime" import "unsafe" // Status codes used for the return values of most functions @@ -47,40 +48,152 @@ func (self Status) String() string { /* Various opaque data types. For each notmuch__t see the various * notmuch_ functions below. */ +type Object interface {} + type Database struct { db *C.notmuch_database_t } +func createDatabase(db *C.notmuch_database_t) *Database { + self := &Database{db: db} + + runtime.SetFinalizer(self, func(x *Database) { + if (x.db != nil) { + C.notmuch_database_destroy(x.db) + } + }) + + return self +} + type Query struct { query *C.notmuch_query_t + owner Object +} + +func createQuery(query *C.notmuch_query_t, owner Object) *Query { + self := &Query{query: query, owner: owner} + + runtime.SetFinalizer(self, func(x *Query) { + if (x.query != nil) { + C.notmuch_query_destroy(x.query) + } + }) + + return self } type Threads struct { threads *C.notmuch_threads_t + owner Object +} + +func createThreads(threads *C.notmuch_threads_t, owner Object) *Threads { + self := &Threads{threads: threads, owner: owner} + + runtime.SetFinalizer(self, func(x *Threads) { + if (x.threads != nil) { + C.notmuch_threads_destroy(x.threads) + } + }) + + return self } type Thread struct { thread *C.notmuch_thread_t + owner Object +} + +func createThread(thread *C.notmuch_thread_t, owner Object) *Thread { + self := &Thread{thread: thread, owner: owner} + + runtime.SetFinalizer(self, func(x *Thread) { + if (x.thread != nil) { + C.notmuch_thread_destroy(x.thread) + } + }) + + return self } type Messages struct { messages *C.notmuch_messages_t + owner Object +} + +func createMessages(messages *C.notmuch_messages_t, owner Object) *Messages { + self := &Messages{messages: messages, owner: owner} + + return self } type Message struct { message *C.notmuch_message_t + owner Object +} + +func createMessage(message *C.notmuch_message_t, owner Object) *Message { + self := &Message{message: message, owner: owner} + +
[PATCH 7/7] go: Bind notmuch_thread_t functions
--- bindings/go/src/notmuch/notmuch.go | 253 +++- 1 files changed, 252 insertions(+), 1 deletions(-) diff --git a/bindings/go/src/notmuch/notmuch.go b/bindings/go/src/notmuch/notmuch.go index be4cb8c..f667dbb 100644 --- a/bindings/go/src/notmuch/notmuch.go +++ b/bindings/go/src/notmuch/notmuch.go @@ -12,6 +12,8 @@ package notmuch */ import "C" import "runtime" +import "strings" +import "time" import "unsafe" // Status codes used for the return values of most functions @@ -700,7 +702,20 @@ func (self *Query) CountMessages() uint { return uint(C.notmuch_query_count_messages(self.query)) } -// TODO: wrap threads and thread +/* Return the number of threads matching a search. + * + * This function performs a search and returns the number of unique thread IDs + * in the matching messages. This is the same as number of threads matching a + * search. + * + * Note that this is a significantly heavier operation than + * notmuch_query_count_messages(). + * + * If an error occurs, this function may return 0. + */ +func (self *Query) CountThreads() uint { + return uint(C.notmuch_query_count_threads(self.query)) +} /* Is the given 'threads' iterator pointing at a valid thread. * @@ -722,6 +737,45 @@ func (self *Threads) Valid() bool { return true } +/* Get the current thread from 'threads' as a notmuch_thread_t. + * + * Note: The returned thread belongs to 'threads' and has a lifetime + * identical to it (and the query to which it belongs). + * + * See the documentation of notmuch_query_search_threads for example + * code showing how to iterate over a notmuch_threads_t object. + * + * If an out-of-memory situation occurs, this function will return + * NULL. + */ +func (self *Threads) Get() *Thread { + if self.threads == nil { + return nil + } + thread := C.notmuch_threads_get(self.threads) + if thread == nil { + return nil + } + return createThread(thread, self) +} + +/* Move the 'threads' iterator to the next thread. + * + * If 'threads' is already pointing at the last thread then the + * iterator will be moved to a point just beyond that last thread, + * (where notmuch_threads_valid will return FALSE and + * notmuch_threads_get will return NULL). + * + * See the documentation of notmuch_query_search_threads for example + * code showing how to iterate over a notmuch_threads_t object. + */ +func (self *Threads) MoveToNext() { + if self.threads == nil { + return + } + C.notmuch_threads_move_to_next(self.threads) +} + /* Destroy a notmuch_threads_t object. * * It's not strictly necessary to call this function. All memory from @@ -735,6 +789,203 @@ func (self *Threads) Destroy() { } } +/* Get the thread ID of 'thread'. + * + * The returned string belongs to 'thread' and as such, should not be + * modified by the caller and will only be valid for as long as the + * thread is valid, (which is until notmuch_thread_destroy or until + * the query from which it derived is destroyed). + */ +func (self *Thread) GetThreadId() string { + if self.thread == nil { + return "" + } + id := C.notmuch_thread_get_thread_id(self.thread) + + if id == nil { + return "" + } + + return C.GoString(id) +} + +/* Get the total number of messages in 'thread'. + * + * This count consists of all messages in the database belonging to + * this thread. Contrast with notmuch_thread_get_matched_messages() . + */ +func (self *Thread) GetTotalMessages() int { + if self.thread == nil { + return 0 + } + return int(C.notmuch_thread_get_total_messages(self.thread)) +} + +/* Get a notmuch_messages_t iterator for the top-level messages in + * 'thread'. + * + * This iterator will not necessarily iterate over all of the messages + * in the thread. It will only iterate over the messages in the thread + * which are not replies to other messages in the thread. + * + * To iterate over all messages in the thread, the caller will need to + * iterate over the result of notmuch_message_get_replies for each + * top-level message (and do that recursively for the resulting + * messages, etc.). + */ +func (self *Thread) GetToplevelMessages() *Messages { + if self.thread == nil { + return nil + } + msgs := C.notmuch_thread_get_toplevel_messages(self.thread) + if msgs == nil { + return nil + } + return createMessages(msgs, self) +} + +/* Get a notmuch_messages_t iterator for the top-level messages in + * 'thread'. + * + * This iterator will not necessarily iterate over all of the messages + * in the thread. It will only iterate over the messages in the thread + * which are not replies to other messages in the thread. + * + * To iterate over all messages in the thread, the caller will need to + * iterate over the result of notmuch_message_get_r
[PATCH 6/7] go: Bind notmuch_database_find_message_by_filename
--- bindings/go/src/notmuch/notmuch.go | 39 1 files changed, 39 insertions(+), 0 deletions(-) diff --git a/bindings/go/src/notmuch/notmuch.go b/bindings/go/src/notmuch/notmuch.go index 384d5a5..be4cb8c 100644 --- a/bindings/go/src/notmuch/notmuch.go +++ b/bindings/go/src/notmuch/notmuch.go @@ -453,6 +453,45 @@ func (self *Database) FindMessage(message_id string) (*Message, Status) { return createMessage(msg, nil), st } +/* Find a message with the given filename. + * + * If the database contains a message with the given filename then, on + * successful return (NOTMUCH_STATUS_SUCCESS) '*message' will be initialized to + * a message object. The caller should call notmuch_message_destroy when done + * with the message. + * + * On any failure or when the message is not found, this function initializes + * '*message' to NULL. This means, when NOTMUCH_STATUS_SUCCESS is returned, the + * caller is supposed to check '*message' for NULL to find out whether the + * message with the given filename is found. + * + * Return value: + * + * NOTMUCH_STATUS_SUCCESS: Successful return, check '*message' + * + * NOTMUCH_STATUS_NULL_POINTER: The given 'message' argument is NULL + * + * NOTMUCH_STATUS_OUT_OF_MEMORY: Out of memory, creating the message object + * + * NOTMUCH_STATUS_XAPIAN_EXCEPTION: A Xapian exception occurred + */ +func (self *Database) FindMessageByFilename(filename string) (*Message, Status) { + + var c_msg_filename *C.char = C.CString(filename) + defer C.free(unsafe.Pointer(c_msg_filename)) + + if c_msg_filename == nil { + return nil, STATUS_OUT_OF_MEMORY + } + + var msg *C.notmuch_message_t + st := Status(C.notmuch_database_find_message_by_filename(self.db, c_msg_filename, &msg)) + if st != STATUS_SUCCESS { + return nil, st + } + return createMessage(msg, nil), st +} + /* Return a list of all tags found in the database. * * This function creates a list of all tags found in the database. The -- 1.7.7.6
[PATCH 5/7] go: Partially bind notmuch_database_upgrade
This binding does not handle the progress callback, but at least allows opening and upgrading a database if needed. --- bindings/go/src/notmuch/notmuch.go | 13 - 1 files changed, 12 insertions(+), 1 deletions(-) diff --git a/bindings/go/src/notmuch/notmuch.go b/bindings/go/src/notmuch/notmuch.go index d8b2739..384d5a5 100644 --- a/bindings/go/src/notmuch/notmuch.go +++ b/bindings/go/src/notmuch/notmuch.go @@ -296,7 +296,18 @@ func (self *Database) NeedsUpgrade() bool { return true } -// TODO: notmuch_database_upgrade +// TODO: Proper notmuch_database_upgrade +/* Upgrade the current database. + * + * After opening a database in read-write mode, the client should + * check if an upgrade is needed (notmuch_database_needs_upgrade) and + * if so, upgrade with this function before making any modifications. + */ +func (self *Database) Upgrade() Status { + st := Status(C.notmuch_database_upgrade(self.db, nil, nil)); + + return st; +} /* Retrieve a directory object from the database for 'path'. * -- 1.7.7.6
[PATCH 4/7] go: Make Destroy functions safe to call several times
Those methods were already checking if the underlying C object was NULL, but they were not setting the pointer to NULL after destroying it. --- bindings/go/src/notmuch/notmuch.go |6 ++ 1 files changed, 6 insertions(+), 0 deletions(-) diff --git a/bindings/go/src/notmuch/notmuch.go b/bindings/go/src/notmuch/notmuch.go index 3f436a0..d8b2739 100644 --- a/bindings/go/src/notmuch/notmuch.go +++ b/bindings/go/src/notmuch/notmuch.go @@ -634,6 +634,7 @@ func (self *Query) SearchMessages() *Messages { func (self *Query) Destroy() { if self.query != nil { C.notmuch_query_destroy(self.query) + self.query = nil } } @@ -680,6 +681,7 @@ func (self *Threads) Valid() bool { func (self *Threads) Destroy() { if self.threads != nil { C.notmuch_threads_destroy(self.threads) + self.threads = nil } } @@ -1143,6 +1145,7 @@ func (self *Message) Destroy() { return } C.notmuch_message_destroy(self.message) + self.message = nil } /* Is the given 'tags' iterator pointing at a valid tag. @@ -1214,6 +1217,7 @@ func (self *Tags) Destroy() { return } C.notmuch_tags_destroy(self.tags) + self.tags = nil } // TODO: wrap notmuch_directory_ @@ -1224,6 +1228,7 @@ func (self *Directory) Destroy() { return } C.notmuch_directory_destroy(self.dir) + self.dir = nil } // TODO: wrap notmuch_filenames_ @@ -1242,6 +1247,7 @@ func (self *Filenames) Destroy() { return } C.notmuch_filenames_destroy(self.fnames) + self.fnames = nil } /* EOF */ -- 1.7.7.6
[PATCH 3/7] go: Allow notmuch objects to be garbage collected
This makes notmuch appropriately free the underlying notmuch C objects when garbage collecting their Go wrappers. To make sure we don't break the underlying links between objects (for example, a notmuch_messages_t being GC'ed before a notmuch_message_t belonging to it), we add for each wraper struct a pointer to the owner object (Go objects with a reference pointing to them don't get garbage collected). --- bindings/go/src/notmuch/notmuch.go | 153 +++- 1 files changed, 134 insertions(+), 19 deletions(-) diff --git a/bindings/go/src/notmuch/notmuch.go b/bindings/go/src/notmuch/notmuch.go index 1d77fd2..3f436a0 100644 --- a/bindings/go/src/notmuch/notmuch.go +++ b/bindings/go/src/notmuch/notmuch.go @@ -11,6 +11,7 @@ package notmuch #include "notmuch.h" */ import "C" +import "runtime" import "unsafe" // Status codes used for the return values of most functions @@ -47,40 +48,152 @@ func (self Status) String() string { /* Various opaque data types. For each notmuch__t see the various * notmuch_ functions below. */ +type Object interface {} + type Database struct { db *C.notmuch_database_t } +func createDatabase(db *C.notmuch_database_t) *Database { + self := &Database{db: db} + + runtime.SetFinalizer(self, func(x *Database) { + if (x.db != nil) { + C.notmuch_database_destroy(x.db) + } + }) + + return self +} + type Query struct { query *C.notmuch_query_t + owner Object +} + +func createQuery(query *C.notmuch_query_t, owner Object) *Query { + self := &Query{query: query, owner: owner} + + runtime.SetFinalizer(self, func(x *Query) { + if (x.query != nil) { + C.notmuch_query_destroy(x.query) + } + }) + + return self } type Threads struct { threads *C.notmuch_threads_t + owner Object +} + +func createThreads(threads *C.notmuch_threads_t, owner Object) *Threads { + self := &Threads{threads: threads, owner: owner} + + runtime.SetFinalizer(self, func(x *Threads) { + if (x.threads != nil) { + C.notmuch_threads_destroy(x.threads) + } + }) + + return self } type Thread struct { thread *C.notmuch_thread_t + owner Object +} + +func createThread(thread *C.notmuch_thread_t, owner Object) *Thread { + self := &Thread{thread: thread, owner: owner} + + runtime.SetFinalizer(self, func(x *Thread) { + if (x.thread != nil) { + C.notmuch_thread_destroy(x.thread) + } + }) + + return self } type Messages struct { messages *C.notmuch_messages_t + owner Object +} + +func createMessages(messages *C.notmuch_messages_t, owner Object) *Messages { + self := &Messages{messages: messages, owner: owner} + + return self } type Message struct { message *C.notmuch_message_t + owner Object +} + +func createMessage(message *C.notmuch_message_t, owner Object) *Message { + self := &Message{message: message, owner: owner} + + runtime.SetFinalizer(self, func(x *Message) { + if (x.message != nil) { + C.notmuch_message_destroy(x.message) + } + }) + + return self } type Tags struct { tags *C.notmuch_tags_t + owner Object +} + +func createTags(tags *C.notmuch_tags_t, owner Object) *Tags { + self := &Tags{tags: tags, owner: owner} + + runtime.SetFinalizer(self, func(x *Tags) { + if (x.tags != nil) { + C.notmuch_tags_destroy(x.tags) + } + }) + + return self } type Directory struct { dir *C.notmuch_directory_t + owner Object +} + +func createDirectory(directory *C.notmuch_directory_t, owner Object) *Directory { + self := &Directory{dir: directory, owner: owner} + + runtime.SetFinalizer(self, func(x *Directory) { + if (x.dir != nil) { + C.notmuch_directory_destroy(x.dir) + } + }) + + return self } type Filenames struct { fnames *C.notmuch_filenames_t + owner Object +} + +func createFilenames(filenames *C.notmuch_filenames_t, owner Object) *Filenames { + self := &Filenames{fnames: filenames, owner: owner} + + runtime.SetFinalizer(self, func(x *Filenames) { + if (x.fnames != nil) { + C.notmuch_filenames_destroy(x.fnames) + } + }) + + return self } type DatabaseMode C.notmuch_database_mode_t @@ -100,12 +213,13 @@ func NewDatabase(path string) (*Database, Status) { return nil, STATUS_OUT_OF_MEMORY } - self := &Database{db: nil} - st := Status(C.notmuch_database_create(c_path, &self.db)) + var db *C.notmuch_database_t; + st := Status
[PATCH 2/7] go: Add missing MESSAGE_FLAG_EXCLUDED in Flag enum
--- bindings/go/src/notmuch/notmuch.go |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/bindings/go/src/notmuch/notmuch.go b/bindings/go/src/notmuch/notmuch.go index ecd7418..1d77fd2 100644 --- a/bindings/go/src/notmuch/notmuch.go +++ b/bindings/go/src/notmuch/notmuch.go @@ -775,6 +775,7 @@ type Flag C.notmuch_message_flag_t const ( MESSAGE_FLAG_MATCH Flag = iota + MESSAGE_FLAG_EXCLUDED ) /* Get a value of a flag for the email corresponding to 'message'. */ -- 1.7.7.6
[PATCH 1/7] go: Use iota in enum bindings
Using iota is the correct way to get the values in the enum increment automatically. The old code would just set all the enum values to 0. --- bindings/go/src/notmuch/notmuch.go |6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/bindings/go/src/notmuch/notmuch.go b/bindings/go/src/notmuch/notmuch.go index 00bd53a..ecd7418 100644 --- a/bindings/go/src/notmuch/notmuch.go +++ b/bindings/go/src/notmuch/notmuch.go @@ -86,7 +86,7 @@ type Filenames struct { type DatabaseMode C.notmuch_database_mode_t const ( - DATABASE_MODE_READ_ONLY DatabaseMode = 0 + DATABASE_MODE_READ_ONLY DatabaseMode = iota DATABASE_MODE_READ_WRITE ) @@ -386,7 +386,7 @@ func (self *Database) CreateQuery(query string) *Query { type Sort C.notmuch_sort_t const ( - SORT_OLDEST_FIRST Sort = 0 + SORT_OLDEST_FIRST Sort = iota SORT_NEWEST_FIRST SORT_MESSAGE_ID SORT_UNSORTED @@ -774,7 +774,7 @@ func (self *Message) GetFileName() string { type Flag C.notmuch_message_flag_t const ( - MESSAGE_FLAG_MATCH Flag = 0 + MESSAGE_FLAG_MATCH Flag = iota ) /* Get a value of a flag for the email corresponding to 'message'. */ -- 1.7.7.6
[PATCH 0/7] Various fixes for the Go bindings
The following patches fix some serious memory management issues with the Go bindings, and add some missing functions as well. Adrien Bustany (7): go: Use iota in enum bindings go: Add missing MESSAGE_FLAG_EXCLUDED in Flag enum go: Allow notmuch objects to be garbage collected go: Make Destroy functions safe to call several times go: Partially bind notmuch_database_upgrade go: Bind notmuch_database_find_message_by_filename go: Bind notmuch_thread_t functions bindings/go/src/notmuch/notmuch.go | 471 ++-- 1 files changed, 447 insertions(+), 24 deletions(-) -- 1.7.7.6
[PATCH 3/7] go: Allow notmuch objects to be garbage collected
This makes notmuch appropriately free the underlying notmuch C objects when garbage collecting their Go wrappers. To make sure we don't break the underlying links between objects (for example, a notmuch_messages_t being GC'ed before a notmuch_message_t belonging to it), we add for each wraper struct a pointer to the owner object (Go objects with a reference pointing to them don't get garbage collected). --- bindings/go/src/notmuch/notmuch.go | 153 +++- 1 files changed, 134 insertions(+), 19 deletions(-) diff --git a/bindings/go/src/notmuch/notmuch.go b/bindings/go/src/notmuch/notmuch.go index 1d77fd2..3f436a0 100644 --- a/bindings/go/src/notmuch/notmuch.go +++ b/bindings/go/src/notmuch/notmuch.go @@ -11,6 +11,7 @@ package notmuch #include "notmuch.h" */ import "C" +import "runtime" import "unsafe" // Status codes used for the return values of most functions @@ -47,40 +48,152 @@ func (self Status) String() string { /* Various opaque data types. For each notmuch__t see the various * notmuch_ functions below. */ +type Object interface {} + type Database struct { db *C.notmuch_database_t } +func createDatabase(db *C.notmuch_database_t) *Database { + self := &Database{db: db} + + runtime.SetFinalizer(self, func(x *Database) { + if (x.db != nil) { + C.notmuch_database_destroy(x.db) + } + }) + + return self +} + type Query struct { query *C.notmuch_query_t + owner Object +} + +func createQuery(query *C.notmuch_query_t, owner Object) *Query { + self := &Query{query: query, owner: owner} + + runtime.SetFinalizer(self, func(x *Query) { + if (x.query != nil) { + C.notmuch_query_destroy(x.query) + } + }) + + return self } type Threads struct { threads *C.notmuch_threads_t + owner Object +} + +func createThreads(threads *C.notmuch_threads_t, owner Object) *Threads { + self := &Threads{threads: threads, owner: owner} + + runtime.SetFinalizer(self, func(x *Threads) { + if (x.threads != nil) { + C.notmuch_threads_destroy(x.threads) + } + }) + + return self } type Thread struct { thread *C.notmuch_thread_t + owner Object +} + +func createThread(thread *C.notmuch_thread_t, owner Object) *Thread { + self := &Thread{thread: thread, owner: owner} + + runtime.SetFinalizer(self, func(x *Thread) { + if (x.thread != nil) { + C.notmuch_thread_destroy(x.thread) + } + }) + + return self } type Messages struct { messages *C.notmuch_messages_t + owner Object +} + +func createMessages(messages *C.notmuch_messages_t, owner Object) *Messages { + self := &Messages{messages: messages, owner: owner} + + return self } type Message struct { message *C.notmuch_message_t + owner Object +} + +func createMessage(message *C.notmuch_message_t, owner Object) *Message { + self := &Message{message: message, owner: owner} + + runtime.SetFinalizer(self, func(x *Message) { + if (x.message != nil) { + C.notmuch_message_destroy(x.message) + } + }) + + return self } type Tags struct { tags *C.notmuch_tags_t + owner Object +} + +func createTags(tags *C.notmuch_tags_t, owner Object) *Tags { + self := &Tags{tags: tags, owner: owner} + + runtime.SetFinalizer(self, func(x *Tags) { + if (x.tags != nil) { + C.notmuch_tags_destroy(x.tags) + } + }) + + return self } type Directory struct { dir *C.notmuch_directory_t + owner Object +} + +func createDirectory(directory *C.notmuch_directory_t, owner Object) *Directory { + self := &Directory{dir: directory, owner: owner} + + runtime.SetFinalizer(self, func(x *Directory) { + if (x.dir != nil) { + C.notmuch_directory_destroy(x.dir) + } + }) + + return self } type Filenames struct { fnames *C.notmuch_filenames_t + owner Object +} + +func createFilenames(filenames *C.notmuch_filenames_t, owner Object) *Filenames { + self := &Filenames{fnames: filenames, owner: owner} + + runtime.SetFinalizer(self, func(x *Filenames) { + if (x.fnames != nil) { + C.notmuch_filenames_destroy(x.fnames) + } + }) + + return self } type DatabaseMode C.notmuch_database_mode_t @@ -100,12 +213,13 @@ func NewDatabase(path string) (*Database, Status) { return nil, STATUS_OUT_OF_MEMORY } - self := &Database{db: nil} - st := Status(C.notmuch_database_create(c_path, &self.db)) + var db *C.notmuch_database_t; +
[PATCH 7/7] go: Bind notmuch_thread_t functions
--- bindings/go/src/notmuch/notmuch.go | 253 +++- 1 files changed, 252 insertions(+), 1 deletions(-) diff --git a/bindings/go/src/notmuch/notmuch.go b/bindings/go/src/notmuch/notmuch.go index be4cb8c..f667dbb 100644 --- a/bindings/go/src/notmuch/notmuch.go +++ b/bindings/go/src/notmuch/notmuch.go @@ -12,6 +12,8 @@ package notmuch */ import "C" import "runtime" +import "strings" +import "time" import "unsafe" // Status codes used for the return values of most functions @@ -700,7 +702,20 @@ func (self *Query) CountMessages() uint { return uint(C.notmuch_query_count_messages(self.query)) } -// TODO: wrap threads and thread +/* Return the number of threads matching a search. + * + * This function performs a search and returns the number of unique thread IDs + * in the matching messages. This is the same as number of threads matching a + * search. + * + * Note that this is a significantly heavier operation than + * notmuch_query_count_messages(). + * + * If an error occurs, this function may return 0. + */ +func (self *Query) CountThreads() uint { + return uint(C.notmuch_query_count_threads(self.query)) +} /* Is the given 'threads' iterator pointing at a valid thread. * @@ -722,6 +737,45 @@ func (self *Threads) Valid() bool { return true } +/* Get the current thread from 'threads' as a notmuch_thread_t. + * + * Note: The returned thread belongs to 'threads' and has a lifetime + * identical to it (and the query to which it belongs). + * + * See the documentation of notmuch_query_search_threads for example + * code showing how to iterate over a notmuch_threads_t object. + * + * If an out-of-memory situation occurs, this function will return + * NULL. + */ +func (self *Threads) Get() *Thread { + if self.threads == nil { + return nil + } + thread := C.notmuch_threads_get(self.threads) + if thread == nil { + return nil + } + return createThread(thread, self) +} + +/* Move the 'threads' iterator to the next thread. + * + * If 'threads' is already pointing at the last thread then the + * iterator will be moved to a point just beyond that last thread, + * (where notmuch_threads_valid will return FALSE and + * notmuch_threads_get will return NULL). + * + * See the documentation of notmuch_query_search_threads for example + * code showing how to iterate over a notmuch_threads_t object. + */ +func (self *Threads) MoveToNext() { + if self.threads == nil { + return + } + C.notmuch_threads_move_to_next(self.threads) +} + /* Destroy a notmuch_threads_t object. * * It's not strictly necessary to call this function. All memory from @@ -735,6 +789,203 @@ func (self *Threads) Destroy() { } } +/* Get the thread ID of 'thread'. + * + * The returned string belongs to 'thread' and as such, should not be + * modified by the caller and will only be valid for as long as the + * thread is valid, (which is until notmuch_thread_destroy or until + * the query from which it derived is destroyed). + */ +func (self *Thread) GetThreadId() string { + if self.thread == nil { + return "" + } + id := C.notmuch_thread_get_thread_id(self.thread) + + if id == nil { + return "" + } + + return C.GoString(id) +} + +/* Get the total number of messages in 'thread'. + * + * This count consists of all messages in the database belonging to + * this thread. Contrast with notmuch_thread_get_matched_messages() . + */ +func (self *Thread) GetTotalMessages() int { + if self.thread == nil { + return 0 + } + return int(C.notmuch_thread_get_total_messages(self.thread)) +} + +/* Get a notmuch_messages_t iterator for the top-level messages in + * 'thread'. + * + * This iterator will not necessarily iterate over all of the messages + * in the thread. It will only iterate over the messages in the thread + * which are not replies to other messages in the thread. + * + * To iterate over all messages in the thread, the caller will need to + * iterate over the result of notmuch_message_get_replies for each + * top-level message (and do that recursively for the resulting + * messages, etc.). + */ +func (self *Thread) GetToplevelMessages() *Messages { + if self.thread == nil { + return nil + } + msgs := C.notmuch_thread_get_toplevel_messages(self.thread) + if msgs == nil { + return nil + } + return createMessages(msgs, self) +} + +/* Get a notmuch_messages_t iterator for the top-level messages in + * 'thread'. + * + * This iterator will not necessarily iterate over all of the messages + * in the thread. It will only iterate over the messages in the thread + * which are not replies to other messages in the thread. + * + * To iterate over all messages in the thread, the caller will need to + * iterate over the result of notmuch_message_
[PATCH 0/7] Various fixes for the Go bindings
The following patches fix some serious memory management issues with the Go bindings, and add some missing functions as well. Adrien Bustany (7): go: Use iota in enum bindings go: Add missing MESSAGE_FLAG_EXCLUDED in Flag enum go: Allow notmuch objects to be garbage collected go: Make Destroy functions safe to call several times go: Partially bind notmuch_database_upgrade go: Bind notmuch_database_find_message_by_filename go: Bind notmuch_thread_t functions bindings/go/src/notmuch/notmuch.go | 471 ++-- 1 files changed, 447 insertions(+), 24 deletions(-) -- 1.7.7.6 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 2/7] go: Add missing MESSAGE_FLAG_EXCLUDED in Flag enum
--- bindings/go/src/notmuch/notmuch.go |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/bindings/go/src/notmuch/notmuch.go b/bindings/go/src/notmuch/notmuch.go index ecd7418..1d77fd2 100644 --- a/bindings/go/src/notmuch/notmuch.go +++ b/bindings/go/src/notmuch/notmuch.go @@ -775,6 +775,7 @@ type Flag C.notmuch_message_flag_t const ( MESSAGE_FLAG_MATCH Flag = iota + MESSAGE_FLAG_EXCLUDED ) /* Get a value of a flag for the email corresponding to 'message'. */ -- 1.7.7.6 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 6/7] go: Bind notmuch_database_find_message_by_filename
--- bindings/go/src/notmuch/notmuch.go | 39 1 files changed, 39 insertions(+), 0 deletions(-) diff --git a/bindings/go/src/notmuch/notmuch.go b/bindings/go/src/notmuch/notmuch.go index 384d5a5..be4cb8c 100644 --- a/bindings/go/src/notmuch/notmuch.go +++ b/bindings/go/src/notmuch/notmuch.go @@ -453,6 +453,45 @@ func (self *Database) FindMessage(message_id string) (*Message, Status) { return createMessage(msg, nil), st } +/* Find a message with the given filename. + * + * If the database contains a message with the given filename then, on + * successful return (NOTMUCH_STATUS_SUCCESS) '*message' will be initialized to + * a message object. The caller should call notmuch_message_destroy when done + * with the message. + * + * On any failure or when the message is not found, this function initializes + * '*message' to NULL. This means, when NOTMUCH_STATUS_SUCCESS is returned, the + * caller is supposed to check '*message' for NULL to find out whether the + * message with the given filename is found. + * + * Return value: + * + * NOTMUCH_STATUS_SUCCESS: Successful return, check '*message' + * + * NOTMUCH_STATUS_NULL_POINTER: The given 'message' argument is NULL + * + * NOTMUCH_STATUS_OUT_OF_MEMORY: Out of memory, creating the message object + * + * NOTMUCH_STATUS_XAPIAN_EXCEPTION: A Xapian exception occurred + */ +func (self *Database) FindMessageByFilename(filename string) (*Message, Status) { + + var c_msg_filename *C.char = C.CString(filename) + defer C.free(unsafe.Pointer(c_msg_filename)) + + if c_msg_filename == nil { + return nil, STATUS_OUT_OF_MEMORY + } + + var msg *C.notmuch_message_t + st := Status(C.notmuch_database_find_message_by_filename(self.db, c_msg_filename, &msg)) + if st != STATUS_SUCCESS { + return nil, st + } + return createMessage(msg, nil), st +} + /* Return a list of all tags found in the database. * * This function creates a list of all tags found in the database. The -- 1.7.7.6 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 1/7] go: Use iota in enum bindings
Using iota is the correct way to get the values in the enum increment automatically. The old code would just set all the enum values to 0. --- bindings/go/src/notmuch/notmuch.go |6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/bindings/go/src/notmuch/notmuch.go b/bindings/go/src/notmuch/notmuch.go index 00bd53a..ecd7418 100644 --- a/bindings/go/src/notmuch/notmuch.go +++ b/bindings/go/src/notmuch/notmuch.go @@ -86,7 +86,7 @@ type Filenames struct { type DatabaseMode C.notmuch_database_mode_t const ( - DATABASE_MODE_READ_ONLY DatabaseMode = 0 + DATABASE_MODE_READ_ONLY DatabaseMode = iota DATABASE_MODE_READ_WRITE ) @@ -386,7 +386,7 @@ func (self *Database) CreateQuery(query string) *Query { type Sort C.notmuch_sort_t const ( - SORT_OLDEST_FIRST Sort = 0 + SORT_OLDEST_FIRST Sort = iota SORT_NEWEST_FIRST SORT_MESSAGE_ID SORT_UNSORTED @@ -774,7 +774,7 @@ func (self *Message) GetFileName() string { type Flag C.notmuch_message_flag_t const ( - MESSAGE_FLAG_MATCH Flag = 0 + MESSAGE_FLAG_MATCH Flag = iota ) /* Get a value of a flag for the email corresponding to 'message'. */ -- 1.7.7.6 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 4/7] go: Make Destroy functions safe to call several times
Those methods were already checking if the underlying C object was NULL, but they were not setting the pointer to NULL after destroying it. --- bindings/go/src/notmuch/notmuch.go |6 ++ 1 files changed, 6 insertions(+), 0 deletions(-) diff --git a/bindings/go/src/notmuch/notmuch.go b/bindings/go/src/notmuch/notmuch.go index 3f436a0..d8b2739 100644 --- a/bindings/go/src/notmuch/notmuch.go +++ b/bindings/go/src/notmuch/notmuch.go @@ -634,6 +634,7 @@ func (self *Query) SearchMessages() *Messages { func (self *Query) Destroy() { if self.query != nil { C.notmuch_query_destroy(self.query) + self.query = nil } } @@ -680,6 +681,7 @@ func (self *Threads) Valid() bool { func (self *Threads) Destroy() { if self.threads != nil { C.notmuch_threads_destroy(self.threads) + self.threads = nil } } @@ -1143,6 +1145,7 @@ func (self *Message) Destroy() { return } C.notmuch_message_destroy(self.message) + self.message = nil } /* Is the given 'tags' iterator pointing at a valid tag. @@ -1214,6 +1217,7 @@ func (self *Tags) Destroy() { return } C.notmuch_tags_destroy(self.tags) + self.tags = nil } // TODO: wrap notmuch_directory_ @@ -1224,6 +1228,7 @@ func (self *Directory) Destroy() { return } C.notmuch_directory_destroy(self.dir) + self.dir = nil } // TODO: wrap notmuch_filenames_ @@ -1242,6 +1247,7 @@ func (self *Filenames) Destroy() { return } C.notmuch_filenames_destroy(self.fnames) + self.fnames = nil } /* EOF */ -- 1.7.7.6 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 5/7] go: Partially bind notmuch_database_upgrade
This binding does not handle the progress callback, but at least allows opening and upgrading a database if needed. --- bindings/go/src/notmuch/notmuch.go | 13 - 1 files changed, 12 insertions(+), 1 deletions(-) diff --git a/bindings/go/src/notmuch/notmuch.go b/bindings/go/src/notmuch/notmuch.go index d8b2739..384d5a5 100644 --- a/bindings/go/src/notmuch/notmuch.go +++ b/bindings/go/src/notmuch/notmuch.go @@ -296,7 +296,18 @@ func (self *Database) NeedsUpgrade() bool { return true } -// TODO: notmuch_database_upgrade +// TODO: Proper notmuch_database_upgrade +/* Upgrade the current database. + * + * After opening a database in read-write mode, the client should + * check if an upgrade is needed (notmuch_database_needs_upgrade) and + * if so, upgrade with this function before making any modifications. + */ +func (self *Database) Upgrade() Status { + st := Status(C.notmuch_database_upgrade(self.db, nil, nil)); + + return st; +} /* Retrieve a directory object from the database for 'path'. * -- 1.7.7.6 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH] Makefile: specify libnotmuch.so location with -rpath
On Sun, 25 Apr 2010 16:38:40 +0100, Chris Wilson wrote: > In order to handle installation into user directories, it is convenient > to encode the library location into the search path for the notmuch > executable. This is achieved for the GNU linker with the -rpath > argument. > --- > Makefile.local |2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/Makefile.local b/Makefile.local > index 5bb570b..77d2c45 100644 > --- a/Makefile.local > +++ b/Makefile.local > @@ -31,7 +31,7 @@ GPG_FILE=$(SHA1_FILE).asc > # Smash together user's values with our extra values > FINAL_CFLAGS = -DNOTMUCH_VERSION=$(VERSION) $(CFLAGS) $(WARN_CFLAGS) > $(CONFIGURE_CFLAGS) $(extra_cflags) > FINAL_CXXFLAGS = $(CXXFLAGS) $(WARN_CXXFLAGS) $(CONFIGURE_CXXFLAGS) > $(extra_cflags) $(extra_cxxflags) > -FINAL_NOTMUCH_LDFLAGS = $(LDFLAGS) -Llib -lnotmuch > +FINAL_NOTMUCH_LDFLAGS = $(LDFLAGS) -Wl,-rpath=$(prefix)/lib -Llib > -lnotmuch > FINAL_NOTMUCH_LINKER = CC > ifneq ($(LINKER_RESOLVES_LIBRARY_DEPENDENCIES),1) > FINAL_NOTMUCH_LDFLAGS += $(CONFIGURE_LDFLAGS) Hello Chris, I know that many distros (among them Fedora, see https://fedoraproject.org/wiki/Packaging/Guidelines#Beware_of_Rpath ) discourage the use of rpath. Do you think it could be an option set at configure time (set rpath or not) ? Cheers Adrien
[PATCH] Makefile: specify libnotmuch.so location with -rpath
On Sun, 25 Apr 2010 16:38:40 +0100, Chris Wilson wrote: > In order to handle installation into user directories, it is convenient > to encode the library location into the search path for the notmuch > executable. This is achieved for the GNU linker with the -rpath > argument. > --- > Makefile.local |2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/Makefile.local b/Makefile.local > index 5bb570b..77d2c45 100644 > --- a/Makefile.local > +++ b/Makefile.local > @@ -31,7 +31,7 @@ GPG_FILE=$(SHA1_FILE).asc > # Smash together user's values with our extra values > FINAL_CFLAGS = -DNOTMUCH_VERSION=$(VERSION) $(CFLAGS) $(WARN_CFLAGS) > $(CONFIGURE_CFLAGS) $(extra_cflags) > FINAL_CXXFLAGS = $(CXXFLAGS) $(WARN_CXXFLAGS) $(CONFIGURE_CXXFLAGS) > $(extra_cflags) $(extra_cxxflags) > -FINAL_NOTMUCH_LDFLAGS = $(LDFLAGS) -Llib -lnotmuch > +FINAL_NOTMUCH_LDFLAGS = $(LDFLAGS) -Wl,-rpath=$(prefix)/lib -Llib > -lnotmuch > FINAL_NOTMUCH_LINKER = CC > ifneq ($(LINKER_RESOLVES_LIBRARY_DEPENDENCIES),1) > FINAL_NOTMUCH_LDFLAGS += $(CONFIGURE_LDFLAGS) Hello Chris, I know that many distros (among them Fedora, see https://fedoraproject.org/wiki/Packaging/Guidelines#Beware_of_Rpath ) discourage the use of rpath. Do you think it could be an option set at configure time (set rpath or not) ? Cheers Adrien
Re: [PATCH] Makefile: specify libnotmuch.so location with -rpath
On Sun, 25 Apr 2010 16:38:40 +0100, Chris Wilson wrote: > In order to handle installation into user directories, it is convenient > to encode the library location into the search path for the notmuch > executable. This is achieved for the GNU linker with the -rpath > argument. > --- > Makefile.local |2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/Makefile.local b/Makefile.local > index 5bb570b..77d2c45 100644 > --- a/Makefile.local > +++ b/Makefile.local > @@ -31,7 +31,7 @@ GPG_FILE=$(SHA1_FILE).asc > # Smash together user's values with our extra values > FINAL_CFLAGS = -DNOTMUCH_VERSION=$(VERSION) $(CFLAGS) $(WARN_CFLAGS) > $(CONFIGURE_CFLAGS) $(extra_cflags) > FINAL_CXXFLAGS = $(CXXFLAGS) $(WARN_CXXFLAGS) $(CONFIGURE_CXXFLAGS) > $(extra_cflags) $(extra_cxxflags) > -FINAL_NOTMUCH_LDFLAGS = $(LDFLAGS) -Llib -lnotmuch > +FINAL_NOTMUCH_LDFLAGS = $(LDFLAGS) -Wl,-rpath=$(prefix)/lib -Llib > -lnotmuch > FINAL_NOTMUCH_LINKER = CC > ifneq ($(LINKER_RESOLVES_LIBRARY_DEPENDENCIES),1) > FINAL_NOTMUCH_LDFLAGS += $(CONFIGURE_LDFLAGS) Hello Chris, I know that many distros (among them Fedora, see https://fedoraproject.org/wiki/Packaging/Guidelines#Beware_of_Rpath ) discourage the use of rpath. Do you think it could be an option set at configure time (set rpath or not) ? Cheers Adrien ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH] Makefile: specify libnotmuch.so location with -rpath
On Sun, 25 Apr 2010 16:38:40 +0100, Chris Wilson wrote: > In order to handle installation into user directories, it is convenient > to encode the library location into the search path for the notmuch > executable. This is achieved for the GNU linker with the -rpath > argument. > --- > Makefile.local |2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/Makefile.local b/Makefile.local > index 5bb570b..77d2c45 100644 > --- a/Makefile.local > +++ b/Makefile.local > @@ -31,7 +31,7 @@ GPG_FILE=$(SHA1_FILE).asc > # Smash together user's values with our extra values > FINAL_CFLAGS = -DNOTMUCH_VERSION=$(VERSION) $(CFLAGS) $(WARN_CFLAGS) > $(CONFIGURE_CFLAGS) $(extra_cflags) > FINAL_CXXFLAGS = $(CXXFLAGS) $(WARN_CXXFLAGS) $(CONFIGURE_CXXFLAGS) > $(extra_cflags) $(extra_cxxflags) > -FINAL_NOTMUCH_LDFLAGS = $(LDFLAGS) -Llib -lnotmuch > +FINAL_NOTMUCH_LDFLAGS = $(LDFLAGS) -Wl,-rpath=$(prefix)/lib -Llib > -lnotmuch > FINAL_NOTMUCH_LINKER = CC > ifneq ($(LINKER_RESOLVES_LIBRARY_DEPENDENCIES),1) > FINAL_NOTMUCH_LDFLAGS += $(CONFIGURE_LDFLAGS) Hello Chris, I know that many distros (among them Fedora, see https://fedoraproject.org/wiki/Packaging/Guidelines#Beware_of_Rpath ) discourage the use of rpath. Do you think it could be an option set at configure time (set rpath or not) ? Cheers Adrien ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH] Makefile.local: Fix LDFLAGS for notmuch-shared
This commit adds GMIME_LDFLAGS and TALLOC_LDFLAGS to the linker flags when linking notmuch-shared. Without these flags, linking fails because of undefined symbols. --- Makefile.local |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/Makefile.local b/Makefile.local index 5bb570b..37dd745 100644 --- a/Makefile.local +++ b/Makefile.local @@ -31,7 +31,7 @@ GPG_FILE=$(SHA1_FILE).asc # Smash together user's values with our extra values FINAL_CFLAGS = -DNOTMUCH_VERSION=$(VERSION) $(CFLAGS) $(WARN_CFLAGS) $(CONFIGURE_CFLAGS) $(extra_cflags) FINAL_CXXFLAGS = $(CXXFLAGS) $(WARN_CXXFLAGS) $(CONFIGURE_CXXFLAGS) $(extra_cflags) $(extra_cxxflags) -FINAL_NOTMUCH_LDFLAGS = $(LDFLAGS) -Llib -lnotmuch +FINAL_NOTMUCH_LDFLAGS = $(LDFLAGS) $(GMIME_LDFLAGS) $(TALLOC_LDFLAGS) -Llib -lnotmuch FINAL_NOTMUCH_LINKER = CC ifneq ($(LINKER_RESOLVES_LIBRARY_DEPENDENCIES),1) FINAL_NOTMUCH_LDFLAGS += $(CONFIGURE_LDFLAGS) -- 1.7.0.1
[PATCH] Makefile.local: Fix LDFLAGS for notmuch-shared
This commit adds GMIME_LDFLAGS and TALLOC_LDFLAGS to the linker flags when linking notmuch-shared. Without these flags, linking fails because of undefined symbols. --- Makefile.local |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/Makefile.local b/Makefile.local index 5bb570b..37dd745 100644 --- a/Makefile.local +++ b/Makefile.local @@ -31,7 +31,7 @@ GPG_FILE=$(SHA1_FILE).asc # Smash together user's values with our extra values FINAL_CFLAGS = -DNOTMUCH_VERSION=$(VERSION) $(CFLAGS) $(WARN_CFLAGS) $(CONFIGURE_CFLAGS) $(extra_cflags) FINAL_CXXFLAGS = $(CXXFLAGS) $(WARN_CXXFLAGS) $(CONFIGURE_CXXFLAGS) $(extra_cflags) $(extra_cxxflags) -FINAL_NOTMUCH_LDFLAGS = $(LDFLAGS) -Llib -lnotmuch +FINAL_NOTMUCH_LDFLAGS = $(LDFLAGS) $(GMIME_LDFLAGS) $(TALLOC_LDFLAGS) -Llib -lnotmuch FINAL_NOTMUCH_LINKER = CC ifneq ($(LINKER_RESOLVES_LIBRARY_DEPENDENCIES),1) FINAL_NOTMUCH_LDFLAGS += $(CONFIGURE_LDFLAGS) -- 1.7.0.1 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH] configure: Add support for GMime 2.6
Just sending the patch again for the 0.3 merge window... Cheers Adrien On Thu, 15 Apr 2010 19:41:55 -0400, Adrien Bustany wrote: > Notmuch compiles just fine with GMime 2.6, so accept it in the configure > script. > --- > configure |5 + > 1 files changed, 5 insertions(+), 0 deletions(-) > > diff --git a/configure b/configure > index eebe075..d4d462f 100755 > --- a/configure > +++ b/configure > @@ -188,6 +188,11 @@ if pkg-config --modversion gmime-2.4 > /dev/null > 2>&1; then > have_gmime=1 > gmime_cflags=$(pkg-config --cflags gmime-2.4) > gmime_ldflags=$(pkg-config --libs gmime-2.4) > +elif pkg-config --modversion gmime-2.6 > /dev/null 2>&1; then > +printf "Yes.\n" > +have_gmime=1 > +gmime_cflags=$(pkg-config --cflags gmime-2.6) > +gmime_ldflags=$(pkg-config --libs gmime-2.6) > else > printf "No.\n" > have_gmime=0
Re: [PATCH] configure: Add support for GMime 2.6
Just sending the patch again for the 0.3 merge window... Cheers Adrien On Thu, 15 Apr 2010 19:41:55 -0400, Adrien Bustany wrote: > Notmuch compiles just fine with GMime 2.6, so accept it in the configure > script. > --- > configure |5 + > 1 files changed, 5 insertions(+), 0 deletions(-) > > diff --git a/configure b/configure > index eebe075..d4d462f 100755 > --- a/configure > +++ b/configure > @@ -188,6 +188,11 @@ if pkg-config --modversion gmime-2.4 > /dev/null > 2>&1; then > have_gmime=1 > gmime_cflags=$(pkg-config --cflags gmime-2.4) > gmime_ldflags=$(pkg-config --libs gmime-2.4) > +elif pkg-config --modversion gmime-2.6 > /dev/null 2>&1; then > +printf "Yes.\n" > +have_gmime=1 > +gmime_cflags=$(pkg-config --cflags gmime-2.6) > +gmime_ldflags=$(pkg-config --libs gmime-2.6) > else > printf "No.\n" > have_gmime=0 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH] configure: Add support for GMime 2.6
Notmuch compiles just fine with GMime 2.6, so accept it in the configure script. --- configure |5 + 1 files changed, 5 insertions(+), 0 deletions(-) diff --git a/configure b/configure index eebe075..d4d462f 100755 --- a/configure +++ b/configure @@ -188,6 +188,11 @@ if pkg-config --modversion gmime-2.4 > /dev/null 2>&1; then have_gmime=1 gmime_cflags=$(pkg-config --cflags gmime-2.4) gmime_ldflags=$(pkg-config --libs gmime-2.4) +elif pkg-config --modversion gmime-2.6 > /dev/null 2>&1; then +printf "Yes.\n" +have_gmime=1 +gmime_cflags=$(pkg-config --cflags gmime-2.6) +gmime_ldflags=$(pkg-config --libs gmime-2.6) else printf "No.\n" have_gmime=0 -- 1.7.0.1
[PATCH] configure: Add support for GMime 2.6
Notmuch compiles just fine with GMime 2.6, so accept it in the configure script. --- configure |5 + 1 files changed, 5 insertions(+), 0 deletions(-) diff --git a/configure b/configure index eebe075..d4d462f 100755 --- a/configure +++ b/configure @@ -188,6 +188,11 @@ if pkg-config --modversion gmime-2.4 > /dev/null 2>&1; then have_gmime=1 gmime_cflags=$(pkg-config --cflags gmime-2.4) gmime_ldflags=$(pkg-config --libs gmime-2.4) +elif pkg-config --modversion gmime-2.6 > /dev/null 2>&1; then +printf "Yes.\n" +have_gmime=1 +gmime_cflags=$(pkg-config --cflags gmime-2.6) +gmime_ldflags=$(pkg-config --libs gmime-2.6) else printf "No.\n" have_gmime=0 -- 1.7.0.1 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[notmuch] vala, this is notmuch. notmuch, this is vala
On Mon, 5 Apr 2010 14:50:04 +0100, Enrico Zini wrote: > On Mon, Apr 05, 2010 at 02:49:23PM +0200, Sebastian Spaeth wrote: > >> So I bound notmuch.so to vala (at least what I needed) and played with >> the code a bit. The resulting 100 lines of vala code are here: > > Ooh, a .vapi for notmuch, that is something that makes me happy. It > could be shipped with notmuch proper, even if it's rough now, and then > improved as people use it. We actually realized with spaetz that we duplicated work on this one... I also have a vapi file, which is working pretty well so far (I've used almost all the functions in it). I guess we should merge them, and include the result in notmuch's tree. See http://git.mymadcat.com/index.php/p/abitmore/source/tree/master/src/notmuch.vapi > > >> Usage: "./vnotmuch Seb" will output all 'to:' addresses according to >> frequency for all messages where to, cc, or bcc matches "Seb*". It also >> filters with AND "from:yourprimarymailaddress". Just >> "./vnotmuch" outputs all addresses that you ever sent mails to. It never >> writes/modifies your db. > > Now I use "lbdb", which gets very slow as time goes. You idea creates a > most definitely superior system. > > >> Just a teaser to make you interested in vala :). > > As it happens, some of us already are interested. > > As soon as automatic gobject introspection based language bindings > become workable for at least python and perl, my plan is to rewrite > buffy[1] in Vala. > > A second plan would be to have buffy show stats for saved notmuch > queries as well as (or instead of) mail folders. > > It's very nice to know I wouldn't be the only person playing with Vala > around here. Make them two ;) Cheers Adrien > > > Ciao, > > Enrico > > [1] http://packages.debian.org/sid/buffy
Re: [notmuch] vala, this is notmuch. notmuch, this is vala
On Mon, 5 Apr 2010 14:50:04 +0100, Enrico Zini wrote: > On Mon, Apr 05, 2010 at 02:49:23PM +0200, Sebastian Spaeth wrote: > >> So I bound notmuch.so to vala (at least what I needed) and played with >> the code a bit. The resulting 100 lines of vala code are here: > > Ooh, a .vapi for notmuch, that is something that makes me happy. It > could be shipped with notmuch proper, even if it's rough now, and then > improved as people use it. We actually realized with spaetz that we duplicated work on this one... I also have a vapi file, which is working pretty well so far (I've used almost all the functions in it). I guess we should merge them, and include the result in notmuch's tree. See http://git.mymadcat.com/index.php/p/abitmore/source/tree/master/src/notmuch.vapi > > >> Usage: "./vnotmuch Seb" will output all 'to:' addresses according to >> frequency for all messages where to, cc, or bcc matches "Seb*". It also >> filters with AND "from:yourprimarymailaddress". Just >> "./vnotmuch" outputs all addresses that you ever sent mails to. It never >> writes/modifies your db. > > Now I use "lbdb", which gets very slow as time goes. You idea creates a > most definitely superior system. > > >> Just a teaser to make you interested in vala :). > > As it happens, some of us already are interested. > > As soon as automatic gobject introspection based language bindings > become workable for at least python and perl, my plan is to rewrite > buffy[1] in Vala. > > A second plan would be to have buffy show stats for saved notmuch > queries as well as (or instead of) mail folders. > > It's very nice to know I wouldn't be the only person playing with Vala > around here. Make them two ;) Cheers Adrien > > > Ciao, > > Enrico > > [1] http://packages.debian.org/sid/buffy ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[notmuch] keeping a copy of sent mail locally
On Mon, 21 Dec 2009 09:27:04 -0800, Carl Worth wrote: > On Sun, 20 Dec 2009 15:29:15 +1100, Alex Ghitza wrote: >> It looks like we need a way to get the primary email address from the >> config file. > > Yes, we definitely need that. > >> Actually, while we're at it, we can consider making this >> more flexible and adding a new option to the config file (e.g. bcc=...) >> which would take a semicolon-separated list of email addresses (in case >> someone wants to always bcc an address other than the primary one). > > And that would be fine too. > >> Is there a nice clean way of getting the config variables in notmuch.el? >> Or should this go into a new file notmuch-compose.c? Note that the >> latter would imply having a new command like "notmuch compose". > > I think we want a new C file to make it easy to ask for options out of > the configuration file. The operation of "notmuch compose" seems simple > enough to not be necessary, (though maybe it would make it easier to get > standard behavior among different interfaces). What about adding a way to get/set config variables from the notmuch command line ? Would that be bloat ? That would even allow changing the internal format of the conf. file without breaking the clients... [Snip] Cheers Adrien
Re: [notmuch] keeping a copy of sent mail locally
On Mon, 21 Dec 2009 09:27:04 -0800, Carl Worth wrote: > On Sun, 20 Dec 2009 15:29:15 +1100, Alex Ghitza wrote: >> It looks like we need a way to get the primary email address from the >> config file. > > Yes, we definitely need that. > >> Actually, while we're at it, we can consider making this >> more flexible and adding a new option to the config file (e.g. bcc=...) >> which would take a semicolon-separated list of email addresses (in case >> someone wants to always bcc an address other than the primary one). > > And that would be fine too. > >> Is there a nice clean way of getting the config variables in notmuch.el? >> Or should this go into a new file notmuch-compose.c? Note that the >> latter would imply having a new command like "notmuch compose". > > I think we want a new C file to make it easy to ask for options out of > the configuration file. The operation of "notmuch compose" seems simple > enough to not be necessary, (though maybe it would make it easier to get > standard behavior among different interfaces). What about adding a way to get/set config variables from the notmuch command line ? Would that be bloat ? That would even allow changing the internal format of the conf. file without breaking the clients... [Snip] Cheers Adrien ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch