From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp2 ([2001:41d0:2:863f::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by ms12 with LMTPS id cJf7HzJhwmDXBAAAsNZ9tg (envelope-from ); Thu, 10 Jun 2021 19:00:02 +0000 Received: from out1.migadu.com ([2001:41d0:2:863f::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp2 with LMTPS id +E5RMC5hwmBWCwAAB5/wlQ (envelope-from ); Thu, 10 Jun 2021 18:59:58 +0000 X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kyleam.com; s=key1; t=1623351598; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=zPDglDwZ9jOJcO5XlZXRTqKWslTzL3F58KJtsZtedII=; b=SgqbIl59C0cc7jU4R8Y/H/4ouWLOtYrTiOsTSsktCLcRCPl7xgiNe6mr+s3wXg2asC92hE cM/vfJVwl0xYRNdifqFZa34a2rii6WVN1KN+cwdujuE6gHVGPKOLgzraW6K8pcRph2rlfz gjreYDQGTlwH9C8qu/kmBkWk6+ps83NuEcMVag6JJlkZkk3GlH13PrtGQ6GO0+Yus0MvbS z/Ntao6ZnUl+lPbldh0+IZH1CAuYVdDAJo6b46rhudW11Hxd4W3+jpb5o9KdUbSGiomLr5 XHRSbKhkg5vbwvRj7FPOME51Og66bwUOB2cXh1LKrMLLiyr99y6ILiwiVe4/Mw== From: Kyle Meyer To: piem@inbox.kyleam.com Cc: Xinglu Chen Subject: [PATCH 4/4] Support reading inboxes from ~/.public-inbox/config Date: Thu, 10 Jun 2021 14:59:43 -0400 Message-Id: <20210610185943.14155-5-kyle@kyleam.com> In-Reply-To: <20210610185943.14155-1-kyle@kyleam.com> References: <20210610185943.14155-1-kyle@kyleam.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Migadu-Flow: FLOW_OUT X-Migadu-Auth-User: kyle@kyleam.com X-TUID: e9MkAo0eLzzJ 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)))))))))) + +;;;; 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)) + ;;;; 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