Re: Not much database creation error

Re: Not much database creation error

[Sebastian Spaeth]

[-- Attachment #1: Type: text/plain, Size: 645 bytes --]

On Sun, 04 Sep 2011 00:34:58 -0400, Martin Owens <doctormo@gmail.com> wrote:
> I am creating a program using notmuch and the python notmuch API and
> have found a problem. when creating or loading a database using a
> unicode path, notmuch returns an error:
> 
> Error: Cannot create directory //.notmuch: Permission denied

Hi Martin,

the underlying notmuch library is agnostic to whatever encoding it is
handed and I indeed had simply assumed strings. I have now added
encoding/decoding in a couple of places where it seemed necessary and
pushed the result to the master branch.

Let me know if this fixes things.

Sebastian

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: Not much database creation error

[Martin Owens]

Thanks Sebastian,

I'll test it as soon as I can.

As a further question, I have a program opening the database for
READ_WRITE access (this process provides a simple write only dbus
interface) and I want clients to connect to notmuch over a read only
connection.

A problem I'm having is that when I add messages to the database,
searching using the read only interface fails to show anything.

Searching using the read-write interface shows each email message being
added successfully.

Is this api topology not expected/recommended/working? Please advise.

Best Regards, Martin Owens

On Mon, 2011-09-05 at 15:57 +0200, Sebastian Spaeth wrote:
> 
> Let me know if this fixes things. 

Re: Not much database creation error

[Sebastian Spaeth]

[-- Attachment #1: Type: text/plain, Size: 1197 bytes --]

On Wed, 07 Sep 2011 01:26:59 -0400, Martin Owens <doctormo@gmail.com> wrote:
> As a further question, I have a program opening the database for
> READ_WRITE access (this process provides a simple write only dbus
> interface) and I want clients to connect to notmuch over a read only
> connection.
> 
> A problem I'm having is that when I add messages to the database,
> searching using the read only interface fails to show anything.

> Is this api topology not expected/recommended/working? Please advise.

hi Martin,

I have to admit, I am not deep into the libnotmuch/xapian internals, so
I might be the wrong person to ask (I just do some python bindings to
whatever libnotmuch provides).

However, when a read-write connection changes content, I believe that
all other existing connections will return an error stating
DATABASE_MODIFIED. I think we throw (or at least are supposed to throw)
an exception in this case.

Are you seeing any exceptions or errors been thrown?  Does it not show
ANYTHING or only not the new messages?

An open database connection provides you only with a snapshot, and you
need to reopen it after content has been modified.

Sebastian

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: Not much database creation error

[Martin Owens]

On Thu, 2011-09-08 at 15:45 +0200, Sebastian Spaeth wrote:
> An open database connection provides you only with a snapshot, and you
> need to reopen it after content has been modified. 

You are correct and I have submitted a patch for libnotmuch and the
python bindings to add reopen and flush to the code. To this notmuch
mailing list.

Errors didn't seem to happen consistently though, I got one error out of
20 tests or so.

Martin,

Python Unicode Was: Not much database creation error

[Martin Owens]

[-- Attachment #1: Type: text/plain, Size: 491 bytes --]

Hello Sebastian,

I've attached a diff for some proposed changes to help make dealing with
unicode and strings in the bindings more regular. I noticed some of the
methods were protected and others were not.

Let me know.

Best Regards, Martin Owens

On Thu, 2011-09-08 at 15:45 +0200, Sebastian Spaeth wrote:
> I have to admit, I am not deep into the libnotmuch/xapian internals,
> so
> I might be the wrong person to ask (I just do some python bindings to
> whatever libnotmuch provides). 

[-- Attachment #2: unicode.patch --]
[-- Type: text/x-patch, Size: 6195 bytes --]

diff --git a/bindings/python/notmuch/database.py b/bindings/python/notmuch/database.py
index b070019..4f44de0 100644
--- a/bindings/python/notmuch/database.py
+++ b/bindings/python/notmuch/database.py
@@ -19,12 +19,11 @@ Copyright 2010 Sebastian Spaeth <Sebastian@SSpaeth.de>'
 
 import os
 from ctypes import c_int, c_char_p, c_void_p, c_uint, c_long, byref
-from notmuch.globals import nmlib, STATUS, NotmuchError, Enum
+from notmuch.globals import nmlib, STATUS, NotmuchError, Enum, _str
 from notmuch.thread import Threads
 from notmuch.message import Messages, Message
 from notmuch.tag import Tags
 
-
 class Database(object):
     """Represents a notmuch database (wraps notmuch_database_t)
 
@@ -105,7 +104,6 @@ class Database(object):
                 Database._std_db_path = self._get_user_default_db()
             path = Database._std_db_path
 
-        assert isinstance(path, basestring), 'Path must be a string or None.'
         if create == False:
             self.open(path, mode)
         else:
@@ -136,7 +134,7 @@ class Database(object):
             raise NotmuchError(message="Cannot create db, this Database() "
                                        "already has an open one.")
 
-        res = Database._create(path, Database.MODE.READ_WRITE)
+        res = Database._create(_str(path), Database.MODE.READ_WRITE)
 
         if res is None:
             raise NotmuchError(
@@ -156,9 +154,7 @@ class Database(object):
         :exception: Raises :exc:`NotmuchError` in case
                     of any failure (after printing an error message on stderr).
         """
-        if isinstance(path, unicode):
-            path = path.encode('utf-8') 
-        res = Database._open(path, mode)
+        res = Database._open(_str(path), mode)
 
         if res is None:
             raise NotmuchError(
@@ -263,12 +259,10 @@ class Database(object):
             #we got a relative path, make it absolute
             abs_dirpath = os.path.abspath(os.path.join(self.get_path(), path))
 
-        if isinstance(path, unicode):
-            path = path.encode('UTF-8')
-        dir_p = Database._get_directory(self._db, path)
+        dir_p = Database._get_directory(self._db, _str(path))
 
         # return the Directory, init it with the absolute path
-        return Directory(abs_dirpath, dir_p, self)
+        return Directory(_str(abs_dirpath), dir_p, self)
 
     def add_message(self, filename, sync_maildir_flags=False):
         """Adds a new message to the database
@@ -325,7 +319,7 @@ class Database(object):
 
         msg_p = c_void_p()
         status = nmlib.notmuch_database_add_message(self._db,
-                                                  filename,
+                                                  _str(filename),
                                                   byref(msg_p))
 
         if not status in [STATUS.SUCCESS, STATUS.DUPLICATE_MESSAGE_ID]:
@@ -394,7 +388,7 @@ class Database(object):
         # Raise a NotmuchError if not initialized
         self._verify_initialized_db()
 
-        msg_p = Database._find_message(self._db, msgid)
+        msg_p = Database._find_message(self._db, _str(msgid))
         if msg_p is None:
             return None
         return Message(msg_p, self)
@@ -404,8 +398,7 @@ class Database(object):
            with this filename.
         """
         self._verify_initialized_db()
-
-        msg_p = Database._find_message_by_filename(self._db, filename)
+        msg_p = Database._find_message_by_filename(self._db, _str(filename))
         return msg_p and Message(msg_p, self) or None
 
     def get_all_tags(self):
@@ -558,11 +551,8 @@ class Query(object):
             raise NotmuchError(STATUS.NOT_INITIALIZED)
         # create reference to parent db to keep it alive
         self._db = db
-        if isinstance(querystr, unicode):
-            # xapian takes utf-8 encoded byte arrays
-            querystr = querystr.encode('utf-8')
         # create query, return None if too little mem available
-        query_p = Query._create(db.db_p, querystr)
+        query_p = Query._create(db.db_p, _str(querystr))
         if query_p is None:
             NotmuchError(STATUS.NULL_POINTER)
         self._query = query_p
diff --git a/bindings/python/notmuch/globals.py b/bindings/python/notmuch/globals.py
index 77f2905..5c81076 100644
--- a/bindings/python/notmuch/globals.py
+++ b/bindings/python/notmuch/globals.py
@@ -98,3 +98,13 @@ class NotmuchError(Exception):
             return self.args[0]
         else:
             return STATUS.status2str(self.args[1])
+
+# C++ code expects strings to be well formatted and
+# unicode strings to have no null bytes.
+def _str(value):
+    if not isinstance(value, basestring):
+        raise TypeError("Expected str or unicode, got %s" % str(type(value)))
+    if isinstance(value, unicode):
+        return value.encode('UTF-8')
+    return str(value)
+
diff --git a/bindings/python/notmuch/message.py b/bindings/python/notmuch/message.py
index ae6ae1b..4f93a2a 100644
--- a/bindings/python/notmuch/message.py
+++ b/bindings/python/notmuch/message.py
@@ -21,7 +21,7 @@ Copyright 2010 Sebastian Spaeth <Sebastian@SSpaeth.de>'
 
 from ctypes import c_char_p, c_void_p, c_long, c_uint, c_int
 from datetime import date
-from notmuch.globals import nmlib, STATUS, NotmuchError, Enum
+from notmuch.globals import nmlib, STATUS, NotmuchError, Enum, _str
 from notmuch.tag import Tags
 from notmuch.filename import Filenames
 import sys
@@ -505,7 +505,7 @@ class Message(object):
         if self._msg is None:
             raise NotmuchError(STATUS.NOT_INITIALIZED)
 
-        status = nmlib.notmuch_message_add_tag(self._msg, tag)
+        status = nmlib.notmuch_message_add_tag(self._msg, _str(tag))
 
         # bail out on failure
         if status != STATUS.SUCCESS:
@@ -549,7 +549,7 @@ class Message(object):
         if self._msg is None:
             raise NotmuchError(STATUS.NOT_INITIALIZED)
 
-        status = nmlib.notmuch_message_remove_tag(self._msg, tag)
+        status = nmlib.notmuch_message_remove_tag(self._msg, _str(tag))
         # bail out on error
         if status != STATUS.SUCCESS:
             raise NotmuchError(status)

Re: Python Unicode Was: Not much database creation error

[Austin Clements]

This seems like a symptom of a much bigger problem.  Shouldn't the
bindings be checking or coercing the type of *every* argument that
gets passed from the caller through to a ctypes-wrapped libnotmuch
function?  Otherwise, a simple type error in a caller, like passing a
number instead of a string to one of the Python methods, would likely
result in a segfault.

On Thu, Sep 15, 2011 at 1:41 PM, Martin Owens <doctormo@gmail.com> wrote:
> Hello Sebastian,
>
> I've attached a diff for some proposed changes to help make dealing with
> unicode and strings in the bindings more regular. I noticed some of the
> methods were protected and others were not.
>
> Let me know.
>
> Best Regards, Martin Owens
>
> On Thu, 2011-09-08 at 15:45 +0200, Sebastian Spaeth wrote:
>> I have to admit, I am not deep into the libnotmuch/xapian internals,
>> so
>> I might be the wrong person to ask (I just do some python bindings to
>> whatever libnotmuch provides).

Re: Python Unicode Was: Not much database creation error

[Sebastian Spaeth]

[-- Attachment #1: Type: text/plain, Size: 679 bytes --]

On Thu, 15 Sep 2011 13:41:11 -0400, Martin Owens wrote:
> I've attached a diff for some proposed changes to help make dealing with
> unicode and strings in the bindings more regular. I noticed some of the
> methods were protected and others were not.

I've now pushed a slightly modified version of the patch (BTW, you have
find_message_by_filename() when I don't, care to send that patch too?
:-))

As discussed on IRC with Bremner and ojwb: Rather than hardcoding UTF-8
we should probably detect the systems locale variable and use that as
encoding for file paths (falling back to utf8).

But your patch is better than what we have before, so it's going in for
now.

Sebastian

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: Python Unicode Was: Not much database creation error

[Sebastian Spaeth]

[-- Attachment #1: Type: text/plain, Size: 152 bytes --]


P.S. perhaps we should be using:

locale.getpreferredencoding()

to determine the default path and tag encoding? Opinions, Experiences,...?

Sebastian

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]