[PATCH v2 5/5] news: Update for changes to notmuch_database_get_directory

2012-05-13 Thread Austin Clements
---
 NEWS |   10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/NEWS b/NEWS
index c67f186..f987811 100644
--- a/NEWS
+++ b/NEWS
@@ -91,12 +91,12 @@ notmuch_database_close and notmuch_database_destroy
   database and thus release the lock associated with it without
   destroying the data structures obtained from it.

-notmuch_database_open and notmuch_database_create now return errors
+notmuch_database_open, notmuch_database_create, and
+notmuch_database_get_directory now return errors

-  The type signatures of notmuch_database_open and
-  notmuch_database_create have changed so that the functions now
-  return a notmuch_status_t and take an out-argument for returning the
-  new database object.
+  The type signatures of these functions have changed so that the
+  functions now return a notmuch_status_t and take an out-argument for
+  returning the new database object or directory object.

 go bindings changes
 ---
-- 
1.7.10



[PATCH v2 4/5] ruby: Update for changes to notmuch_database_get_directory

2012-05-13 Thread Austin Clements
---
 bindings/ruby/database.c |   11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/bindings/ruby/database.c b/bindings/ruby/database.c
index 409d54f..e84f726 100644
--- a/bindings/ruby/database.c
+++ b/bindings/ruby/database.c
@@ -252,6 +252,7 @@ VALUE
 notmuch_rb_database_get_directory (VALUE self, VALUE pathv)
 {
 const char *path;
+notmuch_status_t ret;
 notmuch_directory_t *dir;
 notmuch_database_t *db;

@@ -260,11 +261,11 @@ notmuch_rb_database_get_directory (VALUE self, VALUE 
pathv)
 SafeStringValue (pathv);
 path = RSTRING_PTR (pathv);

-dir = notmuch_database_get_directory (db, path);
-if (!dir)
-rb_raise (notmuch_rb_eXapianError, "Xapian exception");
-
-return Data_Wrap_Struct (notmuch_rb_cDirectory, NULL, NULL, dir);
+ret = notmuch_database_get_directory (db, path, );
+notmuch_rb_status_raise (ret);
+if (dir)
+   return Data_Wrap_Struct (notmuch_rb_cDirectory, NULL, NULL, dir);
+return Qnil;
 }

 /*
-- 
1.7.10



[PATCH v2 3/5] python: Update for changes to notmuch_database_get_directory

2012-05-13 Thread Austin Clements
notmuch_database_get_directory now returns
NOTMUCH_STATUS_READ_ONLY_DATABASE on its own (rather than crashing) so
the workaround in Database.get_directory is no longer necessary.
---
 bindings/python/notmuch/database.py |   19 +--
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/bindings/python/notmuch/database.py 
b/bindings/python/notmuch/database.py
index 1b1ddc3..797554d 100644
--- a/bindings/python/notmuch/database.py
+++ b/bindings/python/notmuch/database.py
@@ -73,8 +73,8 @@ class Database(object):

 """notmuch_database_get_directory"""
 _get_directory = nmlib.notmuch_database_get_directory
-_get_directory.argtypes = [NotmuchDatabaseP, c_char_p]
-_get_directory.restype = NotmuchDirectoryP
+_get_directory.argtypes = [NotmuchDatabaseP, c_char_p, 
POINTER(NotmuchDirectoryP)]
+_get_directory.restype = c_uint

 """notmuch_database_get_path"""
 _get_path = nmlib.notmuch_database_get_path
@@ -359,13 +359,6 @@ class Database(object):
 """
 self._assert_db_is_initialized()

-# work around libnotmuch calling exit(3), see
-# id:20120221002921.8534.57091 at thinkbox.jade-hamburg.de
-# TODO: remove once this issue is resolved
-if self.mode != Database.MODE.READ_WRITE:
-raise ReadOnlyDatabaseError('The database has to be opened in '
-'read-write mode for get_directory')
-
 # sanity checking if path is valid, and make path absolute
 if path and path[0] == os.sep:
 # we got an absolute path
@@ -378,7 +371,13 @@ class Database(object):
 #we got a relative path, make it absolute
 abs_dirpath = os.path.abspath(os.path.join(self.get_path(), path))

-dir_p = Database._get_directory(self._db, _str(path))
+dir_p = NotmuchDirectoryP()
+status = Database._get_directory(self._db, _str(path), byref(dir_p))
+
+if status != STATUS.SUCCESS:
+raise NotmuchError(status)
+if not dir_p:
+return None

 # return the Directory, init it with the absolute path
 return Directory(abs_dirpath, dir_p, self)
-- 
1.7.10



[PATCH v2 2/5] go: Update for changes to notmuch_database_get_directory

2012-05-13 Thread Austin Clements
---
 bindings/go/src/notmuch/notmuch.go |   13 +++--
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/bindings/go/src/notmuch/notmuch.go 
b/bindings/go/src/notmuch/notmuch.go
index 12de4c8..00bd53a 100644
--- a/bindings/go/src/notmuch/notmuch.go
+++ b/bindings/go/src/notmuch/notmuch.go
@@ -191,19 +191,20 @@ func (self *Database) NeedsUpgrade() bool {
  *
  * Can return NULL if a Xapian exception occurs.
  */
-func (self *Database) GetDirectory(path string) *Directory {
+func (self *Database) GetDirectory(path string) (*Directory, Status) {
var c_path *C.char = C.CString(path)
defer C.free(unsafe.Pointer(c_path))

if c_path == nil {
-   return nil
+   return nil, STATUS_OUT_OF_MEMORY
}

-   c_dir := C.notmuch_database_get_directory(self.db, c_path)
-   if c_dir == nil {
-   return nil
+   var c_dir *C.notmuch_directory_t
+   st := Status(C.notmuch_database_get_directory(self.db, c_path, _dir))
+   if st != STATUS_SUCCESS || c_dir == nil {
+   return nil, st
}
-   return {dir: c_dir}
+   return {dir: c_dir}, st
 }

 /* Add a new message to the given notmuch database.
-- 
1.7.10



[PATCH v2 1/5] lib/cli: Make notmuch_database_get_directory return a status code

2012-05-13 Thread Austin Clements
Previously, notmuch_database_get_directory had no way to indicate how
it had failed.  This changes its prototype to return a status code and
set an out-argument to the retrieved directory, like similar functions
in the library API.  This does *not* change its currently broken
behavior of creating directory objects when they don't exist, but it
does document it and paves the way for fixing this.  Also, it can now
check for a read-only database and return
NOTMUCH_STATUS_READ_ONLY_DATABASE instead of crashing.

In the interest of atomicity, this also updates calls from the CLI so
that notmuch still compiles.
---
 lib/database.cc |   22 --
 lib/notmuch.h   |   23 ---
 notmuch-new.c   |   21 ++---
 3 files changed, 50 insertions(+), 16 deletions(-)

diff --git a/lib/database.cc b/lib/database.cc
index 07f186e..f8c4a7d 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -955,8 +955,8 @@ notmuch_database_upgrade (notmuch_database_t *notmuch,
mtime = Xapian::sortable_unserialise (
document.get_value (NOTMUCH_VALUE_TIMESTAMP));

-   directory = notmuch_database_get_directory (notmuch,
-   term.c_str() + 10);
+   directory = _notmuch_directory_create (notmuch, term.c_str() + 
10,
+  );
notmuch_directory_set_mtime (directory, mtime);
notmuch_directory_destroy (directory);
}
@@ -1304,20 +1304,30 @@ _notmuch_database_relative_path (notmuch_database_t 
*notmuch,
 return relative;
 }

-notmuch_directory_t *
+notmuch_status_t
 notmuch_database_get_directory (notmuch_database_t *notmuch,
-   const char *path)
+   const char *path,
+   notmuch_directory_t **directory)
 {
 notmuch_status_t status;

+if (directory == NULL)
+   return NOTMUCH_STATUS_NULL_POINTER;
+*directory = NULL;
+
+/* XXX Handle read-only databases properly */
+if (notmuch->mode == NOTMUCH_DATABASE_MODE_READ_ONLY)
+   return NOTMUCH_STATUS_READ_ONLY_DATABASE;
+
 try {
-   return _notmuch_directory_create (notmuch, path, );
+   *directory = _notmuch_directory_create (notmuch, path, );
 } catch (const Xapian::Error ) {
fprintf (stderr, "A Xapian exception occurred getting directory: %s.\n",
 error.get_msg().c_str());
notmuch->exception_reported = TRUE;
-   return NULL;
+   status = NOTMUCH_STATUS_XAPIAN_EXCEPTION;
 }
+return status;
 }

 /* Allocate a document ID that satisfies the following criteria:
diff --git a/lib/notmuch.h b/lib/notmuch.h
index e6e5cc2..bbb17e4 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -300,11 +300,28 @@ notmuch_database_end_atomic (notmuch_database_t *notmuch);
  * (see notmuch_database_get_path), or else should be an absolute path
  * with initial components that match the path of 'database'.
  *
- * Can return NULL if a Xapian exception occurs.
+ * Note: Currently this will create the directory object if it doesn't
+ * exist.  In the future, when a directory object does not exist this
+ * will return NOTMUCH_STATUS_SUCCESS and set *directory to NULL.
+ * Callers should be prepared for this.
+ *
+ * Return value:
+ *
+ * NOTMUCH_STATUS_SUCCESS: Successfully retrieved directory.
+ *
+ * NOTMUCH_STATUS_NULL_POINTER: The given 'directory' argument is NULL.
+ *
+ * NOTMUCH_STATUS_XAPIAN_EXCEPTION: A Xapian exception occurred;
+ * directory not retrieved.
+ *
+ * NOTMUCH_STATUS_READ_ONLY_DATABASE: Database was opened in read-only
+ * mode so the directory cannot be created (this case will be
+ * removed in the future).
  */
-notmuch_directory_t *
+notmuch_status_t
 notmuch_database_get_directory (notmuch_database_t *database,
-   const char *path);
+   const char *path,
+   notmuch_directory_t **directory);

 /* Add a new message to the given notmuch database or associate an
  * additional filename with an existing message.
diff --git a/notmuch-new.c b/notmuch-new.c
index cb720cc..a3a8bec 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -250,7 +250,7 @@ add_files_recursive (notmuch_database_t *notmuch,
 notmuch_status_t status, ret = NOTMUCH_STATUS_SUCCESS;
 notmuch_message_t *message = NULL;
 struct dirent **fs_entries = NULL;
-int i, num_fs_entries;
+int i, num_fs_entries = 0;
 notmuch_directory_t *directory;
 notmuch_filenames_t *db_files = NULL;
 notmuch_filenames_t *db_subdirs = NULL;
@@ -274,8 +274,12 @@ add_files_recursive (notmuch_database_t *notmuch,

 fs_mtime = st.st_mtime;

-directory = notmuch_database_get_directory (notmuch, path);
-db_mtime = notmuch_directory_get_mtime (directory);
+status = notmuch_database_get_directory (notmuch, 

[PATCH v2 0/5] Fix notmuch_database_get_directory API

2012-05-13 Thread Austin Clements
This version moves a Python bindings change that had slipped into the
Go patch into the Python patch and words the future-proofing warning
on notmuch_database_get_directory more strongly.  There are no code
changes from v1.



[PATCH 2/5] go: Update for changes to notmuch_database_get_directory

2012-05-13 Thread Austin Clements
Quoth Justus Winter on May 13 at 11:51 pm:
> Hi Austin,
> 
> thanks for taking care of this.
> 
> Quoting Austin Clements (2012-05-13 21:57:06)
> > ---
> >  bindings/go/src/notmuch/notmuch.go  |   13 +++--
> >  bindings/python/notmuch/database.py |4 ++--
> 
> A change to the python bindings slipped into this one.

Sure enough.  I'll send a v2 that moves that hunk to the python patch.

> Other than that this looks good.
> 
> I just saw that I marked another function as problematic,
> notmuch_database_find_message_by_filename ends up calling
> _notmuch_directory_create that aborts if the database is not
> writable. But this might have to wait and might not even requira an
> api change.

Right.  notmuch_database_find_message_by_filename already has a status
return, so its API is fine.  (Actually, I was starting to fix its
implementation when I realized notmuch_database_get_directory was
problematic.)

> Justus


[PATCH] news: Update for changes to notmuch_database_get_directory

2012-05-13 Thread Austin Clements
Sorry for the spam.  This was supposed to be a test message to myself
and I forgot I had git-send-email configured to sent to
notmuch at notmuchmail.org by default.


[PATCH] news: Update for changes to notmuch_database_get_directory

2012-05-13 Thread Austin Clements
---
 NEWS |   10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/NEWS b/NEWS
index c67f186..f987811 100644
--- a/NEWS
+++ b/NEWS
@@ -91,12 +91,12 @@ notmuch_database_close and notmuch_database_destroy
   database and thus release the lock associated with it without
   destroying the data structures obtained from it.

-notmuch_database_open and notmuch_database_create now return errors
+notmuch_database_open, notmuch_database_create, and
+notmuch_database_get_directory now return errors

-  The type signatures of notmuch_database_open and
-  notmuch_database_create have changed so that the functions now
-  return a notmuch_status_t and take an out-argument for returning the
-  new database object.
+  The type signatures of these functions have changed so that the
+  functions now return a notmuch_status_t and take an out-argument for
+  returning the new database object or directory object.

 go bindings changes
 ---
-- 
1.7.10



[PATCH 5/5] news: Update for changes to notmuch_database_get_directory

2012-05-13 Thread Austin Clements
---
 NEWS |   10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/NEWS b/NEWS
index c67f186..f987811 100644
--- a/NEWS
+++ b/NEWS
@@ -91,12 +91,12 @@ notmuch_database_close and notmuch_database_destroy
   database and thus release the lock associated with it without
   destroying the data structures obtained from it.

-notmuch_database_open and notmuch_database_create now return errors
+notmuch_database_open, notmuch_database_create, and
+notmuch_database_get_directory now return errors

-  The type signatures of notmuch_database_open and
-  notmuch_database_create have changed so that the functions now
-  return a notmuch_status_t and take an out-argument for returning the
-  new database object.
+  The type signatures of these functions have changed so that the
+  functions now return a notmuch_status_t and take an out-argument for
+  returning the new database object or directory object.

 go bindings changes
 ---
-- 
1.7.10



[PATCH 4/5] ruby: Update for changes to notmuch_database_get_directory

2012-05-13 Thread Austin Clements
---
 bindings/ruby/database.c |   11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/bindings/ruby/database.c b/bindings/ruby/database.c
index 409d54f..e84f726 100644
--- a/bindings/ruby/database.c
+++ b/bindings/ruby/database.c
@@ -252,6 +252,7 @@ VALUE
 notmuch_rb_database_get_directory (VALUE self, VALUE pathv)
 {
 const char *path;
+notmuch_status_t ret;
 notmuch_directory_t *dir;
 notmuch_database_t *db;

@@ -260,11 +261,11 @@ notmuch_rb_database_get_directory (VALUE self, VALUE 
pathv)
 SafeStringValue (pathv);
 path = RSTRING_PTR (pathv);

-dir = notmuch_database_get_directory (db, path);
-if (!dir)
-rb_raise (notmuch_rb_eXapianError, "Xapian exception");
-
-return Data_Wrap_Struct (notmuch_rb_cDirectory, NULL, NULL, dir);
+ret = notmuch_database_get_directory (db, path, );
+notmuch_rb_status_raise (ret);
+if (dir)
+   return Data_Wrap_Struct (notmuch_rb_cDirectory, NULL, NULL, dir);
+return Qnil;
 }

 /*
-- 
1.7.10



[PATCH 3/5] python: Update for changes to notmuch_database_get_directory

2012-05-13 Thread Austin Clements
notmuch_database_get_directory now returns
NOTMUCH_STATUS_READ_ONLY_DATABASE on its own (rather than crashing) so
the workaround in Database.get_directory is no longer necessary.
---
 bindings/python/notmuch/database.py |   15 +++
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/bindings/python/notmuch/database.py 
b/bindings/python/notmuch/database.py
index 0a58dd0..797554d 100644
--- a/bindings/python/notmuch/database.py
+++ b/bindings/python/notmuch/database.py
@@ -359,13 +359,6 @@ class Database(object):
 """
 self._assert_db_is_initialized()

-# work around libnotmuch calling exit(3), see
-# id:20120221002921.8534.57091 at thinkbox.jade-hamburg.de
-# TODO: remove once this issue is resolved
-if self.mode != Database.MODE.READ_WRITE:
-raise ReadOnlyDatabaseError('The database has to be opened in '
-'read-write mode for get_directory')
-
 # sanity checking if path is valid, and make path absolute
 if path and path[0] == os.sep:
 # we got an absolute path
@@ -378,7 +371,13 @@ class Database(object):
 #we got a relative path, make it absolute
 abs_dirpath = os.path.abspath(os.path.join(self.get_path(), path))

-dir_p = Database._get_directory(self._db, _str(path))
+dir_p = NotmuchDirectoryP()
+status = Database._get_directory(self._db, _str(path), byref(dir_p))
+
+if status != STATUS.SUCCESS:
+raise NotmuchError(status)
+if not dir_p:
+return None

 # return the Directory, init it with the absolute path
 return Directory(abs_dirpath, dir_p, self)
-- 
1.7.10



[PATCH 2/5] go: Update for changes to notmuch_database_get_directory

2012-05-13 Thread Austin Clements
---
 bindings/go/src/notmuch/notmuch.go  |   13 +++--
 bindings/python/notmuch/database.py |4 ++--
 2 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/bindings/go/src/notmuch/notmuch.go 
b/bindings/go/src/notmuch/notmuch.go
index 12de4c8..00bd53a 100644
--- a/bindings/go/src/notmuch/notmuch.go
+++ b/bindings/go/src/notmuch/notmuch.go
@@ -191,19 +191,20 @@ func (self *Database) NeedsUpgrade() bool {
  *
  * Can return NULL if a Xapian exception occurs.
  */
-func (self *Database) GetDirectory(path string) *Directory {
+func (self *Database) GetDirectory(path string) (*Directory, Status) {
var c_path *C.char = C.CString(path)
defer C.free(unsafe.Pointer(c_path))

if c_path == nil {
-   return nil
+   return nil, STATUS_OUT_OF_MEMORY
}

-   c_dir := C.notmuch_database_get_directory(self.db, c_path)
-   if c_dir == nil {
-   return nil
+   var c_dir *C.notmuch_directory_t
+   st := Status(C.notmuch_database_get_directory(self.db, c_path, _dir))
+   if st != STATUS_SUCCESS || c_dir == nil {
+   return nil, st
}
-   return {dir: c_dir}
+   return {dir: c_dir}, st
 }

 /* Add a new message to the given notmuch database.
diff --git a/bindings/python/notmuch/database.py 
b/bindings/python/notmuch/database.py
index 1b1ddc3..0a58dd0 100644
--- a/bindings/python/notmuch/database.py
+++ b/bindings/python/notmuch/database.py
@@ -73,8 +73,8 @@ class Database(object):

 """notmuch_database_get_directory"""
 _get_directory = nmlib.notmuch_database_get_directory
-_get_directory.argtypes = [NotmuchDatabaseP, c_char_p]
-_get_directory.restype = NotmuchDirectoryP
+_get_directory.argtypes = [NotmuchDatabaseP, c_char_p, 
POINTER(NotmuchDirectoryP)]
+_get_directory.restype = c_uint

 """notmuch_database_get_path"""
 _get_path = nmlib.notmuch_database_get_path
-- 
1.7.10



[PATCH 1/5] lib/cli: Make notmuch_database_get_directory return a status code

2012-05-13 Thread Austin Clements
Previously, notmuch_database_get_directory had no way to indicate how
it had failed.  This changes its prototype to return a status code and
set an out-argument to the retrieved directory, like similar functions
in the library API.  This does *not* change its currently broken
behavior of creating directory objects when they don't exist, but it
does document it and paves the way for fixing this.  Also, it can now
check for a read-only database and return
NOTMUCH_STATUS_READ_ONLY_DATABASE instead of crashing.

In the interest of atomicity, this also updates calls from the CLI so
that notmuch still compiles.
---
 lib/database.cc |   22 --
 lib/notmuch.h   |   24 +---
 notmuch-new.c   |   21 ++---
 3 files changed, 51 insertions(+), 16 deletions(-)

diff --git a/lib/database.cc b/lib/database.cc
index 07f186e..f8c4a7d 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -955,8 +955,8 @@ notmuch_database_upgrade (notmuch_database_t *notmuch,
mtime = Xapian::sortable_unserialise (
document.get_value (NOTMUCH_VALUE_TIMESTAMP));

-   directory = notmuch_database_get_directory (notmuch,
-   term.c_str() + 10);
+   directory = _notmuch_directory_create (notmuch, term.c_str() + 
10,
+  );
notmuch_directory_set_mtime (directory, mtime);
notmuch_directory_destroy (directory);
}
@@ -1304,20 +1304,30 @@ _notmuch_database_relative_path (notmuch_database_t 
*notmuch,
 return relative;
 }

-notmuch_directory_t *
+notmuch_status_t
 notmuch_database_get_directory (notmuch_database_t *notmuch,
-   const char *path)
+   const char *path,
+   notmuch_directory_t **directory)
 {
 notmuch_status_t status;

+if (directory == NULL)
+   return NOTMUCH_STATUS_NULL_POINTER;
+*directory = NULL;
+
+/* XXX Handle read-only databases properly */
+if (notmuch->mode == NOTMUCH_DATABASE_MODE_READ_ONLY)
+   return NOTMUCH_STATUS_READ_ONLY_DATABASE;
+
 try {
-   return _notmuch_directory_create (notmuch, path, );
+   *directory = _notmuch_directory_create (notmuch, path, );
 } catch (const Xapian::Error ) {
fprintf (stderr, "A Xapian exception occurred getting directory: %s.\n",
 error.get_msg().c_str());
notmuch->exception_reported = TRUE;
-   return NULL;
+   status = NOTMUCH_STATUS_XAPIAN_EXCEPTION;
 }
+return status;
 }

 /* Allocate a document ID that satisfies the following criteria:
diff --git a/lib/notmuch.h b/lib/notmuch.h
index e6e5cc2..d414198 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -300,11 +300,29 @@ notmuch_database_end_atomic (notmuch_database_t *notmuch);
  * (see notmuch_database_get_path), or else should be an absolute path
  * with initial components that match the path of 'database'.
  *
- * Can return NULL if a Xapian exception occurs.
+ * Note: Currently this will create the directory object if it doesn't
+ * exist.  This is not intentional behavior and is likely to change in
+ * the near future.  To future-proof, when this returns
+ * NOTMUCH_STATUS_SUCCESS, callers should check that *directory is not
+ * NULL.
+ *
+ * Return value:
+ *
+ * NOTMUCH_STATUS_SUCCESS: Successfully retrieved directory.
+ *
+ * NOTMUCH_STATUS_NULL_POINTER: The given 'directory' argument is NULL.
+ *
+ * NOTMUCH_STATUS_XAPIAN_EXCEPTION: A Xapian exception occurred;
+ * directory not retrieved.
+ *
+ * NOTMUCH_STATUS_READ_ONLY_DATABASE: Database was opened in read-only
+ * mode so the directory cannot be created (this case will be
+ * removed in the future).
  */
-notmuch_directory_t *
+notmuch_status_t
 notmuch_database_get_directory (notmuch_database_t *database,
-   const char *path);
+   const char *path,
+   notmuch_directory_t **directory);

 /* Add a new message to the given notmuch database or associate an
  * additional filename with an existing message.
diff --git a/notmuch-new.c b/notmuch-new.c
index cb720cc..a3a8bec 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -250,7 +250,7 @@ add_files_recursive (notmuch_database_t *notmuch,
 notmuch_status_t status, ret = NOTMUCH_STATUS_SUCCESS;
 notmuch_message_t *message = NULL;
 struct dirent **fs_entries = NULL;
-int i, num_fs_entries;
+int i, num_fs_entries = 0;
 notmuch_directory_t *directory;
 notmuch_filenames_t *db_files = NULL;
 notmuch_filenames_t *db_subdirs = NULL;
@@ -274,8 +274,12 @@ add_files_recursive (notmuch_database_t *notmuch,

 fs_mtime = st.st_mtime;

-directory = notmuch_database_get_directory (notmuch, path);
-db_mtime = notmuch_directory_get_mtime (directory);
+status = 

[PATCH 0/5] Fix notmuch_database_get_directory API

2012-05-13 Thread Austin Clements
This is a proposed last-minute change for 0.13.  It fixes the
notmuch_database_get_directory API in the same way we're fixing
notmuch_database_open, etc in this release.  Since this is a
backwards-incompatible change, it would be nice to lump it with the
other API-breaking changes.

To keep the patch simple, this does not change the behavior of
notmuch_database_get_directory, but it puts us in a good position to
fix it in the future.



[PATCH] ruby: extern linkage portability improvement

2012-05-13 Thread Austin Clements
Quoth Tomi Ollila on May 10 at  8:12 pm:
> Some C compilers are stricter when it comes to (tentative) definition
> of a variable -- in those compilers introducing variable without 'extern'
> keyword always allocates new 'storage' to the variable and linking all
> these modules fails due to duplicate symbols.
> 
> This change uses some macro trickery to avoid writing every variable twice.
> 
> This is reimplementation of Charlie Allom's patch:
> id:"1336481467-66356-1-git-send-email-charlie at mediasp.com"
> 
> combining information from other change made by Ali Polatel.
> ---
> 
> Charlie: could you test whether this patch actually work ? :)
> 
> Everyone: what do you think of the "hiding extern" macro trick ?

This seems like a hacky and nonstandard way to do this.  Granted, the
standard way to do this---always declare variables extern in .h files
and also give a non-extern definition in exactly one .c file---is more
verbose, but nobody will be surprised or confused by it.

>  bindings/ruby/defs.h |   56 +++--
>  bindings/ruby/init.c |2 +
>  2 files changed, 33 insertions(+), 25 deletions(-)
> 
> diff --git a/bindings/ruby/defs.h b/bindings/ruby/defs.h
> index 85d8205..2531760 100644
> --- a/bindings/ruby/defs.h
> +++ b/bindings/ruby/defs.h
> @@ -24,31 +24,37 @@
>  #include 
>  #include "notmuch.h"
>  
> -VALUE notmuch_rb_cDatabase;
> -VALUE notmuch_rb_cDirectory;
> -VALUE notmuch_rb_cFileNames;
> -VALUE notmuch_rb_cQuery;
> -VALUE notmuch_rb_cThreads;
> -VALUE notmuch_rb_cThread;
> -VALUE notmuch_rb_cMessages;
> -VALUE notmuch_rb_cMessage;
> -VALUE notmuch_rb_cTags;
> -
> -VALUE notmuch_rb_eBaseError;
> -VALUE notmuch_rb_eDatabaseError;
> -VALUE notmuch_rb_eMemoryError;
> -VALUE notmuch_rb_eReadOnlyError;
> -VALUE notmuch_rb_eXapianError;
> -VALUE notmuch_rb_eFileError;
> -VALUE notmuch_rb_eFileNotEmailError;
> -VALUE notmuch_rb_eNullPointerError;
> -VALUE notmuch_rb_eTagTooLongError;
> -VALUE notmuch_rb_eUnbalancedFreezeThawError;
> -VALUE notmuch_rb_eUnbalancedAtomicError;
> -
> -ID ID_call;
> -ID ID_db_create;
> -ID ID_db_mode;
> +#ifdef RUBY_INIT_C
> +#define extern
> +#endif
> +
> +extern VALUE notmuch_rb_cDatabase;
> +extern VALUE notmuch_rb_cDirectory;
> +extern VALUE notmuch_rb_cFileNames;
> +extern VALUE notmuch_rb_cQuery;
> +extern VALUE notmuch_rb_cThreads;
> +extern VALUE notmuch_rb_cThread;
> +extern VALUE notmuch_rb_cMessages;
> +extern VALUE notmuch_rb_cMessage;
> +extern VALUE notmuch_rb_cTags;
> +
> +extern VALUE notmuch_rb_eBaseError;
> +extern VALUE notmuch_rb_eDatabaseError;
> +extern VALUE notmuch_rb_eMemoryError;
> +extern VALUE notmuch_rb_eReadOnlyError;
> +extern VALUE notmuch_rb_eXapianError;
> +extern VALUE notmuch_rb_eFileError;
> +extern VALUE notmuch_rb_eFileNotEmailError;
> +extern VALUE notmuch_rb_eNullPointerError;
> +extern VALUE notmuch_rb_eTagTooLongError;
> +extern VALUE notmuch_rb_eUnbalancedFreezeThawError;
> +extern VALUE notmuch_rb_eUnbalancedAtomicError;
> +
> +extern ID ID_call;
> +extern ID ID_db_create;
> +extern ID ID_db_mode;
> +
> +#undef extern
>  
>  /* RSTRING_PTR() is new in ruby-1.9 */
>  #if !defined(RSTRING_PTR)
> diff --git a/bindings/ruby/init.c b/bindings/ruby/init.c
> index 3fe60fb..b2dc7f6 100644
> --- a/bindings/ruby/init.c
> +++ b/bindings/ruby/init.c
> @@ -18,7 +18,9 @@
>   * Author: Ali Polatel 
>   */
>  
> +#define RUBY_INIT_C
>  #include "defs.h"
> +#undef RUBY_INIT_C
>  
>  /*
>   * Document-module: Notmuch


Re: [PATCH] ruby: extern linkage portability improvement

2012-05-13 Thread Austin Clements
Quoth Tomi Ollila on May 10 at  8:12 pm:
 Some C compilers are stricter when it comes to (tentative) definition
 of a variable -- in those compilers introducing variable without 'extern'
 keyword always allocates new 'storage' to the variable and linking all
 these modules fails due to duplicate symbols.
 
 This change uses some macro trickery to avoid writing every variable twice.
 
 This is reimplementation of Charlie Allom's patch:
 id:1336481467-66356-1-git-send-email-char...@mediasp.com
 
 combining information from other change made by Ali Polatel.
 ---
 
 Charlie: could you test whether this patch actually work ? :)
 
 Everyone: what do you think of the hiding extern macro trick ?

This seems like a hacky and nonstandard way to do this.  Granted, the
standard way to do this---always declare variables extern in .h files
and also give a non-extern definition in exactly one .c file---is more
verbose, but nobody will be surprised or confused by it.

  bindings/ruby/defs.h |   56 +++--
  bindings/ruby/init.c |2 +
  2 files changed, 33 insertions(+), 25 deletions(-)
 
 diff --git a/bindings/ruby/defs.h b/bindings/ruby/defs.h
 index 85d8205..2531760 100644
 --- a/bindings/ruby/defs.h
 +++ b/bindings/ruby/defs.h
 @@ -24,31 +24,37 @@
  #include ruby.h
  #include notmuch.h
  
 -VALUE notmuch_rb_cDatabase;
 -VALUE notmuch_rb_cDirectory;
 -VALUE notmuch_rb_cFileNames;
 -VALUE notmuch_rb_cQuery;
 -VALUE notmuch_rb_cThreads;
 -VALUE notmuch_rb_cThread;
 -VALUE notmuch_rb_cMessages;
 -VALUE notmuch_rb_cMessage;
 -VALUE notmuch_rb_cTags;
 -
 -VALUE notmuch_rb_eBaseError;
 -VALUE notmuch_rb_eDatabaseError;
 -VALUE notmuch_rb_eMemoryError;
 -VALUE notmuch_rb_eReadOnlyError;
 -VALUE notmuch_rb_eXapianError;
 -VALUE notmuch_rb_eFileError;
 -VALUE notmuch_rb_eFileNotEmailError;
 -VALUE notmuch_rb_eNullPointerError;
 -VALUE notmuch_rb_eTagTooLongError;
 -VALUE notmuch_rb_eUnbalancedFreezeThawError;
 -VALUE notmuch_rb_eUnbalancedAtomicError;
 -
 -ID ID_call;
 -ID ID_db_create;
 -ID ID_db_mode;
 +#ifdef RUBY_INIT_C
 +#define extern
 +#endif
 +
 +extern VALUE notmuch_rb_cDatabase;
 +extern VALUE notmuch_rb_cDirectory;
 +extern VALUE notmuch_rb_cFileNames;
 +extern VALUE notmuch_rb_cQuery;
 +extern VALUE notmuch_rb_cThreads;
 +extern VALUE notmuch_rb_cThread;
 +extern VALUE notmuch_rb_cMessages;
 +extern VALUE notmuch_rb_cMessage;
 +extern VALUE notmuch_rb_cTags;
 +
 +extern VALUE notmuch_rb_eBaseError;
 +extern VALUE notmuch_rb_eDatabaseError;
 +extern VALUE notmuch_rb_eMemoryError;
 +extern VALUE notmuch_rb_eReadOnlyError;
 +extern VALUE notmuch_rb_eXapianError;
 +extern VALUE notmuch_rb_eFileError;
 +extern VALUE notmuch_rb_eFileNotEmailError;
 +extern VALUE notmuch_rb_eNullPointerError;
 +extern VALUE notmuch_rb_eTagTooLongError;
 +extern VALUE notmuch_rb_eUnbalancedFreezeThawError;
 +extern VALUE notmuch_rb_eUnbalancedAtomicError;
 +
 +extern ID ID_call;
 +extern ID ID_db_create;
 +extern ID ID_db_mode;
 +
 +#undef extern
  
  /* RSTRING_PTR() is new in ruby-1.9 */
  #if !defined(RSTRING_PTR)
 diff --git a/bindings/ruby/init.c b/bindings/ruby/init.c
 index 3fe60fb..b2dc7f6 100644
 --- a/bindings/ruby/init.c
 +++ b/bindings/ruby/init.c
 @@ -18,7 +18,9 @@
   * Author: Ali Polatel a...@exherbo.org
   */
  
 +#define RUBY_INIT_C
  #include defs.h
 +#undef RUBY_INIT_C
  
  /*
   * Document-module: Notmuch
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH 0/5] Fix notmuch_database_get_directory API

2012-05-13 Thread Austin Clements
This is a proposed last-minute change for 0.13.  It fixes the
notmuch_database_get_directory API in the same way we're fixing
notmuch_database_open, etc in this release.  Since this is a
backwards-incompatible change, it would be nice to lump it with the
other API-breaking changes.

To keep the patch simple, this does not change the behavior of
notmuch_database_get_directory, but it puts us in a good position to
fix it in the future.

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH 2/5] go: Update for changes to notmuch_database_get_directory

2012-05-13 Thread Austin Clements
---
 bindings/go/src/notmuch/notmuch.go  |   13 +++--
 bindings/python/notmuch/database.py |4 ++--
 2 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/bindings/go/src/notmuch/notmuch.go 
b/bindings/go/src/notmuch/notmuch.go
index 12de4c8..00bd53a 100644
--- a/bindings/go/src/notmuch/notmuch.go
+++ b/bindings/go/src/notmuch/notmuch.go
@@ -191,19 +191,20 @@ func (self *Database) NeedsUpgrade() bool {
  *
  * Can return NULL if a Xapian exception occurs.
  */
-func (self *Database) GetDirectory(path string) *Directory {
+func (self *Database) GetDirectory(path string) (*Directory, Status) {
var c_path *C.char = C.CString(path)
defer C.free(unsafe.Pointer(c_path))
 
if c_path == nil {
-   return nil
+   return nil, STATUS_OUT_OF_MEMORY
}
 
-   c_dir := C.notmuch_database_get_directory(self.db, c_path)
-   if c_dir == nil {
-   return nil
+   var c_dir *C.notmuch_directory_t
+   st := Status(C.notmuch_database_get_directory(self.db, c_path, c_dir))
+   if st != STATUS_SUCCESS || c_dir == nil {
+   return nil, st
}
-   return Directory{dir: c_dir}
+   return Directory{dir: c_dir}, st
 }
 
 /* Add a new message to the given notmuch database.
diff --git a/bindings/python/notmuch/database.py 
b/bindings/python/notmuch/database.py
index 1b1ddc3..0a58dd0 100644
--- a/bindings/python/notmuch/database.py
+++ b/bindings/python/notmuch/database.py
@@ -73,8 +73,8 @@ class Database(object):
 
 notmuch_database_get_directory
 _get_directory = nmlib.notmuch_database_get_directory
-_get_directory.argtypes = [NotmuchDatabaseP, c_char_p]
-_get_directory.restype = NotmuchDirectoryP
+_get_directory.argtypes = [NotmuchDatabaseP, c_char_p, 
POINTER(NotmuchDirectoryP)]
+_get_directory.restype = c_uint
 
 notmuch_database_get_path
 _get_path = nmlib.notmuch_database_get_path
-- 
1.7.10

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH 5/5] news: Update for changes to notmuch_database_get_directory

2012-05-13 Thread Austin Clements
---
 NEWS |   10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/NEWS b/NEWS
index c67f186..f987811 100644
--- a/NEWS
+++ b/NEWS
@@ -91,12 +91,12 @@ notmuch_database_close and notmuch_database_destroy
   database and thus release the lock associated with it without
   destroying the data structures obtained from it.
 
-notmuch_database_open and notmuch_database_create now return errors
+notmuch_database_open, notmuch_database_create, and
+notmuch_database_get_directory now return errors
 
-  The type signatures of notmuch_database_open and
-  notmuch_database_create have changed so that the functions now
-  return a notmuch_status_t and take an out-argument for returning the
-  new database object.
+  The type signatures of these functions have changed so that the
+  functions now return a notmuch_status_t and take an out-argument for
+  returning the new database object or directory object.
 
 go bindings changes
 ---
-- 
1.7.10

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH 1/5] lib/cli: Make notmuch_database_get_directory return a status code

2012-05-13 Thread Austin Clements
Previously, notmuch_database_get_directory had no way to indicate how
it had failed.  This changes its prototype to return a status code and
set an out-argument to the retrieved directory, like similar functions
in the library API.  This does *not* change its currently broken
behavior of creating directory objects when they don't exist, but it
does document it and paves the way for fixing this.  Also, it can now
check for a read-only database and return
NOTMUCH_STATUS_READ_ONLY_DATABASE instead of crashing.

In the interest of atomicity, this also updates calls from the CLI so
that notmuch still compiles.
---
 lib/database.cc |   22 --
 lib/notmuch.h   |   24 +---
 notmuch-new.c   |   21 ++---
 3 files changed, 51 insertions(+), 16 deletions(-)

diff --git a/lib/database.cc b/lib/database.cc
index 07f186e..f8c4a7d 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -955,8 +955,8 @@ notmuch_database_upgrade (notmuch_database_t *notmuch,
mtime = Xapian::sortable_unserialise (
document.get_value (NOTMUCH_VALUE_TIMESTAMP));
 
-   directory = notmuch_database_get_directory (notmuch,
-   term.c_str() + 10);
+   directory = _notmuch_directory_create (notmuch, term.c_str() + 
10,
+  status);
notmuch_directory_set_mtime (directory, mtime);
notmuch_directory_destroy (directory);
}
@@ -1304,20 +1304,30 @@ _notmuch_database_relative_path (notmuch_database_t 
*notmuch,
 return relative;
 }
 
-notmuch_directory_t *
+notmuch_status_t
 notmuch_database_get_directory (notmuch_database_t *notmuch,
-   const char *path)
+   const char *path,
+   notmuch_directory_t **directory)
 {
 notmuch_status_t status;
 
+if (directory == NULL)
+   return NOTMUCH_STATUS_NULL_POINTER;
+*directory = NULL;
+
+/* XXX Handle read-only databases properly */
+if (notmuch-mode == NOTMUCH_DATABASE_MODE_READ_ONLY)
+   return NOTMUCH_STATUS_READ_ONLY_DATABASE;
+
 try {
-   return _notmuch_directory_create (notmuch, path, status);
+   *directory = _notmuch_directory_create (notmuch, path, status);
 } catch (const Xapian::Error error) {
fprintf (stderr, A Xapian exception occurred getting directory: %s.\n,
 error.get_msg().c_str());
notmuch-exception_reported = TRUE;
-   return NULL;
+   status = NOTMUCH_STATUS_XAPIAN_EXCEPTION;
 }
+return status;
 }
 
 /* Allocate a document ID that satisfies the following criteria:
diff --git a/lib/notmuch.h b/lib/notmuch.h
index e6e5cc2..d414198 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -300,11 +300,29 @@ notmuch_database_end_atomic (notmuch_database_t *notmuch);
  * (see notmuch_database_get_path), or else should be an absolute path
  * with initial components that match the path of 'database'.
  *
- * Can return NULL if a Xapian exception occurs.
+ * Note: Currently this will create the directory object if it doesn't
+ * exist.  This is not intentional behavior and is likely to change in
+ * the near future.  To future-proof, when this returns
+ * NOTMUCH_STATUS_SUCCESS, callers should check that *directory is not
+ * NULL.
+ *
+ * Return value:
+ *
+ * NOTMUCH_STATUS_SUCCESS: Successfully retrieved directory.
+ *
+ * NOTMUCH_STATUS_NULL_POINTER: The given 'directory' argument is NULL.
+ *
+ * NOTMUCH_STATUS_XAPIAN_EXCEPTION: A Xapian exception occurred;
+ * directory not retrieved.
+ *
+ * NOTMUCH_STATUS_READ_ONLY_DATABASE: Database was opened in read-only
+ * mode so the directory cannot be created (this case will be
+ * removed in the future).
  */
-notmuch_directory_t *
+notmuch_status_t
 notmuch_database_get_directory (notmuch_database_t *database,
-   const char *path);
+   const char *path,
+   notmuch_directory_t **directory);
 
 /* Add a new message to the given notmuch database or associate an
  * additional filename with an existing message.
diff --git a/notmuch-new.c b/notmuch-new.c
index cb720cc..a3a8bec 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -250,7 +250,7 @@ add_files_recursive (notmuch_database_t *notmuch,
 notmuch_status_t status, ret = NOTMUCH_STATUS_SUCCESS;
 notmuch_message_t *message = NULL;
 struct dirent **fs_entries = NULL;
-int i, num_fs_entries;
+int i, num_fs_entries = 0;
 notmuch_directory_t *directory;
 notmuch_filenames_t *db_files = NULL;
 notmuch_filenames_t *db_subdirs = NULL;
@@ -274,8 +274,12 @@ add_files_recursive (notmuch_database_t *notmuch,
 
 fs_mtime = st.st_mtime;
 
-directory = notmuch_database_get_directory (notmuch, path);
-db_mtime = notmuch_directory_get_mtime 

[PATCH 4/5] ruby: Update for changes to notmuch_database_get_directory

2012-05-13 Thread Austin Clements
---
 bindings/ruby/database.c |   11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/bindings/ruby/database.c b/bindings/ruby/database.c
index 409d54f..e84f726 100644
--- a/bindings/ruby/database.c
+++ b/bindings/ruby/database.c
@@ -252,6 +252,7 @@ VALUE
 notmuch_rb_database_get_directory (VALUE self, VALUE pathv)
 {
 const char *path;
+notmuch_status_t ret;
 notmuch_directory_t *dir;
 notmuch_database_t *db;
 
@@ -260,11 +261,11 @@ notmuch_rb_database_get_directory (VALUE self, VALUE 
pathv)
 SafeStringValue (pathv);
 path = RSTRING_PTR (pathv);
 
-dir = notmuch_database_get_directory (db, path);
-if (!dir)
-rb_raise (notmuch_rb_eXapianError, Xapian exception);
-
-return Data_Wrap_Struct (notmuch_rb_cDirectory, NULL, NULL, dir);
+ret = notmuch_database_get_directory (db, path, dir);
+notmuch_rb_status_raise (ret);
+if (dir)
+   return Data_Wrap_Struct (notmuch_rb_cDirectory, NULL, NULL, dir);
+return Qnil;
 }
 
 /*
-- 
1.7.10

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH 3/5] python: Update for changes to notmuch_database_get_directory

2012-05-13 Thread Austin Clements
notmuch_database_get_directory now returns
NOTMUCH_STATUS_READ_ONLY_DATABASE on its own (rather than crashing) so
the workaround in Database.get_directory is no longer necessary.
---
 bindings/python/notmuch/database.py |   15 +++
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/bindings/python/notmuch/database.py 
b/bindings/python/notmuch/database.py
index 0a58dd0..797554d 100644
--- a/bindings/python/notmuch/database.py
+++ b/bindings/python/notmuch/database.py
@@ -359,13 +359,6 @@ class Database(object):
 
 self._assert_db_is_initialized()
 
-# work around libnotmuch calling exit(3), see
-# id:20120221002921.8534.57...@thinkbox.jade-hamburg.de
-# TODO: remove once this issue is resolved
-if self.mode != Database.MODE.READ_WRITE:
-raise ReadOnlyDatabaseError('The database has to be opened in '
-'read-write mode for get_directory')
-
 # sanity checking if path is valid, and make path absolute
 if path and path[0] == os.sep:
 # we got an absolute path
@@ -378,7 +371,13 @@ class Database(object):
 #we got a relative path, make it absolute
 abs_dirpath = os.path.abspath(os.path.join(self.get_path(), path))
 
-dir_p = Database._get_directory(self._db, _str(path))
+dir_p = NotmuchDirectoryP()
+status = Database._get_directory(self._db, _str(path), byref(dir_p))
+
+if status != STATUS.SUCCESS:
+raise NotmuchError(status)
+if not dir_p:
+return None
 
 # return the Directory, init it with the absolute path
 return Directory(abs_dirpath, dir_p, self)
-- 
1.7.10

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH] news: Update for changes to notmuch_database_get_directory

2012-05-13 Thread Austin Clements
---
 NEWS |   10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/NEWS b/NEWS
index c67f186..f987811 100644
--- a/NEWS
+++ b/NEWS
@@ -91,12 +91,12 @@ notmuch_database_close and notmuch_database_destroy
   database and thus release the lock associated with it without
   destroying the data structures obtained from it.
 
-notmuch_database_open and notmuch_database_create now return errors
+notmuch_database_open, notmuch_database_create, and
+notmuch_database_get_directory now return errors
 
-  The type signatures of notmuch_database_open and
-  notmuch_database_create have changed so that the functions now
-  return a notmuch_status_t and take an out-argument for returning the
-  new database object.
+  The type signatures of these functions have changed so that the
+  functions now return a notmuch_status_t and take an out-argument for
+  returning the new database object or directory object.
 
 go bindings changes
 ---
-- 
1.7.10

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH] news: Update for changes to notmuch_database_get_directory

2012-05-13 Thread Austin Clements
Sorry for the spam.  This was supposed to be a test message to myself
and I forgot I had git-send-email configured to sent to
notmuch@notmuchmail.org by default.
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH 0/5] Fix notmuch_database_get_directory API

2012-05-13 Thread Tomi Ollila
On Sun, May 13 2012, Austin Clements amdra...@mit.edu wrote:

 This is a proposed last-minute change for 0.13.  It fixes the
 notmuch_database_get_directory API in the same way we're fixing
 notmuch_database_open, etc in this release.  Since this is a
 backwards-incompatible change, it would be nice to lump it with the
 other API-breaking changes.

 To keep the patch simple, this does not change the behavior of
 notmuch_database_get_directory, but it puts us in a good position to
 fix it in the future.

Looks good to me (and applied in my current environment). The c/c++ 
changes were easy to understand and the python/go/ruby binding
changes looks like the old changes -- but those who understand more
(and actually uses those bindings) could do better review.

It would be nice to get those in so that we may have chance not
updating SONAME for notmuch 0.14.


Tomi
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH 2/5] go: Update for changes to notmuch_database_get_directory

2012-05-13 Thread Justus Winter
Hi Austin,

thanks for taking care of this.

Quoting Austin Clements (2012-05-13 21:57:06)
 ---
  bindings/go/src/notmuch/notmuch.go  |   13 +++--
  bindings/python/notmuch/database.py |4 ++--

A change to the python bindings slipped into this one.

Other than that this looks good.

I just saw that I marked another function as problematic,
notmuch_database_find_message_by_filename ends up calling
_notmuch_directory_create that aborts if the database is not
writable. But this might have to wait and might not even requira an
api change.

Justus
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH 0/5] Fix notmuch_database_get_directory API

2012-05-13 Thread Justus Winter
Quoting Tomi Ollila (2012-05-13 22:49:58)
 On Sun, May 13 2012, Austin Clements amdra...@mit.edu wrote:
 
  This is a proposed last-minute change for 0.13.  It fixes the
  notmuch_database_get_directory API in the same way we're fixing
  notmuch_database_open, etc in this release.  Since this is a
  backwards-incompatible change, it would be nice to lump it with the
  other API-breaking changes.
 
  To keep the patch simple, this does not change the behavior of
  notmuch_database_get_directory, but it puts us in a good position to
  fix it in the future.
 
 Looks good to me (and applied in my current environment). The c/c++ 
 changes were easy to understand and the python/go/ruby binding
 changes looks like the old changes -- but those who understand more
 (and actually uses those bindings) could do better review.

I'd say the changes to the python and go bindings are fine.

 It would be nice to get those in so that we may have chance not
 updating SONAME for notmuch 0.14.

Yes, I'd love to see this in 0.13 as well, but I don't really see the
problem with breaking the api if this fixes design issues. After all,
notmuch is young and so are all the projects building uppon it.

Justus
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH 2/5] go: Update for changes to notmuch_database_get_directory

2012-05-13 Thread Austin Clements
Quoth Justus Winter on May 13 at 11:51 pm:
 Hi Austin,
 
 thanks for taking care of this.
 
 Quoting Austin Clements (2012-05-13 21:57:06)
  ---
   bindings/go/src/notmuch/notmuch.go  |   13 +++--
   bindings/python/notmuch/database.py |4 ++--
 
 A change to the python bindings slipped into this one.

Sure enough.  I'll send a v2 that moves that hunk to the python patch.

 Other than that this looks good.
 
 I just saw that I marked another function as problematic,
 notmuch_database_find_message_by_filename ends up calling
 _notmuch_directory_create that aborts if the database is not
 writable. But this might have to wait and might not even requira an
 api change.

Right.  notmuch_database_find_message_by_filename already has a status
return, so its API is fine.  (Actually, I was starting to fix its
implementation when I realized notmuch_database_get_directory was
problematic.)

 Justus
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH v2 1/5] lib/cli: Make notmuch_database_get_directory return a status code

2012-05-13 Thread Austin Clements
Previously, notmuch_database_get_directory had no way to indicate how
it had failed.  This changes its prototype to return a status code and
set an out-argument to the retrieved directory, like similar functions
in the library API.  This does *not* change its currently broken
behavior of creating directory objects when they don't exist, but it
does document it and paves the way for fixing this.  Also, it can now
check for a read-only database and return
NOTMUCH_STATUS_READ_ONLY_DATABASE instead of crashing.

In the interest of atomicity, this also updates calls from the CLI so
that notmuch still compiles.
---
 lib/database.cc |   22 --
 lib/notmuch.h   |   23 ---
 notmuch-new.c   |   21 ++---
 3 files changed, 50 insertions(+), 16 deletions(-)

diff --git a/lib/database.cc b/lib/database.cc
index 07f186e..f8c4a7d 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -955,8 +955,8 @@ notmuch_database_upgrade (notmuch_database_t *notmuch,
mtime = Xapian::sortable_unserialise (
document.get_value (NOTMUCH_VALUE_TIMESTAMP));
 
-   directory = notmuch_database_get_directory (notmuch,
-   term.c_str() + 10);
+   directory = _notmuch_directory_create (notmuch, term.c_str() + 
10,
+  status);
notmuch_directory_set_mtime (directory, mtime);
notmuch_directory_destroy (directory);
}
@@ -1304,20 +1304,30 @@ _notmuch_database_relative_path (notmuch_database_t 
*notmuch,
 return relative;
 }
 
-notmuch_directory_t *
+notmuch_status_t
 notmuch_database_get_directory (notmuch_database_t *notmuch,
-   const char *path)
+   const char *path,
+   notmuch_directory_t **directory)
 {
 notmuch_status_t status;
 
+if (directory == NULL)
+   return NOTMUCH_STATUS_NULL_POINTER;
+*directory = NULL;
+
+/* XXX Handle read-only databases properly */
+if (notmuch-mode == NOTMUCH_DATABASE_MODE_READ_ONLY)
+   return NOTMUCH_STATUS_READ_ONLY_DATABASE;
+
 try {
-   return _notmuch_directory_create (notmuch, path, status);
+   *directory = _notmuch_directory_create (notmuch, path, status);
 } catch (const Xapian::Error error) {
fprintf (stderr, A Xapian exception occurred getting directory: %s.\n,
 error.get_msg().c_str());
notmuch-exception_reported = TRUE;
-   return NULL;
+   status = NOTMUCH_STATUS_XAPIAN_EXCEPTION;
 }
+return status;
 }
 
 /* Allocate a document ID that satisfies the following criteria:
diff --git a/lib/notmuch.h b/lib/notmuch.h
index e6e5cc2..bbb17e4 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -300,11 +300,28 @@ notmuch_database_end_atomic (notmuch_database_t *notmuch);
  * (see notmuch_database_get_path), or else should be an absolute path
  * with initial components that match the path of 'database'.
  *
- * Can return NULL if a Xapian exception occurs.
+ * Note: Currently this will create the directory object if it doesn't
+ * exist.  In the future, when a directory object does not exist this
+ * will return NOTMUCH_STATUS_SUCCESS and set *directory to NULL.
+ * Callers should be prepared for this.
+ *
+ * Return value:
+ *
+ * NOTMUCH_STATUS_SUCCESS: Successfully retrieved directory.
+ *
+ * NOTMUCH_STATUS_NULL_POINTER: The given 'directory' argument is NULL.
+ *
+ * NOTMUCH_STATUS_XAPIAN_EXCEPTION: A Xapian exception occurred;
+ * directory not retrieved.
+ *
+ * NOTMUCH_STATUS_READ_ONLY_DATABASE: Database was opened in read-only
+ * mode so the directory cannot be created (this case will be
+ * removed in the future).
  */
-notmuch_directory_t *
+notmuch_status_t
 notmuch_database_get_directory (notmuch_database_t *database,
-   const char *path);
+   const char *path,
+   notmuch_directory_t **directory);
 
 /* Add a new message to the given notmuch database or associate an
  * additional filename with an existing message.
diff --git a/notmuch-new.c b/notmuch-new.c
index cb720cc..a3a8bec 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -250,7 +250,7 @@ add_files_recursive (notmuch_database_t *notmuch,
 notmuch_status_t status, ret = NOTMUCH_STATUS_SUCCESS;
 notmuch_message_t *message = NULL;
 struct dirent **fs_entries = NULL;
-int i, num_fs_entries;
+int i, num_fs_entries = 0;
 notmuch_directory_t *directory;
 notmuch_filenames_t *db_files = NULL;
 notmuch_filenames_t *db_subdirs = NULL;
@@ -274,8 +274,12 @@ add_files_recursive (notmuch_database_t *notmuch,
 
 fs_mtime = st.st_mtime;
 
-directory = notmuch_database_get_directory (notmuch, path);
-db_mtime = notmuch_directory_get_mtime (directory);
+status = 

[PATCH v2 2/5] go: Update for changes to notmuch_database_get_directory

2012-05-13 Thread Austin Clements
---
 bindings/go/src/notmuch/notmuch.go |   13 +++--
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/bindings/go/src/notmuch/notmuch.go 
b/bindings/go/src/notmuch/notmuch.go
index 12de4c8..00bd53a 100644
--- a/bindings/go/src/notmuch/notmuch.go
+++ b/bindings/go/src/notmuch/notmuch.go
@@ -191,19 +191,20 @@ func (self *Database) NeedsUpgrade() bool {
  *
  * Can return NULL if a Xapian exception occurs.
  */
-func (self *Database) GetDirectory(path string) *Directory {
+func (self *Database) GetDirectory(path string) (*Directory, Status) {
var c_path *C.char = C.CString(path)
defer C.free(unsafe.Pointer(c_path))
 
if c_path == nil {
-   return nil
+   return nil, STATUS_OUT_OF_MEMORY
}
 
-   c_dir := C.notmuch_database_get_directory(self.db, c_path)
-   if c_dir == nil {
-   return nil
+   var c_dir *C.notmuch_directory_t
+   st := Status(C.notmuch_database_get_directory(self.db, c_path, c_dir))
+   if st != STATUS_SUCCESS || c_dir == nil {
+   return nil, st
}
-   return Directory{dir: c_dir}
+   return Directory{dir: c_dir}, st
 }
 
 /* Add a new message to the given notmuch database.
-- 
1.7.10

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH v2 4/5] ruby: Update for changes to notmuch_database_get_directory

2012-05-13 Thread Austin Clements
---
 bindings/ruby/database.c |   11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/bindings/ruby/database.c b/bindings/ruby/database.c
index 409d54f..e84f726 100644
--- a/bindings/ruby/database.c
+++ b/bindings/ruby/database.c
@@ -252,6 +252,7 @@ VALUE
 notmuch_rb_database_get_directory (VALUE self, VALUE pathv)
 {
 const char *path;
+notmuch_status_t ret;
 notmuch_directory_t *dir;
 notmuch_database_t *db;
 
@@ -260,11 +261,11 @@ notmuch_rb_database_get_directory (VALUE self, VALUE 
pathv)
 SafeStringValue (pathv);
 path = RSTRING_PTR (pathv);
 
-dir = notmuch_database_get_directory (db, path);
-if (!dir)
-rb_raise (notmuch_rb_eXapianError, Xapian exception);
-
-return Data_Wrap_Struct (notmuch_rb_cDirectory, NULL, NULL, dir);
+ret = notmuch_database_get_directory (db, path, dir);
+notmuch_rb_status_raise (ret);
+if (dir)
+   return Data_Wrap_Struct (notmuch_rb_cDirectory, NULL, NULL, dir);
+return Qnil;
 }
 
 /*
-- 
1.7.10

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH v2 3/5] python: Update for changes to notmuch_database_get_directory

2012-05-13 Thread Austin Clements
notmuch_database_get_directory now returns
NOTMUCH_STATUS_READ_ONLY_DATABASE on its own (rather than crashing) so
the workaround in Database.get_directory is no longer necessary.
---
 bindings/python/notmuch/database.py |   19 +--
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/bindings/python/notmuch/database.py 
b/bindings/python/notmuch/database.py
index 1b1ddc3..797554d 100644
--- a/bindings/python/notmuch/database.py
+++ b/bindings/python/notmuch/database.py
@@ -73,8 +73,8 @@ class Database(object):
 
 notmuch_database_get_directory
 _get_directory = nmlib.notmuch_database_get_directory
-_get_directory.argtypes = [NotmuchDatabaseP, c_char_p]
-_get_directory.restype = NotmuchDirectoryP
+_get_directory.argtypes = [NotmuchDatabaseP, c_char_p, 
POINTER(NotmuchDirectoryP)]
+_get_directory.restype = c_uint
 
 notmuch_database_get_path
 _get_path = nmlib.notmuch_database_get_path
@@ -359,13 +359,6 @@ class Database(object):
 
 self._assert_db_is_initialized()
 
-# work around libnotmuch calling exit(3), see
-# id:20120221002921.8534.57...@thinkbox.jade-hamburg.de
-# TODO: remove once this issue is resolved
-if self.mode != Database.MODE.READ_WRITE:
-raise ReadOnlyDatabaseError('The database has to be opened in '
-'read-write mode for get_directory')
-
 # sanity checking if path is valid, and make path absolute
 if path and path[0] == os.sep:
 # we got an absolute path
@@ -378,7 +371,13 @@ class Database(object):
 #we got a relative path, make it absolute
 abs_dirpath = os.path.abspath(os.path.join(self.get_path(), path))
 
-dir_p = Database._get_directory(self._db, _str(path))
+dir_p = NotmuchDirectoryP()
+status = Database._get_directory(self._db, _str(path), byref(dir_p))
+
+if status != STATUS.SUCCESS:
+raise NotmuchError(status)
+if not dir_p:
+return None
 
 # return the Directory, init it with the absolute path
 return Directory(abs_dirpath, dir_p, self)
-- 
1.7.10

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH v2 5/5] news: Update for changes to notmuch_database_get_directory

2012-05-13 Thread Austin Clements
---
 NEWS |   10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/NEWS b/NEWS
index c67f186..f987811 100644
--- a/NEWS
+++ b/NEWS
@@ -91,12 +91,12 @@ notmuch_database_close and notmuch_database_destroy
   database and thus release the lock associated with it without
   destroying the data structures obtained from it.
 
-notmuch_database_open and notmuch_database_create now return errors
+notmuch_database_open, notmuch_database_create, and
+notmuch_database_get_directory now return errors
 
-  The type signatures of notmuch_database_open and
-  notmuch_database_create have changed so that the functions now
-  return a notmuch_status_t and take an out-argument for returning the
-  new database object.
+  The type signatures of these functions have changed so that the
+  functions now return a notmuch_status_t and take an out-argument for
+  returning the new database object or directory object.
 
 go bindings changes
 ---
-- 
1.7.10

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH 1/2] cli: also use Delivered-To header to figure out the reply from address

2012-05-13 Thread Michael Hudson-Doyle
Jani Nikula j...@nikula.org writes:

 Add another fallback header Delivered-To for guessing the user's from
 address for notmuch reply before using the Received
 headers. Apparently some MTAs use Delivered-To instead of
 X-Original-To (which already exists as a fallback).

 Reported-by: Michael Hudson-Doyle michael.hud...@canonical.com
 Signed-off-by: Jani Nikula j...@nikula.org

Firstly, thank you so much for doing this!

Unfortunately it doesn't work for me :( Poking around a bit reveals why:
I have two Delivered-To: headers in the problem mails, and only the
lower/earlier one containst the email address I want to be used as
From:.  And it seems that the only header one can get all values for is
Received...

Cheers,
mwh
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch