Use libnotmuch config / database search in python-cffi bindings

Use libnotmuch config / database search in python-cffi bindings

[David Bremner]

This is my first "serious" attempt at modifying the python-cffi
bindings, so I would appreciate feedback, particularly on the API changes.

In my dev branch this is applied on top the series [1] and the patch
[2].  The first is mandatory, the second might be optional, I didn't
test.

[1]: id:20211028013419.3996019-2-david@tethera.net
[2]: id:20211029145459.563484-1-david@tethera.net

[PATCH 1/4] python-cffi: fix typos in docstring for Database.default_path

[David Bremner]

These generate warnings from sphinx doc, which makes it harder to
debug documentation changes. They also corrupt the output.
---
 bindings/python-cffi/notmuch2/_database.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/bindings/python-cffi/notmuch2/_database.py b/bindings/python-cffi/notmuch2/_database.py
index 868f4408..c1fb88eb 100644
--- a/bindings/python-cffi/notmuch2/_database.py
+++ b/bindings/python-cffi/notmuch2/_database.py
@@ -187,8 +187,8 @@ class Database(base.NotmuchObject):
 
         :param cfg_path: The pathname of the notmuch configuration file.
            If not specified tries to use the pathname provided in the
-           :env:`NOTMUCH_CONFIG` environment variable and falls back
-           to :file:`~/.notmuch-config.
+           :envvar:`NOTMUCH_CONFIG` environment variable and falls back
+           to :file:`~/.notmuch-config`.
         :type cfg_path: str, bytes, os.PathLike or pathlib.Path.
 
         :returns: The path of the database, which does not necessarily
@@ -198,7 +198,7 @@ class Database(base.NotmuchObject):
            be opened.
         :raises configparser.Error: or subclasses if the configuration
            file can not be parsed.
-        :raises NotmuchError if the config file does not have the
+        :raises NotmuchError: if the config file does not have the
            database.path setting.
         """
         if not cfg_path:
-- 
2.33.0

[PATCH 2/4] test: add python-cffi bindings to path for test_python

[David Bremner]

This will allow testing the new python bindings using test_python, in
addition to the current invocation of pytest.
---
 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

[PATCH 3/4] test: add known broken tests for python bindings in split configs

[David Bremner]

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

diff --git a/test/T055-path-config.sh b/test/T055-path-config.sh
index ef22e964..d6494b92 100755
--- a/test/T055-path-config.sh
+++ b/test/T055-path-config.sh
@@ -306,7 +306,24 @@ EOF
 	   output2=$(notmuch --config='' config get ${key})
 	   notmuch config set ${key}
 	   test_expect_equal "${output}+${output2}" "${value}+"
-	   ;;
+	   ;&
+       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()
+m=db.find('20091117232137.GA7669@griffis1.net')
+to=m.header('To')
+print(to)
+EOF
+	   test_expect_equal_file EXPECTED OUTPUT
+	   ;& # fall through
+   esac
+
+   case $config in
+       split|XDG*)
    esac
    restore_config
    rm -rf home/.local
-- 
2.33.0

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

[David Bremner]

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

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

[Tomi Ollila]

On Sat, Oct 30 2021, David Bremner wrote:

> 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.

Generally this series looks good to me -- some suspicious newlines I see,
some (if not all) of those might be ok...

>
> [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
> +

first one above --- in database.py in current HEAD I don't see that there
is newline before :raises lines...

>      :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.
> +

IIRC usually no empty line before ending """

>      """
>  
>      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)
> +

hard to say above -- I might have done the same ;D

>          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
> +

Nw that I look this the same amount of newlines as it used to be ...

>          :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.
> +

...but here clearly added newline.

>          """
> -        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

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

[David Bremner]

Floris Bruynooghe <flub@devork.be> writes:

> On Sat 30 Oct 2021 at 13:22 -0300, David Bremner wrote:
>> +
>> +        .. deprecated:: 0.35
>> +           Use the ``cfg_path`` parameter instead.
>
> Do you mean the ``config`` parameter to :meth:`__init__` or
> :meth:`__create__` here instead of ``cfg_path``?
>

Good catch, that is indeed what I meant.


[rest of patch snipped]

> LGTM I think.
>
> As an aside, what is the oldest python version we still want to support?
> I'm wondering if we can get rid of this `if not hasattr(os, 'PathLike')`
> construct which only exists because of python 3.5.

I usually go by what is in Debian oldstable, which is 3.7.x at this
point. But maybe Tomi wants to comment, he is usually expert at finding
old distros to run notmuch on ;)

d

PS: I took the liberty of replying to the list, hope that's OK.

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

[Tomi Ollila]

On Tue, Nov 02 2021, David Bremner wrote:

> Floris Bruynooghe <flub@devork.be> writes:
>
>> On Sat 30 Oct 2021 at 13:22 -0300, David Bremner wrote:
>>> +
>>> +        .. deprecated:: 0.35
>>> +           Use the ``cfg_path`` parameter instead.
>>
>> Do you mean the ``config`` parameter to :meth:`__init__` or
>> :meth:`__create__` here instead of ``cfg_path``?
>>
>
> Good catch, that is indeed what I meant.
>
>
> [rest of patch snipped]
>
>> LGTM I think.
>>
>> As an aside, what is the oldest python version we still want to support?
>> I'm wondering if we can get rid of this `if not hasattr(os, 'PathLike')`
>> construct which only exists because of python 3.5.
>
> I usually go by what is in Debian oldstable, which is 3.7.x at this
> point. But maybe Tomi wants to comment, he is usually expert at finding
> old distros to run notmuch on ;)

I run nmbug using python 2.7 >;D

Last time I recall python 3.6 is the oldest supported version
(i.e. python 3.5 is EOL'd).

>
> d
>
> PS: I took the liberty of replying to the list, hope that's OK.

Tomi

Remembered this after spending some time creating svg image -- so
backupping it too :D

⠠⢄⣀⡀⠀⢾⡷
⢀⣠⣣⣉⠙⢺⡗⠦⢤⣀⡀
⠸⣌⣋⡼⠀⣸⡇⠀⠀⡜⡌⠁
⠀⠀⠀⠀⠀⣿⣷⠀⠺⠶⠾⠂
⠀⠀⠀⠀⠀⣿⣿
⠀⠀⣠⣴⣶⣿⣿⣶⣦⣄
\r

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

[Floris Bruynooghe]

On Tue 02 Nov 2021 at 21:32 -0300, David Bremner wrote:

> PS: I took the liberty of replying to the list, hope that's OK.

whoops, totally.   I keep forgetting to hit the right reply key.
Personal feature request: notmuch-reply-dwim, bound to r, which replies
to the list for mailing list posts. ;-)


Cheers,
Floris

Re: [PATCH 1/4] python-cffi: fix typos in docstring for Database.default_path

[David Bremner]

David Bremner <david@tethera.net> writes:

> These generate warnings from sphinx doc, which makes it harder to
> debug documentation changes. They also corrupt the output.

Series applied to release and master.

d