Migrate to notmuch2 module

[mjg]

Here are some contributions on the way to using notmuch2:

I based my work on @GuillaumeSeren `s branch and added my work on top, resulting in a passing test suite. The test suite does not cover all code paths. The last commit in this PR covers changes which are not caught by the suite, and I have not run this branch against my "real" mail database yet.

Also, I use a generator expression at some point, and this may fail on older python versions - I'm not sure which ones afew wants to support.

So, basically: do not merge yet ;)

[benmezger]

Question: would it be interesting to run this method asynchronously to speed things up? I believe this will speed things up when someone has a bunch of emails to process. Perhaps having a private method that handles the asynchronous call and calls add? I've done something similar here: https://github.com/afewmail/afew/pull/316/files#r744197423

[mjg]

This is just in the test suite, which has 10 e-mails or so.

That mail mover test suite is not exactly "pretty" anyways, the proposed changes are the minimal ones to let it pass with the notmuch2 bindings. I would refactor the e-mail comparisons quite a bit, for example.

[benmezger]

My bad, I didn’t pay attention to the file name.

On Sat, 27 Nov 2021 at 10:57 Michael J Gruber ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In afew/tests/test_mailmover.py <#320 (comment)>: > + ret = set() + for msg in db.open().messages('folder:{}'.format(folder)): + with open(msg.path) as f: + ret.add((os.path.basename(msg.messageid), + email.message_from_file(f).get_payload())) + return ret This is just in the test suite, which has 10 e-mails or so. That mail mover test suite is not exactly "pretty" anyways, the proposed changes are the minimal ones to let it pass with the notmuch2 bindings. I would refactor the e-mail comparisons quite a bit, for example. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#320 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAG7RFQFWCGGQMKVQOKAGJLUODPVBANCNFSM5I3ML44A> .

-- Ben Mezger

[GuillaumeSeren]

Hey @mjg !
Thank you to work on this.

This is a big subject as the eco-system is gona migrate to the new python module soon (notmuch, alot and afew),
also as you have seen in some old issue the notmuch package of your distro may stop shipping the old python module,
due to security concern, so we have to have ready.

But I didn't merge it right away because many distro still ship older version of notmuch (0.2x I think),
and the newer CFFI module is in 0.3x so we still have some time.

Is there something going on with the ci runners ?

Maybe there is an issue un GH now ?

[mjg]

I see that lieer is trying to still support 18LTS versions of ubuntu by catering for both versions of the notmuch bindings. Is that something you would prefer, too? In this case this PR needs to be done differently, of course. (Also, I haven't looked into it since.)

[GuillaumeSeren]

Hey @mjg,
I didn't know this lieer project it looks very cool.

If we can refactor the branch to able to support both, it would nice because we could merge now,
and start testing in the next release.
But it could also lead to more issue and complexity just to support old version.

In the end it depends on the implementation.

[hbog]

Is there something I can do to help to get this merged ? The current notmuch bindings bother me by causing segmentation faults is certain cases.

[GuillaumeSeren]

Hey @hbog ,
if you want to help you can try this branch, but I don't think it is ready yet (if you have problem with main branch open a different issue).

[mjg]

What would help (without having to port any bindings) is adding more tests so that at least each filter is covered.
As a second step, if we still want to support the legacy bindings, some refactoring should be done in order to isolate any uses of the notmuch module. Then we can reimplement that interface for notmuch2.

[mjg]

I"ve reworked the series and used it in "production", together with #350 and the mailcap dead battery, on Fedora 41with python 3.13.
Running it in in production uncovered a lot of codepaths especially in filters which are not covered by the tests. All filters which I use are ported in this series, and as much as I can test, I applied the same changes to the rest.

Side note: upcoming notmuch 0.39 will drop the legacy bindings. I see no point in refactoring just to keep supporting the legacy bindings when users can use a specific afew version from pip.

[GuillaumeSeren]

Hey @mjg ,
thank you so much for pushing this patch for nearly 4 years.
I can't really review now, but I will in the next week, I think now notmuch should be available,
so we should be good to merge.

Also would you be interested to co-maintain afew ? (I need help)

[mjg]

Hi @GuillaumeSeren ,
Thanks, but this series is a joint effort :)

A few (huh) days ago I removed python-notmuch from my system and discovered some uses of notmuch.error which i turned into notmuch2._error with the previous push. Today I encountered a message without subject header (duh!) which triggered an exception in FolderNameFilter, and I fixed that with today's push.
There may still be undiscovered cases where header access is not within in an try-except-block. That (not simply returning an empty header if absent) is one of the changes which make the new bindings more pythonic, but also affect the migration. I fixed when/where I spotted it rather than refactoring with a helper method. This is something to look out for during review.

As for comaintership: I feel honoured by the offer but don't want to make promises which I cannot uphold. (Also, last time I took over comaintership somewhere the maintainer disappeared ...)
I'll certainly be around as a user/commenter/contributor. I maintain notmuch in Fedora and use early versions before I push them to Fedora, and that can expose necessary changes in afew, alot, neomutt which I use with notmuch (and possibly aerc in the future)

[bremner]

I tried running this patched version of afew with python 3.13 on Debian Testing, and I get the following

raceback (most recent call last):
  File "/usr/bin/afew", line 33, in <module>
    sys.exit(load_entry_point('afew==3.0.1.post63', 'console_scripts', 'afew')())
             ~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/bin/afew", line 25, in importlib_load_entry_point
    return next(matches).load()
           ~~~~~~~~~~~~~~~~~~^^
  File "/usr/lib/python3.13/importlib/metadata/__init__.py", line 179, in load
    module = import_module(match.group('module'))
  File "/usr/lib/python3.13/importlib/__init__.py", line 88, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
           ~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "<frozen importlib._bootstrap>", line 1387, in _gcd_import
  File "<frozen importlib._bootstrap>", line 1360, in _find_and_load
  File "<frozen importlib._bootstrap>", line 1331, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 935, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 1026, in exec_module
  File "<frozen importlib._bootstrap>", line 488, in _call_with_frames_removed
  File "/usr/lib/python3/dist-packages/afew/commands.py", line 12, in <module>
    from afew.Settings import user_config_dir, get_filter_chain, \
        get_mail_move_rules, get_mail_move_age, get_mail_move_rename
  File "/usr/lib/python3/dist-packages/afew/Settings.py", line 21, in <module>
    settings.readfp(open(os.path.join(os.path.dirname(__file__), 'defaults', 'afew.config')))

I don't know that much about python packaging on Debian, so it could be specific to the way I generated a Debian package.

[mjg]

Hi @bremner , how do you run afew? Do you have a config file? Does your method of packaging work without this branch?
I use afew installed via pip from a checkout.

[bremner]

Michael J Gruber ***@***.***> writes:

mjg left a comment (afewmail/afew#320) Hi @bremner , how do you run afew? Do you have a config file?

I just ran "afew" without any arguments. I don't have a config file.

Does your method of packaging work without this branch?

Without this branch I get "You need to specify an action", which seems like (limited) success.

I use afew installed via pip from a checkout.

I am not actually an afew user, just trying to unbreak the Debian package. So your method would not work for me. EDIT: The underlying problem looks to be readfp is gone as of python 3.12 https://docs.python.org/3/whatsnew/3.12.html#configparser

[mjg]

Sure, that's why I wrote "together with #350" ;-)

[bremner]

Michael J Gruber ***@***.***> writes:

mjg left a comment (afewmail/afew#320) Sure, that's why I wrote "together with #350" ;-)

Ah. Yes, I'm (clearly) not really good at working with github; I just cherry picked a range of commits and ignored the surounding commentary. Anyway, with 350 it seems to work. Someone(TM) should really do a release.

[GuillaumeSeren]

Hey @mjg,
I think this migration will be the cause a new major release (maybe v4 ?),
while we wait for this and ideally #189 to be ready at least, we could also tag a minor upgrade on the actual branch (v3).

Anyway thank you a lot for helping this project

[jirijakes]

@mjg, would you mind rebasing your branch on afew's current master? I would like to try this PR using Arch Linux package manager (directly from your branch), but it seems that your branch is missing some important commits from master. Thanks!

[mjg]

Rebased on top of master which contains the readfp fix now.

[jirijakes]

Unsure how related it is but I get this error when afew -t "tag:unread":

Traceback (most recent call last):
  File "/usr/bin/afew", line 8, in <module>
    sys.exit(main())
             ~~~~^^
  File "/usr/lib/python3.13/site-packages/afew/commands.py", line 160, in main
    inner_main(args, database, query_string)
    ~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.13/site-packages/afew/main.py", line 20, in main
    filter_.commit(options.dry_run)
    ~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.13/site-packages/afew/filters/BaseFilter.py", line 105, in commit
    message.tags.remove(tag)
    ~~~~~~~~~~~~~~~~~~~^^^^^
  File "<frozen _collections_abc>", line 733, in remove
KeyError: 'new'

I use build instructions from Arch's afew package with mjg's branch.

Later I will try to use build scripts directly and with minimal config unless someone already knows what's the problem in the meantime.

[tohojo]

I'm seeing the same error as @jirijakes - fixed it with this patch:

diff --git a/afew/filters/BaseFilter.py b/afew/filters/BaseFilter.py
index 3803d7b..716f8b0 100644
--- a/afew/filters/BaseFilter.py
+++ b/afew/filters/BaseFilter.py
@@ -102,6 +102,9 @@ class Filter:
                     message.tags.add(tag)
 
                 for tag in self._remove_tags.get(message_id, []):
-                    message.tags.remove(tag)
+                    try:
+                        message.tags.remove(tag)
+                    except KeyError:
+                        pass
 
         self.flush_changes()

[mjg]

Thanks, I guess neither the test nor my setup remove tags which are not present. I vaguely remember a discussion whether this should even be possible to occur or caught earlier.
As a general rule, the notmuch2 bindings raise an error where notmuch bindings would silently return None or '' or do nothing when a key is not present. In this particular case, I'm not 100% sure whether at this point in the codepath afew should even try to remove a tag which is not present, that is whether @tohojo 's fix masks a different problem. What do you think, @GuillaumeSeren ?

[jirijakes]

fixed it with this patch: […]

Thanks, @tohojo. Yeah, applying this patch makes it all work.

But I understand the concern with this kind of fix.

[tohojo]

Yeah, may well be there should be a check somewhere else. Didn't look that closely, just needed a quick fix to get my email working again :)

OTOH, if the notmuch bindings have previously just accepted this behaviour there's a good chance that this is how things have always functioned, and suppressing the error just makes it visible what the notmuch bindings were silently doing before?

[mjg]

Yes, checked the usage, and you're supposed to be able to just remove tags, present or not. I've rebased and put @tohojo 's fix second but last - the last ones enables CI and needs be rewritten. Are the new notmuch bindings not available for py 3.9? I could try one by one, of course, and annoy everyone with forced pushes ...

[tohojo]

Are the new notmuch bindings not available for py 3.9

Well, the Python bindings are just wrappers around C functions, so they are not really portable across Python versions. The way the CI is setup, it just links the distribution packaged version of the bindings into the Python venv, so that will only work with whatever version of Python the distribution ships. And maybe not even that, since the setup-python action AFAIU installs binaries from upstream Python, which I'm not sure are compatible with the one compiled by the distribution.

So to get CI working with these bindings, it will have to be changed to clone the notmuch git repository and compile the bindings inside the venv...

[mjg]

Are the new notmuch bindings not available for py 3.9

Well, the Python bindings are just wrappers around C functions, so they are not really portable across Python versions. The way the CI is setup, it just links the distribution packaged version of the bindings into the Python venv, so that will only work with whatever version of Python the distribution ships. And maybe not even that, since the setup-python action AFAIU installs binaries from upstream Python, which I'm not sure are compatible with the one compiled by the distribution.

So to get CI working with these bindings, it will have to be changed to clone the notmuch git repository and compile the bindings inside the venv...

I'm kinda wondering how this worked before. I expected a wheel to be available and to be used. Difficult to test locally, but I may try some pushes ...

[mjg]

Yeah 🥇

[tohojo]

I'm kinda wondering how this worked before.

Looks like the v1 bindings open the libnotmuch.so library from the Python side, whereas v2 goes the other way and compiles a Python extension from the C sources.

[andram]

Just to say that 126f89f works for me as well. Thanks!

My setup:

arch linux
python 3.13
notmuch 0.19

[tohojo]

Yeah 🥇

Huh, interesting that pip install works - the version of notmuch2 on PyPi is 5 years old. I guess as long as the API doesn't change on the notmuch (C) side, it should keep working...

[mjg]

Yeah 🥇

Huh, interesting that pip install works - the version of notmuch2 on PyPi is 5 years old. I guess as long as the API doesn't change on the notmuch (C) side, it should keep working...

This is more of a POC/minimal change to get afew's update to notmuch2 going. I'll check with notmuch's maintainer what he prefers - there was discussion about shutting down the github mirror of notmuch, which could be a base for pip. But I have no experience with packaging for pip/pypa.

The alternative is to trim down the test matrix to default install of python+notmuch, with maybe more distro varsions or variants.

[mjg]

FTR: alot (the notmuch-based client) clones notmuch.git and builds it in CI, apparantly after giving up on a PR for the pypi notmuch2 project :|

[GuillaumeSeren]

Hey,
as we expected for a while now, (see #320) the legacy python module for notmuch was going to be removed,
it is the case for the 0.39 release (module was still here in 0.38.3).
Also the gentoo maintainer want to drop the package, I think it is fair,
but that is annoying for the users:

- mail-filter/afew-3.0.1-r1::gentoo (masked by: package.mask)                                                                                                                                    
/var/db/repos/gentoo/profiles/package.mask:                                                                                                                                                      
# Michał Górny <[email protected]> (2025-03-22)                                                                                                                                                  
# Depends on deprecated notmuch bindings.  The migration to new bindings                                                                                                                         
# have not been finished in 5 years already.  Still uses legacy                                                                                                                                  
# distutils-r1.eclass mode.                                                                     
# Removal on 2025-04-21.  Bug #751541. 

[GuillaumeSeren]

Thanks, I guess neither the test nor my setup remove tags which are not present. I vaguely remember a discussion whether this should even be possible to occur or caught earlier. As a general rule, the notmuch2 bindings raise an error where notmuch bindings would silently return None or '' or do nothing when a key is not present. In this particular case, I'm not 100% sure whether at this point in the codepath afew should even try to remove a tag which is not present, that is whether @tohojo 's fix masks a different problem. What do you think, @GuillaumeSeren ?

I think we should aim for keeping the behavior as close as we can but upgrading the module,
here the @tohojo / @jirijakes looks ok and I guess it should be in this branch as it would be needed with the new module.

[mjg]

Thanks, I guess neither the test nor my setup remove tags which are not present. I vaguely remember a discussion whether this should even be possible to occur or caught earlier. As a general rule, the notmuch2 bindings raise an error where notmuch bindings would silently return None or '' or do nothing when a key is not present. In this particular case, I'm not 100% sure whether at this point in the codepath afew should even try to remove a tag which is not present, that is whether @tohojo 's fix masks a different problem. What do you think, @GuillaumeSeren ?

I think we should aim for keeping the behavior as close as we can but upgrading the module, here the @tohojo / @jirijakes looks ok and I guess it should be in this branch as it would be needed with the new module.

It is already 👍