[PATCH 4/4] python-cffi: switch to notmuch_database_{open,create}_with_config

[David Bremner <david@tethera.net>]

Since release 0.32, libnotmuch provides searching for database and
configuration paths. This commit changes the python module notmuch2 to
use those facilities.

This fixes the bug reported in [1], along with a couple of the
deprecation warnings in the python bindings.

Database.default_path is deprecated, since it no longer faithfully
reflects what libnotmuch is doing, and it is also no longer used in
the bindings themselves.

This commit choose the default of config=CONFIG.EMPTY (equivalent to
passing "" to notmuch_database_open_with_config).  This makes the
change upward compatible API-wise (at least as far as the test suite
verifies), but changing the default to CONFIG.SEARCH would probably be
more convenient for bindings users.

[1]: id:87h7d4wp6b.fsf@tethera.net
---
 bindings/python-cffi/notmuch2/_build.py    | 26 ++++---
 bindings/python-cffi/notmuch2/_database.py | 81 ++++++++++++++++------
 doc/man1/notmuch-config.rst                |  2 +
 test/T055-path-config.sh                   |  5 +-
 test/T391-python-cffi.sh                   |  8 ++-
 5 files changed, 82 insertions(+), 40 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 c1fb88eb..92bfdef2 100644
--- a/bindings/python-cffi/notmuch2/_database.py
+++ b/bindings/python-cffi/notmuch2/_database.py
@@ -31,6 +31,9 @@ class Mode(enum.Enum):
     READ_ONLY = capi.lib.NOTMUCH_DATABASE_MODE_READ_ONLY
     READ_WRITE = capi.lib.NOTMUCH_DATABASE_MODE_READ_WRITE
 
+class ConfigFile(enum.Enum):
+    EMPTY = b''
+    SEARCH = capi.ffi.NULL
 
 class QuerySortOrder(enum.Enum):
     OLDEST_FIRST = capi.lib.NOTMUCH_SORT_OLDEST_FIRST
@@ -71,6 +74,9 @@ class Database(base.NotmuchObject):
     :cvar EXCLUDE: Which messages to exclude from queries, ``TRUE``,
        ``FLAG``, ``FALSE`` or ``ALL``.  See the query documentation
        for details.
+    :cvar CONFIG: Control loading of config file. Enumeration of
+       ``EMPTY`` (don't load a config file), and ``SEARCH`` (search as
+       in :ref:`config_search`)
     :cvar AddedMessage: A namedtuple ``(msg, dup)`` used by
        :meth:`add` as return value.
     :cvar STR_MODE_MAP: A map mapping strings to :attr:`MODE` items.
@@ -81,9 +87,8 @@ class Database(base.NotmuchObject):
        still open.
 
     :param path: The directory of where the database is stored.  If
-       ``None`` the location will be read from the user's
-       configuration file, respecting the ``NOTMUCH_CONFIG``
-       environment variable if set.
+       ``None`` the location will be searched according to
+       :ref:`database`
     :type path: str, bytes, os.PathLike or pathlib.Path
     :param mode: The mode to open the database in.  One of
        :attr:`MODE.READ_ONLY` OR :attr:`MODE.READ_WRITE`.  For
@@ -91,17 +96,22 @@ class Database(base.NotmuchObject):
        :attr:`MODE.READ_ONLY` and ``rw`` for :attr:`MODE.READ_WRITE`.
     :type mode: :attr:`MODE` or str.
 
+    :param config: Where to load the configuration from, if any.
+    :type config: :attr:`CONFIG.EMPTY`, :attr:`CONFIG.SEARCH`, str, bytes, os.PathLike, pathlib.Path
+
     :raises KeyError: if an unknown mode string is used.
     :raises OSError: or subclasses if the configuration file can not
        be opened.
     :raises configparser.Error: or subclasses if the configuration
        file can not be parsed.
     :raises NotmuchError: or subclasses for other failures.
+
     """
 
     MODE = Mode
     SORT = QuerySortOrder
     EXCLUDE = QueryExclude
+    CONFIG = ConfigFile
     AddedMessage = collections.namedtuple('AddedMessage', ['msg', 'dup'])
     _db_p = base.MemoryPointer()
     STR_MODE_MAP = {
@@ -109,18 +119,40 @@ class Database(base.NotmuchObject):
         'rw': MODE.READ_WRITE,
     }
 
-    def __init__(self, path=None, mode=MODE.READ_ONLY):
+    @staticmethod
+    def _cfg_path_encode(path):
+        if isinstance(path,ConfigFile):
+            path = path.value
+        elif 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
+
+    @staticmethod
+    def _db_path_encode(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, config=CONFIG.EMPTY):
         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._db_path_encode(path),
+                                                         mode.value,
+                                                         self._cfg_path_encode(config),
+                                                         capi.ffi.NULL,
+                                                         db_pp, cmsg)
         if cmsg[0]:
             msg = capi.ffi.string(cmsg[0]).decode(errors='replace')
             capi.lib.free(cmsg[0])
@@ -132,18 +164,20 @@ class Database(base.NotmuchObject):
         self.closed = False
 
     @classmethod
-    def create(cls, path=None):
+    def create(cls, path=None, config=ConfigFile.EMPTY):
         """Create and open database in READ_WRITE mode.
 
         This is creates a new notmuch database and returns an opened
         instance in :attr:`MODE.READ_WRITE` mode.
 
-        :param path: The directory of where the database is stored.  If
-           ``None`` the location will be read from the user's
-           configuration file, respecting the ``NOTMUCH_CONFIG``
-           environment variable if set.
+        :param path: The directory of where the database is stored.
+           If ``None`` the location will be read searched by the
+           notmuch library (see notmuch(3)::notmuch_open_with_config).
         :type path: str, bytes or os.PathLike
 
+        :param config: The pathname of the notmuch configuration file.
+        :type config: :attr:`CONFIG.EMPTY`, :attr:`CONFIG.SEARCH`, str, bytes, os.PathLike, pathlib.Path
+
         :raises OSError: or subclasses if the configuration file can not
            be opened.
         :raises configparser.Error: or subclasses if the configuration
@@ -153,15 +187,15 @@ class Database(base.NotmuchObject):
         :raises FileError: if the database already exists.
 
         :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._db_path_encode(path),
+                                                           cls._cfg_path_encode(config),
+                                                           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 +210,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, config=config)
 
     @staticmethod
     def default_path(cfg_path=None):
@@ -200,6 +234,9 @@ class Database(base.NotmuchObject):
            file can not be parsed.
         :raises NotmuchError: if the config file does not have the
            database.path setting.
+
+        .. deprecated:: 0.35
+           Use the ``cfg_path`` parameter instead.
         """
         if not cfg_path:
             cfg_path = _config_pathname()
diff --git a/doc/man1/notmuch-config.rst b/doc/man1/notmuch-config.rst
index 7d901758..36e57ea6 100644
--- a/doc/man1/notmuch-config.rst
+++ b/doc/man1/notmuch-config.rst
@@ -259,6 +259,8 @@ paths are presumed relative to `$HOME` for items in section
 FILES
 =====
 
+.. _config_search:
+
 CONFIGURATION
 -------------
 
diff --git a/test/T055-path-config.sh b/test/T055-path-config.sh
index d6494b92..6d9fb402 100755
--- a/test/T055-path-config.sh
+++ b/test/T055-path-config.sh
@@ -309,11 +309,10 @@ EOF
 	   ;&
        split)
 	   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()
+from notmuch2 import Database
+db=Database(config=Database.CONFIG.SEARCH)
 m=db.find('20091117232137.GA7669@griffis1.net')
 to=m.header('To')
 print(to)
diff --git a/test/T391-python-cffi.sh b/test/T391-python-cffi.sh
index d54bad27..30872af0 100755
--- a/test/T391-python-cffi.sh
+++ b/test/T391-python-cffi.sh
@@ -7,8 +7,14 @@ if [ $NOTMUCH_HAVE_PYTHON3_CFFI -eq 0 -o $NOTMUCH_HAVE_PYTHON3_PYTEST -eq 0 ]; t
 fi
 
 
-test_begin_subtest "python cffi tests"
+test_begin_subtest "python cffi tests (NOTMUCH_CONFIG set)"
 pytest_dir=$NOTMUCH_BUILDDIR/bindings/python-cffi/build/stage
 printf "[pytest]\nminversion = 3.0\naddopts = -ra\n" > $pytest_dir/pytest.ini
 test_expect_success "(cd $pytest_dir && ${NOTMUCH_PYTHON} -m pytest --verbose --log-file=$TMP_DIRECTORY/test.output)"
+
+test_begin_subtest "python cffi tests (NOTMUCH_CONFIG unset)"
+pytest_dir=$NOTMUCH_BUILDDIR/bindings/python-cffi/build/stage
+printf "[pytest]\nminversion = 3.0\naddopts = -ra\n" > $pytest_dir/pytest.ini
+unset NOTMUCH_CONFIG
+test_expect_success "(cd $pytest_dir && ${NOTMUCH_PYTHON} -m pytest --verbose --log-file=$TMP_DIRECTORY/test.output)"
 test_done
-- 
2.33.0