Skip to content

mujmap: add module#2960

Merged
rycee merged 2 commits intonix-community:masterfrom
elizagamedev:mujmap
Jun 21, 2022
Merged

mujmap: add module#2960
rycee merged 2 commits intonix-community:masterfrom
elizagamedev:mujmap

Conversation

@elizagamedev
Copy link
Copy Markdown
Contributor

mujmap is a tool which synchronizes mail between a mail server and
notmuch via JMAP. It's very similar to lieer, so I heavily based the
implementation of the notmuch module on lieer's. I did not include an
equivalent to lieer's periodic synchronization service, however, because
I plan to soon introduce a daemon mode to mujmap.

https://github.com/elizagamedev/mujmap

This isn't quite ready for merge yet, as I'm still waiting on
NixOS/nixpkgs#172648. However, this is my first module contribution to
home-manager, and I thought it was worth it to receive early feedback.

Checklist

  • Change is backwards compatible.

  • Code formatted with ./format.

  • Code tested through nix-shell --pure tests -A run.all.

  • Test cases updated/added. See example.

  • Commit messages are formatted like

    {component}: {description}
    
    {long description}
    

    See CONTRIBUTING for more information and recent commit messages for examples.

  • If this PR adds a new module

    • Added myself as module maintainer. See example.

    • Added myself and the module files to .github/CODEOWNERS.

@elizagamedev elizagamedev requested a review from rycee as a code owner May 14, 2022 04:54
@elizagamedev elizagamedev changed the title Description mujmap: add module May 14, 2022
@elizagamedev elizagamedev marked this pull request as draft May 14, 2022 04:56
@teto
Copy link
Copy Markdown
Collaborator

teto commented May 19, 2022

Great job !

My generated mujmap.toml contained password_command = ["/nix/store/acfizib24wk6hq20qg5x9f3n8398q6lh-pass-show/bin/pass-show", "perso/fastmail_mc"] which is apparently not supported by mujmap (it expects a string).

Also to setup the module I had to set mujmap.fqdn: in modules/accounts/email.nix there is a setting called "flavor" which lists "fastmail.com" and could be used to autoset fqdn

@elizagamedev
Copy link
Copy Markdown
Contributor Author

Pushed a commit to fix both of these issues.

@teto
Copy link
Copy Markdown
Collaborator

teto commented May 23, 2022

@elizagamedev CI fails for mundane reasons (can't find you as a maintainer), could you fix it ? other than that it looks good we have tests, integration with the mail backend, a pretty good first module. I will do a better review after testing/CI passes

@elizagamedev
Copy link
Copy Markdown
Contributor Author

@teto I'm not actually sure how to fix CI, since I added myself to the maintainers list but it's still failing. Is there another place I missed?

@teto
Copy link
Copy Markdown
Collaborator

teto commented May 23, 2022

ha yeah I've tried it locally and the maintainer infra is complex. @rycee any idea ?

@elizagamedev elizagamedev marked this pull request as ready for review June 7, 2022 01:57
@teto teto mentioned this pull request Jun 7, 2022
13 tasks
@teto
Copy link
Copy Markdown
Collaborator

teto commented Jun 7, 2022

@rycee do we have a style guide / CONTRIBUTING.md anywhere ? I think we prefer to use camelCase for module options but I could not refer to any documents: shall we refernece a nixpkgs one (but even then I am not sure what to link)

@elizagamedev
Copy link
Copy Markdown
Contributor Author

I used snake case so that it could directly be serialized to the config file like the lieer module does, but if it is preferred to have them as camel case it should not be too hard to translate the names before serialization.

@teto
Copy link
Copy Markdown
Collaborator

teto commented Jun 7, 2022

dont do it yet as I am unsure, waiting confirmation from the HM gods :)

@teto
Copy link
Copy Markdown
Collaborator

teto commented Jun 16, 2022

while doing a nixos-rebuild

error: store path 'm34v88fkaklilvsvv75vld6q384zqyra-mujmap-MYUSERNAME@fastmail.com.toml' contains illegal character '@'

       … while evaluating 'check'

Once could do
lib.replaceStrings ["@"] ["a"] "t@to"
or call sanitizeDerivationName from lib/strings.nix

@teto
Copy link
Copy Markdown
Collaborator

teto commented Jun 16, 2022

so using the module I get

$ mujmap sync -v
[2022-06-16T22:19:30Z WARN  mujmap::sync] Could not read mujmap state file `./mujmap.state.json': No such file or directory (os error 2)
Retrieving metadata... (11157 possibly changed)
Applying changes to notmuch database... (11157 new, 11157 changed, 0 destroyed)
error: Could not sync mail: Could not add new local email: Something went wrong trying to read or write a file

the error is not verbose enough for a proper diagnostic. I still have my maildir on another drive (ext4), can it be it ?

ls -l ~/maildir
lrwxrwxrwx 1 teto users 16 2021-10-02  /home/teto/maildir -> /mnt/ext/maildir

@elizagamedev
Copy link
Copy Markdown
Contributor Author

Once could do lib.replaceStrings ["@"] ["a"] "t@to" or call sanitizeDerivationName from lib/strings.nix

Done.

the error is not verbose enough for a proper diagnostic. I still have my maildir on another drive (ext4), can it be it ?

This is possible. I added some new config options for v2.0 which I forgot to add to this module which solve that problem (the troubleshooting section of the readme now addresses this). They have been added to the module. Does messing with those settings help?

I also tweaked fqdnXorSessionUrlAssertions, since v2.0 can now auto-detect the fqdn from the domain part of the username. Now it just checks to make sure you don't specify both.

@teto
Copy link
Copy Markdown
Collaborator

teto commented Jun 17, 2022

I've found the piece that recommands camelCase https://nix-community.github.io/home-manager/index.html#sec-code-style

I get the same error error: store path 'm34v88fkaklilvsvv75vld6q384zqyra-mujmap-MYUSERNAME@fastmail.com.toml' contains illegal character '@' with your change. Actually I think sanitizeDerivationName is broken, I've discovered and had the same problem recently. For now lib.replaceStrings ["@"] ["a"] "t@to" is probably enough.

As for the tests, seems like they are broken on master so I couldn't check that the above error was tested.

@elizagamedev
Copy link
Copy Markdown
Contributor Author

I get the same error error: store path 'm34v88fkaklilvsvv75vld6q384zqyra-mujmap-MYUSERNAME@fastmail.com.toml' contains illegal character '@' with your change. Actually I think sanitizeDerivationName is broken, I've discovered and had the same problem recently. For now lib.replaceStrings ["@"] ["a"] "t@to" is probably enough.

I changed it to replace "@" with "_at_" since that seems pretty unique. I don't know why it was working for me and not you, maybe a difference in nix version?

I've found the piece that recommands camelCase https://nix-community.github.io/home-manager/index.html#sec-code-style

It actually looks like there's an exception to this in the manual. (Section 5.2.3, Add only valuable options)

If the software uses a structured configuration format like a JSON, YAML, INI, TOML, or even a plain list of key/value pairs then consider using a settings option as described in Nix RFC 42.

The RFC linked is what lieer does, and because this is modeled after lieer's module, I inherited this way of doing things without realizing it's the "new" way. So changing the module to use camelCase would actually be going backwards. Perhaps the code style sction you linked ought to be amended to point this exception out to prevent future confusion.

That said, the "essential-ness" of some of the config options I explicitly included is a bit questionable. I removed the following explicit options:

  • convert_dos_to_unix
  • timeout
  • retries
  • concurrent_downloads

@elizagamedev
Copy link
Copy Markdown
Contributor Author

Thank you for your feedback, @rycee. I added a JMAP config option and modified the mujmap module to use it. Let me know if you want additional changes or if this is good to merge.

Co-authored-by: Eliza Velasquez <4576666+elizagamedev@users.noreply.github.com>
mujmap is a tool that synchronizes mail between a mail server and
notmuch via JMAP. It's very similar to lieer, so I heavily based the
implementation of the notmuch module on lieer's. I did not include an
equivalent to lieer's periodic synchronization service, however,
because I plan to soon introduce a daemon mode to mujmap.

https://github.com/elizagamedev/mujmap
@rycee rycee merged commit d059b94 into nix-community:master Jun 21, 2022
@rycee
Copy link
Copy Markdown
Member

rycee commented Jun 21, 2022

Nice! I've squashed and merged to master now. Thanks for the contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants