* [PATCH 0/2] Use make-process when available @ 2017-08-17 17:57 Vladimir Panteleev 2017-08-17 17:57 ` [PATCH 1/2] emacs: Refactor subprocess stderr propagation Vladimir Panteleev 2017-08-17 17:57 ` [PATCH 2/2] emacs: Use make-process when available Vladimir Panteleev 0 siblings, 2 replies; 5+ messages in thread From: Vladimir Panteleev @ 2017-08-17 17:57 UTC (permalink / raw) To: notmuch Hopefully that should avoid any issues with the hardcoded /bin/sh path for users of Emacs 25 and newer. Vladimir Panteleev (2): emacs: Refactor subprocess stderr propagation emacs: Use make-process when available emacs/notmuch-lib.el | 105 ++++++++++++++++++++++++++++++++------------------- 1 file changed, 66 insertions(+), 39 deletions(-) -- 2.13.3 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/2] emacs: Refactor subprocess stderr propagation 2017-08-17 17:57 [PATCH 0/2] Use make-process when available Vladimir Panteleev @ 2017-08-17 17:57 ` Vladimir Panteleev 2017-08-19 18:45 ` Tomi Ollila 2017-08-20 12:11 ` David Bremner 2017-08-17 17:57 ` [PATCH 2/2] emacs: Use make-process when available Vladimir Panteleev 1 sibling, 2 replies; 5+ messages in thread From: Vladimir Panteleev @ 2017-08-17 17:57 UTC (permalink / raw) To: notmuch Load subprocess error output to a string in the callers, and propagate the error messages as a string parameter instead of a path to file names. Required to be able to avoid using temporary files for subprocess error output. * notmuch-lib.el: Update notmuch-check-async-exit-status, notmuch-check-exit-status: accept an err parameter instead of err-file; shift the responsibility of loading error messages from files up the call stack. --- emacs/notmuch-lib.el | 47 ++++++++++++++++++++++++++--------------------- 1 file changed, 26 insertions(+), 21 deletions(-) diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el index 337b20ac..1f743eff 100644 --- a/emacs/notmuch-lib.el +++ b/emacs/notmuch-lib.el @@ -768,23 +768,23 @@ signaled error. This function does not return." (error "%s" (concat msg (when extra " (see *Notmuch errors* for more details)")))) -(defun notmuch-check-async-exit-status (proc msg &optional command err-file) +(defun notmuch-check-async-exit-status (proc msg &optional command err) "If PROC exited abnormally, pop up an error buffer and signal an error. This is a wrapper around `notmuch-check-exit-status' for asynchronous process sentinels. PROC and MSG must be the -arguments passed to the sentinel. COMMAND and ERR-FILE, if -provided, are passed to `notmuch-check-exit-status'. If COMMAND -is not provided, it is taken from `process-command'." +arguments passed to the sentinel. COMMAND and ERR, if provided, +are passed to `notmuch-check-exit-status'. If COMMAND is not +provided, it is taken from `process-command'." (let ((exit-status (case (process-status proc) ((exit) (process-exit-status proc)) ((signal) msg)))) (when exit-status (notmuch-check-exit-status exit-status (or command (process-command proc)) - nil err-file)))) + nil err)))) -(defun notmuch-check-exit-status (exit-status command &optional output err-file) +(defun notmuch-check-exit-status (exit-status command &optional output err) "If EXIT-STATUS is non-zero, pop up an error buffer and signal an error. If EXIT-STATUS is non-zero, pop up a notmuch error buffer @@ -793,9 +793,9 @@ be a number indicating the exit status code of a process or a string describing the signal that terminated the process (such as returned by `call-process'). COMMAND must be a list giving the command and its arguments. OUTPUT, if provided, is a string -giving the output of command. ERR-FILE, if provided, is the name -of a file containing the error output of command. OUTPUT and the -contents of ERR-FILE will be included in the error message." +giving the output of command. ERR, if provided, is the error +output of command. OUTPUT and ERR will be included in the error +message." (cond ((eq exit-status 0) t) @@ -808,12 +808,7 @@ You may need to restart Emacs or upgrade your notmuch Emacs package.")) Emacs requested a newer output format than supported by the notmuch CLI. You may need to restart Emacs or upgrade your notmuch package.")) (t - (let* ((err (when err-file - (with-temp-buffer - (insert-file-contents err-file) - (unless (eobp) - (buffer-string))))) - (command-string + (let* ((command-string (mapconcat (lambda (arg) (shell-quote-argument (cond ((stringp arg) arg) @@ -889,9 +884,13 @@ error." (with-temp-buffer (let ((err-file (make-temp-file "nmerr"))) (unwind-protect - (let ((status (notmuch-call-notmuch--helper (list t err-file) args))) + (let ((status (notmuch-call-notmuch--helper (list t err-file) args)) + (err (with-temp-buffer + (insert-file-contents err-file) + (unless (eobp) + (buffer-string))))) (notmuch-check-exit-status status (cons notmuch-command args) - (buffer-string) err-file) + (buffer-string) err) (goto-char (point-min)) (read (current-buffer))) (delete-file err-file))))) @@ -931,9 +930,14 @@ status." proc)) (defun notmuch-start-notmuch-sentinel (proc event) - (let ((err-file (process-get proc 'err-file)) - (sub-sentinel (process-get proc 'sub-sentinel)) - (real-command (process-get proc 'real-command))) + "Process sentinel function used by `notmuch-start-notmuch'." + (let* ((err-file (process-get proc 'err-file)) + (err (with-temp-buffer + (insert-file-contents err-file) + (unless (eobp) + (buffer-string)))) + (sub-sentinel (process-get proc 'sub-sentinel)) + (real-command (process-get proc 'real-command))) (condition-case err (progn ;; Invoke the sub-sentinel, if any @@ -945,12 +949,13 @@ status." ;; and there's no point in telling the user that (but we ;; still check for and report stderr output below). (when (buffer-live-p (process-buffer proc)) - (notmuch-check-async-exit-status proc event real-command err-file)) + (notmuch-check-async-exit-status proc event real-command err)) ;; If that didn't signal an error, then any error output was ;; really warning output. Show warnings, if any. (let ((warnings (with-temp-buffer (unless (= (second (insert-file-contents err-file)) 0) + (goto-char (point-min)) (end-of-line) ;; Show first line; stuff remaining lines in the ;; errors buffer. -- 2.13.3 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] emacs: Refactor subprocess stderr propagation 2017-08-17 17:57 ` [PATCH 1/2] emacs: Refactor subprocess stderr propagation Vladimir Panteleev @ 2017-08-19 18:45 ` Tomi Ollila 2017-08-20 12:11 ` David Bremner 1 sibling, 0 replies; 5+ messages in thread From: Tomi Ollila @ 2017-08-19 18:45 UTC (permalink / raw) To: Vladimir Panteleev, notmuch On Thu, Aug 17 2017, Vladimir Panteleev wrote: > Load subprocess error output to a string in the callers, and propagate > the error messages as a string parameter instead of a path to file > names. > > Required to be able to avoid using temporary files for subprocess > error output. > > * notmuch-lib.el: Update notmuch-check-async-exit-status, > notmuch-check-exit-status: accept an err parameter instead of > err-file; shift the responsibility of loading error messages from > files up the call stack. both changes Look Good To Me, and works on emacs 25.2 (changed notmuch-command to "git" and saw error buffer populated as expected, then M-x debug-on-entry make-process to verify that make-process is actually called) Tomi > --- > emacs/notmuch-lib.el | 47 ++++++++++++++++++++++++++--------------------- > 1 file changed, 26 insertions(+), 21 deletions(-) > > diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el > index 337b20ac..1f743eff 100644 > --- a/emacs/notmuch-lib.el > +++ b/emacs/notmuch-lib.el > @@ -768,23 +768,23 @@ signaled error. This function does not return." > (error "%s" (concat msg (when extra > " (see *Notmuch errors* for more details)")))) > > -(defun notmuch-check-async-exit-status (proc msg &optional command err-file) > +(defun notmuch-check-async-exit-status (proc msg &optional command err) > "If PROC exited abnormally, pop up an error buffer and signal an error. > > This is a wrapper around `notmuch-check-exit-status' for > asynchronous process sentinels. PROC and MSG must be the > -arguments passed to the sentinel. COMMAND and ERR-FILE, if > -provided, are passed to `notmuch-check-exit-status'. If COMMAND > -is not provided, it is taken from `process-command'." > +arguments passed to the sentinel. COMMAND and ERR, if provided, > +are passed to `notmuch-check-exit-status'. If COMMAND is not > +provided, it is taken from `process-command'." > (let ((exit-status > (case (process-status proc) > ((exit) (process-exit-status proc)) > ((signal) msg)))) > (when exit-status > (notmuch-check-exit-status exit-status (or command (process-command proc)) > - nil err-file)))) > + nil err)))) > > -(defun notmuch-check-exit-status (exit-status command &optional output err-file) > +(defun notmuch-check-exit-status (exit-status command &optional output err) > "If EXIT-STATUS is non-zero, pop up an error buffer and signal an error. > > If EXIT-STATUS is non-zero, pop up a notmuch error buffer > @@ -793,9 +793,9 @@ be a number indicating the exit status code of a process or a > string describing the signal that terminated the process (such as > returned by `call-process'). COMMAND must be a list giving the > command and its arguments. OUTPUT, if provided, is a string > -giving the output of command. ERR-FILE, if provided, is the name > -of a file containing the error output of command. OUTPUT and the > -contents of ERR-FILE will be included in the error message." > +giving the output of command. ERR, if provided, is the error > +output of command. OUTPUT and ERR will be included in the error > +message." > > (cond > ((eq exit-status 0) t) > @@ -808,12 +808,7 @@ You may need to restart Emacs or upgrade your notmuch Emacs package.")) > Emacs requested a newer output format than supported by the notmuch CLI. > You may need to restart Emacs or upgrade your notmuch package.")) > (t > - (let* ((err (when err-file > - (with-temp-buffer > - (insert-file-contents err-file) > - (unless (eobp) > - (buffer-string))))) > - (command-string > + (let* ((command-string > (mapconcat (lambda (arg) > (shell-quote-argument > (cond ((stringp arg) arg) > @@ -889,9 +884,13 @@ error." > (with-temp-buffer > (let ((err-file (make-temp-file "nmerr"))) > (unwind-protect > - (let ((status (notmuch-call-notmuch--helper (list t err-file) args))) > + (let ((status (notmuch-call-notmuch--helper (list t err-file) args)) > + (err (with-temp-buffer > + (insert-file-contents err-file) > + (unless (eobp) > + (buffer-string))))) > (notmuch-check-exit-status status (cons notmuch-command args) > - (buffer-string) err-file) > + (buffer-string) err) > (goto-char (point-min)) > (read (current-buffer))) > (delete-file err-file))))) > @@ -931,9 +930,14 @@ status." > proc)) > > (defun notmuch-start-notmuch-sentinel (proc event) > - (let ((err-file (process-get proc 'err-file)) > - (sub-sentinel (process-get proc 'sub-sentinel)) > - (real-command (process-get proc 'real-command))) > + "Process sentinel function used by `notmuch-start-notmuch'." > + (let* ((err-file (process-get proc 'err-file)) > + (err (with-temp-buffer > + (insert-file-contents err-file) > + (unless (eobp) > + (buffer-string)))) > + (sub-sentinel (process-get proc 'sub-sentinel)) > + (real-command (process-get proc 'real-command))) > (condition-case err > (progn > ;; Invoke the sub-sentinel, if any > @@ -945,12 +949,13 @@ status." > ;; and there's no point in telling the user that (but we > ;; still check for and report stderr output below). > (when (buffer-live-p (process-buffer proc)) > - (notmuch-check-async-exit-status proc event real-command err-file)) > + (notmuch-check-async-exit-status proc event real-command err)) > ;; If that didn't signal an error, then any error output was > ;; really warning output. Show warnings, if any. > (let ((warnings > (with-temp-buffer > (unless (= (second (insert-file-contents err-file)) 0) > + (goto-char (point-min)) > (end-of-line) > ;; Show first line; stuff remaining lines in the > ;; errors buffer. > -- > 2.13.3 > > _______________________________________________ > notmuch mailing list > notmuch@notmuchmail.org > https://notmuchmail.org/mailman/listinfo/notmuch ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] emacs: Refactor subprocess stderr propagation 2017-08-17 17:57 ` [PATCH 1/2] emacs: Refactor subprocess stderr propagation Vladimir Panteleev 2017-08-19 18:45 ` Tomi Ollila @ 2017-08-20 12:11 ` David Bremner 1 sibling, 0 replies; 5+ messages in thread From: David Bremner @ 2017-08-20 12:11 UTC (permalink / raw) To: Vladimir Panteleev, notmuch Vladimir Panteleev <notmuch@thecybershadow.net> writes: > Load subprocess error output to a string in the callers, and propagate > the error messages as a string parameter instead of a path to file > names. series pushed d ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 2/2] emacs: Use make-process when available 2017-08-17 17:57 [PATCH 0/2] Use make-process when available Vladimir Panteleev 2017-08-17 17:57 ` [PATCH 1/2] emacs: Refactor subprocess stderr propagation Vladimir Panteleev @ 2017-08-17 17:57 ` Vladimir Panteleev 1 sibling, 0 replies; 5+ messages in thread From: Vladimir Panteleev @ 2017-08-17 17:57 UTC (permalink / raw) To: notmuch make-process is a new function introduced in Emacs 25, which provides greater control over process creation. Crucially, it allows separately redirecting stderr directly to a buffer, which allows us to avoid needing to use the shell to redirect to a temporary file in order to correctly distinguish stdout and stderr. * notmuch-lib.el: Use make-process when it is available; fall back to the previous method when not. --- emacs/notmuch-lib.el | 66 ++++++++++++++++++++++++++++++++++------------------ 1 file changed, 44 insertions(+), 22 deletions(-) diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el index 1f743eff..010be454 100644 --- a/emacs/notmuch-lib.el +++ b/emacs/notmuch-lib.el @@ -909,21 +909,42 @@ invoke `set-process-sentinel' directly on the returned process, as that will interfere with the handling of stderr and the exit status." - ;; There is no way (as of Emacs 24.3) to capture stdout and stderr - ;; separately for asynchronous processes, or even to redirect stderr - ;; to a file, so we use a trivial shell wrapper to send stderr to a - ;; temporary file and clean things up in the sentinel. - (let* ((err-file (make-temp-file "nmerr")) - ;; Use a pipe - (process-connection-type nil) - ;; Find notmuch using Emacs' `exec-path' - (command (or (executable-find notmuch-command) - (error "command not found: %s" notmuch-command))) - (proc (apply #'start-process name buffer - "/bin/sh" "-c" - "exec 2>\"$1\"; shift; exec \"$0\" \"$@\"" - command err-file args))) - (process-put proc 'err-file err-file) + (let (err-file err-buffer proc + ;; Find notmuch using Emacs' `exec-path' + (command (or (executable-find notmuch-command) + (error "Command not found: %s" notmuch-command)))) + (if (fboundp 'make-process) + (progn + (setq err-buffer (generate-new-buffer " *notmuch-stderr*")) + ;; Emacs 25 and newer has `make-process', which allows + ;; redirecting stderr independently from stdout to a + ;; separate buffer. As this allows us to avoid using a + ;; temporary file and shell invocation, use it when + ;; available. + (setq proc (make-process + :name name + :buffer buffer + :command (cons command args) + :connection-type 'pipe + :stderr err-buffer)) + (process-put proc 'err-buffer err-buffer) + ;; Silence "Process NAME stderr finished" in stderr by adding a + ;; no-op sentinel to the fake stderr process object + (set-process-sentinel (get-buffer-process err-buffer) #'ignore)) + + ;; On Emacs versions before 25, there is no way to capture + ;; stdout and stderr separately for asynchronous processes, or + ;; even to redirect stderr to a file, so we use a trivial shell + ;; wrapper to send stderr to a temporary file and clean things + ;; up in the sentinel. + (setq err-file (make-temp-file "nmerr")) + (let ((process-connection-type nil)) ;; Use a pipe + (setq proc (apply #'start-process name buffer + "/bin/sh" "-c" + "exec 2>\"$1\"; shift; exec \"$0\" \"$@\"" + command err-file args))) + (process-put proc 'err-file err-file)) + (process-put proc 'sub-sentinel sentinel) (process-put proc 'real-command (cons notmuch-command args)) (set-process-sentinel proc #'notmuch-start-notmuch-sentinel) @@ -932,10 +953,10 @@ status." (defun notmuch-start-notmuch-sentinel (proc event) "Process sentinel function used by `notmuch-start-notmuch'." (let* ((err-file (process-get proc 'err-file)) - (err (with-temp-buffer - (insert-file-contents err-file) - (unless (eobp) - (buffer-string)))) + (err-buffer (or (process-get proc 'err-buffer) + (find-file-noselect err-file))) + (err (when (not (zerop (buffer-size err-buffer))) + (with-current-buffer err-buffer (buffer-string)))) (sub-sentinel (process-get proc 'sub-sentinel)) (real-command (process-get proc 'real-command))) (condition-case err @@ -953,8 +974,8 @@ status." ;; If that didn't signal an error, then any error output was ;; really warning output. Show warnings, if any. (let ((warnings - (with-temp-buffer - (unless (= (second (insert-file-contents err-file)) 0) + (when err + (with-current-buffer err-buffer (goto-char (point-min)) (end-of-line) ;; Show first line; stuff remaining lines in the @@ -969,7 +990,8 @@ status." ;; Emacs behaves strangely if an error escapes from a sentinel, ;; so turn errors into messages. (message "%s" (error-message-string err)))) - (ignore-errors (delete-file err-file)))) + (when err-buffer (kill-buffer err-buffer)) + (when err-file (ignore-errors (delete-file err-file))))) ;; This variable is used only buffer local, but it needs to be ;; declared globally first to avoid compiler warnings. -- 2.13.3 ^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-08-20 12:11 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-08-17 17:57 [PATCH 0/2] Use make-process when available Vladimir Panteleev 2017-08-17 17:57 ` [PATCH 1/2] emacs: Refactor subprocess stderr propagation Vladimir Panteleev 2017-08-19 18:45 ` Tomi Ollila 2017-08-20 12:11 ` David Bremner 2017-08-17 17:57 ` [PATCH 2/2] emacs: Use make-process when available Vladimir Panteleev
Code repositories for project(s) associated with this public inbox https://yhetil.org/notmuch.git/ This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).