[PATCH 4/4] Support reading inboxes from ~/.public-inbox/config

[Kyle Meyer <kyle@kyleam.com>]

A client may mirror and configure inboxes locally.  Doing so enables
fast local access to public-inbox-httpd and public-inbox-nntpd.  And
with the next pubic-inbox release (v1.7), it will be necessary to set
up local externals for lei.

That can lead to a good amount of information being duplicated between
the piem-inboxes option and ~/.public-inbox/config.  To avoid this,
let users set an option to enable collecting information from
public-inbox's configuration.

This relies on code getting the list of inboxes with
piem-merged-inboxes rather than inspecting piem-inboxes directly.
That should be okay because at this point there should be very few
third-party callers.  An alternative would be to merge values from the
configuration into the value of piem-inboxes.  That'd let callers
continue to inspect public-inboxes, but I'd prefer not to touch the
value of a user option.
---

    Xinglu, just a heads up: it looks like git-email-piem.el uses
    piem-inboxes.  That will continue to work after this patch, but
    it'd be good to eventually switch those spots over to calling
    piem-merged-inboxes.

 Documentation/piem.texi   |  46 +++++++++++
 piem-b4.el                |  12 +--
 piem-notmuch.el           |  10 ++-
 piem.el                   | 164 ++++++++++++++++++++++++++++++++++----
 tests/piem-rmail-tests.el |   6 +-
 tests/piem-tests.el       |  70 +++++++++++++++-
 6 files changed, 279 insertions(+), 29 deletions(-)

diff --git a/Documentation/piem.texi b/Documentation/piem.texi
index 3e3ccf9e..73d277ea 100644
--- a/Documentation/piem.texi
+++ b/Documentation/piem.texi
@@ -165,6 +165,52 @@ Registering inboxes
 information is required to apply patches from an archive to a local code
 repository (@pxref{Applying patches}).
 
+@findex piem-merged-inboxes
+@vindex piem-get-inboxes-from-config
+If you mirror some inboxes locally (e.g., for fast local access or for
+use with lei), you don't need to duplicate the information from your
+public-inbox configuration
+(@url{https://public-inbox.org/public-inbox-config.txt,public-inbox-config(5)}).
+When the option @code{piem-get-inboxes-from-config} is non-nil, the
+function @code{piem-merged-inboxes}, which all code should use for
+accessing the registered inboxes, returns a combined set of inboxes
+derived from @code{piem-inboxes} and public-inbox's configuration.
+Merging is done at the level of inbox properties (e.g., an inbox's URL
+may be defined in @code{piem-inboxes} and the inbox's address in
+public-inbox's configuration).  When a value is defined in both sources,
+the one in @code{piem-inboxes} takes precedence.
+
+Properties described for @code{piem-inboxes} are constructed by mapping
+
+@example
+[publicinbox $inbox]
+$name = $value
+@end example
+
+@noindent
+to
+
+@example
+  ($inbox :$name $value ...)
+@end example
+
+@noindent
+The one exception is @code{:coderepo}.  In public-inbox's configuration,
+the value of @code{publicinbox.$inbox.coderepo} points to another
+configuration option, @code{coderepo.$value.dir}, which in turn points
+to a repository's git directory.  The @code{:coderepo} of
+@code{piem-inboxes}, however, should be set to the @emph{working tree},
+so @code{:coderepo} is derived from the value of
+@code{coderepo.$value.dir}, stripping a trailing @file{/.git} if
+present.
+
+@findex piem-clear-merged-inboxes
+Note that @code{piem-merged-inboxes} reads from the public-inbox
+configuration once, generates the merged set of inboxes, and then caches
+the result.  If you change @code{piem-inboxes} outside the customize
+interface or change public-inbox's configuration, you need to call the
+command @code{piem-clear-merged-inboxes} to clear the cache.
+
 @node Enabling integration libraries
 @section Enabling integration libraries
 @findex piem-elfeed-mode
diff --git a/piem-b4.el b/piem-b4.el
index ddb18d52..5e0a6640 100644
--- a/piem-b4.el
+++ b/piem-b4.el
@@ -69,8 +69,8 @@ (defun piem-b4--get-am-files (mid coderepo args)
         (unless (= (point-max) 1)
           (setq local-mbox-p t))))
     ;; `piem-mid-to-thread-functions' didn't generate an mbox.  Next
-    ;; try to download it from a URL at `piem-inboxes'.  Finally, fall
-    ;; back to b4's configuration.
+    ;; try to download it from an inbox's URL.  Finally, fall back to
+    ;; b4's configuration.
     (unless local-mbox-p
       (when-let ((url (and (equal mid (piem-mid))
                            (piem-inbox-get :url))))
@@ -133,10 +133,10 @@ (defun piem-b4-am-from-mid (mid &optional args toggle-worktree)
 
 Try to generate a thread for the Message-Id MID with
 `piem-mid-to-thread-functions'.  If that fails, try to download
-the thread from the `piem-inboxes' URL associated with the
-current buffer, provided that the current buffer's message ID
-matches MID.  And if that doesn't work, let `b4 am' download the
-thread according to its own configuration.
+the thread from an inbox URL associated with the current buffer,
+provided that the current buffer's message ID matches MID.  And
+if that doesn't work, let `b4 am' download the thread according
+to its own configuration.
 
 After calling `b4 am' with ARGS to prepare an am-ready mbox, feed
 the result to `git am'.
diff --git a/piem-notmuch.el b/piem-notmuch.el
index 2a5c525f..1da554a4 100644
--- a/piem-notmuch.el
+++ b/piem-notmuch.el
@@ -108,7 +108,8 @@ (defun piem-notmuch-am-ready-mbox ()
 
 (defun piem-notmuch-show-get-public-inbox-link (mid)
   "Given the message-id MID, return the public-inbox url.
-This will lookup the url in the `piem-inboxes' variable."
+This will lookup the url in the inboxes returned by
+`piem-merged-inboxes'."
   (piem-mid-url mid
                 (or (piem-notmuch-get-inbox)
                     (user-error "No inbox associated with current buffer"))))
@@ -123,9 +124,10 @@ (define-minor-mode piem-notmuch-mode
 
 This will add a new entry to
 `notmuch-show-stash-mlarchive-link-alist' which will determine
-the archive url by reading the `piem-inboxes' variable.  You can
-also set `notmuch-show-stash-mlarchive-link-default' to \"piem\"
-to make this the default behavior when calling
+the archive url by searching the inboxes returned by
+`piem-merged-inboxes'.  You can also set
+`notmuch-show-stash-mlarchive-link-default' to \"piem\" to make
+this the default behavior when calling
 `notmuch-show-stash-mlarchive-link'."
   :global t
   :init-value nil
diff --git a/piem.el b/piem.el
index cf176bdb..0c5f9cf3 100644
--- a/piem.el
+++ b/piem.el
@@ -58,10 +58,6 @@ (defgroup piem ()
   :link '(info-link "(piem)Top")
   :group 'tools)
 
-;; TODO: These intentionally follow public-inbox's configuration
-;; names.  Eventually reading values from there as well should be
-;; supported.
-;;
 ;; TODO: Decide how to deal with inboxes that map to more than one
 ;; coderepo.  This is important to support for people that want to
 ;; use a catchall inbox for small projects which they don't think
@@ -93,10 +89,40 @@ (defcustom piem-inboxes nil
      :address \"meta@public-inbox.org\"
      :listid \"meta.public-inbox.org\"
      :url \"https://public-inbox.org/meta/\"
-     :maildir \"~/.local/share/mail/.lists.mail.public-inbox\")"
+     :maildir \"~/.local/share/mail/.lists.mail.public-inbox\")
+
+Inboxes may also be fully or partially defined via public-inbox
+configuration when `piem-get-inboxes-from-config' is non-nil.
+Prefer the function `piem-merged-inboxes' over inspecting
+`piem-inboxes' directly so that values from the public-inbox
+configuration are considered."
+  ;; Note: When adding a property to the list above, update
+  ;; `piem--merge-config-inboxes' so that the value can also be set in
+  ;; ~/.public-inbox/config.
   :type '(alist :key-type string
                 :value-type
-                (plist :value-type string)))
+                (plist :value-type string))
+  :set (lambda (var val)
+         (set var val)
+         (when (fboundp 'piem-clear-merged-inboxes)
+           (piem-clear-merged-inboxes))))
+
+(defcustom piem-get-inboxes-from-config nil
+  "Whether to construct inboxes from public-inbox's configuration.
+
+If any inboxes are mirrored locally, some of the information in
+the option `piem-inboxes' may already be present in
+~/.public-inbox/config.  When this option is non-nil, the
+function `piem-merged-inboxes' combines these two sources, with
+values from `piem-inboxes' taking precedence.  For details, see
+Info node `(piem) Registering inboxes'.
+
+If you change the option `piem-inboxes' outside of the
+`customize' interface or change public-inbox's configuration, the
+merged representation can be updated by calling
+`piem-clear-merged-inboxes'."
+  :package-version '(piem . "0.3.0")
+  :type 'boolean)
 
 (defcustom piem-get-inbox-functions nil
   "Functions tried to get the inbox of the current buffer.
@@ -315,6 +341,110 @@ (defun piem-process-call-with-buffer-input
                      (list (format "%s call in %s failed"
                                    program default-directory))))))))))
 
+\f
+;;;; Integration with public-inbox configuration
+
+(defun piem--git-config-list (&optional file)
+  "Return a hash table that maps git-config keys to values.
+
+The value for each key is a list in the order that would be
+reported by `git config --get-all $key'.  The first value has the
+highest precedence (i.e. what would be reported by `git config
+--get $key').
+
+If FILE is non-nil, report configuration values from that file.
+Otherwise report values from all standard Git configuration
+files."
+  (with-temp-buffer
+    (unless (= 0 (apply #'call-process piem-git-executable nil '(t nil) nil
+                        "config" "--list" "-z"
+                        (append (and file (list "--file" file)))))
+      (error "git-config call failed"))
+    (goto-char (point-min))
+    (let ((cfg (make-hash-table :test #'equal)))
+      (while (not (eobp))
+        (let* ((key-end (line-end-position))
+               (key (buffer-substring (point) key-end))
+               (value (progn (skip-chars-forward "^\0")
+                             (buffer-substring (1+ key-end) (point)))))
+          (puthash key (cons value (gethash key cfg)) cfg)
+          (forward-char 1)))
+      cfg)))
+
+(defvar piem--inboxes 'unset)
+
+(defun piem--merge-config-inboxes ()
+  (let ((cfg-file (or (getenv "PI_CONFIG")
+                      (expand-file-name "~/.public-inbox/config"))))
+    (if (not (file-readable-p cfg-file))
+        (setq piem--inboxes piem-inboxes)
+      (let ((case-fold-search t)
+            (pi-cfg (piem--git-config-list cfg-file))
+            cfg-inboxes)
+        (maphash
+         (lambda (key val)
+           (when (string-match
+                  (rx string-start "publicinbox."
+                      (group (one-or-more not-newline)) "."
+                      (group
+                       (or "address" "coderepo" "listid" "maildir" "url"))
+                      string-end)
+                  key)
+             (let* ((inbox-name (match-string 1 key))
+                    (inbox-item (assoc inbox-name cfg-inboxes))
+                    (prop-name (intern (concat ":" (match-string 2 key))))
+                    (prop-pair (list prop-name (car val))))
+               (when-let ((coderepo
+                           (and (eq prop-name :coderepo)
+                                (car (gethash
+                                      (format "coderepo.%s.dir" (car val))
+                                      pi-cfg)))))
+                 (setq prop-pair
+                       (list :coderepo
+                             (replace-regexp-in-string
+                              "/\\.git/?\\'" "" coderepo))))
+               (if inbox-item
+                   (setcdr inbox-item (nconc prop-pair (cdr inbox-item)))
+                 (push (cons inbox-name prop-pair) cfg-inboxes)))))
+         pi-cfg)
+        (let (merged)
+          (dolist (name (delete-dups
+                         (mapcar #'car (append cfg-inboxes piem-inboxes))))
+            (push (append (list name)
+                          (alist-get name piem-inboxes nil nil #'equal)
+                          (alist-get name cfg-inboxes nil nil #'equal))
+                  merged))
+          (setq piem--inboxes merged))))))
+
+(defun piem-merged-inboxes ()
+  "Return list of inboxes.
+
+This list has the same form as described in the option
+`piem-inboxes'.
+
+If `piem-get-inboxes-from-config' is non-nil, the return value is
+constructed by merging inboxes defined in public-inbox's
+configuration with the inboxes from `piem-inboxes', with the
+values in the latter taking precedence.  If
+`piem-get-inboxes-from-config' is nil, this value matches
+`piem-inboxes'."
+  (if (not piem-get-inboxes-from-config)
+      piem-inboxes
+    (when (eq piem--inboxes 'unset)
+      (piem--merge-config-inboxes))
+    piem--inboxes))
+
+(defun piem-clear-merged-inboxes ()
+  "Clear inboxes cached by `piem-merged-inboxes'.
+When `piem-get-inboxes-from-config' is set to a non-nil value,
+`piem-merged-inboxes' constructs a set of inboxes by merging
+`piem-inboxes' and public-inbox's configuration and then caches
+the result.  If you change `piem-inboxes' outside of the
+customize interface or change public-inbox's configuration, call
+this command to clear the cached value."
+  (interactive)
+  (setq piem--inboxes 'unset))
+
 \f
 ;;;; Extractors
 
@@ -353,7 +483,7 @@ (defun piem-inbox-by-header-match ()
   (pcase-let ((`(,listid ,to ,cc)
                (piem--message-fetch-decoded-fields '("list-id" "to" "cc"))))
     (catch 'hit
-      (dolist (inbox piem-inboxes)
+      (dolist (inbox (piem-merged-inboxes))
         (let* ((info (cdr inbox))
                (p-listid (plist-get info :listid)))
           (when (and listid
@@ -372,10 +502,12 @@ (defun piem-inbox ()
   (run-hook-with-args-until-success 'piem-get-inbox-functions))
 
 (defun piem-inbox-get (key &optional inbox)
-  "Get info KEY for INBOX's entry in `piem-inboxes'.
-If INBOX is nil, use the inbox returned by `piem-inbox'."
+  "Return value of KEY associated with INBOX.
+The key-value pair may be defined in `piem-inboxes' or
+public-inbox's configuration.  If INBOX is nil, use the inbox
+returned by `piem-inbox'."
   (when-let ((p (or inbox (piem-inbox))))
-    (plist-get (cdr (assoc p piem-inboxes)) key)))
+    (plist-get (cdr (assoc p (piem-merged-inboxes))) key)))
 
 (defun piem-inbox-coderepo (&optional inbox)
   "Return the code repository of current buffer's inbox."
@@ -383,10 +515,11 @@ (defun piem-inbox-coderepo (&optional inbox)
     (file-name-as-directory (expand-file-name repo))))
 
 (defun piem-inbox-maildir-directory (&optional inbox)
-  "Return the maildir for INBOX's entry in `piem-inboxes'.
+  "Return the maildir for INBOX.
 
 If INBOX is nil, use the inbox returned by `piem-inbox'.  If the
-INBOX doesn't have a maildir configured, return the value of
+INBOX doesn't have a maildir configured (via `piem-inboxes' or
+public-inbox's configuration), return the value of
 `piem-maildir-directory'."
   (or (piem-inbox-get :maildir inbox)
       piem-maildir-directory))
@@ -395,7 +528,7 @@ (defun piem-inbox-by-url-match (url)
   "Return inbox based on matching URL against `:url'."
   (setq url (piem--ensure-trailing-slash url))
   (catch 'hit
-    (dolist (inbox piem-inboxes)
+    (dolist (inbox (piem-merged-inboxes))
       (when-let ((info (cdr inbox))
                  (p-url (plist-get info :url)))
         (setq p-url (piem--ensure-trailing-slash p-url))
@@ -500,8 +633,9 @@ (defun piem-escape-mid (mid)
 
 (defun piem-mid-url (mid &optional inbox)
   "Return a public-inbox URL for MID.
-The URL is determined by INBOX's entry in `piem-inboxes'.  If
-INBOX is nil, use the inbox returned by `piem-inbox'."
+The URL for INBOX may be defined in `piem-inboxes' or
+public-inbox's configuration.  If INBOX is nil, use the inbox
+returned by `piem-inbox'."
   (concat
    (piem--ensure-trailing-slash
     (or (piem-inbox-get :url inbox)
diff --git a/tests/piem-rmail-tests.el b/tests/piem-rmail-tests.el
index b4895941..dbf655fc 100644
--- a/tests/piem-rmail-tests.el
+++ b/tests/piem-rmail-tests.el
@@ -53,7 +53,8 @@ (ert-deftest piem-rmail-get-inbox ()
           (with-temp-buffer
             (insert piem-rmail-tests-mbox-text)
             (rmail-mode)
-            (let ((piem-inboxes '(("foo" :address "i@inbox.example.com"))))
+            (let ((piem-get-inboxes-from-config nil)
+                  (piem-inboxes '(("foo" :address "i@inbox.example.com"))))
               (piem-rmail-get-inbox))))))
 
 (ert-deftest piem-rmail-get-mid ()
@@ -63,7 +64,8 @@ (ert-deftest piem-rmail-get-mid ()
             (insert piem-rmail-tests-mbox-text)
             (rmail-mode)
             (rmail-first-message)
-            (let ((piem-inboxes '(("foo" :address "i@inbox.example.com"))))
+            (let ((piem-get-inboxes-from-config nil)
+                  (piem-inboxes '(("foo" :address "i@inbox.example.com"))))
               (list (piem-rmail-get-mid)
                     (progn
                       (rmail-next-message 1)
diff --git a/tests/piem-tests.el b/tests/piem-tests.el
index 91beb9a5..490901e3 100644
--- a/tests/piem-tests.el
+++ b/tests/piem-tests.el
@@ -19,11 +19,75 @@
 
 ;;; Code:
 
+(require 'cl-lib)
 (require 'ert)
 (require 'piem)
 (require 'piem-lei-tests)
 (require 'piem-rmail-tests)
 
+(defmacro piem-tests-with-pi-config (content &rest body)
+  "Point public-inbox's configuration to CONTENT and evaluate BODY."
+  (declare (indent 1) (debug t))
+  (let ((temp-file (cl-gensym "temp-file"))
+        (pi-config-orig (cl-gensym "pi-config-org")))
+    `(let ((,temp-file (make-temp-file "piem-tests-" nil nil ,content))
+           (,pi-config-orig (getenv "PI_CONFIG")))
+       (unwind-protect
+           (progn (setenv "PI_CONFIG" ,temp-file)
+                  ,@body)
+         (setenv "PI_CONFIG" ,pi-config-orig)
+         (delete-file ,temp-file)))))
+
+(defvar piem-tests-sample-pi-config "
+[publicinbox \"foo\"]
+        address = foo@example.com
+        url = https://example.com/foo
+        inboxdir = /inboxes/foo
+        coderepo = foo.git
+
+[coderepo \"foo.git\"]
+          dir = /code/foo/.git
+")
+
+(ert-deftest piem-merged-inboxes:from-config-disabled ()
+  (let ((piem-get-inboxes-from-config nil)
+        (piem-inboxes nil))
+    (should-not (piem-merged-inboxes))
+    (should-not (piem-tests-with-pi-config ""
+                  (piem-merged-inboxes)))
+    (should-not (piem-tests-with-pi-config piem-tests-sample-pi-config
+                  (piem-merged-inboxes))))
+  (let ((piem-get-inboxes-from-config nil)
+        (piem-inboxes '(("inbox" :url "inbox-url"))))
+    (should (equal (piem-inbox-get :url "inbox") "inbox-url"))))
+
+(ert-deftest piem-merged-inboxes:from-config ()
+  (piem-clear-merged-inboxes)
+  (let ((piem-get-inboxes-from-config t)
+        (piem-inboxes nil))
+    (piem-tests-with-pi-config piem-tests-sample-pi-config
+      (should (equal (piem-inbox-get :address "foo")
+                     "foo@example.com"))
+      (should (equal (piem-inbox-get :url "foo")
+                     "https://example.com/foo"))
+      (should (equal (piem-inbox-coderepo "foo")
+                     "/code/foo/")))
+    (piem-tests-with-pi-config ""
+      (should (equal (piem-inbox-get :address "foo")
+                     "foo@example.com"))
+      (piem-clear-merged-inboxes)
+      (should-not (piem-inbox-get :address "foo")))))
+
+(ert-deftest piem-merged-inboxes:override-config ()
+  (piem-clear-merged-inboxes)
+  (let ((piem-get-inboxes-from-config t)
+        (piem-inboxes '(("foo" :coderepo "/tmp/override/"))))
+    (piem-tests-with-pi-config piem-tests-sample-pi-config
+      (should (equal (piem-inbox-coderepo "foo")
+                     "/tmp/override/"))
+      (should (equal (piem-inbox-get :address "foo")
+                     "foo@example.com")))))
+
 (ert-deftest piem-message-link-re ()
   (should-not (string-match-p
                (piem-message-link-re "https://example.com/inbox")
@@ -52,7 +116,8 @@ (ert-deftest piem-escape-mid ()
   (should (equal (piem-escape-mid "m/g@id") "m%2Fg@id")))
 
 (ert-deftest piem-mid-url ()
-  (let ((piem-inboxes '(("inbox-a" :url "https://example.com/a/")
+  (let ((piem-get-inboxes-from-config nil)
+        (piem-inboxes '(("inbox-a" :url "https://example.com/a/")
                         ("inbox-b" :url "https://example.com/b/"))))
     (should (equal (piem-mid-url "msg@id" "inbox-a")
                    "https://example.com/a/msg@id"))
@@ -60,7 +125,8 @@ (ert-deftest piem-mid-url ()
                    "https://example.com/b/m%2Fsg@id"))
     (should-error (piem-mid-url "msg@id")
                   :type 'user-error))
-  (let ((piem-inboxes '(("inbox-a"))))
+  (let ((piem-get-inboxes-from-config nil)
+        (piem-inboxes '(("inbox-a"))))
     (should-error (piem-mid-url "msg@id" "inbox-a")
                   :type 'user-error)))
 
-- 
2.32.0