unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
* problem with message headers in python bindings
@ 2021-10-26  1:27 David Bremner
  2021-10-26  1:56 ` David Bremner
  0 siblings, 1 reply; 6+ messages in thread
From: David Bremner @ 2021-10-26  1:27 UTC (permalink / raw)
  To: notmuch

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


On a host running Debian Stable with python 3.9.2, and notmuch 0.31.4,
both of the attached programs work fine. On my developement host running
notmuch 0.34, both segfault when trying to fetch the 'to' header. This
invokes a more complicated code path, which seems to have broken
sometime between 0.31.4 and 0.34.

I guess something changed in the library that the bindings need to catch
up with.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: test.py --]
[-- Type: text/x-python, Size: 162 bytes --]

import notmuch2

db=notmuch2.Database()

m=db.find('20211026002252.2823593-1-austin@austinray.io')

frm=m.header('from')
print(frm)

to=m.header('To')
print(to)


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: test2.py --]
[-- Type: text/x-python, Size: 176 bytes --]

import notmuch

db=notmuch.Database()

m=db.find_message('20211026002252.2823593-1-austin@austinray.io')

frm=m.get_header('from')
print(frm)

to=m.get_header('To')
print(to)


[-- Attachment #4: Type: text/plain, Size: 0 bytes --]



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: problem with message headers in python bindings
  2021-10-26  1:27 problem with message headers in python bindings David Bremner
@ 2021-10-26  1:56 ` David Bremner
  2021-10-26  2:11   ` [PATCH] test: add known broken tests for python bindings in split configs David Bremner
  0 siblings, 1 reply; 6+ messages in thread
From: David Bremner @ 2021-10-26  1:56 UTC (permalink / raw)
  To: notmuch

David Bremner <david@tethera.net> writes:

> On a host running Debian Stable with python 3.9.2, and notmuch 0.31.4,
> both of the attached programs work fine. On my developement host running
> notmuch 0.34, both segfault when trying to fetch the 'to' header. This
> invokes a more complicated code path, which seems to have broken
> sometime between 0.31.4 and 0.34.
>
> I guess something changed in the library that the bindings need to catch
> up with.

Yep, the change is split configurations (database.path !=
database.mail_root). The python bindings (both of them) need to be
updated to handle split configurations. In a sense this is related to
notmuch version, since split configurations are not supported prior to
0.32, but the workaround is (for now) don't use split configurations if
you need the python bindings.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] test: add known broken tests for python bindings in split configs
  2021-10-26  1:56 ` David Bremner
@ 2021-10-26  2:11   ` David Bremner
  2021-10-27  2:36     ` WIP fix split-config for notmuch-python2 David Bremner
  0 siblings, 1 reply; 6+ messages in thread
From: David Bremner @ 2021-10-26  2:11 UTC (permalink / raw)
  To: David Bremner, notmuch

This reproduces the bug(s) reported in id:87h7d4wp6b.fsf@tethera.net
---
 test/T055-path-config.sh | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/test/T055-path-config.sh b/test/T055-path-config.sh
index ef22e964..c06962e4 100755
--- a/test/T055-path-config.sh
+++ b/test/T055-path-config.sh
@@ -308,6 +308,34 @@ EOF
 	   test_expect_equal "${output}+${output2}" "${value}+"
 	   ;;
    esac
+
+   case $config in
+       split|XDG*)
+	   test_begin_subtest "'to' header does not crash (python) ($config)"
+	   test_subtest_known_broken
+	   echo 'notmuch@notmuchmail.org' > EXPECTED
+	   test_python <<EOF
+import notmuch
+db = notmuch.Database(mode=notmuch.Database.MODE.READ_ONLY)
+m=db.find_message('20091117232137.GA7669@griffis1.net')
+to=m.get_header('to')
+print(to)
+EOF
+	   test_expect_equal_file EXPECTED OUTPUT
+
+	   test_begin_subtest "'to' header does not crash (python-cffi) ($config)"
+	   test_subtest_known_broken
+	   echo 'notmuch@notmuchmail.org' > EXPECTED
+	   test_python <<EOF
+import notmuch2
+db=notmuch2.Database()
+m=db.find('20091117232137.GA7669@griffis1.net')
+to=m.header('To')
+print(to)
+EOF
+	   test_expect_equal_file EXPECTED OUTPUT
+	   ;;
+   esac
    restore_config
    rm -rf home/.local
    rm -rf home/.config
-- 
2.33.0

^ permalink raw reply	[flat|nested] 6+ messages in thread

* WIP fix split-config for notmuch-python2
  2021-10-26  2:11   ` [PATCH] test: add known broken tests for python bindings in split configs David Bremner
@ 2021-10-27  2:36     ` David Bremner
  2021-10-27  2:36       ` [PATCH 1/2] WIP: add python-cffi bindings to path David Bremner
  2021-10-27  2:36       ` [PATCH 2/2] WIP/python2: switch to notmuch_database_{open,create}_with_config David Bremner
  0 siblings, 2 replies; 6+ messages in thread
From: David Bremner @ 2021-10-27  2:36 UTC (permalink / raw)
  To: David Bremner, notmuch

Posting these proof-of-concept patches because I want to go back and
look carefully at some of the API subtleties that arise, mainly the
question of what happens if the user passes a database_path, and the
library finds a config file which conflicts with that path. Currently
the config file wins, which doesn't seem quite right, but might or
might not be easy to fix.

Also I note that fixing the bug with split configurations for _one_ of
the python frameworks isn't handled really well by the tests in a loop
of id:20211026021131.3162922-1-david@tethera.net

I guess a final question is if we should even bother fixing split
configs for the old python bindings? It is a new feature, so in a
sense it's reasonable not provide it in the deprecated bindings.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/2] WIP: add python-cffi bindings to path
  2021-10-27  2:36     ` WIP fix split-config for notmuch-python2 David Bremner
@ 2021-10-27  2:36       ` David Bremner
  2021-10-27  2:36       ` [PATCH 2/2] WIP/python2: switch to notmuch_database_{open,create}_with_config David Bremner
  1 sibling, 0 replies; 6+ messages in thread
From: David Bremner @ 2021-10-27  2:36 UTC (permalink / raw)
  To: David Bremner, notmuch

this will probably break test as installed stuff
---
 test/test-lib.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index 32f710a5..e476a69b 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -909,7 +909,7 @@ test_done () {
 test_python () {
     # Note: if there is need to print debug information from python program,
     # use stdout = os.fdopen(6, 'w') or stderr = os.fdopen(7, 'w')
-    PYTHONPATH="$NOTMUCH_SRCDIR/bindings/python${PYTHONPATH:+:$PYTHONPATH}" \
+    PYTHONPATH="$NOTMUCH_BUILDDIR/bindings/python-cffi/build/stage:$NOTMUCH_SRCDIR/bindings/python${PYTHONPATH:+:$PYTHONPATH}" \
 	$NOTMUCH_PYTHON -B - > OUTPUT
 }
 
-- 
2.33.0

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 2/2] WIP/python2: switch to notmuch_database_{open,create}_with_config
  2021-10-27  2:36     ` WIP fix split-config for notmuch-python2 David Bremner
  2021-10-27  2:36       ` [PATCH 1/2] WIP: add python-cffi bindings to path David Bremner
@ 2021-10-27  2:36       ` David Bremner
  1 sibling, 0 replies; 6+ messages in thread
From: David Bremner @ 2021-10-27  2:36 UTC (permalink / raw)
  To: David Bremner, notmuch

This fixes two deprecation warnings, and is a (partial?) fix for split
configurations.

The main changes to the tests are to force the pytest tests to ignore
the configuration found via NOTMUCH_CONFIG. It is possible that there
is a misfeature at the library level to be addressed, namely that an
implicitely found configuration file should not override an
explicitely passed database path.
---
 bindings/python-cffi/notmuch2/_build.py     | 26 +++++++-------
 bindings/python-cffi/notmuch2/_database.py  | 39 +++++++++++++--------
 bindings/python-cffi/tests/test_database.py |  2 +-
 bindings/python-cffi/tests/test_message.py  |  4 +--
 bindings/python-cffi/tests/test_tags.py     |  8 ++---
 5 files changed, 43 insertions(+), 36 deletions(-)

diff --git a/bindings/python-cffi/notmuch2/_build.py b/bindings/python-cffi/notmuch2/_build.py
index 24df939e..f6184b97 100644
--- a/bindings/python-cffi/notmuch2/_build.py
+++ b/bindings/python-cffi/notmuch2/_build.py
@@ -103,20 +103,18 @@ ffibuilder.cdef(
     notmuch_status_to_string (notmuch_status_t status);
 
     notmuch_status_t
-    notmuch_database_create_verbose (const char *path,
-                                     notmuch_database_t **database,
-                                     char **error_message);
-    notmuch_status_t
-    notmuch_database_create (const char *path, notmuch_database_t **database);
-    notmuch_status_t
-    notmuch_database_open_verbose (const char *path,
-                                   notmuch_database_mode_t mode,
-                                   notmuch_database_t **database,
-                                   char **error_message);
-    notmuch_status_t
-    notmuch_database_open (const char *path,
-                           notmuch_database_mode_t mode,
-                           notmuch_database_t **database);
+    notmuch_database_create_with_config (const char *database_path,
+                                         const char *config_path,
+                                         const char *profile,
+                                         notmuch_database_t **database,
+                                         char **error_message);
+    notmuch_status_t
+    notmuch_database_open_with_config (const char *database_path,
+                                       notmuch_database_mode_t mode,
+                                       const char *config_path,
+                                       const char *profile,
+                                       notmuch_database_t **database,
+                                       char **error_message);
     notmuch_status_t
     notmuch_database_close (notmuch_database_t *database);
     notmuch_status_t
diff --git a/bindings/python-cffi/notmuch2/_database.py b/bindings/python-cffi/notmuch2/_database.py
index 868f4408..9f059652 100644
--- a/bindings/python-cffi/notmuch2/_database.py
+++ b/bindings/python-cffi/notmuch2/_database.py
@@ -109,18 +109,28 @@ class Database(base.NotmuchObject):
         'rw': MODE.READ_WRITE,
     }
 
-    def __init__(self, path=None, mode=MODE.READ_ONLY):
+    @staticmethod
+    def _encode_for_open(path):
+        if path is None:
+            path = capi.ffi.NULL
+        elif not hasattr(os, 'PathLike') and isinstance(path, pathlib.Path):
+            path = bytes(path)
+        else:
+            path = os.fsencode(path)
+        return path
+
+    def __init__(self, path=None, mode=MODE.READ_ONLY, cfg_path=None):
         if isinstance(mode, str):
             mode = self.STR_MODE_MAP[mode]
         self.mode = mode
-        if path is None:
-            path = self.default_path()
-        if not hasattr(os, 'PathLike') and isinstance(path, pathlib.Path):
-            path = bytes(path)
+
         db_pp = capi.ffi.new('notmuch_database_t **')
         cmsg = capi.ffi.new('char**')
-        ret = capi.lib.notmuch_database_open_verbose(os.fsencode(path),
-                                                     mode.value, db_pp, cmsg)
+        ret = capi.lib.notmuch_database_open_with_config(self._encode_for_open(path),
+                                                         mode.value,
+                                                         self._encode_for_open(cfg_path),
+                                                         capi.ffi.NULL,
+                                                         db_pp, cmsg)
         if cmsg[0]:
             msg = capi.ffi.string(cmsg[0]).decode(errors='replace')
             capi.lib.free(cmsg[0])
@@ -132,7 +142,7 @@ class Database(base.NotmuchObject):
         self.closed = False
 
     @classmethod
-    def create(cls, path=None):
+    def create(cls, path=None, cfg_path=None):
         """Create and open database in READ_WRITE mode.
 
         This is creates a new notmuch database and returns an opened
@@ -154,14 +164,13 @@ class Database(base.NotmuchObject):
 
         :returns: The newly created instance.
         """
-        if path is None:
-            path = cls.default_path()
-        if not hasattr(os, 'PathLike') and isinstance(path, pathlib.Path):
-            path = bytes(path)
+
         db_pp = capi.ffi.new('notmuch_database_t **')
         cmsg = capi.ffi.new('char**')
-        ret = capi.lib.notmuch_database_create_verbose(os.fsencode(path),
-                                                       db_pp, cmsg)
+        ret = capi.lib.notmuch_database_create_with_config(cls._encode_for_open(path),
+                                                           cls._encode_for_open(cfg_path),
+                                                           capi.ffi.NULL,
+                                                           db_pp, cmsg)
         if cmsg[0]:
             msg = capi.ffi.string(cmsg[0]).decode(errors='replace')
             capi.lib.free(cmsg[0])
@@ -176,7 +185,7 @@ class Database(base.NotmuchObject):
         ret = capi.lib.notmuch_database_destroy(db_pp[0])
         if ret != capi.lib.NOTMUCH_STATUS_SUCCESS:
             raise errors.NotmuchError(ret)
-        return cls(path, cls.MODE.READ_WRITE)
+        return cls(path, cls.MODE.READ_WRITE, cfg_path=cfg_path)
 
     @staticmethod
     def default_path(cfg_path=None):
diff --git a/bindings/python-cffi/tests/test_database.py b/bindings/python-cffi/tests/test_database.py
index 9b3219c0..136ab184 100644
--- a/bindings/python-cffi/tests/test_database.py
+++ b/bindings/python-cffi/tests/test_database.py
@@ -13,7 +13,7 @@ import notmuch2._message as message
 
 @pytest.fixture
 def db(maildir):
-    with dbmod.Database.create(maildir.path) as db:
+    with dbmod.Database.create(path=maildir.path, cfg_path="") as db:
         yield db
 
 
diff --git a/bindings/python-cffi/tests/test_message.py b/bindings/python-cffi/tests/test_message.py
index 532bf921..61658812 100644
--- a/bindings/python-cffi/tests/test_message.py
+++ b/bindings/python-cffi/tests/test_message.py
@@ -17,7 +17,7 @@ class TestMessage:
 
     @pytest.fixture
     def db(self, maildir):
-        with notmuch2.Database.create(maildir.path) as db:
+        with notmuch2.Database.create(maildir.path, cfg_path="") as db:
             yield db
 
     @pytest.fixture
@@ -142,7 +142,7 @@ class TestProperties:
     @pytest.fixture
     def props(self, maildir):
         msgid, path = maildir.deliver()
-        with notmuch2.Database.create(maildir.path) as db:
+        with notmuch2.Database.create(maildir.path, cfg_path="") as db:
             msg, dup = db.add(path, sync_flags=False)
             yield msg.properties
 
diff --git a/bindings/python-cffi/tests/test_tags.py b/bindings/python-cffi/tests/test_tags.py
index faf3947b..feb20a0e 100644
--- a/bindings/python-cffi/tests/test_tags.py
+++ b/bindings/python-cffi/tests/test_tags.py
@@ -23,7 +23,7 @@ class TestImmutable:
         """
         maildir.deliver()
         notmuch('new')
-        with database.Database(maildir.path) as db:
+        with database.Database(maildir.path, cfg_path="") as db:
             yield db.tags
 
     def test_type(self, tagset):
@@ -159,7 +159,7 @@ class TestMutableTagset:
         _, pathname = maildir.deliver()
         notmuch('new')
         with database.Database(maildir.path,
-                               mode=database.Mode.READ_WRITE) as db:
+                               mode=database.Mode.READ_WRITE, cfg_path="") as db:
             msg = db.get(pathname)
             yield msg.tags
 
@@ -195,7 +195,7 @@ class TestMutableTagset:
         _, pathname = maildir.deliver(flagged=True)
         notmuch('new')
         with database.Database(maildir.path,
-                               mode=database.Mode.READ_WRITE) as db:
+                               mode=database.Mode.READ_WRITE, cfg_path="") as db:
             msg = db.get(pathname)
             msg.tags.discard('flagged')
             msg.tags.from_maildir_flags()
@@ -205,7 +205,7 @@ class TestMutableTagset:
         _, pathname = maildir.deliver(flagged=True)
         notmuch('new')
         with database.Database(maildir.path,
-                               mode=database.Mode.READ_WRITE) as db:
+                               mode=database.Mode.READ_WRITE, cfg_path="") as db:
             msg = db.get(pathname)
             flags = msg.path.name.split(',')[-1]
             assert 'F' in flags
-- 
2.33.0

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-10-27  2:37 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-26  1:27 problem with message headers in python bindings David Bremner
2021-10-26  1:56 ` David Bremner
2021-10-26  2:11   ` [PATCH] test: add known broken tests for python bindings in split configs David Bremner
2021-10-27  2:36     ` WIP fix split-config for notmuch-python2 David Bremner
2021-10-27  2:36       ` [PATCH 1/2] WIP: add python-cffi bindings to path David Bremner
2021-10-27  2:36       ` [PATCH 2/2] WIP/python2: switch to notmuch_database_{open,create}_with_config David Bremner

Code repositories for project(s) associated with this inbox:

	notmuch.git.git (no URL configured)

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).