Re: New Python bindings (notmuch2 module) fail to exclude tags

[Jorge P. de Morais Neto <jorge+list@disroot.org>]

Hi Floris.

Em [2020-11-20 sex 19:17:56+0100], Floris Bruynooghe escreveu:

> Looking at the implementation I don't seem much that could have gone
> wrong.  However I did notice the bindings fail to check the return code
> in one call where it probably should, you could try with this patch?
>
> diff --git a/bindings/python-cffi/notmuch2/_database.py b/bindings/python-cffi/notmuch2/_database.py
> index 5ab0f20a..5dbfe68e 100644
> --- a/bindings/python-cffi/notmuch2/_database.py
> +++ b/bindings/python-cffi/notmuch2/_database.py
> @@ -579,7 +579,10 @@ class Database(base.NotmuchObject):
>              for tag in exclude_tags:
>                  if isinstance(tag, str):
>                      tag = str.encode('utf-8')
> -                capi.lib.notmuch_query_add_tag_exclude(query_p, tag)
> +                ret = capi.lib.notmuch_query_add_tag_exclude(query_p, tag)
> +                if ret not in [capi.lib.NOTMUCH_STATUS_SUCCESS,
> +                               capi.lib.NOTMUCH_STATUS_IGNORED]:
> +                    raise errors.NotmuchError(ret)
>          return querymod.Query(self, query_p)
>  
>      def messages(self, query, *,
>
After applying your patch, the call to nm_db.count_messages() fails:
    AttributeError: cffi library 'notmuch2._capi' has no function, constant or global variable named 'NOTMUCH_STATUS_IGNORED'

However, I then found the bug.  The patch below fixes it.

$ diff -u notmuch2-orig/_database.py notmuch2/_database.py
--- notmuch2-orig/_database.py	2020-11-21 18:02:17.560240619 -0300
+++ notmuch2/_database.py	2020-11-21 18:43:44.827879141 -0300
@@ -578,7 +578,7 @@
         if exclude_tags is not None:
             for tag in exclude_tags:
                 if isinstance(tag, str):
-                    tag = str.encode('utf-8')
+                    tag = tag.encode('utf-8')
                 capi.lib.notmuch_query_add_tag_exclude(query_p, tag)
         return querymod.Query(self, query_p)


However, I think you should *also* add the error checking.  The only
reason my patch omits error checking is that I don't know how to define
capi.lib.NOTMUCH_STATUS_IGNORED.  After you apply my patch and also add
the error checking, you could send me the combined patch, so I don't
have to wait for the next release to get the desirable error checking.

Regards

-- 
- <https://jorgemorais.gitlab.io/justice-for-rms/>
- If an email of mine arrives at your spam box, please notify me.
- Please adopt free/libre formats like PDF, ODF, Org, LaTeX, Opus, WebM and 7z.
- Free/libre software for Replicant, LineageOS and Android: https://f-droid.org
- [[https://www.gnu.org/philosophy/free-sw.html][What is free software?]]