[Joerg Jaspert] Bug#922536: notmuch-emacs: notmuch breaks on directory removal

[Joerg Jaspert] Bug#922536: notmuch-emacs: notmuch breaks on directory removal

[David Bremner]

[-- Attachment #1: Type: text/plain, Size: 157 bytes --]


Seems like reasonable complaint. It should be possible to change to the
maildir root, but probably requires shelling out to notmuch config 
get the value.


[-- Attachment #2: Type: message/rfc822, Size: 6382 bytes --]

From: Joerg Jaspert <joerg@debian.org>
To: Debian Bug Tracking System <submit@bugs.debian.org>
Subject: Bug#922536: notmuch-emacs: notmuch breaks on directory removal
Date: Sun, 17 Feb 2019 21:07:09 +0100
Message-ID: <87bm3auqky.fsf@delenn.ganneff.de>

Package: notmuch-emacs
Version: 0.28-2~bpo9+1
Severity: normal

Dear Maintainer,

*** Reporter, please consider answering these questions, where appropriate ***

   * What led up to the situation?

Open notmuch "from" a random buffer. Keep the notmuch-hello open.

Remove the directory of that buffer (and the buffer too).

Try refreshing notmuch.

Get greeted by:

"apply: Setting current directory: No such file or directory, 
/the/removed/directory/"

   * What outcome did you expect instead?

Notmuch should change its own buffers to a safe dir that always exists.

-- 
bye, Joerg

Re: [Joerg Jaspert] Bug#922536: notmuch-emacs: notmuch breaks on directory removal

[Tomi Ollila]

On Sun, Feb 17 2019, David Bremner wrote:

> Seems like reasonable complaint. It should be possible to change to the
> maildir root, but probably requires shelling out to notmuch config 
> get the value.

Hmm. what does other emacs software do in that case ?

Since OTOH I can think of a situation where I run emacs when located on
a directory. from *scratch* buffer I launch notmuch-hello, then I press
'm' to compose new mail and c-x i (insert-file) ... Now I'd expect
to be located on the same directory instead of my mail buffer is suddenly
cd'd to totally different location (*)

Tomi

(*) that would be comparable annoying like my experience when using
    ultimaker cura: have to run it with 
    $ HOME=$PWD /path/to/cura $PWD/file.stl
    to have it working as i'd expect software to locate input and
    output directories/files (it sure never default to current dir)




>
> From: Joerg Jaspert <joerg@debian.org>
> Subject: Bug#922536: notmuch-emacs: notmuch breaks on directory removal
> To: Debian Bug Tracking System <submit@bugs.debian.org>
> Date: Sun, 17 Feb 2019 21:07:09 +0100
>
> Package: notmuch-emacs
> Version: 0.28-2~bpo9+1
> Severity: normal
>
> Dear Maintainer,
>
> *** Reporter, please consider answering these questions, where appropriate ***
>
>    * What led up to the situation?
>
> Open notmuch "from" a random buffer. Keep the notmuch-hello open.
>
> Remove the directory of that buffer (and the buffer too).
>
> Try refreshing notmuch.
>
> Get greeted by:
>
> "apply: Setting current directory: No such file or directory, 
> /the/removed/directory/"
>
>    * What outcome did you expect instead?
>
> Notmuch should change its own buffers to a safe dir that always exists.
>
> -- 
> bye, Joerg
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> https://notmuchmail.org/mailman/listinfo/notmuch

Re: [Joerg Jaspert] Bug#922536: notmuch-emacs: notmuch breaks on directory removal

[Matt Armstrong]

Tomi Ollila <tomi.ollila@iki.fi> writes:

> On Sun, Feb 17 2019, David Bremner wrote:
>
>> Seems like reasonable complaint. It should be possible to change to
>> the maildir root, but probably requires shelling out to notmuch
>> config get the value.
>
> Hmm. what does other emacs software do in that case ?
>
> Since OTOH I can think of a situation where I run emacs when located
> on a directory. from *scratch* buffer I launch notmuch-hello, then I
> press 'm' to compose new mail and c-x i (insert-file) ... Now I'd
> expect to be located on the same directory instead of my mail buffer
> is suddenly cd'd to totally different location (*)
>
> Tomi
>
> (*) that would be comparable annoying like my experience when using
>     ultimaker cura: have to run it with 
>     $ HOME=$PWD /path/to/cura $PWD/file.stl
>     to have it working as i'd expect software to locate input and
>     output directories/files (it sure never default to current dir)

I think David's original message was not specific about exactly how the
directory might be changed.  There are two separable questions here:

a) should notmuch-lib.el use whatever `default-directory' was in effect
when, say, M-x notmuch was run when running notmuch sub-commands
(i.e. when executing `call-process').

b) if (a) is "no", then in what way should `default-directory' be
changed?

I think (a) is probably "yes" -- it is possible to run M-x notmuch while
under any directory, but it makes little sense to run the notmuch
sub-commands from the same directory.

I think (b) has multiple answers.

1) The `default-directory' could be set to something predictable each
notmuch mode (hello, etc.).

2) The `default-directory' could be let-bound to something reasonable,
like (expand-file-name "~"), around each call to `call-proccess'.

3) Notmuch could limit option (2) to those cases where the current
diretory seems inaccessible.

I think (2) is probably the best option.

Emacs itself sets a bit of a precedent here.  If `default-directory' is
a remote file system (I suppose this is tramp?) then `call-process' runs
the command under "~" instead.

Re: [Joerg Jaspert] Bug#922536: notmuch-emacs: notmuch breaks on directory removal

[Tomi Ollila]

On Wed, Feb 20 2019, Matt Armstrong wrote:

> Tomi Ollila <tomi.ollila@iki.fi> writes:
>
>> On Sun, Feb 17 2019, David Bremner wrote:
>>
>>> Seems like reasonable complaint. It should be possible to change to
>>> the maildir root, but probably requires shelling out to notmuch
>>> config get the value.
>>
>> Hmm. what does other emacs software do in that case ?
>>
>> Since OTOH I can think of a situation where I run emacs when located
>> on a directory. from *scratch* buffer I launch notmuch-hello, then I
>> press 'm' to compose new mail and c-x i (insert-file) ... Now I'd
>> expect to be located on the same directory instead of my mail buffer
>> is suddenly cd'd to totally different location (*)
>>
>> Tomi
>>
>> (*) that would be comparable annoying like my experience when using
>>     ultimaker cura: have to run it with 
>>     $ HOME=$PWD /path/to/cura $PWD/file.stl
>>     to have it working as i'd expect software to locate input and
>>     output directories/files (it sure never default to current dir)
>
> I think David's original message was not specific about exactly how the
> directory might be changed.  There are two separable questions here:
>
> a) should notmuch-lib.el use whatever `default-directory' was in effect
> when, say, M-x notmuch was run when running notmuch sub-commands
> (i.e. when executing `call-process').
>
> b) if (a) is "no", then in what way should `default-directory' be
> changed?
>
> I think (a) is probably "yes" -- it is possible to run M-x notmuch while
> under any directory, but it makes little sense to run the notmuch
> sub-commands from the same directory.
>
> I think (b) has multiple answers.
>
> 1) The `default-directory' could be set to something predictable each
> notmuch mode (hello, etc.).
>
> 2) The `default-directory' could be let-bound to something reasonable,
> like (expand-file-name "~"), around each call to `call-proccess'.

Yes, (2) this seems pretty good alternative (3) is just... 
Is `(expand-file-name` -part needed: for me default-directory seems to
start with ~ whenever I test-start emacs in subdirectory of $HOME (*)

Tomi

(*) Emacs expands ~ to $HOME (the usual case) -- some other software 
(uses the home directory in password database to expand ~
(e.g. in openssh config example ControlPath ~/.ssh/master-%r@%h:%p).

> 3) Notmuch could limit option (2) to those cases where the current
> diretory seems inaccessible.
>
> I think (2) is probably the best option.
>
> Emacs itself sets a bit of a precedent here.  If `default-directory' is
> a remote file system (I suppose this is tramp?) then `call-process' runs
> the command under "~" instead.
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> https://notmuchmail.org/mailman/listinfo/notmuch

Re: [Joerg Jaspert] Bug#922536: notmuch-emacs: notmuch breaks on directory removal

[David Bremner]

Tomi Ollila <tomi.ollila@iki.fi> writes:

> Yes, (2) this seems pretty good alternative (3) is just... 
> Is `(expand-file-name` -part needed: for me default-directory seems to
> start with ~ whenever I test-start emacs in subdirectory of $HOME (*)
>

Not sure I understood what you wrote correctly, but `default-directory`
is expanded (~ is replaced by /home/bremner) for me on Emacs 26.1 /
Debian.

Wrap process creating primitives with safe CWD

[David Bremner]

This a bit more of a change than I was expecting, but there
is nothing very tricky here (except getting the tests for exceptions in emacs working took a bit of effort).

I decided against trying to normalize the use of process-creating
primitives (e.g. to all make-process or whatever) in this series, but
that seems like it would be a reasonable project.

[PATCH 1/9] test/emacs: provide macro test-log-error

[David Bremner]

Because of the way emacs reports errors, a test form can crash and not
change the main buffer. To work around this, capture both signalled
errors and any other messages.
---
 test/test-lib.el | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/test/test-lib.el b/test/test-lib.el
index 32d53736..e12034c3 100644
--- a/test/test-lib.el
+++ b/test/test-lib.el
@@ -159,6 +159,18 @@ running, quit if it terminated."
 	 (lambda (x) `(prog1 ,x (notmuch-post-command)))
 	 body)))
 
+;; Log any signalled error (and other messages) to MESSAGES
+;; Log "COMPLETE" if forms complete without error.
+(defmacro test-log-error (&rest body)
+  `(progn
+     (with-current-buffer "*Messages*"
+       (let ((inhibit-read-only t)) (erase-buffer)))
+     (condition-case err
+       (progn ,@body
+	  (message "COMPLETE"))
+       (t (message "%s" err)))
+     (with-current-buffer "*Messages*" (test-output "MESSAGES"))))
+
 ;; For historical reasons, we hide deleted tags by default in the test
 ;; suite
 (setq notmuch-tag-deleted-formats
-- 
2.33.0

[PATCH 2/9] test/emacs: run notmuch-hello with a nonexisting default dir

[David Bremner]

This replicates the problem reported in Debian bug #922536.
---
 test/T440-emacs-hello.sh | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/test/T440-emacs-hello.sh b/test/T440-emacs-hello.sh
index bafccd1f..9d1e5a9c 100755
--- a/test/T440-emacs-hello.sh
+++ b/test/T440-emacs-hello.sh
@@ -68,4 +68,13 @@ test_emacs '(notmuch-hello)
 notmuch tag -$tag '*'
 test_expect_equal_file $EXPECTED/notmuch-hello-long-names OUTPUT
 
+test_begin_subtest "notmuch-hello with nonexistent CWD"
+test_subtest_known_broken
+test_emacs '
+      (notmuch-hello)
+      (test-log-error
+        (let ((default-directory "/nonexistent"))
+	  (notmuch-hello-update)))'
+test_expect_equal "$(cat MESSAGES)" "COMPLETE"
+
 test_done
-- 
2.33.0

[PATCH 3/9] emacs: wrap process-lines

[David Bremner]

Initially just set the working directory, to avoid (the implicit)
call-process crashing when the default-directory points to a
non-existent location.

Use of a macro here is over-engineering for this change, but the same
change needs to be applied to several other process creation
primitives.
---
 emacs/notmuch-address.el | 2 +-
 emacs/notmuch-draft.el   | 2 +-
 emacs/notmuch-hello.el   | 6 +++---
 emacs/notmuch-lib.el     | 9 +++++++++
 emacs/notmuch-tree.el    | 2 +-
 emacs/notmuch.el         | 2 +-
 6 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/emacs/notmuch-address.el b/emacs/notmuch-address.el
index 9fc13bc5..1a4cdda2 100644
--- a/emacs/notmuch-address.el
+++ b/emacs/notmuch-address.el
@@ -217,7 +217,7 @@ requiring external commands."
       ;; harvest if necessary.
       (notmuch-address-harvest-trigger)))
    (t
-    (process-lines notmuch-address-command original))))
+    (notmuch--process-lines notmuch-address-command original))))
 
 (defun notmuch-address-expand-name ()
   (cond
diff --git a/emacs/notmuch-draft.el b/emacs/notmuch-draft.el
index a68b7d8d..1f156746 100644
--- a/emacs/notmuch-draft.el
+++ b/emacs/notmuch-draft.el
@@ -239,7 +239,7 @@ applied to newly inserted messages)."
 (defun notmuch-draft-resume (id)
   "Resume editing of message with id ID."
   ;; Used by command `notmuch-show-resume-message'.
-  (let* ((tags (process-lines notmuch-command "search" "--output=tags"
+  (let* ((tags (notmuch--process-lines notmuch-command "search" "--output=tags"
 			      "--exclude=false" id))
 	 (draft (equal tags (notmuch-update-tags tags notmuch-draft-tags))))
     (when (or draft
diff --git a/emacs/notmuch-hello.el b/emacs/notmuch-hello.el
index 1e66555b..450cfcfb 100644
--- a/emacs/notmuch-hello.el
+++ b/emacs/notmuch-hello.el
@@ -494,7 +494,7 @@ diagonal."
 		    (widget-get widget :notmuch-search-oldest-first)))))
 
 (defun notmuch-saved-search-count (search)
-  (car (process-lines notmuch-command "count" search)))
+  (car (notmuch--process-lines notmuch-command "count" search)))
 
 (defun notmuch-hello-tags-per-line (widest)
   "Determine how many tags to show per line and how wide they
@@ -746,7 +746,7 @@ Complete list of currently available key bindings:
 		    (list (cons tag
 				(concat "tag:"
 					(notmuch-escape-boolean-term tag))))))
-	     (process-lines notmuch-command "search" "--output=tags" "*")))
+	     (notmuch--process-lines notmuch-command "search" "--output=tags" "*")))
 
 (defun notmuch-hello-insert-header ()
   "Insert the default notmuch-hello header."
@@ -784,7 +784,7 @@ Complete list of currently available key bindings:
 		   :help-echo "Refresh"
 		   (notmuch-hello-nice-number
 		    (string-to-number
-		     (car (process-lines notmuch-command "count")))))
+		     (car (notmuch--process-lines notmuch-command "count")))))
     (widget-insert " messages.\n")))
 
 (defun notmuch-hello-insert-saved-searches ()
diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
index c7bb2091..928286c3 100644
--- a/emacs/notmuch-lib.el
+++ b/emacs/notmuch-lib.el
@@ -860,6 +860,15 @@ You may need to restart Emacs or upgrade your notmuch package."))
       ;; `notmuch-logged-error' does not return.
       ))))
 
+(defmacro notmuch--apply-with-env (func &rest args)
+  `(let ((default-directory "~"))
+     (apply ,func ,@args)))
+
+(defun notmuch--process-lines (program &rest args)
+  "Wrap process-lines, binding DEFAULT-DIRECTORY to a safe
+default"
+  (notmuch--apply-with-env #'process-lines program args))
+
 (defun notmuch-call-notmuch--helper (destination args)
   "Helper for synchronous notmuch invocation commands.
 
diff --git a/emacs/notmuch-tree.el b/emacs/notmuch-tree.el
index 2f508128..49192e95 100644
--- a/emacs/notmuch-tree.el
+++ b/emacs/notmuch-tree.el
@@ -1098,7 +1098,7 @@ the same as for the function notmuch-tree."
 				   (concat " and (" query-context ")"))))
 	 (sort-arg (if oldest-first "--sort=oldest-first" "--sort=newest-first"))
 	 (message-arg (if unthreaded "--unthreaded" "--entire-thread")))
-    (when (equal (car (process-lines notmuch-command "count" search-args)) "0")
+    (when (equal (car (notmuch--process-lines notmuch-command "count" search-args)) "0")
       (setq search-args basic-query))
     (notmuch-tag-clear-cache)
     (let ((proc (notmuch-start-notmuch
diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index 739cb93b..c81155e6 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -935,7 +935,7 @@ See `notmuch-tag' for information on the format of TAG-CHANGES."
 PROMPT is the string to prompt with."
   (let* ((all-tags
 	  (mapcar (lambda (tag) (notmuch-escape-boolean-term tag))
-		  (process-lines notmuch-command "search" "--output=tags" "*")))
+		  (notmuch--process-lines notmuch-command "search" "--output=tags" "*")))
 	 (completions
 	  (append (list "folder:" "path:" "thread:" "id:" "date:" "from:" "to:"
 			"subject:" "attachment:")
-- 
2.33.0

[PATCH 4/9] emacs: wrap call-process-region

[David Bremner]

As with notmuch--process-lines, initial purpose is to provide a safe
binding for default-directory. This is enough to make notmuch-hello
robust against non-existent or corrupt values default-directory, but
probably not other views.
---
 emacs/notmuch-hello.el   |  2 +-
 emacs/notmuch-lib.el     | 10 +++++++++-
 emacs/notmuch-print.el   |  2 +-
 test/T440-emacs-hello.sh |  1 -
 4 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/emacs/notmuch-hello.el b/emacs/notmuch-hello.el
index 450cfcfb..30e6bd8e 100644
--- a/emacs/notmuch-hello.el
+++ b/emacs/notmuch-hello.el
@@ -567,7 +567,7 @@ options will be handled as specified for
 					(or (plist-get options :filter-count)
 					    (plist-get options :filter))))
 	 "\n")))
-    (unless (= (call-process-region (point-min) (point-max) notmuch-command
+    (unless (= (notmuch--call-process-region (point-min) (point-max) notmuch-command
 				    t t nil "count" "--batch") 0)
       (notmuch-logged-error
        "notmuch count --batch failed"
diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
index 928286c3..207ea4c2 100644
--- a/emacs/notmuch-lib.el
+++ b/emacs/notmuch-lib.el
@@ -869,6 +869,14 @@ You may need to restart Emacs or upgrade your notmuch package."))
 default"
   (notmuch--apply-with-env #'process-lines program args))
 
+(defun notmuch--call-process-region (start end program
+					   &optional delete buffer display
+					   &rest args)
+  "Wrap call-process-region, binding DEFAULT-DIRECTORY to a safe
+default"
+  (notmuch--apply-with-env
+   #'call-process-region start end program delete buffer display args))
+
 (defun notmuch-call-notmuch--helper (destination args)
   "Helper for synchronous notmuch invocation commands.
 
@@ -885,7 +893,7 @@ for `call-process'.  ARGS is as described for
     (if (null stdin-string)
 	(apply #'call-process notmuch-command nil destination nil args)
       (insert stdin-string)
-      (apply #'call-process-region (point-min) (point-max)
+      (apply #'notmuch--call-process-region (point-min) (point-max)
 	     notmuch-command t destination nil args))))
 
 (defun notmuch-call-notmuch-process (&rest args)
diff --git a/emacs/notmuch-print.el b/emacs/notmuch-print.el
index d0061499..85fa1f21 100644
--- a/emacs/notmuch-print.el
+++ b/emacs/notmuch-print.el
@@ -48,7 +48,7 @@
   "Pass the contents of the current buffer to 'muttprint'.
 
 Optional OUTPUT allows passing a list of flags to muttprint."
-  (apply #'call-process-region (point-min) (point-max)
+  (apply #'notmuch--call-process-region (point-min) (point-max)
 	 ;; Reads from stdin.
 	 "muttprint"
 	 nil nil nil
diff --git a/test/T440-emacs-hello.sh b/test/T440-emacs-hello.sh
index 9d1e5a9c..a1ed1c2b 100755
--- a/test/T440-emacs-hello.sh
+++ b/test/T440-emacs-hello.sh
@@ -69,7 +69,6 @@ notmuch tag -$tag '*'
 test_expect_equal_file $EXPECTED/notmuch-hello-long-names OUTPUT
 
 test_begin_subtest "notmuch-hello with nonexistent CWD"
-test_subtest_known_broken
 test_emacs '
       (notmuch-hello)
       (test-log-error
-- 
2.33.0

[PATCH 5/9] test/emacs: test for notmuch-search with nonexistent CWD

[David Bremner]

(At least) notmuch-start-notmuch needs to be updated to set a safe
working directory.
---
 test/T310-emacs.sh | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/test/T310-emacs.sh b/test/T310-emacs.sh
index d69d94a3..3434fd0b 100755
--- a/test/T310-emacs.sh
+++ b/test/T310-emacs.sh
@@ -1154,4 +1154,11 @@ This text added by the hook.
 EOF
 test_expect_equal_file EXPECTED OUTPUT
 
+test_begin_subtest "notmuch-search with nonexistent CWD"
+test_subtest_known_broken
+test_emacs '(test-log-error
+	      (let ((default-directory "/nonexistent"))
+	        (notmuch-search "*")))'
+test_expect_equal "$(cat MESSAGES)" "COMPLETE"
+
 test_done
-- 
2.33.0

[PATCH 6/9] emacs: wrap make-process

[David Bremner]

Provide a safe working directory.
---
 emacs/notmuch-crypto.el | 2 +-
 emacs/notmuch-lib.el    | 7 ++++++-
 test/T310-emacs.sh      | 1 -
 3 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/emacs/notmuch-crypto.el b/emacs/notmuch-crypto.el
index de4d9aea..3ffb5654 100644
--- a/emacs/notmuch-crypto.el
+++ b/emacs/notmuch-crypto.el
@@ -224,7 +224,7 @@ corresponding key when the status button is pressed."
 	  (with-current-buffer buffer
 	    (goto-char (point-max))
 	    (insert (format "--- Retrieving key %s:\n" keyid)))
-	  (let ((p (make-process
+	  (let ((p (notmuch--make-process
 		    :name "notmuch GPG key retrieval"
 		    :connection-type 'pipe
 		    :buffer buffer
diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
index 207ea4c2..9693185f 100644
--- a/emacs/notmuch-lib.el
+++ b/emacs/notmuch-lib.el
@@ -869,6 +869,11 @@ You may need to restart Emacs or upgrade your notmuch package."))
 default"
   (notmuch--apply-with-env #'process-lines program args))
 
+(defun notmuch--make-process (&rest args)
+  "Wrap make-process, binding DEFAULT-DIRECTORY to a safe
+default"
+  (notmuch--apply-with-env #'make-process args))
+
 (defun notmuch--call-process-region (start end program
 					   &optional delete buffer display
 					   &rest args)
@@ -950,7 +955,7 @@ status."
   (let* ((command (or (executable-find notmuch-command)
 		      (error "Command not found: %s" notmuch-command)))
 	 (err-buffer (generate-new-buffer " *notmuch-stderr*"))
-	 (proc (make-process
+	 (proc (notmuch--make-process
 		:name name
 		:buffer buffer
 		:command (cons command args)
diff --git a/test/T310-emacs.sh b/test/T310-emacs.sh
index 3434fd0b..fd0ea11d 100755
--- a/test/T310-emacs.sh
+++ b/test/T310-emacs.sh
@@ -1155,7 +1155,6 @@ EOF
 test_expect_equal_file EXPECTED OUTPUT
 
 test_begin_subtest "notmuch-search with nonexistent CWD"
-test_subtest_known_broken
 test_emacs '(test-log-error
 	      (let ((default-directory "/nonexistent"))
 	        (notmuch-search "*")))'
-- 
2.33.0

[PATCH 7/9] test/emacs: test for notmuch-show with nonexistent CWD

[David Bremner]

Somewhat predictably, the other code path in
notmuch-call-notmuch--helper also needs to be fixed.
---
 test/T450-emacs-show.sh | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/test/T450-emacs-show.sh b/test/T450-emacs-show.sh
index a750cc4d..05e3692e 100755
--- a/test/T450-emacs-show.sh
+++ b/test/T450-emacs-show.sh
@@ -230,4 +230,12 @@ test_emacs '(let ((notmuch-crypto-process-mime nil))
              (test-visible-output))'
 test_expect_equal_file $EXPECTED/notmuch-show-decrypted-message-no-crypto OUTPUT
 
+test_begin_subtest "notmuch-show with nonexistent CWD"
+tid=$(notmuch search --limit=1 --output=threads '*' | sed s/thread://)
+test_subtest_known_broken
+test_emacs "(test-log-error
+	      (let ((default-directory \"/nonexistent\"))
+	        (notmuch-show \"$tid\")))"
+test_expect_equal "$(cat MESSAGES)" "COMPLETE"
+
 test_done
-- 
2.33.0

[PATCH 8/9] emacs: wrap call-process

[David Bremner]

Provide a safe working directory
---
 emacs/notmuch-crypto.el |  6 +++---
 emacs/notmuch-draft.el  |  2 +-
 emacs/notmuch-lib.el    | 14 +++++++++-----
 emacs/notmuch-mua.el    |  2 +-
 emacs/notmuch-show.el   |  6 +++---
 emacs/notmuch-tag.el    |  2 +-
 test/T450-emacs-show.sh |  1 -
 7 files changed, 18 insertions(+), 15 deletions(-)

diff --git a/emacs/notmuch-crypto.el b/emacs/notmuch-crypto.el
index 3ffb5654..a1cf3ddd 100644
--- a/emacs/notmuch-crypto.el
+++ b/emacs/notmuch-crypto.el
@@ -164,7 +164,7 @@ mode."
 	(goto-char (point-max))
 	(insert (format "-- Key %s in message %s:\n"
 			fingerprint id))
-	(call-process notmuch-crypto-gpg-program nil t t
+	(notmuch--call-process notmuch-crypto-gpg-program nil t t
 		      "--batch" "--no-tty" "--list-keys" fingerprint))
       (recenter -1))))
 
@@ -240,9 +240,9 @@ corresponding key when the status button is pressed."
 	  (with-current-buffer buffer
 	    (goto-char (point-max))
 	    (insert (format "--- Retrieving key %s:\n" keyid))
-	    (call-process notmuch-crypto-gpg-program nil t t "--recv-keys" keyid)
+	    (notmuch--call-process notmuch-crypto-gpg-program nil t t "--recv-keys" keyid)
 	    (insert "\n")
-	    (call-process notmuch-crypto-gpg-program nil t t "--list-keys" keyid))
+	    (notmuch--call-process notmuch-crypto-gpg-program nil t t "--list-keys" keyid))
 	  (recenter -1))
 	(notmuch-show-refresh-view)))))
 
diff --git a/emacs/notmuch-draft.el b/emacs/notmuch-draft.el
index 1f156746..d87a4805 100644
--- a/emacs/notmuch-draft.el
+++ b/emacs/notmuch-draft.el
@@ -249,7 +249,7 @@ applied to newly inserted messages)."
       (setq buffer-read-only nil)
       (erase-buffer)
       (let ((coding-system-for-read 'no-conversion))
-	(call-process notmuch-command nil t nil "show" "--format=raw" id))
+	(notmuch--call-process notmuch-command nil t nil "show" "--format=raw" id))
       (mime-to-mml)
       (goto-char (point-min))
       (when (re-search-forward "^$" nil t)
diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
index 9693185f..9afdc165 100644
--- a/emacs/notmuch-lib.el
+++ b/emacs/notmuch-lib.el
@@ -195,7 +195,7 @@ will be signaled.
 
 Otherwise the output will be returned."
   (with-temp-buffer
-    (let ((status (apply #'call-process notmuch-command nil t nil args))
+    (let ((status (apply #'notmuch--call-process notmuch-command nil t nil args))
 	  (output (buffer-string)))
       (notmuch-check-exit-status status (cons notmuch-command args) output)
       output)))
@@ -206,7 +206,7 @@ Otherwise the output will be returned."
 (defun notmuch-cli-sane-p ()
   "Return t if the cli seems to be configured sanely."
   (unless notmuch--cli-sane-p
-    (let ((status (call-process notmuch-command nil nil nil
+    (let ((status (notmuch--call-process notmuch-command nil nil nil
 				"config" "get" "user.primary_email")))
       (setq notmuch--cli-sane-p (= status 0))))
   notmuch--cli-sane-p)
@@ -286,7 +286,7 @@ depending on the value of `notmuch-poll-script'."
   (message "Polling mail...")
   (if (stringp notmuch-poll-script)
       (unless (string-empty-p notmuch-poll-script)
-	(unless (equal (call-process notmuch-poll-script nil nil) 0)
+	(unless (equal (notmuch--call-process notmuch-poll-script nil nil) 0)
 	  (error "Notmuch: poll script `%s' failed!" notmuch-poll-script)))
     (notmuch-call-notmuch-process "new"))
   (message "Polling mail...done"))
@@ -639,7 +639,7 @@ the given type."
 				  ;; charset is US-ASCII. RFC6657
 				  ;; complicates this somewhat.
 				  'us-ascii)))))
-		       (apply #'call-process
+		       (apply #'notmuch--call-process
 			      notmuch-command nil '(t nil) nil args)
 		       (buffer-string))))))
     (when (and cache data)
@@ -882,6 +882,10 @@ default"
   (notmuch--apply-with-env
    #'call-process-region start end program delete buffer display args))
 
+(defun notmuch--call-process (program &optional infile destination display &rest args)
+  (notmuch--apply-with-env
+   #'call-process program infile destination display args))
+
 (defun notmuch-call-notmuch--helper (destination args)
   "Helper for synchronous notmuch invocation commands.
 
@@ -896,7 +900,7 @@ for `call-process'.  ARGS is as described for
 	(otherwise
 	 (error "Unknown keyword argument: %s" (car args)))))
     (if (null stdin-string)
-	(apply #'call-process notmuch-command nil destination nil args)
+	(apply #'notmuch--call-process notmuch-command nil destination nil args)
       (insert stdin-string)
       (apply #'notmuch--call-process-region (point-min) (point-max)
 	     notmuch-command t destination nil args))))
diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
index f510c043..c679373b 100644
--- a/emacs/notmuch-mua.el
+++ b/emacs/notmuch-mua.el
@@ -474,7 +474,7 @@ the From: address."
 		(with-current-buffer temp-buffer
 		  (erase-buffer)
 		  (let ((coding-system-for-read 'no-conversion))
-		    (call-process notmuch-command nil t nil
+		    (notmuch--call-process notmuch-command nil t nil
 				  "show" "--format=raw" id))
 		  ;; Because we process the messages in reverse order,
 		  ;; always generate a forwarded subject, then use the
diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 0f96c4fe..aea72e74 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -279,7 +279,7 @@ position of the message in the thread."
        (let ((buf (generate-new-buffer (concat "*notmuch-msg-" id "*"))))
 	 (with-current-buffer buf
 	   (let ((coding-system-for-read 'no-conversion))
-	     (call-process notmuch-command nil t nil "show" "--format=raw" id))
+	     (notmuch--call-process notmuch-command nil t nil "show" "--format=raw" id))
 	   ,@body)
 	 (kill-buffer buf)))))
 
@@ -2032,7 +2032,7 @@ to show, nil otherwise."
     (pop-to-buffer-same-window buf)
     (erase-buffer)
     (let ((coding-system-for-read 'no-conversion))
-      (call-process notmuch-command nil t nil "show" "--format=raw" id))
+      (notmuch--call-process notmuch-command nil t nil "show" "--format=raw" id))
     (goto-char (point-min))
     (set-buffer-modified-p nil)
     (setq buffer-read-only t)
@@ -2083,7 +2083,7 @@ message."
 	;; Use the originating buffer's working directory instead of
 	;; that of the pipe buffer.
 	(cd cwd)
-	(let ((exit-code (call-process-shell-command shell-command nil buf)))
+	(let ((exit-code (notmuch--call-process-shell-command shell-command nil buf)))
 	  (goto-char (point-max))
 	  (set-buffer-modified-p nil)
 	  (setq buffer-read-only t)
diff --git a/emacs/notmuch-tag.el b/emacs/notmuch-tag.el
index ebccb5a0..6d9fc2f9 100644
--- a/emacs/notmuch-tag.el
+++ b/emacs/notmuch-tag.el
@@ -397,7 +397,7 @@ Return all tags if no search terms are given."
   (split-string
    (with-output-to-string
      (with-current-buffer standard-output
-       (apply 'call-process notmuch-command nil t
+       (apply 'notmuch--call-process notmuch-command nil t
 	      nil "search" "--output=tags" "--exclude=false" search-terms)))
    "\n+" t))
 
diff --git a/test/T450-emacs-show.sh b/test/T450-emacs-show.sh
index 05e3692e..7cb142fa 100755
--- a/test/T450-emacs-show.sh
+++ b/test/T450-emacs-show.sh
@@ -232,7 +232,6 @@ test_expect_equal_file $EXPECTED/notmuch-show-decrypted-message-no-crypto OUTPUT
 
 test_begin_subtest "notmuch-show with nonexistent CWD"
 tid=$(notmuch search --limit=1 --output=threads '*' | sed s/thread://)
-test_subtest_known_broken
 test_emacs "(test-log-error
 	      (let ((default-directory \"/nonexistent\"))
 	        (notmuch-show \"$tid\")))"
-- 
2.33.0

[PATCH 9/9] test/emacs: tests for notmuch-{tree,unthreaded} with bad CWD

[David Bremner]

These work thanks to the previous wrapping of process creation
primitives.
---
 test/T460-emacs-tree.sh       | 6 ++++++
 test/T465-emacs-unthreaded.sh | 6 ++++++
 2 files changed, 12 insertions(+)

diff --git a/test/T460-emacs-tree.sh b/test/T460-emacs-tree.sh
index 405d7ee7..aaece220 100755
--- a/test/T460-emacs-tree.sh
+++ b/test/T460-emacs-tree.sh
@@ -179,4 +179,10 @@ output=$(test_emacs '(notmuch-tree "tag:inbox")
 		     (notmuch-show-stash-message-id)')
 test_expect_equal "$output" "\"Stashed: id:1258493565-13508-1-git-send-email-keithp@keithp.com\""
 
+test_begin_subtest "notmuch-tree with nonexistent CWD"
+test_emacs '(test-log-error
+	      (let ((default-directory "/nonexistent"))
+	        (notmuch-tree "*")))'
+test_expect_equal "$(cat MESSAGES)" "COMPLETE"
+
 test_done
diff --git a/test/T465-emacs-unthreaded.sh b/test/T465-emacs-unthreaded.sh
index f9a09426..830cefaf 100755
--- a/test/T465-emacs-unthreaded.sh
+++ b/test/T465-emacs-unthreaded.sh
@@ -34,4 +34,10 @@ output=$(test_emacs '(let ((max-lisp-eval-depth 10))
 		       "SUCCESS")' )
 test_expect_equal "$output" '"SUCCESS"'
 
+test_begin_subtest "notmuch-unthreaded with nonexistent CWD"
+test_emacs '(test-log-error
+	      (let ((default-directory "/nonexistent"))
+	        (notmuch-unthreaded "*")))'
+test_expect_equal "$(cat MESSAGES)" "COMPLETE"
+
 test_done
-- 
2.33.0

Re: Wrap process creating primitives with safe CWD

[David Bremner]

David Bremner <david@tethera.net> writes:

> This a bit more of a change than I was expecting, but there
> is nothing very tricky here (except getting the tests for exceptions in emacs working took a bit of effort).
>
> I decided against trying to normalize the use of process-creating
> primitives (e.g. to all make-process or whatever) in this series, but
> that seems like it would be a reasonable project.

I have applied this series to master.

d

Re: [Joerg Jaspert] Bug#922536: notmuch-emacs: notmuch breaks on directory removal

[David Bremner]

David Bremner <david@tethera.net> writes:

> Seems like reasonable complaint. It should be possible to change to the
> maildir root, but probably requires shelling out to notmuch config 
> get the value.

This should be fixed in commit 21e365f51