Address code review comments for getmail service

This patch started by addresssing the code review comments to close
https://github.com/rycee/home-manager/pull/290. However initiating a new
pull request it became clear, that home-manager changed significantly
since then.

This changes the initial pull request to be consistent with the email
account management in home-manager now. It also adds a simple test and support
for multiple accounts.

modules/accounts/email.nix

@@ -388,6 +388,7 @@ in
         mailAccountOpts
         (import ../programs/alot-accounts.nix pkgs)
         (import ../programs/astroid-accounts.nix)
+        (import ../programs/getmail-accounts.nix)
         (import ../programs/mbsync-accounts.nix)
         (import ../programs/msmtp-accounts.nix)
         (import ../programs/notmuch-accounts.nix)

modules/modules.nix

@@ -51,6 +51,7 @@ let
     (loadModule ./programs/firefox.nix { })
     (loadModule ./programs/fish.nix { })
     (loadModule ./programs/fzf.nix { })
+    (loadModule ./programs/getmail.nix { })
     (loadModule ./programs/git.nix { })
     (loadModule ./programs/gnome-terminal.nix { })
     (loadModule ./programs/go.nix { })
@@ -99,7 +100,7 @@ let
     (loadModule ./services/gnome-keyring.nix { })
     (loadModule ./services/gpg-agent.nix { })
     (loadModule ./services/imapnotify.nix { condition = hostPlatform.isLinux; })
-    (loadModule ./services/getmail.nix { })
+    (loadModule ./services/getmail.nix { condition = hostPlatform.isLinux; })
     (loadModule ./services/kbfs.nix { })
     (loadModule ./services/kdeconnect.nix { })
     (loadModule ./services/keepassx.nix { })

modules/programs/getmail-accounts.nix

@@ -0,0 +1,49 @@
+{ config, lib, ... }:
+
+with lib;
+
+{
+  options.getmail = {
+    enable = mkEnableOption "the getmail mail retriever for this account";
+
+    destinationCommand = mkOption {
+      type = types.nullOr types.str;
+      default = null;
+      example = "\${pkgs.maildrop}/bin/maildrop";
+      description = ''
+        Specify a command delivering the incoming mail to your maildir.
+      '';
+    };
+
+    mailboxes = mkOption {
+      type = types.nonEmptyListOf types.str;
+      default = [];
+      example = ["INBOX" "INBOX.spam"];
+      description = ''
+        A non-empty list of mailboxes. To download all mail you can
+        use the <literal>ALL</literal> mailbox.
+      '';
+    };
+
+    delete = mkOption {
+      type = types.bool;
+      default = false;
+      description = ''
+        Enable if you want to delete read messages from the server. Most
+        users should either enable <literal>delete</literal> or disable
+        <literal>readAll</literal>.
+      '';
+    };
+
+    readAll = mkOption {
+      type = types.bool;
+      default = true;
+      description = ''
+        Enable if you want to fetch all, even the read messages from the
+        server. Most users should either enable <literal>delete</literal> or
+        disable <literal>readAll</literal>.
+      '';
+    };
+
+  };
+}

modules/programs/getmail.nix

@@ -0,0 +1,59 @@
+{ config, lib, pkgs, ... }:
+
+with lib;
+
+let
+
+  accounts = filter (a: a.getmail.enable)
+    (attrValues config.accounts.email.accounts);
+
+  renderAccountConfig = account: with account;
+    let
+      passCmd = concatMapStringsSep ", " (x: "'${x}'") passwordCommand;
+      renderedMailboxes = concatMapStringsSep ", " (x: "'${x}'") getmail.mailboxes;
+      retrieverType = if imap.tls.enable
+        then "SimpleIMAPSSLRetriever"
+        else "SimpleIMAPRetriever";
+      destination = if getmail.destinationCommand != null
+        then
+        {
+          destinationType = "MDA_external";
+          destinationPath = getmail.destinationCommand;
+        }
+        else
+        {
+          destinationType = "Maildir";
+          destinationPath = "${maildir.absPath}/";
+        };
+      renderGetmailBoolean = v: if v then "true" else "false";
+    in ''
+      # Generated by Home-Manager.
+      [retriever]
+      type = ${retrieverType}
+      server = ${imap.host}
+      username = ${userName}
+      password_command = (${passCmd})
+      mailboxes = ( ${renderedMailboxes} )
+
+      [destination]
+      type = ${destination.destinationType}
+      path = ${destination.destinationPath}
+
+      [options]
+      delete = ${renderGetmailBoolean getmail.delete}
+      read_all = ${renderGetmailBoolean getmail.readAll}
+    '';
+  getmailEnabled = length (filter (a: a.getmail.enable) accounts) > 0;
+  # Watch out! This is used by the getmail.service too!
+  renderConfigFilepath = a: ".getmail/getmail${if a.primary then "rc" else a.name}";
+in
+
+  {
+    config = mkIf getmailEnabled {
+      home.file = map (a:
+        { target = renderConfigFilepath a;
+          text = renderAccountConfig a;
+        }) accounts;
+
+    };
+  }

modules/services/getmail.nix

@@ -4,188 +4,43 @@ with lib;
 
 let
 
-  cfg = config.programs.getmail;
+  cfg = config.services.getmail;
 
-  retrieverModule = types.submodule ({config,...}: {
+  accounts = filter (a: a.getmail.enable)
-    options = {
+    (attrValues config.accounts.email.accounts);
-      type = mkOption {
-        type = types.enum [
-          "SimplePOP3Retriever"
-          "SimplePOP3SSLRetriever"
-          "SimpleIMAPRetriever"
-          "SimpleIMAPSSLRetriever"
-        ];
-        default = "SimpleIMAPSSLRetriever";
-        description = "Type of the retriever.";
-      };
-
-      server = mkOption {
-        type = types.string;
-        default = "";
-        description = "The remote server.";
-      };
-
-      username = mkOption {
-        type = types.string;
-        default = "";
-        description = "The server username.";
-      };
-
-      password = mkOption {
-        type = types.nullOr types.string;
-        default = null;
-        description = ''
-          The server password. Note that the passwords are stored clear in the
-          nix store, so it is recommended to not use this field, but instead
-          either leave empty or use <literal>passwordCommand</literal> instead.
-        '';
-      };
-
-      passwordCommand = mkOption {
-        type = types.nullOr (types.listOf types.string);
-        default = null;
-        example = ["${pkgs.gnupg}/bin/gpg" "--decrypt" "file.gpg"];
-        description = ''
-          The server password. With this the password is retrieved with the
-          given command. The list value is given escaped to the implementation.
-        '';
-      };
-
-      mailboxes = mkOption {
-        type = types.listOf types.string;
-        default = [];
-        description = "A list of mailboxes";
-      };
-    };
-  });
-
-  destinationModule = types.submodule ({config,...}: {
-    options = {
-      type = mkOption {
-        type = types.enum [
-          "MDA_external"
-          "Maildir"
-        ];
-        default = "Maildir";
-        description = "Destination type.";
-      };
-
-      path = mkOption {
-        type = types.string;
-        default = "$HOME/Mail";
-        example = "${pkgs.procmail}/bin/procmail";
-        description = ''
-          The destination path. For <literal>Maildir</literal> it's the file
-          path and for <literal>MDA_external</literal> it's the destination
-          application.
-        '';
-      };
-    };
-  });
-
-  optionsModule = types.submodule ({config,...}: {
-    options = {
-      delete = mkOption {
-        type = types.bool;
-        default = false;
-        description = ''
-          Enable if you want to delete read messages from the server. Most
-          users should either enable <literal>delete</literal> or disable
-          <literal>readAll</literal>.
-        '';
-      };
-
-      readAll = mkOption {
-        type = types.bool;
-        default = true;
-        description = ''
-          Enable if you want to fetch all, even the read messages from the
-          server. Most users should either enable <literal>delete</literal> or
-          disable <literal>readAll</literal>.
-        '';
-      };
-    };
-  });
 
+  # Note: The getmail service does not expect a path, but just the filename!
+  renderConfigFilepath = a: if a.primary then "getmailrc" else "getmail${a.name}";
+  configFiles = concatMapStringsSep " " (a: " --rcfile ${renderConfigFilepath a}") accounts;
 in
-
 {
   options = {
-    programs.getmail = {
+    services.getmail = {
-      enable = mkEnableOption "Enable getmail";
+      enable = mkEnableOption "the getmail systemd service to automatically retrieve mail";
-
-      retriever = mkOption {
-        type = retrieverModule;
-        default = {};
-        description = "The server section.";
-      };
-
-      destination = mkOption {
-        type = destinationModule;
-        default = {};
-        description = "The destination section.";
-      };
-
-      options = mkOption {
-        type = optionsModule;
-        default = {};
-        description = "The options section.";
-      };
 
       frequency = mkOption {
-        type = types.string;
+        type = types.str;
-        default = "*:0/15";
+        default = "*:0/5";
         example = "hourly";
         description = ''
           The refresh frequency. Check <literal>man systemd.time</literal> for
-          more information on the syntax.
+          more information on the syntax. If you use a gpg-agent in
+          combination with the passwordCommand, keep the poll
+          frequency below the cache-ttl value (as set by the
+          <literal>default</literal>) to avoid pinentry asking
+          permanently for a password.
         '';
       };
     };
   };
 
   config = mkIf cfg.enable {
-    home.file.".getmail/getmailrc".text =
-    let
-      quoted = x: "\"${escape ["\""] x}\"";
-
-      passwordCommand = concatStringsSep ", " (map quoted cfg.retriever.passwordCommand);
-
-      password = if cfg.retriever.passwordCommand != null
-        then "password_command = (${passwordCommand})"
-        else optionalString (cfg.retriever.password != null) "password = \"${quoted cfg.retriever.password}\"";
-      mailboxInner = concatStringsSep ", " (
-        map quoted cfg.retriever.mailboxes);
-
-      mailboxes = "(${mailboxInner})";
-
-      in
-
-    ''
-      [retriever]
-      type = ${cfg.retriever.type}
-      server = ${cfg.retriever.server}
-      username = ${cfg.retriever.username}
-      ${password}
-      mailboxes = ${mailboxes}
-
-      [destination]
-      type = ${cfg.destination.type}
-      path = ${cfg.destination.path}
-
-      [options]
-      delete = ${toString cfg.options.delete}
-      read_all = ${toString cfg.options.readAll}
-    '';
-
     systemd.user.services.getmail = {
       Unit = {
         Description = "getmail email fetcher";
-        PartOf = ["network-online.target"];
       };
       Service = {
-        Type = "simple";
+        ExecStart = "${pkgs.getmail}/bin/getmail ${configFiles}";
-        ExecStart = "${pkgs.getmail}/bin/getmail";
       };
     };
 
@@ -201,5 +56,6 @@ in
         WantedBy = [ "timers.target" ];
       };
     };
+
   };
 }

tests/default.nix

@@ -25,6 +25,7 @@ import nmt {
     git-with-most-options = ./modules/programs/git.nix;
     git-with-str-extra-config = ./modules/programs/git-with-str-extra-config.nix;
     mbsync = ./modules/programs/mbsync.nix;
+    getmail = ./modules/programs/getmail.nix;
     texlive-minimal = ./modules/programs/texlive-minimal.nix;
     xresources = ./modules/xresources.nix;
   }

tests/modules/programs/getmail-expected.conf

@@ -0,0 +1,15 @@
+# Generated by Home-Manager.
+[retriever]
+type = SimpleIMAPSSLRetriever
+server = imap.example.com
+username = home.manager
+password_command = ('password-command')
+mailboxes = ( 'INBOX', 'Sent', 'Work' )
+
+[destination]
+type = MDA_external
+path = /bin/maildrop
+
+[options]
+delete = false
+read_all = true

tests/modules/programs/getmail.nix

@@ -0,0 +1,26 @@
+{ config, lib, pkgs, ... }:
+
+with lib;
+
+{
+  imports = [ ../accounts/email-test-accounts.nix ];
+
+  config = {
+    home.username = "hm-user";
+    home.homeDirectory = "/home/hm-user";
+
+    accounts.email.accounts = {
+      "hm@example.com".getmail = {
+        enable = true;
+        mailboxes = ["INBOX" "Sent" "Work"];
+        destinationCommand = "/bin/maildrop";
+        delete = false;
+      };
+    };
+
+    nmt.script = ''
+      assertFileExists home-files/.getmail/getmailhm@example.com
+      assertFileContent home-files/.getmail/getmailhm@example.com ${./getmail-expected.conf}
+    '';
+  };
+}