bug#66066: 30.0.50; [PATCH] Add support for more-complex Eshell commands in the background

[Jim Porter <jporterbugs@gmail.com>]

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

Currently, only very simple Eshell commands can be run in the 
background. Anything more complicated, such as Eshell script files or 
things involving variable interpolation fail. For example, this should 
print "hi", but it doesn't print anything (aside from messages about the 
processes that started/stopped):

   *echo ${*echo hi} &

The attached patches fix this.

Note: In reality, this is just a preliminary set of changes to add full 
job control to Eshell, but that's more complex. (Hopefully, I'll get 
that working next though.)

[-- Attachment #2: 0001-Make-eshell-resume-eval-take-the-command-to-resume.patch --]
[-- Type: text/plain, Size: 2536 bytes --]

From c635e28b4b293968647ee1db8e05cf4db16fb879 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Sun, 17 Sep 2023 13:56:51 -0700
Subject: [PATCH 1/2] ; Make 'eshell-resume-eval' take the command to resume

* lisp/eshell/esh-cmd.el (eshell-resume-eval): Take COMMAND to resume.
Update callers.
---
 lisp/eshell/esh-cmd.el | 31 ++++++++++++++-----------------
 1 file changed, 14 insertions(+), 17 deletions(-)

diff --git a/lisp/eshell/esh-cmd.el b/lisp/eshell/esh-cmd.el
index 0d73b2d6e69..9b0948c77ca 100644
--- a/lisp/eshell/esh-cmd.el
+++ b/lisp/eshell/esh-cmd.el
@@ -1003,7 +1003,8 @@ eshell-eval-command
     (setq eshell-current-command command)
     (let* (result
            (delim (catch 'eshell-incomplete
-                    (ignore (setq result (eshell-resume-eval))))))
+                    (ignore (setq result (eshell-resume-eval
+                                          eshell-current-command))))))
       (when delim
         (error "Unmatched delimiter: %S" delim))
       result)))
@@ -1019,24 +1020,20 @@ eshell-resume-command
              ;; that all of those processes are now dead.
              (member proc eshell-last-async-procs)
              (not (seq-some #'process-live-p eshell-last-async-procs)))
-    (eshell-resume-eval)))
+    (eshell-resume-eval eshell-current-command)))
 
-(defun eshell-resume-eval ()
-  "Destructively evaluate a form which may need to be deferred."
+(defun eshell-resume-eval (command)
+  "Destructively evaluate a COMMAND form which may need to be deferred."
   (setq eshell-last-async-procs nil)
-  (when eshell-current-command
-    (eshell-condition-case err
-        (let* (retval
-               (procs (catch 'eshell-defer
-                        (ignore
-                         (setq retval
-                               (eshell-do-eval
-                                eshell-current-command))))))
-          (if retval
-              (cadr retval)
-            (ignore (setq eshell-last-async-procs procs))))
-      (error
-       (error (error-message-string err))))))
+  (eshell-condition-case err
+      (let* (retval
+             (procs (catch 'eshell-defer
+                      (ignore (setq retval (eshell-do-eval command))))))
+        (if retval
+            (cadr retval)
+          (ignore (setq eshell-last-async-procs procs))))
+    (error
+     (error (error-message-string err)))))
 
 (defmacro eshell-manipulate (form tag &rest body)
   "Manipulate a command FORM with BODY, using TAG as a debug identifier."
-- 
2.25.1


[-- Attachment #3: 0002-Support-Eshell-iterative-evaluation-in-the-backgroun.patch --]
[-- Type: text/plain, Size: 11812 bytes --]

From 7ed112c39dd3424b2cb69732c7e27509b7c3da0d Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Sun, 17 Sep 2023 20:17:59 -0700
Subject: [PATCH 2/2] Support Eshell iterative evaluation in the background

* lisp/eshell/esh-cmd.el (eshell-deferred-commands): New variable.
(eshell-parse-command): 'eshell-do-subjob' should be the outermost
call, if present.
(eshell-do-subjob): Call 'eshell-resume-eval' to split this command
off from its parent forms.
(eshell-resume-command): Consult 'eshell-deferred-commands'.
(eshell-resume-eval): New argument BACKGROUND.
(eshell-do-eval): Remove check for 'eshell-current-subjob-p'.  This is
handled differently now.

* lisp/eshell/esh-proc.el (eshell-subjob-messages): New variable...
(eshell-record-process-object, eshell-remove-process-entry):... use it.

* lisp/eshell/em-script.el (eshell-source-file): Disable subjob
messages.

* test/lisp/eshell/esh-cmd-tests.el
(esh-cmd-test/background/simple-command)
(esh-cmd-test/background/subcommand)
* test/lisp/eshell/em-script-tests.el
(em-script-test/source-script/background): New tests.
---
 lisp/eshell/em-script.el            |  7 ++-
 lisp/eshell/esh-cmd.el              | 67 ++++++++++++++++++-----------
 lisp/eshell/esh-proc.el             | 18 +++++---
 test/lisp/eshell/em-script-tests.el | 13 ++++++
 test/lisp/eshell/esh-cmd-tests.el   | 26 +++++++++++
 5 files changed, 98 insertions(+), 33 deletions(-)

diff --git a/lisp/eshell/em-script.el b/lisp/eshell/em-script.el
index 9f6f720b8b0..3a4c315ad15 100644
--- a/lisp/eshell/em-script.el
+++ b/lisp/eshell/em-script.el
@@ -94,7 +94,12 @@ eshell-source-file
       (setq cmd `(eshell-as-subcommand ,cmd)))
     (throw 'eshell-replace-command
            `(let ((eshell-command-name ',file)
-                  (eshell-command-arguments ',args))
+                  (eshell-command-arguments ',args)
+                  ;; Don't print subjob messages by default.
+                  ;; Otherwise, if this function was called as a
+                  ;; subjob, then *all* commands in the script would
+                  ;; print start/stop messages.
+                  (eshell-subjob-messages nil))
               ,cmd))))
 
 (defun eshell/source (&rest args)
diff --git a/lisp/eshell/esh-cmd.el b/lisp/eshell/esh-cmd.el
index 9b0948c77ca..b80e3f51d87 100644
--- a/lisp/eshell/esh-cmd.el
+++ b/lisp/eshell/esh-cmd.el
@@ -263,6 +263,7 @@ eshell-ensure-newline-p
 
 ;;; Internal Variables:
 
+(defvar-local eshell-deferred-commands nil)
 (defvar eshell-current-command nil)
 (defvar eshell-command-name nil)
 (defvar eshell-command-arguments nil)
@@ -418,8 +419,6 @@ eshell-parse-command
        (lambda (cmd)
          (let ((sep (pop sep-terms)))
            (setq cmd (eshell-parse-pipeline cmd))
-           (when (equal sep "&")
-             (setq cmd `(eshell-do-subjob (cons :eshell-background ,cmd))))
            (unless eshell-in-pipeline-p
              (setq cmd `(eshell-trap-errors ,cmd)))
            ;; Copy I/O handles so each full statement can manipulate
@@ -427,6 +426,8 @@ eshell-parse-command
            ;; command in the list; we won't use the originals again
            ;; anyway.
            (setq cmd `(eshell-with-copied-handles ,cmd ,(not sep)))
+           (when (equal sep "&")
+             (setq cmd `(eshell-do-subjob ,cmd)))
            cmd))
        sub-chains)))
     (if toplevel
@@ -750,10 +751,13 @@ eshell-separate-commands
 
 (defmacro eshell-do-subjob (object)
   "Evaluate a command OBJECT as a subjob.
-We indicate that the process was run in the background by returning it
-ensconced in a list."
-  `(let ((eshell-current-subjob-p t))
-     ,object))
+We indicate that the process was run in the background by
+returning it as (:eshell-background . PROCS)."
+  `(let ((eshell-current-subjob-p t)
+         ;; Print subjob messages.  This could have been cleared
+         ;; (e.g. by `eshell-source-file', which see).
+         (eshell-subjob-messages t))
+     (eshell-resume-eval ',object 'background)))
 
 (defmacro eshell-commands (object &optional silent)
   "Place a valid set of handles, and context, around command OBJECT."
@@ -988,7 +992,7 @@ eshell-eval-command
 COMMAND, if any.  If COMMAND is a background command, return the
 process(es) in a cons cell like:
 
-  (:eshell-background . PROCESS)"
+  (:eshell-background . PROCS)"
   (if eshell-current-command
       ;; We can just stick the new command at the end of the current
       ;; one, and everything will happen as it should.
@@ -1010,28 +1014,42 @@ eshell-eval-command
       result)))
 
 (defun eshell-resume-command (proc status)
-  "Resume the current command when a pipeline ends."
-  (when (and proc
-             ;; Make sure STATUS is something we want to handle.
-             (stringp status)
-             (not (string= "stopped" status))
-             (not (string-match eshell-reset-signals status))
-             ;; Make sure PROC is one of our foreground processes and
-             ;; that all of those processes are now dead.
-             (member proc eshell-last-async-procs)
-             (not (seq-some #'process-live-p eshell-last-async-procs)))
-    (eshell-resume-eval eshell-current-command)))
-
-(defun eshell-resume-eval (command)
-  "Destructively evaluate a COMMAND form which may need to be deferred."
-  (setq eshell-last-async-procs nil)
+  "Resume the current command when a pipeline ends.
+PROC is the process that invoked this from its sentinel, and
+STATUS is its status."
+  (when-let (proc
+             ((stringp status))
+             ((not (string= "stopped" status)))
+             ((not (string-match eshell-reset-signals status)))
+             ;; Find the deferred command associated with this process.
+             (deferred (assoc proc eshell-deferred-commands
+                              (lambda (i j) (memq j i))))
+             ;; Make sure that all of the processes in this pipeline
+             ;; are now dead.
+             ((not (seq-some #'process-live-p (car deferred)))))
+    ;; Remove this command from `eshell-deferred-commands'.  If it
+    ;; gets deferred again, `eshell-resume-eval' will re-add it.
+    (setq eshell-deferred-commands (delq deferred eshell-deferred-commands))
+    (apply #'eshell-resume-eval (cdr deferred))))
+
+(defun eshell-resume-eval (command &optional background)
+  "Destructively evaluate a COMMAND form which may need to be deferred.
+Return the result of COMMAND if it wasn't deferred.  If
+BACKGROUND is non-nil and Eshell defers COMMAND, return a list of
+the form (:eshell-background . PROCS)."
+  (unless background
+    (setq eshell-last-async-procs nil))
   (eshell-condition-case err
       (let* (retval
              (procs (catch 'eshell-defer
                       (ignore (setq retval (eshell-do-eval command))))))
         (if retval
             (cadr retval)
-          (ignore (setq eshell-last-async-procs procs))))
+          (push (list procs command background)
+                eshell-deferred-commands)
+          (if background
+              (cons :eshell-background procs)
+            (ignore (setq eshell-last-async-procs procs)))))
     (error
      (error (error-message-string err)))))
 
@@ -1241,13 +1259,12 @@ eshell-do-eval
 		    (setcdr form (cdr new-form)))
 		  (eshell-do-eval form synchronous-p))
               (if-let (((memq (car form) eshell-deferrable-commands))
-                       ((not eshell-current-subjob-p))
                        (procs (eshell-make-process-list result)))
                   (if synchronous-p
 		      (apply #'eshell/wait procs)
 		    (eshell-manipulate form "inserting ignore form"
 		      (setcar form 'ignore)
-		      (setcdr form nil))
+		      (setcdr form nul))
 		    (throw 'eshell-defer procs))
                 (list 'quote result))))))))))))
 
diff --git a/lisp/eshell/esh-proc.el b/lisp/eshell/esh-proc.el
index aed8f8af93d..2cb7f92516c 100644
--- a/lisp/eshell/esh-proc.el
+++ b/lisp/eshell/esh-proc.el
@@ -100,6 +100,8 @@ eshell-kill-hook
 (defvar eshell-supports-asynchronous-processes (fboundp 'make-process)
   "Non-nil if Eshell can create asynchronous processes.")
 
+(defvar eshell-subjob-messages t
+  "Non-nil if we should print process start/end messages for subjobs.")
 (defvar eshell-current-subjob-p nil)
 
 (defvar eshell-process-list nil
@@ -235,8 +237,9 @@ eshell-insert-process
 
 (defsubst eshell-record-process-object (object)
   "Record OBJECT as now running."
-  (when (and (eshell-processp object)
-	     eshell-current-subjob-p)
+  (when (and eshell-subjob-messages
+             eshell-current-subjob-p
+             (eshell-processp object))
     (require 'esh-mode)
     (declare-function eshell-interactive-print "esh-mode" (string))
     (eshell-interactive-print
@@ -245,11 +248,12 @@ eshell-record-process-object
 
 (defun eshell-remove-process-entry (entry)
   "Record the process ENTRY as fully completed."
-  (if (and (eshell-processp (car entry))
-	   (cdr entry)
-	   eshell-done-messages-in-minibuffer)
-      (message "[%s]+ Done %s" (process-name (car entry))
-	       (process-command (car entry))))
+  (when (and eshell-subjob-messages
+             eshell-done-messages-in-minibuffer
+             (eshell-processp (car entry))
+             (cdr entry))
+    (message "[%s]+ Done %s" (process-name (car entry))
+             (process-command (car entry))))
   (setq eshell-process-list
 	(delq entry eshell-process-list)))
 
diff --git a/test/lisp/eshell/em-script-tests.el b/test/lisp/eshell/em-script-tests.el
index 74328844778..02e4125d827 100644
--- a/test/lisp/eshell/em-script-tests.el
+++ b/test/lisp/eshell/em-script-tests.el
@@ -63,6 +63,19 @@ em-script-test/source-script/redirect/dev-null
         "\\`\\'"))
       (should (equal (buffer-string) "hibye")))))
 
+(ert-deftest em-script-test/source-script/background ()
+  "Test sourcing a script in the background."
+  (skip-unless (executable-find "echo"))
+  (ert-with-temp-file temp-file
+    :text "*echo hi\nif {[ foo = foo ]} {*echo bye}"
+    (eshell-with-temp-buffer bufname "old"
+      (with-temp-eshell
+       (eshell-match-command-output
+        (format "source %s > #<%s> &" temp-file bufname)
+        "\\`\\'")
+       (eshell-wait-for-subprocess t))
+      (should (equal (buffer-string) "hi\nbye\n")))))
+
 (ert-deftest em-script-test/source-script/arg-vars ()
   "Test sourcing script with $0, $1, ... variables."
   (ert-with-temp-file temp-file :text "printnl $0 \"$1 $2\""
diff --git a/test/lisp/eshell/esh-cmd-tests.el b/test/lisp/eshell/esh-cmd-tests.el
index 7c384471e93..0a973a89563 100644
--- a/test/lisp/eshell/esh-cmd-tests.el
+++ b/test/lisp/eshell/esh-cmd-tests.el
@@ -103,6 +103,32 @@ esh-cmd-test/let-rebinds-after-defer
             "}")
     "value\nexternal\nvalue\n")))
 
+\f
+;; Background command invocation
+
+(ert-deftest esh-cmd-test/background/simple-command ()
+  "Test invocation with a simple background command."
+  (skip-unless (executable-find "echo"))
+  (eshell-with-temp-buffer bufname ""
+    (with-temp-eshell
+     (eshell-match-command-output
+      (format "*echo hi > #<%s> &" bufname)
+      (rx "[echo" (? ".exe") "] " (+ digit) "\n"))
+     (eshell-wait-for-subprocess t))
+    (should (equal (buffer-string) "hi\n"))))
+
+(ert-deftest esh-cmd-test/background/subcommand ()
+  "Test invocation with a background command containing subcommands."
+  (skip-unless (and (executable-find "echo")
+                    (executable-find "rev")))
+  (eshell-with-temp-buffer bufname ""
+    (with-temp-eshell
+     (eshell-match-command-output
+      (format "*echo ${*echo hello | rev} > #<%s> &" bufname)
+      (rx "[echo" (? ".exe") "] " (+ digit) "\n"))
+     (eshell-wait-for-subprocess t))
+    (should (equal (buffer-string) "olleh\n"))))
+
 \f
 ;; Lisp forms
 
-- 
2.25.1