From: Tomi Ollila <tomi.ollila@iki.fi>
To: David Bremner <david@tethera.net>, notmuch@notmuchmail.org
Subject: Re: [PATCH 4/4] python-cffi: switch to notmuch_database_{open,create}_with_config
Date: Sun, 31 Oct 2021 21:46:00 +0200 [thread overview]
Message-ID: <m2ilxdkmfb.fsf@guru.guru-group.fi> (raw)
In-Reply-To: <20211030162235.1203886-5-david@tethera.net>
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
next prev parent reply other threads:[~2021-10-31 19:46 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-10-30 16:22 Use libnotmuch config / database search in python-cffi bindings David Bremner
2021-10-30 16:22 ` [PATCH 1/4] python-cffi: fix typos in docstring for Database.default_path David Bremner
2021-12-04 13:47 ` David Bremner
2021-10-30 16:22 ` [PATCH 2/4] test: add python-cffi bindings to path for test_python David Bremner
2021-10-30 16:22 ` [PATCH 3/4] test: add known broken tests for python bindings in split configs David Bremner
2021-10-30 16:22 ` [PATCH 4/4] python-cffi: switch to notmuch_database_{open,create}_with_config David Bremner
2021-10-31 19:46 ` Tomi Ollila [this message]
[not found] ` <87tugu9r4a.fsf@powell.devork.be>
2021-11-03 0:32 ` David Bremner
2021-11-03 21:49 ` Tomi Ollila
2021-11-05 18:17 ` Floris Bruynooghe
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://notmuchmail.org/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=m2ilxdkmfb.fsf@guru.guru-group.fi \
--to=tomi.ollila@iki.fi \
--cc=david@tethera.net \
--cc=notmuch@notmuchmail.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://yhetil.org/notmuch.git/
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).