bug#62556: 29.0.60; [PATCH] Fix regression when calling 'eshell-command' with a pipeline in the background

bug#62556: 29.0.60; [PATCH] Fix regression when calling 'eshell-command' with a pipeline in the background

[Jim Porter]

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

Starting from "emacs -Q":

   M-x eshell-command RET *echo hi | *cat & RET

You'll see the following error in the *Messages* buffer instead of the 
result:

   eshell-eval-command: Unmatched delimiter: (#<process echo> . 
#<process cat>)

This is a regression from my fix for bug#53715, which changed how
Eshell pipelines return the processes in the pipeline. Attached are some 
patches to fix it. One for the emacs-29 branch, and one for master.

Eli, is it ok to merge the patch for emacs-29? I tried to keep the 
change as minimal as possible for that branch.

The patch for master is a bit more extensive, and also fixes another 
issue where this would fail due to incorrect syntax in the Eshell 
command form:

   (eshell-command "*echo hi &" t)

Previously, it turned the command into "*echo hi & >>> 
#<current-buffer>", but that's not right; the "&" needs to go last.

For master, I also thought it would be nice to clean up 'eshell-command' 
slightly (see patch 0002); this just changes it to handle its arguments 
in the interactive spec.

[-- Attachment #2: emacs-29--0001-Fix-using-background-commands-in-eshell-command.patch --]
[-- Type: text/plain, Size: 4687 bytes --]

From 70b62fc6173ec25d6c4c82e86af7cb4bacc75baa Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Thu, 30 Mar 2023 17:39:24 -0700
Subject: [PATCH] Fix using background commands in 'eshell-command'

Do not merge to master.

This regressed due to the patch for bug#53715, which changed how
Eshell pipelines return the processes in the pipeline.

* lisp/eshell/esh-cmd.el (eshell-eval-command): Allow process-pairs.

* test/lisp/eshell/eshell-tests.el (eshell-test/eshell-command/simple)
(eshell-test/eshell-command/pipeline)
(eshell-test/eshell-command/background)
(eshell-test/eshell-command/background-pipeline): New tests.
---
 lisp/eshell/esh-cmd.el           |  8 ++++--
 test/lisp/eshell/eshell-tests.el | 47 ++++++++++++++++++++++++++++++++
 2 files changed, 52 insertions(+), 3 deletions(-)

diff --git a/lisp/eshell/esh-cmd.el b/lisp/eshell/esh-cmd.el
index f4ac384ccc5..706477a5f45 100644
--- a/lisp/eshell/esh-cmd.el
+++ b/lisp/eshell/esh-cmd.el
@@ -1032,18 +1032,20 @@ eshell-eval-command
     (setq eshell-current-command command)
     (let* ((delim (catch 'eshell-incomplete
                     (eshell-resume-eval)))
-           (val (car-safe delim)))
+           (val (car-safe delim))
+           (val-is-process (or (eshell-processp val)
+                               (eshell-process-pair-p val))))
       ;; If the return value of `eshell-resume-eval' is wrapped in a
       ;; list, it indicates that the command was run asynchronously.
       ;; In that case, unwrap the value before checking the delimiter
       ;; value.
       (if (and val
-               (not (eshell-processp val))
+               (not val-is-process)
                (not (eq val t)))
           (error "Unmatched delimiter: %S" val)
         ;; Eshell-command expect a list like (<process>) to know if the
         ;; command should be async or not.
-        (or (and (eshell-processp val) delim) val)))))
+        (or (and val-is-process delim) val)))))
 
 (defun eshell-resume-command (proc status)
   "Resume the current command when a process ends."
diff --git a/test/lisp/eshell/eshell-tests.el b/test/lisp/eshell/eshell-tests.el
index 3c4a8ec97ea..b57abe3226c 100644
--- a/test/lisp/eshell/eshell-tests.el
+++ b/test/lisp/eshell/eshell-tests.el
@@ -105,6 +105,53 @@ eshell-test/lisp-reset-in-pipeline
      (format template "format \"%s\" eshell-in-pipeline-p")
      "nil")))
 
+(ert-deftest eshell-test/eshell-command/simple ()
+  "Test that the `eshell-command' function writes to the current buffer."
+  (skip-unless (executable-find "echo"))
+  (ert-with-temp-directory eshell-directory-name
+    (let ((eshell-history-file-name nil))
+      (with-temp-buffer
+        (eshell-command "*echo hi" t)
+        (should (equal (buffer-string) "hi\n"))))))
+
+(ert-deftest eshell-test/eshell-command/pipeline ()
+  "Test that the `eshell-command' function writes to the current buffer.
+This test uses a pipeline for the command."
+  (skip-unless (and (executable-find "echo")
+                    (executable-find "cat")))
+  (ert-with-temp-directory eshell-directory-name
+    (let ((eshell-history-file-name nil))
+      (with-temp-buffer
+        (eshell-command "*echo hi | *cat" t)
+        (should (equal (buffer-string) "hi\n"))))))
+
+(ert-deftest eshell-test/eshell-command/background ()
+  "Test that `eshell-command' works for background commands."
+  (skip-unless (executable-find "echo"))
+  (ert-with-temp-directory eshell-directory-name
+    (let ((eshell-history-file-name nil))
+      ;; XXX: We can't write to the current buffer here, since
+      ;; `eshell-command' will produce an invalid command in that
+      ;; case.  Just make sure the command runs and produces an output
+      ;; buffer.
+      (eshell-command "*echo hi &")
+      (with-current-buffer "*Eshell Async Command Output*"
+        (goto-char (point-min))
+        (should (looking-at "\\[echo\\(<[0-9]+>\\)?\\]"))))))
+
+(ert-deftest eshell-test/eshell-command/background-pipeline ()
+  "Test that `eshell-command' works for background commands.
+This test uses a pipeline for the command."
+  (skip-unless (and (executable-find "echo")
+                    (executable-find "cat")))
+  (ert-with-temp-directory eshell-directory-name
+    (let ((eshell-history-file-name nil))
+      ;; XXX: As above, we can't write to the current buffer here.
+      (eshell-command "*echo hi | *cat &")
+      (with-current-buffer "*Eshell Async Command Output*"
+        (goto-char (point-min))
+        (should (looking-at "\\[cat\\(<[0-9]+>\\)?\\]"))))))
+
 (ert-deftest eshell-test/command-running-p ()
   "Modeline should show no command running"
   (with-temp-eshell
-- 
2.25.1


[-- Attachment #3: emacs-30--0001-Fix-using-background-commands-in-eshell-command.patch --]
[-- Type: text/plain, Size: 7579 bytes --]

From bed83773f46c5fc72e9694139ddbc20ba0990c62 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Thu, 30 Mar 2023 19:31:30 -0700
Subject: [PATCH 1/2] Fix using background commands in 'eshell-command'

This regressed due to the patch for bug#53715, which changed how
Eshell pipelines return the processes in the pipeline.

* lisp/eshell/esh-cmd.el (eshell-parse-command): When creating
background commands, wrap the process(es) in a cons cell whose CAR is
':eshell-background'.  This lets us use fewer heuristics...
(eshell-eval-command): ... here.  Additionally, keep the result and
the incomplete delimiter separate.

* lisp/eshell/eshell.el (eshell-command): Check ':eshell-background'
and use a more-robust method for setting the output target.

* test/lisp/eshell/eshell-tests.el (eshell-test/eshell-command/simple)
(eshell-test/eshell-command/pipeline)
(eshell-test/eshell-command/background)
(eshell-test/eshell-command/background-pipeline): New tests.
---
 lisp/eshell/esh-cmd.el           | 30 ++++++++++------------
 lisp/eshell/eshell.el            | 21 +++++----------
 test/lisp/eshell/eshell-tests.el | 44 ++++++++++++++++++++++++++++++++
 3 files changed, 65 insertions(+), 30 deletions(-)

diff --git a/lisp/eshell/esh-cmd.el b/lisp/eshell/esh-cmd.el
index d5237ee1f04..600a2c3af0b 100644
--- a/lisp/eshell/esh-cmd.el
+++ b/lisp/eshell/esh-cmd.el
@@ -421,7 +421,8 @@ eshell-parse-command
                            (string= (car eshell--sep-terms) ";"))
                        (eshell-parse-pipeline cmd)
                      `(eshell-do-subjob
-                       (list ,(eshell-parse-pipeline cmd)))))
+                       (cons :eshell-background
+                             ,(eshell-parse-pipeline cmd)))))
              (setq eshell--sep-terms (cdr eshell--sep-terms))
              (if eshell-in-pipeline-p
                  cmd
@@ -1030,7 +1031,12 @@ eshell-eval-argument
     (cadr result)))
 
 (defun eshell-eval-command (command &optional input)
-  "Evaluate the given COMMAND iteratively."
+  "Evaluate the given COMMAND iteratively.
+Return the process (or head and tail processes) created by
+COMMAND, if any.  If COMMAND is a background command, return the
+process(es) in a cons cell like:
+
+  (:eshell-background . PROCESS)"
   (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.
@@ -1046,20 +1052,12 @@ eshell-eval-command
            (erase-buffer)
            (insert "command: \"" input "\"\n")))
     (setq eshell-current-command command)
-    (let* ((delim (catch 'eshell-incomplete
-                    (eshell-resume-eval)))
-           (val (car-safe delim)))
-      ;; If the return value of `eshell-resume-eval' is wrapped in a
-      ;; list, it indicates that the command was run asynchronously.
-      ;; In that case, unwrap the value before checking the delimiter
-      ;; value.
-      (if (and val
-               (not (eshell-processp val))
-               (not (eq val t)))
-          (error "Unmatched delimiter: %S" val)
-        ;; Eshell-command expect a list like (<process>) to know if the
-        ;; command should be async or not.
-        (or (and (eshell-processp val) delim) val)))))
+    (let* (result
+           (delim (catch 'eshell-incomplete
+                    (ignore (setq result (eshell-resume-eval))))))
+      (when delim
+        (error "Unmatched delimiter: %S" delim))
+      result)))
 
 (defun eshell-resume-command (proc status)
   "Resume the current command when a process ends."
diff --git a/lisp/eshell/eshell.el b/lisp/eshell/eshell.el
index 7d2c0335db2..b71f283bf9f 100644
--- a/lisp/eshell/eshell.el
+++ b/lisp/eshell/eshell.el
@@ -290,25 +290,18 @@ eshell-command
 	    (eshell-add-input-to-history command)))))
   (unless command
     (error "No command specified!"))
-  ;; redirection into the current buffer is achieved by adding an
-  ;; output redirection to the end of the command, of the form
-  ;; 'COMMAND >>> #<buffer BUFFER>'.  This will not interfere with
-  ;; other redirections, since multiple redirections merely cause the
-  ;; output to be copied to multiple target locations
-  (if arg
-      (setq command
-	    (concat command
-		    (format " >>> #<buffer %s>"
-			    (buffer-name (current-buffer))))))
   (save-excursion
-    (let ((buf (set-buffer (generate-new-buffer " *eshell cmd*")))
+    (let ((stdout (if arg (current-buffer) t))
+          (buf (set-buffer (generate-new-buffer " *eshell cmd*")))
 	  (eshell-non-interactive-p t))
       (eshell-mode)
       (let* ((proc (eshell-eval-command
-		    (list 'eshell-commands
-			  (eshell-parse-command command))))
+                    `(let ((eshell-current-handles
+                            (eshell-create-handles ,stdout 'insert))
+                           (eshell-current-subjob-p))
+		       ,(eshell-parse-command command))))
 	     intr
-	     (bufname (if (and proc (listp proc))
+	     (bufname (if (eq (car-safe proc) :eshell-background)
 			  "*Eshell Async Command Output*"
 			(setq intr t)
 			"*Eshell Command Output*")))
diff --git a/test/lisp/eshell/eshell-tests.el b/test/lisp/eshell/eshell-tests.el
index 743cc28b9b5..390f75cfbb9 100644
--- a/test/lisp/eshell/eshell-tests.el
+++ b/test/lisp/eshell/eshell-tests.el
@@ -107,6 +107,50 @@ eshell-test/lisp-reset-in-pipeline
      (format template "format \"%s\" eshell-in-pipeline-p")
      "nil")))
 
+(ert-deftest eshell-test/eshell-command/simple ()
+  "Test that the `eshell-command' function writes to the current buffer."
+  (skip-unless (executable-find "echo"))
+  (ert-with-temp-directory eshell-directory-name
+    (let ((eshell-history-file-name nil))
+      (with-temp-buffer
+        (eshell-command "*echo hi" t)
+        (should (equal (buffer-string) "hi\n"))))))
+
+(ert-deftest eshell-test/eshell-command/pipeline ()
+  "Test that the `eshell-command' function writes to the current buffer.
+This test uses a pipeline for the command."
+  (skip-unless (and (executable-find "echo")
+                    (executable-find "cat")))
+  (ert-with-temp-directory eshell-directory-name
+    (let ((eshell-history-file-name nil))
+      (with-temp-buffer
+        (eshell-command "*echo hi | *cat" t)
+        (should (equal (buffer-string) "hi\n"))))))
+
+(ert-deftest eshell-test/eshell-command/background ()
+  "Test that `eshell-command' works for background commands."
+  (skip-unless (executable-find "echo"))
+  (ert-with-temp-directory eshell-directory-name
+    (let ((orig-processes (process-list))
+          (eshell-history-file-name nil))
+      (with-temp-buffer
+        (eshell-command "*echo hi &" t)
+        (eshell-wait-for (lambda () (equal (process-list) orig-processes)))
+        (should (equal (buffer-string) "hi\n"))))))
+
+(ert-deftest eshell-test/eshell-command/background-pipeline ()
+  "Test that `eshell-command' works for background commands.
+This test uses a pipeline for the command."
+  (skip-unless (and (executable-find "echo")
+                    (executable-find "cat")))
+  (ert-with-temp-directory eshell-directory-name
+    (let ((orig-processes (copy-tree (process-list)))
+          (eshell-history-file-name nil))
+      (with-temp-buffer
+        (eshell-command "*echo hi | *cat &" t)
+        (eshell-wait-for (lambda () (equal (process-list) orig-processes)))
+        (should (equal (buffer-string) "hi\n"))))))
+
 (ert-deftest eshell-test/command-running-p ()
   "Modeline should show no command running"
   (with-temp-eshell
-- 
2.25.1


[-- Attachment #4: emacs-30--0002-Use-the-interactive-spec-to-set-arguments-for-eshell.patch --]
[-- Type: text/plain, Size: 2807 bytes --]

From e506ea46e24967fddff6d86ff9597686764d8244 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Thu, 30 Mar 2023 19:38:30 -0700
Subject: [PATCH 2/2] Use the 'interactive' spec to set arguments for
 'eshell-command'

* lisp/eshell/eshell.el (eshell-read-command): New function...
(eshell-command): ... use it.  Additionally, require the COMMAND
argument, and rename ARG to TO-CURRENT-BUFFER.
---
 lisp/eshell/eshell.el | 34 ++++++++++++++++++----------------
 1 file changed, 18 insertions(+), 16 deletions(-)

diff --git a/lisp/eshell/eshell.el b/lisp/eshell/eshell.el
index b71f283bf9f..15fc2ae6310 100644
--- a/lisp/eshell/eshell.el
+++ b/lisp/eshell/eshell.el
@@ -272,26 +272,28 @@ eshell-non-interactive-p
 
 (declare-function eshell-add-input-to-history "em-hist" (input))
 
-;;;###autoload
-(defun eshell-command (&optional command arg)
-  "Execute the Eshell command string COMMAND.
-With prefix ARG, insert output into the current buffer at point."
-  (interactive)
-  (unless arg
-    (setq arg current-prefix-arg))
-  (let ((eshell-non-interactive-p t))
+(defun eshell-read-command (&optional prompt)
+  "Read an Eshell command from the minibuffer, prompting with PROMPT."
+  (let ((prompt (or prompt "Emacs shell command: "))
+        (eshell-non-interactive-p t))
     ;; Enable `eshell-mode' only in this minibuffer.
     (minibuffer-with-setup-hook (lambda ()
                                   (eshell-mode)
                                   (eshell-command-mode +1))
-      (unless command
-        (setq command (read-from-minibuffer "Emacs shell command: "))
-	(if (eshell-using-module 'eshell-hist)
-	    (eshell-add-input-to-history command)))))
-  (unless command
-    (error "No command specified!"))
+      (let ((command (read-from-minibuffer prompt)))
+        (when (eshell-using-module 'eshell-hist)
+          (eshell-add-input-to-history command))
+        command))))
+
+;;;###autoload
+(defun eshell-command (command &optional to-current-buffer)
+  "Execute the Eshell command string COMMAND.
+If TO-CURRENT-BUFFER is non-nil (interactively, with the prefix
+argument), then insert output into the current buffer at point."
+  (interactive (list (eshell-read-command)
+                     current-prefix-arg))
   (save-excursion
-    (let ((stdout (if arg (current-buffer) t))
+    (let ((stdout (if to-current-buffer (current-buffer) t))
           (buf (set-buffer (generate-new-buffer " *eshell cmd*")))
 	  (eshell-non-interactive-p t))
       (eshell-mode)
@@ -319,7 +321,7 @@ eshell-command
 	  (while (and (bolp) (not (bobp)))
 	    (delete-char -1)))
 	(cl-assert (and buf (buffer-live-p buf)))
-	(unless arg
+	(unless to-current-buffer
 	  (let ((len (if (not intr) 2
 		       (count-lines (point-min) (point-max)))))
 	    (cond
-- 
2.25.1

bug#62556: 29.0.60; [PATCH] Fix regression when calling 'eshell-command' with a pipeline in the background

[Eli Zaretskii]

> Date: Thu, 30 Mar 2023 21:18:10 -0700
> From: Jim Porter <jporterbugs@gmail.com>
> 
> This is a regression from my fix for bug#53715, which changed how
> Eshell pipelines return the processes in the pipeline. Attached are some 
> patches to fix it. One for the emacs-29 branch, and one for master.

Looks like there are _two_ patches for master, not one?

> Eli, is it ok to merge the patch for emacs-29?

Yes, thanks.

bug#62556: 29.0.60; [PATCH] Fix regression when calling 'eshell-command' with a pipeline in the background

[Jim Porter]

On 3/30/2023 11:29 PM, Eli Zaretskii wrote:
>> Date: Thu, 30 Mar 2023 21:18:10 -0700
>> From: Jim Porter <jporterbugs@gmail.com>
>>
>> This is a regression from my fix for bug#53715, which changed how
>> Eshell pipelines return the processes in the pipeline. Attached are some
>> patches to fix it. One for the emacs-29 branch, and one for master.
> 
> Looks like there are _two_ patches for master, not one?

Yeah, sorry. That was clumsy wording on my part.

>> Eli, is it ok to merge the patch for emacs-29?
> 
> Yes, thanks.

Thanks. Merged to emacs-29 as 6419d78fa6f. I'll wait a couple days 
before merging the patches for master in case anyone else has comments, 
since they're a bit more substantial.

bug#62556: 29.0.60; [PATCH] Fix regression when calling 'eshell-command' with a pipeline in the background

[Eli Zaretskii]

> Date: Fri, 31 Mar 2023 13:09:24 -0700
> Cc: 62556@debbugs.gnu.org
> From: Jim Porter <jporterbugs@gmail.com>
> 
> Thanks. Merged to emacs-29 as 6419d78fa6f.

Some of the new tests (and one old as well) fail on MS-Windows:

  3 unexpected results:
     FAILED  eshell-test/eshell-command/background
     FAILED  eshell-test/eshell-command/background-pipeline
     FAILED  eshell-test/subcommand-reset-in-pipeline

I fixed the first 2, but I don't understand what goes wrong in the
last one and why:

  Test eshell-test/subcommand-reset-in-pipeline backtrace:
    signal(ert-test-failed (((should (eshell-command-result--equal comma
    ert-fail(((should (eshell-command-result--equal command (eshell-test
    (if (unwind-protect (setq value-7 (apply fn-5 args-6)) (setq form-de
    (let (form-description-9) (if (unwind-protect (setq value-7 (apply f
    (let ((value-7 'ert-form-evaluation-aborted-8)) (let (form-descripti
    (let* ((fn-5 #'eshell-command-result--equal) (args-6 (condition-case
    eshell-command-result-equal("*cat $<echo $eshell-in-pipeline-p | ech
    (let ((template (car tail))) (eshell-command-result-equal (format te
    (while tail (let ((template (car tail))) (eshell-command-result-equa
    (let ((tail '("echo {%s} | *cat" "echo ${%s} | *cat" "*cat $<%s> | *
    (closure (t) nil (let* ((fn-31 #'executable-find) (args-32 (conditio
    ert--run-test-internal(#s(ert--test-execution-info :test #s(ert-test
    ert-run-test(#s(ert-test :name eshell-test/subcommand-reset-in-pipel
    ert-run-or-rerun-test(#s(ert--stats :selector ... :tests ... :test-m
    ert-run-tests((not (or (tag :unstable) (tag :nativecomp))) #f(compil
    ert-run-tests-batch((not (or (tag :unstable) (tag :nativecomp))))
    ert-run-tests-batch-and-exit((not (or (tag :unstable) (tag :nativeco
    eval((ert-run-tests-batch-and-exit '(not (or (tag :unstable) (tag :n
    command-line-1(("-L" ";." "-l" "ert" "-l" "lisp/eshell/eshell-tests.
    command-line()
    normal-top-level()
  Test eshell-test/subcommand-reset-in-pipeline condition:
      (ert-test-failed
       ((should
	 (eshell-command-result--equal command
				       (eshell-test-command-result command)
				       result))
	:form
	(eshell-command-result--equal "*cat $<echo $eshell-in-pipeline-p | echo> | *cat" nil "first")
	:value nil :explanation
	(nonequal-result
	 (command "*cat $<echo $eshell-in-pipeline-p | echo> | *cat")
	 (result nil)
	 (expected "first"))))
     FAILED  16/16  eshell-test/subcommand-reset-in-pipeline (1.796875 sec) at lisp/eshell/eshell-tests.el:80

The test that fails is this one:

    (eshell-command-result-equal
     (format template "echo $eshell-in-pipeline-p | echo")
     "first")

when it is run for the last template, "*cat $<%s> | *cat".

As another annoyance, I get this prompt, and must type "yes RET":

     passed   1/16  eshell-test/command-running-p (0.203125 sec)
     passed   2/16  eshell-test/eshell-command/background (0.015625 sec)
  Buffer "*Eshell Async Command Output*" has a running process; kill it? (yes or no) yes
     passed   3/16  eshell-test/eshell-command/background-pipeline (2.453125 sec)

Any suggestions?

bug#62556: 29.0.60; [PATCH] Fix regression when calling 'eshell-command' with a pipeline in the background

[Jim Porter]

On 3/31/2023 11:35 PM, Eli Zaretskii wrote:
> I fixed the first 2, but I don't understand what goes wrong in the
> last one and why:
[snip]
>    Test eshell-test/subcommand-reset-in-pipeline condition:
>        (ert-test-failed
>         ((should
> 	 (eshell-command-result--equal command
> 				       (eshell-test-command-result command)
> 				       result))
> 	:form
> 	(eshell-command-result--equal "*cat $<echo $eshell-in-pipeline-p | echo> | *cat" nil "first")
> 	:value nil :explanation
> 	(nonequal-result
> 	 (command "*cat $<echo $eshell-in-pipeline-p | echo> | *cat")
> 	 (result nil)
> 	 (expected "first"))))
>       FAILED  16/16  eshell-test/subcommand-reset-in-pipeline (1.796875 sec) at lisp/eshell/eshell-tests.el:80

Lars reported a similar issue a while back too: 
<https://lists.gnu.org/archive/html/emacs-devel/2022-09/msg00314.html>. 
I *think* it's due to a race condition somewhere, but I've had quite a 
bit of difficulty fixing it. I have a couple ideas though, but they're 
probably too invasive for the release branch.

> As another annoyance, I get this prompt, and must type "yes RET":
> 
>       passed   1/16  eshell-test/command-running-p (0.203125 sec)
>       passed   2/16  eshell-test/eshell-command/background (0.015625 sec)
>    Buffer "*Eshell Async Command Output*" has a running process; kill it? (yes or no) yes
>       passed   3/16  eshell-test/eshell-command/background-pipeline (2.453125 sec)
> 
> Any suggestions?

Does this patch fix it?

----------------------------------------

diff --git a/test/lisp/eshell/eshell-tests.el 
b/test/lisp/eshell/eshell-tests.el
index bf7ec0389f0..fbff51a8873 100644
--- a/test/lisp/eshell/eshell-tests.el
+++ b/test/lisp/eshell/eshell-tests.el
@@ -136,6 +136,8 @@ eshell-test/eshell-command/background
        ;; buffer.
        (eshell-command "*echo hi &")
        (with-current-buffer "*Eshell Async Command Output*"
+        (while (get-buffer-process (current-buffer))
+          (accept-process-output))
          (goto-char (point-min))
          (should (looking-at "\\[echo\\(\\.exe\\)?\\(<[0-9]+>\\)?\\]"))))))

@@ -149,6 +151,8 @@ eshell-test/eshell-command/background-pipeline
        ;; XXX: As above, we can't write to the current buffer here.
        (eshell-command "*echo hi | *cat &")
        (with-current-buffer "*Eshell Async Command Output*"
+        (while (get-buffer-process (current-buffer))
+          (accept-process-output))
          (goto-char (point-min))
          (should (looking-at "\\[cat\\(\\.exe\\)?\\(<[0-9]+>\\)?\\]"))))))

bug#62556: 29.0.60; [PATCH] Fix regression when calling 'eshell-command' with a pipeline in the background

[Eli Zaretskii]

> Date: Sat, 1 Apr 2023 00:36:33 -0700
> Cc: 62556@debbugs.gnu.org
> From: Jim Porter <jporterbugs@gmail.com>
> 
> > As another annoyance, I get this prompt, and must type "yes RET":
> > 
> >       passed   1/16  eshell-test/command-running-p (0.203125 sec)
> >       passed   2/16  eshell-test/eshell-command/background (0.015625 sec)
> >    Buffer "*Eshell Async Command Output*" has a running process; kill it? (yes or no) yes
> >       passed   3/16  eshell-test/eshell-command/background-pipeline (2.453125 sec)
> > 
> > Any suggestions?
> 
> Does this patch fix it?

Yes, thanks.

bug#62556: 29.0.60; [PATCH] Fix regression when calling 'eshell-command' with a pipeline in the background

[Jim Porter]

On 4/1/2023 1:05 AM, Eli Zaretskii wrote:
>> Date: Sat, 1 Apr 2023 00:36:33 -0700
>> Cc: 62556@debbugs.gnu.org
>> From: Jim Porter <jporterbugs@gmail.com>
>>
>> Does this patch fix it?
> 
> Yes, thanks.

Thanks. Pushed as 89e337c3fc9.

I'll make sure these issues are addressed in the patches for master and 
then push them in a day or so, too.

bug#62556: 29.0.60; [PATCH] Fix regression when calling 'eshell-command' with a pipeline in the background

[Jim Porter]

On 4/1/2023 10:31 AM, Jim Porter wrote:
> I'll make sure these issues are addressed in the patches for master and 
> then push them in a day or so, too.

Pushed to master as 093a360251a. Closing this now.

Hopefully this works on MS-Windows; I think I wrote it in a way where it 
should behave the same on all platforms. If not, just let me know, and 
I'll try to fix it.

bug#62556: 29.0.60; [PATCH] Fix regression when calling 'eshell-command' with a pipeline in the background

[Eli Zaretskii]

> Date: Sun, 2 Apr 2023 15:09:01 -0700
> From: Jim Porter <jporterbugs@gmail.com>
> Cc: 62556-done@debbugs.gnu.org
> 
> On 4/1/2023 10:31 AM, Jim Porter wrote:
> > I'll make sure these issues are addressed in the patches for master and 
> > then push them in a day or so, too.
> 
> Pushed to master as 093a360251a. Closing this now.
> 
> Hopefully this works on MS-Windows; I think I wrote it in a way where it 
> should behave the same on all platforms. If not, just let me know, and 
> I'll try to fix it.

Which tests are those?  The commit you name has no changes for the
test suite, so I'm unsure which tests to run.

bug#62556: 29.0.60; [PATCH] Fix regression when calling 'eshell-command' with a pipeline in the background

[Jim Porter]

On 4/6/2023 3:32 AM, Eli Zaretskii wrote:
> Which tests are those?  The commit you name has no changes for the
> test suite, so I'm unsure which tests to run.

The commit is 267fca267fe, and the test file I changed is 
test/lisp/eshell/eshell-tests.el. (That was the commit immediately prior 
to 093a360251a in my patch series.)

bug#62556: 29.0.60; [PATCH] Fix regression when calling 'eshell-command' with a pipeline in the background

[Eli Zaretskii]

> Date: Thu, 6 Apr 2023 11:08:05 -0700
> Cc: 62556@debbugs.gnu.org
> From: Jim Porter <jporterbugs@gmail.com>
> 
> On 4/6/2023 3:32 AM, Eli Zaretskii wrote:
> > Which tests are those?  The commit you name has no changes for the
> > test suite, so I'm unsure which tests to run.
> 
> The commit is 267fca267fe, and the test file I changed is 
> test/lisp/eshell/eshell-tests.el. (That was the commit immediately prior 
> to 093a360251a in my patch series.)

Thanks, those tests work on MS-Windows.