afewmail / afew

an initial tagging script for notmuch mail
ISC License
325 stars 99 forks source link

Migrate to notmuch2 module #320

Open mjg opened 2 years ago

mjg commented 2 years ago

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 commented 2 years ago

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 https://github.com/afewmail/afew/pull/320#discussion_r757784347:

  • 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 https://github.com/afewmail/afew/pull/320#discussion_r757784347, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAG7RFQFWCGGQMKVQOKAGJLUODPVBANCNFSM5I3ML44A .

-- Ben Mezger

GuillaumeSeren commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 1 year ago

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 commented 12 months ago

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 commented 8 months ago

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.