From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp1 ([2001:41d0:2:4a6f::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by ms0.migadu.com with LMTPS id 0Pe5GYbyfmG1gAEAgWs5BA (envelope-from ) for ; Sun, 31 Oct 2021 20:46:14 +0100 Received: from aspmx1.migadu.com ([2001:41d0:2:4a6f::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp1 with LMTPS id +FE3FYbyfmFpdQAAbx9fmQ (envelope-from ) for ; Sun, 31 Oct 2021 19:46:14 +0000 Received: from mail.notmuchmail.org (nmbug.tethera.net [144.217.243.247]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by aspmx1.migadu.com (Postfix) with ESMTPS id E9C2E2D87C for ; Sun, 31 Oct 2021 20:46:11 +0100 (CET) Received: from nmbug.tethera.net (localhost [127.0.0.1]) by mail.notmuchmail.org (Postfix) with ESMTP id 3F2E21FD53; Sun, 31 Oct 2021 15:46:06 -0400 (EDT) Received: from lahtoruutu.iki.fi (lahtoruutu.iki.fi [185.185.170.37]) by mail.notmuchmail.org (Postfix) with ESMTPS id DEEA31FCD5 for ; Sun, 31 Oct 2021 15:46:03 -0400 (EDT) Received: from guru.guru-group.fi (unknown [IPv6:2a02:2380:1:9:5054:ff:feb7:a4bc]) (using TLSv1.2 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) (Authenticated sender: too) by lahtoruutu.iki.fi (Postfix) with ESMTPSA id 350091B0008C; Sun, 31 Oct 2021 21:46:02 +0200 (EET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=iki.fi; s=lahtoruutu; t=1635709562; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=12MNoLP56mTbUl+QTrXhZqypvLCnxN+LO4zGRUckfKA=; b=PZlfWFtP/qoab4vP1QjZ5A5CU1jdHrFBEhDSIxohxJhg0rEiA5AGLYq3b03GIhmKm/u6CF vPeQox1jAbnn6qHKa6fJsoAE1+JJOVbiiieZczoL+u4VL13pDm+RuerYWfxBB4ZaTOHESd dnJ50/kTn5AtcLNYWAK/oGRmbwOvaW9MJULvD3g94d463yPDXB78UHPnGkGp2zBUv02xjg 1QKYBGNVpegmZuZY9iM5aPiIEmjucgbbBRr2vjFXiACYmkQr8nNaWPjOPRbwm0uHzQjRHQ xGoQC/0G068YE6lcbzVqQByZ/aY9j3oLkfnqdJuKjvaffOmOXyhXbK9w8Ld8lQ== From: Tomi Ollila To: David Bremner , notmuch@notmuchmail.org Subject: Re: [PATCH 4/4] python-cffi: switch to notmuch_database_{open,create}_with_config In-Reply-To: <20211030162235.1203886-5-david@tethera.net> References: <20211030162235.1203886-1-david@tethera.net> <20211030162235.1203886-5-david@tethera.net> User-Agent: Notmuch/0.33+54~gcaf4513 (https://notmuchmail.org) Emacs/27.1 X-Face: HhBM'cA~ MIME-Version: 1.0 ARC-Seal: i=1; s=lahtoruutu; d=iki.fi; t=1635709562; a=rsa-sha256; cv=none; b=BS5Cv0TlTJeXgE0Ge9lvbVZmAKHvUNwZySSe6UV3NLS/qmznYwKX11AgACdRZ1UPpwI14W RhEI82PQWsBZQexLAYWf0ZoxFo2Y+eesYxz5Kkf0dHzKZC27tt42ZKP5n7gelm7UcNC94b moVYt8iX2Nn8g9Zcp1sTOKun6Hs5iJS3gJS0Bmxn1vBbFjZ4Ab5veFcpj/brso2/6gKDlj 5Y2NjbtuAT+foYvduwrcPTe6ZzBAXVPp+Efqzg4MhKwQcXRd8eVcbq4G3V2XaSAPCRYEtD 6NaDdHDYCkRxgpJdGWM0+VXXZy1nFfYwLH1yM51IssAnduPjebIOAUBnvk2nkg== ARC-Authentication-Results: i=1; ORIGINATING; auth=pass smtp.auth=too smtp.mailfrom=tomi.ollila@iki.fi ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=iki.fi; s=lahtoruutu; t=1635709562; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=12MNoLP56mTbUl+QTrXhZqypvLCnxN+LO4zGRUckfKA=; b=ZVREUDy3xEgUC2kOei+d+9T218t7CHE6PR61i4Wl+xevqrhw2b4UfcEXgsKAQ0w5qj8zL7 EOgAmou9FPK+wc3pprrdGBWGMfiVb4V3Nq1vYtdkbydFfZUlVJekJ+ZDeTLqIzK1MqbEqq e2tloF0yoAuj4xRST4iM5SqDzbLcTSWlm738oe2B08z/VtF3qgdwY1awMBvfRWEw+as7Rn od1dAFUFyzeGXadEdYqpYNWp56hUkVM9+0y6FMQVLRNbskZ6xTHanMskAJ4TYfIwzXseT/ N0TAn5IxGOMU2wzNt3AbRq14vgg/8x/cWZJS9sdBkbT0bPXZ7+GXshUZMZXwQA== Message-ID-Hash: 2A7RAAZVKEOFSFLYATYHEBJXJD3L5UXC X-Message-ID-Hash: 2A7RAAZVKEOFSFLYATYHEBJXJD3L5UXC X-MailFrom: tomi.ollila@iki.fi X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; header-match-notmuch.notmuchmail.org-0; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.3 Precedence: list List-Id: "Use and development of the notmuch mail system." List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit X-Migadu-Flow: FLOW_IN ARC-Message-Signature: i=2; a=rsa-sha256; c=relaxed/relaxed; d=yhetil.org; s=key1; t=1635709572; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:list-id:list-help: list-owner:list-unsubscribe:list-subscribe:list-post:dkim-signature; bh=HjOjHNUz7wb+OGwu7kTKIH4H3pRXAbZ6DrudBLrVhu8=; b=CwR3xSTHUSFcQhiW5oS+O4Ft30KvTWfJzA1sUZ02RuIr4oHv4rW71Hi1jIW4dHVB4SpIpd qp26i8Q1VGRBAKWEYahRPP/XWbAplQ2xoUvE2jIGNHoGNlbukfWGUnhaE1fDB/4wGnB4bl RW+kJ8YbwzVglH5roSOA5IVsnUsjS5d3BfdYh7dJtlBiWr/nFTF2fNNpzeDsDuPRxFBdIh kx5/vU0COHVhK+3BP/1KKHeNzhYRnF7WNHUSDxqMdU4STlbJ+WYzSCUZy1AtHAXr80J/2y vaL4embDGhwZxhy+fQW0yCg6KVw/9A8PgMJTg5jfuJFmNtAFvY5BLnuShmotOg== ARC-Seal: i=2; s=key1; d=yhetil.org; t=1635709572; a=rsa-sha256; cv=fail; b=ngQP8oIBv/INFgZyPl25J4hb7GhfplMyZNiEwjsrlDLQY++yqQDlPh/qnilIyjbcs9ifxN XhIfBJ9svXtLX/Il21UweTI5aCcVisOtyqvZGWallaERYYhRHwDqvyLmNp3urPnsktYE1B q3kz8+6hHegWBmW31piaJZBuAnCvIx9Wb2WCffsdrvrDanXg2jfn6GAamCAHFxh6B8zpoW Nl2v6qQcJJxfbqEwyb3x5gzogPQBii94LkvfDIG8N/77k8FcdQ8u4kW2Yzu2Ily7/RVssA Y9S8ZGMOKknpdoXyWeUUcdnRosx6mWuLFq5YaJoA5MrUZ0/yMES21kuxB2werA== ARC-Authentication-Results: i=2; aspmx1.migadu.com; dkim=fail ("body hash did not verify") header.d=iki.fi header.s=lahtoruutu header.b=PZlfWFtP; arc=reject ("signature check failed: fail, {[1] = sig:iki.fi:reject}"); dmarc=none; spf=pass (aspmx1.migadu.com: domain of notmuch-bounces@notmuchmail.org designates 144.217.243.247 as permitted sender) smtp.mailfrom=notmuch-bounces@notmuchmail.org X-Migadu-Spam-Score: 0.86 Authentication-Results: aspmx1.migadu.com; dkim=fail ("body hash did not verify") header.d=iki.fi header.s=lahtoruutu header.b=PZlfWFtP; arc=reject ("signature check failed: fail, {[1] = sig:iki.fi:reject}"); dmarc=none; spf=pass (aspmx1.migadu.com: domain of notmuch-bounces@notmuchmail.org designates 144.217.243.247 as permitted sender) smtp.mailfrom=notmuch-bounces@notmuchmail.org X-Migadu-Queue-Id: E9C2E2D87C X-Spam-Score: 0.86 X-Migadu-Scanner: scn0.migadu.com X-TUID: ZfyEETs0biSB 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 < -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