use new config API in python-cffi config iterator

use new config API in python-cffi config iterator

[David Bremner]

See [1/2] for for the rationale.

This obsoletes id:20220210122531.898466-1-david@tethera.net

[PATCH 1/2] test: known broken test for list(db.config) in python-cffi bindings

[David Bremner]

As of notmuch 0.34.2 [1], the python-cffi bindings make available the
configuration from both a config file and the database when accessing
Database.config like a dictionary.  It is therefore confusing that the
iterator operations only work on the configuration information stored
in the database.

[1]: d7f95724132bf658fd151630185899737e2ed829
---
 test/T055-path-config.sh | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/test/T055-path-config.sh b/test/T055-path-config.sh
index 1df240dd..71823039 100755
--- a/test/T055-path-config.sh
+++ b/test/T055-path-config.sh
@@ -293,6 +293,27 @@ user.primary_email=test_suite@notmuchmail.org
 EOF
    test_expect_equal_file EXPECTED OUTPUT
 
+   test_begin_subtest "Config list from python ($config)"
+   test_subtest_known_broken
+   test_python <<EOF > OUTPUT
+from notmuch2 import Database
+db=Database(config=Database.CONFIG.SEARCH)
+for key in list(db.config):
+    print(key)
+EOF
+   cat <<EOF > EXPECTED
+database.autocommit
+database.backup_dir
+database.hook_dir
+database.mail_root
+database.path
+maildir.synchronize_flags
+new.tags
+user.name
+user.other_email
+user.primary_email
+EOF
+   test_expect_equal_file EXPECTED OUTPUT
    case $config in
        XDG*)
 	   test_begin_subtest "Set shadowed config value in database ($config)"
-- 
2.34.1

[PATCH 2/2] python-cffi: use config_pairs API in ConfigIterator

[David Bremner]

This returns all of the config keys with non-empty values, not just
those that happen to be stored in the database.
---
 bindings/python-cffi/notmuch2/_build.py   | 16 +++++-----
 bindings/python-cffi/notmuch2/_config.py  | 37 +++++++++++++++--------
 bindings/python-cffi/tests/test_config.py | 22 ++++++++------
 test/T055-path-config.sh                  |  1 -
 4 files changed, 44 insertions(+), 32 deletions(-)

diff --git a/bindings/python-cffi/notmuch2/_build.py b/bindings/python-cffi/notmuch2/_build.py
index a55b484f..349bb79d 100644
--- a/bindings/python-cffi/notmuch2/_build.py
+++ b/bindings/python-cffi/notmuch2/_build.py
@@ -97,7 +97,7 @@ ffibuilder.cdef(
     typedef struct _notmuch_string_map_iterator notmuch_message_properties_t;
     typedef struct _notmuch_directory notmuch_directory_t;
     typedef struct _notmuch_filenames notmuch_filenames_t;
-    typedef struct _notmuch_config_list notmuch_config_list_t;
+    typedef struct _notmuch_config_pairs notmuch_config_pairs_t;
     typedef struct _notmuch_indexopts notmuch_indexopts_t;
 
     const char *
@@ -325,18 +325,18 @@ ffibuilder.cdef(
     notmuch_database_set_config (notmuch_database_t *db, const char *key, const char *value);
     notmuch_status_t
     notmuch_database_get_config (notmuch_database_t *db, const char *key, char **value);
-    notmuch_status_t
-    notmuch_database_get_config_list (notmuch_database_t *db, const char *prefix, notmuch_config_list_t **out);
+    notmuch_config_pairs_t *
+    notmuch_config_get_pairs (notmuch_database_t *db, const char *prefix);
     notmuch_bool_t
-    notmuch_config_list_valid (notmuch_config_list_t *config_list);
+    notmuch_config_pairs_valid (notmuch_config_pairs_t *config_list);
     const char *
-    notmuch_config_list_key (notmuch_config_list_t *config_list);
+    notmuch_config_pairs_key (notmuch_config_pairs_t *config_list);
     const char *
-    notmuch_config_list_value (notmuch_config_list_t *config_list);
+    notmuch_config_pairs_value (notmuch_config_pairs_t *config_list);
     void
-    notmuch_config_list_move_to_next (notmuch_config_list_t *config_list);
+    notmuch_config_pairs_move_to_next (notmuch_config_pairs_t *config_list);
     void
-    notmuch_config_list_destroy (notmuch_config_list_t *config_list);
+    notmuch_config_pairs_destroy (notmuch_config_pairs_t *config_list);
     """
 )
 
diff --git a/bindings/python-cffi/notmuch2/_config.py b/bindings/python-cffi/notmuch2/_config.py
index 29de6495..d73539c5 100644
--- a/bindings/python-cffi/notmuch2/_config.py
+++ b/bindings/python-cffi/notmuch2/_config.py
@@ -13,27 +13,39 @@ class ConfigIter(base.NotmuchIter):
     def __init__(self, parent, iter_p):
         super().__init__(
             parent, iter_p,
-            fn_destroy=capi.lib.notmuch_config_list_destroy,
-            fn_valid=capi.lib.notmuch_config_list_valid,
-            fn_get=capi.lib.notmuch_config_list_key,
-            fn_next=capi.lib.notmuch_config_list_move_to_next)
+            fn_destroy=capi.lib.notmuch_config_pairs_destroy,
+            fn_valid=capi.lib.notmuch_config_pairs_valid,
+            fn_get=capi.lib.notmuch_config_pairs_key,
+            fn_next=capi.lib.notmuch_config_pairs_move_to_next)
 
     def __next__(self):
-        item = super().__next__()
-        return base.BinString.from_cffi(item)
-
+        # skip pairs whose value is NULL
+        while capi.lib.notmuch_config_pairs_valid (super()._iter_p):
+            val_p = capi.lib.notmuch_config_pairs_value (super()._iter_p)
+            key_p = capi.lib.notmuch_config_pairs_key (super()._iter_p)
+            key = base.BinString.from_cffi(key_p)
+            capi.lib.notmuch_config_pairs_move_to_next (super()._iter_p)
+            if val_p != capi.ffi.NULL and base.BinString.from_cffi(val_p) != "":
+                return key
+        self._destroy()
+        raise StopIteration
 
 class ConfigMapping(base.NotmuchObject, collections.abc.MutableMapping):
-    """The config key/value pairs stored in the database.
+    """The config key/value pairs loaded from the database, config file,
+    and and/or defaults.
 
     The entries are exposed as a :class:`collections.abc.MutableMapping` object.
     Note that setting a value to an empty string is the same as deleting it.
 
+    Mutating (deleting or updating values) in the map persists only in
+    the database, which can be shadowed by config files.
+
     :param parent: the parent object
     :param ptr_name: the name of the attribute on the parent which will
        return the memory pointer.  This allows this object to
        access the pointer via the parent's descriptor and thus
        trigger :class:`MemoryPointer`'s memory safety.
+
     """
 
     def __init__(self, parent, ptr_name):
@@ -77,11 +89,10 @@ class ConfigMapping(base.NotmuchObject, collections.abc.MutableMapping):
 
         :raises NullPointerError: If the iterator can not be created.
         """
-        configlist_pp = capi.ffi.new('notmuch_config_list_t**')
-        ret = capi.lib.notmuch_database_get_config_list(self._ptr(), b'', configlist_pp)
-        if ret != capi.lib.NOTMUCH_STATUS_SUCCESS:
-            raise errors.NotmuchError(ret)
-        return ConfigIter(self._parent, configlist_pp[0])
+        config_pairs_p = capi.lib.notmuch_config_get_pairs(self._ptr(), b'')
+        if config_pairs_p == capi.ffi.NULL:
+            raise KeyError
+        return ConfigIter(self._parent, config_pairs_p)
 
     def __len__(self):
         return sum(1 for t in self)
diff --git a/bindings/python-cffi/tests/test_config.py b/bindings/python-cffi/tests/test_config.py
index 67b0dea4..c62aeb98 100644
--- a/bindings/python-cffi/tests/test_config.py
+++ b/bindings/python-cffi/tests/test_config.py
@@ -34,20 +34,22 @@ class TestIter:
             print(repr(val))
 
     def test_iter(self, db):
-        assert list(db.config) == []
-        db.config['spam'] = 'ham'
-        db.config['eggs'] = 'bacon'
-        assert set(db.config) == {'spam', 'eggs'}
-        assert set(db.config.keys()) == {'spam', 'eggs'}
-        assert set(db.config.values()) == {'ham', 'bacon'}
-        assert set(db.config.items()) == {('spam', 'ham'), ('eggs', 'bacon')}
+        import re
+        prefix = re.compile(r"^TEST[.]")
+        assert [ x for x in list(db.config) if prefix.match(x) ] == []
+        db.config['TEST.spam'] = 'TEST.ham'
+        db.config['TEST.eggs'] = 'TEST.bacon'
+        assert { x for x in set(db.config) if prefix.match(x) } == {'TEST.spam', 'TEST.eggs'}
+        assert { x for x in set(db.config.keys()) if prefix.match (x) } == {'TEST.spam', 'TEST.eggs'}
+        assert { x for x in set(db.config.values()) if prefix.match (x) } == {'TEST.ham', 'TEST.bacon'}
+        assert { (x, y) for (x,y) in set(db.config.items()) if prefix.match(x) } == {('TEST.spam', 'TEST.ham'), ('TEST.eggs', 'TEST.bacon')}
 
     def test_len(self, db):
-        assert len(db.config) == 0
+        defaults = len(db.config)
         db.config['spam'] = 'ham'
-        assert len(db.config) == 1
+        assert len(db.config) == defaults + 1
         db.config['eggs'] = 'bacon'
-        assert len(db.config) == 2
+        assert len(db.config) == defaults + 2
 
     def test_del(self, db):
         db.config['spam'] = 'ham'
diff --git a/test/T055-path-config.sh b/test/T055-path-config.sh
index 71823039..4897c814 100755
--- a/test/T055-path-config.sh
+++ b/test/T055-path-config.sh
@@ -294,7 +294,6 @@ EOF
    test_expect_equal_file EXPECTED OUTPUT
 
    test_begin_subtest "Config list from python ($config)"
-   test_subtest_known_broken
    test_python <<EOF > OUTPUT
 from notmuch2 import Database
 db=Database(config=Database.CONFIG.SEARCH)
-- 
2.34.1

Re: [PATCH 2/2] python-cffi: use config_pairs API in ConfigIterator

[Floris Bruynooghe]

On Fri 11 Feb 2022 at 09:04 -0400, David Bremner wrote:

> This returns all of the config keys with non-empty values, not just
> those that happen to be stored in the database.
> ---
>  bindings/python-cffi/notmuch2/_build.py   | 16 +++++-----
>  bindings/python-cffi/notmuch2/_config.py  | 37 +++++++++++++++--------
>  bindings/python-cffi/tests/test_config.py | 22 ++++++++------
>  test/T055-path-config.sh                  |  1 -
>  4 files changed, 44 insertions(+), 32 deletions(-)
>
> diff --git a/bindings/python-cffi/notmuch2/_build.py b/bindings/python-cffi/notmuch2/_build.py
> index a55b484f..349bb79d 100644
> --- a/bindings/python-cffi/notmuch2/_build.py
> +++ b/bindings/python-cffi/notmuch2/_build.py
> @@ -97,7 +97,7 @@ ffibuilder.cdef(
>      typedef struct _notmuch_string_map_iterator notmuch_message_properties_t;
>      typedef struct _notmuch_directory notmuch_directory_t;
>      typedef struct _notmuch_filenames notmuch_filenames_t;
> -    typedef struct _notmuch_config_list notmuch_config_list_t;
> +    typedef struct _notmuch_config_pairs notmuch_config_pairs_t;
>      typedef struct _notmuch_indexopts notmuch_indexopts_t;
>  
>      const char *
> @@ -325,18 +325,18 @@ ffibuilder.cdef(
>      notmuch_database_set_config (notmuch_database_t *db, const char *key, const char *value);
>      notmuch_status_t
>      notmuch_database_get_config (notmuch_database_t *db, const char *key, char **value);
> -    notmuch_status_t
> -    notmuch_database_get_config_list (notmuch_database_t *db, const char *prefix, notmuch_config_list_t **out);
> +    notmuch_config_pairs_t *
> +    notmuch_config_get_pairs (notmuch_database_t *db, const char *prefix);
>      notmuch_bool_t
> -    notmuch_config_list_valid (notmuch_config_list_t *config_list);
> +    notmuch_config_pairs_valid (notmuch_config_pairs_t *config_list);
>      const char *
> -    notmuch_config_list_key (notmuch_config_list_t *config_list);
> +    notmuch_config_pairs_key (notmuch_config_pairs_t *config_list);
>      const char *
> -    notmuch_config_list_value (notmuch_config_list_t *config_list);
> +    notmuch_config_pairs_value (notmuch_config_pairs_t *config_list);
>      void
> -    notmuch_config_list_move_to_next (notmuch_config_list_t *config_list);
> +    notmuch_config_pairs_move_to_next (notmuch_config_pairs_t *config_list);
>      void
> -    notmuch_config_list_destroy (notmuch_config_list_t *config_list);
> +    notmuch_config_pairs_destroy (notmuch_config_pairs_t *config_list);
>      """
>  )
>  
> diff --git a/bindings/python-cffi/notmuch2/_config.py b/bindings/python-cffi/notmuch2/_config.py
> index 29de6495..d73539c5 100644
> --- a/bindings/python-cffi/notmuch2/_config.py
> +++ b/bindings/python-cffi/notmuch2/_config.py
> @@ -13,27 +13,39 @@ class ConfigIter(base.NotmuchIter):
>      def __init__(self, parent, iter_p):
>          super().__init__(
>              parent, iter_p,
> -            fn_destroy=capi.lib.notmuch_config_list_destroy,
> -            fn_valid=capi.lib.notmuch_config_list_valid,
> -            fn_get=capi.lib.notmuch_config_list_key,
> -            fn_next=capi.lib.notmuch_config_list_move_to_next)
> +            fn_destroy=capi.lib.notmuch_config_pairs_destroy,
> +            fn_valid=capi.lib.notmuch_config_pairs_valid,
> +            fn_get=capi.lib.notmuch_config_pairs_key,
> +            fn_next=capi.lib.notmuch_config_pairs_move_to_next)
>  
>      def __next__(self):
> -        item = super().__next__()
> -        return base.BinString.from_cffi(item)
> -
> +        # skip pairs whose value is NULL
> +        while capi.lib.notmuch_config_pairs_valid (super()._iter_p):

FWIW having spaces between the function name and parentheses is rather
uncommon for python style.  Though of course complaining about style
without using an auto-formatter is pretty meh these days :)

> +            val_p = capi.lib.notmuch_config_pairs_value (super()._iter_p)
> +            key_p = capi.lib.notmuch_config_pairs_key (super()._iter_p)
> +            key = base.BinString.from_cffi(key_p)

does key_p need a NULL check first or can it never be NULL?

> +            capi.lib.notmuch_config_pairs_move_to_next (super()._iter_p)
> +            if val_p != capi.ffi.NULL and base.BinString.from_cffi(val_p) != "":
> +                return key
> +        self._destroy()
> +        raise StopIteration
>  
>  class ConfigMapping(base.NotmuchObject, collections.abc.MutableMapping):
> -    """The config key/value pairs stored in the database.
> +    """The config key/value pairs loaded from the database, config file,
> +    and and/or defaults.
>  
>      The entries are exposed as a :class:`collections.abc.MutableMapping` object.
>      Note that setting a value to an empty string is the same as deleting it.
>  
> +    Mutating (deleting or updating values) in the map persists only in
> +    the database, which can be shadowed by config files.
> +
>      :param parent: the parent object
>      :param ptr_name: the name of the attribute on the parent which will
>         return the memory pointer.  This allows this object to
>         access the pointer via the parent's descriptor and thus
>         trigger :class:`MemoryPointer`'s memory safety.
> +
>      """
>  
>      def __init__(self, parent, ptr_name):
> @@ -77,11 +89,10 @@ class ConfigMapping(base.NotmuchObject, collections.abc.MutableMapping):
>  
>          :raises NullPointerError: If the iterator can not be created.
>          """
> -        configlist_pp = capi.ffi.new('notmuch_config_list_t**')
> -        ret = capi.lib.notmuch_database_get_config_list(self._ptr(), b'', configlist_pp)
> -        if ret != capi.lib.NOTMUCH_STATUS_SUCCESS:
> -            raise errors.NotmuchError(ret)
> -        return ConfigIter(self._parent, configlist_pp[0])
> +        config_pairs_p = capi.lib.notmuch_config_get_pairs(self._ptr(), b'')
> +        if config_pairs_p == capi.ffi.NULL:
> +            raise KeyError
> +        return ConfigIter(self._parent, config_pairs_p)
>  
>      def __len__(self):
>          return sum(1 for t in self)
> diff --git a/bindings/python-cffi/tests/test_config.py b/bindings/python-cffi/tests/test_config.py
> index 67b0dea4..c62aeb98 100644
> --- a/bindings/python-cffi/tests/test_config.py
> +++ b/bindings/python-cffi/tests/test_config.py
> @@ -34,20 +34,22 @@ class TestIter:
>              print(repr(val))
>  
>      def test_iter(self, db):
> -        assert list(db.config) == []
> -        db.config['spam'] = 'ham'
> -        db.config['eggs'] = 'bacon'
> -        assert set(db.config) == {'spam', 'eggs'}
> -        assert set(db.config.keys()) == {'spam', 'eggs'}
> -        assert set(db.config.values()) == {'ham', 'bacon'}
> -        assert set(db.config.items()) == {('spam', 'ham'), ('eggs', 'bacon')}
> +        import re
> +        prefix = re.compile(r"^TEST[.]")
> +        assert [ x for x in list(db.config) if prefix.match(x) ] == []

`x.startswith('TEST.')` is probably lighter weight here, maybe easier to
read too that's subjective i guess

You can even try something like this to further make it more readable:

has_prefix = lambda x: x.startswith('TEST.')

> +        db.config['TEST.spam'] = 'TEST.ham'
> +        db.config['TEST.eggs'] = 'TEST.bacon'
> +        assert { x for x in set(db.config) if prefix.match(x) } == {'TEST.spam', 'TEST.eggs'}
> +        assert { x for x in set(db.config.keys()) if prefix.match (x) } == {'TEST.spam', 'TEST.eggs'}

I'm not sure why you need to wrap `db.config.keys()` in `set()`?  This
explicitly creates a set out of things before turning it back into an
interator while you're fine with the original iterator I think?

> +        assert { x for x in set(db.config.values()) if prefix.match (x) } == {'TEST.ham', 'TEST.bacon'}
> +        assert { (x, y) for (x,y) in set(db.config.items()) if prefix.match(x) } == {('TEST.spam', 'TEST.ham'), ('TEST.eggs', 'TEST.bacon')}
>  
>      def test_len(self, db):
> -        assert len(db.config) == 0
> +        defaults = len(db.config)
>          db.config['spam'] = 'ham'
> -        assert len(db.config) == 1
> +        assert len(db.config) == defaults + 1
>          db.config['eggs'] = 'bacon'
> -        assert len(db.config) == 2
> +        assert len(db.config) == defaults + 2
>  
>      def test_del(self, db):
>          db.config['spam'] = 'ham'
> diff --git a/test/T055-path-config.sh b/test/T055-path-config.sh
> index 71823039..4897c814 100755
> --- a/test/T055-path-config.sh
> +++ b/test/T055-path-config.sh
> @@ -294,7 +294,6 @@ EOF
>     test_expect_equal_file EXPECTED OUTPUT
>  
>     test_begin_subtest "Config list from python ($config)"
> -   test_subtest_known_broken
>     test_python <<EOF > OUTPUT
>  from notmuch2 import Database
>  db=Database(config=Database.CONFIG.SEARCH)

(no subject)

[David Bremner]

Floris wrote: 
> FWIW having spaces between the function name and parentheses is rather
>uncommon for python style.  Though of course complaining about style
>without using an auto-formatter is pretty meh these days :)
>

Yeah fair enough, it's the default in the C code, but we pretty
clearly have different styles going on in different languages.

>> +            val_p = capi.lib.notmuch_config_pairs_value (super()._iter_p)
>> +            key_p = capi.lib.notmuch_config_pairs_key (super()._iter_p)
>> +            key = base.BinString.from_cffi(key_p)
>
>does key_p need a NULL check first or can it never be NULL?

I think it can never be NULL, but if it is, better to raise an exception I think.

>>      def test_iter(self, db):
>> -        assert list(db.config) == []
>> -        db.config['spam'] = 'ham'
>> -        db.config['eggs'] = 'bacon'
>> -        assert set(db.config) == {'spam', 'eggs'}
>> -        assert set(db.config.keys()) == {'spam', 'eggs'}
>> -        assert set(db.config.values()) == {'ham', 'bacon'}
>> -        assert set(db.config.items()) == {('spam', 'ham'), ('eggs', 'bacon')}
>> +        import re
>> +        prefix = re.compile(r"^TEST[.]")
>> +        assert [ x for x in list(db.config) if prefix.match(x) ] == []
>
>`x.startswith('TEST.')` is probably lighter weight here, maybe easier to
>read too that's subjective i guess
>
>You can even try something like this to further make it more readable:
>
>has_prefix = lambda x: x.startswith('TEST.')

I did variation on this, defining an inner function instead of using a lambda.

>
>> +        db.config['TEST.spam'] = 'TEST.ham'
>> +        db.config['TEST.eggs'] = 'TEST.bacon'
>> +        assert { x for x in set(db.config) if prefix.match(x) } == {'TEST.spam', 'TEST.eggs'}
>> +        assert { x for x in set(db.config.keys()) if prefix.match (x) } == {'TEST.spam', 'TEST.eggs'}
>
>I'm not sure why you need to wrap `db.config.keys()` in `set()`?  This
>explicitly creates a set out of things before turning it back into an
>interator while you're fine with the original iterator I think?

Good question. That seems to apply to list(db.config) above also? 

[PATCH v2 1/2] test: known broken test for list(db.config) in python-cffi bindings

[David Bremner]

As of notmuch 0.34.2 [1], the python-cffi bindings make available the
configuration from both a config file and the database when accessing
Database.config like a dictionary.  It is therefore confusing that the
iterator operations only work on the configuration information stored
in the database.

[1]: d7f95724132bf658fd151630185899737e2ed829
---
 test/T055-path-config.sh | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/test/T055-path-config.sh b/test/T055-path-config.sh
index 1df240dd..71823039 100755
--- a/test/T055-path-config.sh
+++ b/test/T055-path-config.sh
@@ -293,6 +293,27 @@ user.primary_email=test_suite@notmuchmail.org
 EOF
    test_expect_equal_file EXPECTED OUTPUT
 
+   test_begin_subtest "Config list from python ($config)"
+   test_subtest_known_broken
+   test_python <<EOF > OUTPUT
+from notmuch2 import Database
+db=Database(config=Database.CONFIG.SEARCH)
+for key in list(db.config):
+    print(key)
+EOF
+   cat <<EOF > EXPECTED
+database.autocommit
+database.backup_dir
+database.hook_dir
+database.mail_root
+database.path
+maildir.synchronize_flags
+new.tags
+user.name
+user.other_email
+user.primary_email
+EOF
+   test_expect_equal_file EXPECTED OUTPUT
    case $config in
        XDG*)
 	   test_begin_subtest "Set shadowed config value in database ($config)"
-- 
2.34.1

[PATCH v2 2/2] python-cffi: use config_pairs API in ConfigIterator

[David Bremner]

This returns all of the config keys with non-empty values, not just
those that happen to be stored in the database.
---
 bindings/python-cffi/notmuch2/_build.py   | 16 ++++-----
 bindings/python-cffi/notmuch2/_config.py  | 40 +++++++++++++++--------
 bindings/python-cffi/tests/test_config.py | 24 ++++++++------
 test/T055-path-config.sh                  |  1 -
 4 files changed, 49 insertions(+), 32 deletions(-)

diff --git a/bindings/python-cffi/notmuch2/_build.py b/bindings/python-cffi/notmuch2/_build.py
index a55b484f..349bb79d 100644
--- a/bindings/python-cffi/notmuch2/_build.py
+++ b/bindings/python-cffi/notmuch2/_build.py
@@ -97,7 +97,7 @@ ffibuilder.cdef(
     typedef struct _notmuch_string_map_iterator notmuch_message_properties_t;
     typedef struct _notmuch_directory notmuch_directory_t;
     typedef struct _notmuch_filenames notmuch_filenames_t;
-    typedef struct _notmuch_config_list notmuch_config_list_t;
+    typedef struct _notmuch_config_pairs notmuch_config_pairs_t;
     typedef struct _notmuch_indexopts notmuch_indexopts_t;
 
     const char *
@@ -325,18 +325,18 @@ ffibuilder.cdef(
     notmuch_database_set_config (notmuch_database_t *db, const char *key, const char *value);
     notmuch_status_t
     notmuch_database_get_config (notmuch_database_t *db, const char *key, char **value);
-    notmuch_status_t
-    notmuch_database_get_config_list (notmuch_database_t *db, const char *prefix, notmuch_config_list_t **out);
+    notmuch_config_pairs_t *
+    notmuch_config_get_pairs (notmuch_database_t *db, const char *prefix);
     notmuch_bool_t
-    notmuch_config_list_valid (notmuch_config_list_t *config_list);
+    notmuch_config_pairs_valid (notmuch_config_pairs_t *config_list);
     const char *
-    notmuch_config_list_key (notmuch_config_list_t *config_list);
+    notmuch_config_pairs_key (notmuch_config_pairs_t *config_list);
     const char *
-    notmuch_config_list_value (notmuch_config_list_t *config_list);
+    notmuch_config_pairs_value (notmuch_config_pairs_t *config_list);
     void
-    notmuch_config_list_move_to_next (notmuch_config_list_t *config_list);
+    notmuch_config_pairs_move_to_next (notmuch_config_pairs_t *config_list);
     void
-    notmuch_config_list_destroy (notmuch_config_list_t *config_list);
+    notmuch_config_pairs_destroy (notmuch_config_pairs_t *config_list);
     """
 )
 
diff --git a/bindings/python-cffi/notmuch2/_config.py b/bindings/python-cffi/notmuch2/_config.py
index 29de6495..603fdcbf 100644
--- a/bindings/python-cffi/notmuch2/_config.py
+++ b/bindings/python-cffi/notmuch2/_config.py
@@ -13,27 +13,42 @@ class ConfigIter(base.NotmuchIter):
     def __init__(self, parent, iter_p):
         super().__init__(
             parent, iter_p,
-            fn_destroy=capi.lib.notmuch_config_list_destroy,
-            fn_valid=capi.lib.notmuch_config_list_valid,
-            fn_get=capi.lib.notmuch_config_list_key,
-            fn_next=capi.lib.notmuch_config_list_move_to_next)
+            fn_destroy=capi.lib.notmuch_config_pairs_destroy,
+            fn_valid=capi.lib.notmuch_config_pairs_valid,
+            fn_get=capi.lib.notmuch_config_pairs_key,
+            fn_next=capi.lib.notmuch_config_pairs_move_to_next)
 
     def __next__(self):
-        item = super().__next__()
-        return base.BinString.from_cffi(item)
-
+        # skip pairs whose value is NULL
+        while capi.lib.notmuch_config_pairs_valid(super()._iter_p):
+            val_p = capi.lib.notmuch_config_pairs_value(super()._iter_p)
+            key_p = capi.lib.notmuch_config_pairs_key(super()._iter_p)
+            if key_p == capi.ffi.NULL:
+                # this should never happen
+                raise errors.NullPointerError
+            key = base.BinString.from_cffi(key_p)
+            capi.lib.notmuch_config_pairs_move_to_next(super()._iter_p)
+            if val_p != capi.ffi.NULL and base.BinString.from_cffi(val_p) != "":
+                return key
+        self._destroy()
+        raise StopIteration
 
 class ConfigMapping(base.NotmuchObject, collections.abc.MutableMapping):
-    """The config key/value pairs stored in the database.
+    """The config key/value pairs loaded from the database, config file,
+    and and/or defaults.
 
     The entries are exposed as a :class:`collections.abc.MutableMapping` object.
     Note that setting a value to an empty string is the same as deleting it.
 
+    Mutating (deleting or updating values) in the map persists only in
+    the database, which can be shadowed by config files.
+
     :param parent: the parent object
     :param ptr_name: the name of the attribute on the parent which will
        return the memory pointer.  This allows this object to
        access the pointer via the parent's descriptor and thus
        trigger :class:`MemoryPointer`'s memory safety.
+
     """
 
     def __init__(self, parent, ptr_name):
@@ -77,11 +92,10 @@ class ConfigMapping(base.NotmuchObject, collections.abc.MutableMapping):
 
         :raises NullPointerError: If the iterator can not be created.
         """
-        configlist_pp = capi.ffi.new('notmuch_config_list_t**')
-        ret = capi.lib.notmuch_database_get_config_list(self._ptr(), b'', configlist_pp)
-        if ret != capi.lib.NOTMUCH_STATUS_SUCCESS:
-            raise errors.NotmuchError(ret)
-        return ConfigIter(self._parent, configlist_pp[0])
+        config_pairs_p = capi.lib.notmuch_config_get_pairs(self._ptr(), b'')
+        if config_pairs_p == capi.ffi.NULL:
+            raise KeyError
+        return ConfigIter(self._parent, config_pairs_p)
 
     def __len__(self):
         return sum(1 for t in self)
diff --git a/bindings/python-cffi/tests/test_config.py b/bindings/python-cffi/tests/test_config.py
index 67b0dea4..2a7f42f0 100644
--- a/bindings/python-cffi/tests/test_config.py
+++ b/bindings/python-cffi/tests/test_config.py
@@ -34,20 +34,24 @@ class TestIter:
             print(repr(val))
 
     def test_iter(self, db):
-        assert list(db.config) == []
-        db.config['spam'] = 'ham'
-        db.config['eggs'] = 'bacon'
-        assert set(db.config) == {'spam', 'eggs'}
-        assert set(db.config.keys()) == {'spam', 'eggs'}
-        assert set(db.config.values()) == {'ham', 'bacon'}
-        assert set(db.config.items()) == {('spam', 'ham'), ('eggs', 'bacon')}
+        def has_prefix(x):
+            return x.startswith('TEST.')
+
+        assert [ x for x in db.config if has_prefix(x) ] == []
+        db.config['TEST.spam'] = 'TEST.ham'
+        db.config['TEST.eggs'] = 'TEST.bacon'
+        assert { x for x in db.config if has_prefix(x) } == {'TEST.spam', 'TEST.eggs'}
+        assert { x for x in db.config.keys() if has_prefix(x) } == {'TEST.spam', 'TEST.eggs'}
+        assert { x for x in db.config.values() if has_prefix(x) } == {'TEST.ham', 'TEST.bacon'}
+        assert { (x, y) for (x,y) in db.config.items() if has_prefix(x) } == \
+            {('TEST.spam', 'TEST.ham'), ('TEST.eggs', 'TEST.bacon')}
 
     def test_len(self, db):
-        assert len(db.config) == 0
+        defaults = len(db.config)
         db.config['spam'] = 'ham'
-        assert len(db.config) == 1
+        assert len(db.config) == defaults + 1
         db.config['eggs'] = 'bacon'
-        assert len(db.config) == 2
+        assert len(db.config) == defaults + 2
 
     def test_del(self, db):
         db.config['spam'] = 'ham'
diff --git a/test/T055-path-config.sh b/test/T055-path-config.sh
index 71823039..4897c814 100755
--- a/test/T055-path-config.sh
+++ b/test/T055-path-config.sh
@@ -294,7 +294,6 @@ EOF
    test_expect_equal_file EXPECTED OUTPUT
 
    test_begin_subtest "Config list from python ($config)"
-   test_subtest_known_broken
    test_python <<EOF > OUTPUT
 from notmuch2 import Database
 db=Database(config=Database.CONFIG.SEARCH)
-- 
2.34.1

Re: [PATCH v2 2/2] python-cffi: use config_pairs API in ConfigIterator

[Floris Bruynooghe]

lgtm i think :)

On Wed 16 Feb 2022 at 22:08 -0400, David Bremner wrote:

> This returns all of the config keys with non-empty values, not just
> those that happen to be stored in the database.
> ---
>  bindings/python-cffi/notmuch2/_build.py   | 16 ++++-----
>  bindings/python-cffi/notmuch2/_config.py  | 40 +++++++++++++++--------
>  bindings/python-cffi/tests/test_config.py | 24 ++++++++------
>  test/T055-path-config.sh                  |  1 -
>  4 files changed, 49 insertions(+), 32 deletions(-)
>
> diff --git a/bindings/python-cffi/notmuch2/_build.py b/bindings/python-cffi/notmuch2/_build.py
> index a55b484f..349bb79d 100644
> --- a/bindings/python-cffi/notmuch2/_build.py
> +++ b/bindings/python-cffi/notmuch2/_build.py
> @@ -97,7 +97,7 @@ ffibuilder.cdef(
>      typedef struct _notmuch_string_map_iterator notmuch_message_properties_t;
>      typedef struct _notmuch_directory notmuch_directory_t;
>      typedef struct _notmuch_filenames notmuch_filenames_t;
> -    typedef struct _notmuch_config_list notmuch_config_list_t;
> +    typedef struct _notmuch_config_pairs notmuch_config_pairs_t;
>      typedef struct _notmuch_indexopts notmuch_indexopts_t;
>  
>      const char *
> @@ -325,18 +325,18 @@ ffibuilder.cdef(
>      notmuch_database_set_config (notmuch_database_t *db, const char *key, const char *value);
>      notmuch_status_t
>      notmuch_database_get_config (notmuch_database_t *db, const char *key, char **value);
> -    notmuch_status_t
> -    notmuch_database_get_config_list (notmuch_database_t *db, const char *prefix, notmuch_config_list_t **out);
> +    notmuch_config_pairs_t *
> +    notmuch_config_get_pairs (notmuch_database_t *db, const char *prefix);
>      notmuch_bool_t
> -    notmuch_config_list_valid (notmuch_config_list_t *config_list);
> +    notmuch_config_pairs_valid (notmuch_config_pairs_t *config_list);
>      const char *
> -    notmuch_config_list_key (notmuch_config_list_t *config_list);
> +    notmuch_config_pairs_key (notmuch_config_pairs_t *config_list);
>      const char *
> -    notmuch_config_list_value (notmuch_config_list_t *config_list);
> +    notmuch_config_pairs_value (notmuch_config_pairs_t *config_list);
>      void
> -    notmuch_config_list_move_to_next (notmuch_config_list_t *config_list);
> +    notmuch_config_pairs_move_to_next (notmuch_config_pairs_t *config_list);
>      void
> -    notmuch_config_list_destroy (notmuch_config_list_t *config_list);
> +    notmuch_config_pairs_destroy (notmuch_config_pairs_t *config_list);
>      """
>  )
>  
> diff --git a/bindings/python-cffi/notmuch2/_config.py b/bindings/python-cffi/notmuch2/_config.py
> index 29de6495..603fdcbf 100644
> --- a/bindings/python-cffi/notmuch2/_config.py
> +++ b/bindings/python-cffi/notmuch2/_config.py
> @@ -13,27 +13,42 @@ class ConfigIter(base.NotmuchIter):
>      def __init__(self, parent, iter_p):
>          super().__init__(
>              parent, iter_p,
> -            fn_destroy=capi.lib.notmuch_config_list_destroy,
> -            fn_valid=capi.lib.notmuch_config_list_valid,
> -            fn_get=capi.lib.notmuch_config_list_key,
> -            fn_next=capi.lib.notmuch_config_list_move_to_next)
> +            fn_destroy=capi.lib.notmuch_config_pairs_destroy,
> +            fn_valid=capi.lib.notmuch_config_pairs_valid,
> +            fn_get=capi.lib.notmuch_config_pairs_key,
> +            fn_next=capi.lib.notmuch_config_pairs_move_to_next)
>  
>      def __next__(self):
> -        item = super().__next__()
> -        return base.BinString.from_cffi(item)
> -
> +        # skip pairs whose value is NULL
> +        while capi.lib.notmuch_config_pairs_valid(super()._iter_p):
> +            val_p = capi.lib.notmuch_config_pairs_value(super()._iter_p)
> +            key_p = capi.lib.notmuch_config_pairs_key(super()._iter_p)
> +            if key_p == capi.ffi.NULL:
> +                # this should never happen
> +                raise errors.NullPointerError
> +            key = base.BinString.from_cffi(key_p)
> +            capi.lib.notmuch_config_pairs_move_to_next(super()._iter_p)
> +            if val_p != capi.ffi.NULL and base.BinString.from_cffi(val_p) != "":
> +                return key
> +        self._destroy()
> +        raise StopIteration
>  
>  class ConfigMapping(base.NotmuchObject, collections.abc.MutableMapping):
> -    """The config key/value pairs stored in the database.
> +    """The config key/value pairs loaded from the database, config file,
> +    and and/or defaults.
>  
>      The entries are exposed as a :class:`collections.abc.MutableMapping` object.
>      Note that setting a value to an empty string is the same as deleting it.
>  
> +    Mutating (deleting or updating values) in the map persists only in
> +    the database, which can be shadowed by config files.
> +
>      :param parent: the parent object
>      :param ptr_name: the name of the attribute on the parent which will
>         return the memory pointer.  This allows this object to
>         access the pointer via the parent's descriptor and thus
>         trigger :class:`MemoryPointer`'s memory safety.
> +
>      """
>  
>      def __init__(self, parent, ptr_name):
> @@ -77,11 +92,10 @@ class ConfigMapping(base.NotmuchObject, collections.abc.MutableMapping):
>  
>          :raises NullPointerError: If the iterator can not be created.
>          """
> -        configlist_pp = capi.ffi.new('notmuch_config_list_t**')
> -        ret = capi.lib.notmuch_database_get_config_list(self._ptr(), b'', configlist_pp)
> -        if ret != capi.lib.NOTMUCH_STATUS_SUCCESS:
> -            raise errors.NotmuchError(ret)
> -        return ConfigIter(self._parent, configlist_pp[0])
> +        config_pairs_p = capi.lib.notmuch_config_get_pairs(self._ptr(), b'')
> +        if config_pairs_p == capi.ffi.NULL:
> +            raise KeyError
> +        return ConfigIter(self._parent, config_pairs_p)
>  
>      def __len__(self):
>          return sum(1 for t in self)
> diff --git a/bindings/python-cffi/tests/test_config.py b/bindings/python-cffi/tests/test_config.py
> index 67b0dea4..2a7f42f0 100644
> --- a/bindings/python-cffi/tests/test_config.py
> +++ b/bindings/python-cffi/tests/test_config.py
> @@ -34,20 +34,24 @@ class TestIter:
>              print(repr(val))
>  
>      def test_iter(self, db):
> -        assert list(db.config) == []
> -        db.config['spam'] = 'ham'
> -        db.config['eggs'] = 'bacon'
> -        assert set(db.config) == {'spam', 'eggs'}
> -        assert set(db.config.keys()) == {'spam', 'eggs'}
> -        assert set(db.config.values()) == {'ham', 'bacon'}
> -        assert set(db.config.items()) == {('spam', 'ham'), ('eggs', 'bacon')}
> +        def has_prefix(x):
> +            return x.startswith('TEST.')
> +
> +        assert [ x for x in db.config if has_prefix(x) ] == []
> +        db.config['TEST.spam'] = 'TEST.ham'
> +        db.config['TEST.eggs'] = 'TEST.bacon'
> +        assert { x for x in db.config if has_prefix(x) } == {'TEST.spam', 'TEST.eggs'}
> +        assert { x for x in db.config.keys() if has_prefix(x) } == {'TEST.spam', 'TEST.eggs'}
> +        assert { x for x in db.config.values() if has_prefix(x) } == {'TEST.ham', 'TEST.bacon'}
> +        assert { (x, y) for (x,y) in db.config.items() if has_prefix(x) } == \
> +            {('TEST.spam', 'TEST.ham'), ('TEST.eggs', 'TEST.bacon')}
>  
>      def test_len(self, db):
> -        assert len(db.config) == 0
> +        defaults = len(db.config)
>          db.config['spam'] = 'ham'
> -        assert len(db.config) == 1
> +        assert len(db.config) == defaults + 1
>          db.config['eggs'] = 'bacon'
> -        assert len(db.config) == 2
> +        assert len(db.config) == defaults + 2
>  
>      def test_del(self, db):
>          db.config['spam'] = 'ham'
> diff --git a/test/T055-path-config.sh b/test/T055-path-config.sh
> index 71823039..4897c814 100755
> --- a/test/T055-path-config.sh
> +++ b/test/T055-path-config.sh
> @@ -294,7 +294,6 @@ EOF
>     test_expect_equal_file EXPECTED OUTPUT
>  
>     test_begin_subtest "Config list from python ($config)"
> -   test_subtest_known_broken
>     test_python <<EOF > OUTPUT
>  from notmuch2 import Database
>  db=Database(config=Database.CONFIG.SEARCH)

Re: [PATCH v2 2/2] python-cffi: use config_pairs API in ConfigIterator

[David Bremner]

Floris Bruynooghe <flub@devork.be> writes:

> lgtm i think :)

Applied to master.

d