Subject: bug#62578: closed (Re: bug#62578: 30.0.50; [PATCH] Add regression tests for synchronous processes in Eshell)

From: [email-masked] (GNU bug Tracking System)
Date: Sun, 02 Apr 2023 21:26:02 +0000

Your bug report

#62578: 30.0.50; [PATCH] Add regression tests for synchronous processes in Eshell

which was filed against the emacs package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to [email-masked].

-- 
62578: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=62578
GNU Bug Tracking System
Contact [email-masked] with problems

Subject: Re: bug#62578: 30.0.50; [PATCH] Add regression tests for synchronous processes in Eshell
From: Jim Porter <[email-masked]>
To: Eli Zaretskii <[email-masked]>
Cc: [email-masked]
Date: Sun, 2 Apr 2023 14:25:35 -0700
List-Id: <debbugs-submit.debbugs.gnu.org>

On 4/1/2023 10:51 PM, Jim Porter wrote:
> I'll probably merge this tomorrow unless you catch any remaining issues.

Merged as 00144fa287e. Closing this now.

Subject: 30.0.50; [PATCH] Add regression tests for synchronous processes in Eshell
From: Jim Porter <[email-masked]>
To: [email-masked]
Date: Fri, 31 Mar 2023 21:41:09 -0700
List-Id: <debbugs-submit.debbugs.gnu.org>

As far as I understand it, Eshell only uses synchronous processes on 
MS-DOS (where 'make-process' doesn't exist). Since I don't build Emacs 
for MS-DOS, and I expect not many others do either, I'm worried that 
this support will regress. To avoid that, here are some regression tests.

It also fixes a small bug with how Eshell updated its internal markers 
when using synchronous processes. This mostly affects the tests, but 
could also cause unexpected behavior when using some of Eshell's 
buffer-navigation commands.

From f6a0f2d74fcac44a4faeb3c0b73184d5498ec525 Mon Sep 17 00:00:00 2001
From: Jim Porter <[email-masked]>
Date: Fri, 31 Mar 2023 21:32:44 -0700
Subject: [PATCH] ; Add tests for synchronous processes in Eshell

Normally, Eshell only uses synchronous processes on MS-DOS, so this is
hard to test.  To get around this, let the tests explicitly request
synchronous processes.

* lisp/eshell/esh-proc.el (eshell-supports-asynchronous-processes):
New variable...
(eshell-gather-process-output): ... use it, and remove some incorrect
code updating Eshell's internal markers (the async code path doesn't
do this, so neither should the sync path).

* lisp/eshell/esh-cmd.el (eshell-execute-pipeline): Use
'eshell-supports-asynchronous-processes'.

* test/lisp/eshell/esh-proc-tests.el
(esh-proc-test/synchronous-proc/simple/interactive)
(esh-proc-test/synchronous-proc/simple/command-result)
(esh-proc-test/synchronous-proc/pipeline/interactive)
(esh-proc-test/synchronous-proc/pipeline/command-result): New tests.
---
 lisp/eshell/esh-cmd.el             |  2 +-
 lisp/eshell/esh-proc.el            |  9 ++++----
 test/lisp/eshell/esh-proc-tests.el | 35 ++++++++++++++++++++++++++++++
 3 files changed, 40 insertions(+), 6 deletions(-)

diff --git a/lisp/eshell/esh-cmd.el b/lisp/eshell/esh-cmd.el
index d5237ee1f04..b80596f2cad 100644
--- a/lisp/eshell/esh-cmd.el
+++ b/lisp/eshell/esh-cmd.el
@@ -892,7 +892,7 @@ eshell-execute-pipeline
      (set headproc nil)
      (set tailproc nil)
      (progn
-       ,(if (fboundp 'make-process)
+       ,(if eshell-supports-asynchronous-processes
 	    `(eshell-do-pipelines ,pipeline)
           `(let ((tail-handles (eshell-duplicate-handles
                                 eshell-current-handles)))
diff --git a/lisp/eshell/esh-proc.el b/lisp/eshell/esh-proc.el
index a86e7502795..2976f5694d7 100644
--- a/lisp/eshell/esh-proc.el
+++ b/lisp/eshell/esh-proc.el
@@ -97,6 +97,9 @@ eshell-kill-hook
 
 ;;; Internal Variables:
 
+(defvar eshell-supports-asynchronous-processes (fboundp 'make-process)
+  "Non-nil if Eshell can create asynchronous processes.")
+
 (defvar eshell-current-subjob-p nil)
 
 (defvar eshell-process-list nil
@@ -296,7 +299,7 @@ eshell-gather-process-output
                 (coding-system-change-eol-conversion locale-coding-system
                                                      'unix))))
     (cond
-     ((fboundp 'make-process)
+     (eshell-supports-asynchronous-processes
       (unless (or ;; FIXME: It's not currently possible to use a
                   ;; stderr process for remote files.
                   (file-remote-p default-directory)
@@ -392,10 +395,6 @@ eshell-gather-process-output
 	    (setq lbeg lend)
 	    (set-buffer proc-buf))
 	  (set-buffer oldbuf))
-        (require 'esh-mode)
-        (declare-function eshell-update-markers "esh-mode" (pmark))
-        (defvar eshell-last-output-end)         ;Defined in esh-mode.el.
-	(eshell-update-markers eshell-last-output-end)
 	;; Simulate the effect of eshell-sentinel.
 	(eshell-close-handles
          (if (numberp exit-status) exit-status -1)
diff --git a/test/lisp/eshell/esh-proc-tests.el b/test/lisp/eshell/esh-proc-tests.el
index 8e02fbb5497..8399fbdeeb9 100644
--- a/test/lisp/eshell/esh-proc-tests.el
+++ b/test/lisp/eshell/esh-proc-tests.el
@@ -191,6 +191,41 @@ esh-proc-test/pipeline-connection-type/last
      (unless (eq system-type 'windows-nt)
        "stdout\nstderr\n"))))
 
+
+;; Synchronous processes
+
+(ert-deftest esh-proc-test/synchronous-proc/simple/interactive ()
+  "Test that synchronous processes work in an interactive shell."
+  (skip-unless (executable-find "echo"))
+  (let ((eshell-supports-asynchronous-processes nil))
+    (with-temp-eshell
+     (eshell-match-command-output "*echo hello"
+                                  "\\`hello\n"))))
+
+(ert-deftest esh-proc-test/synchronous-proc/simple/command-result ()
+  "Test that synchronous processes work via `eshell-command-result'."
+  (skip-unless (executable-find "echo"))
+  (let ((eshell-supports-asynchronous-processes nil))
+    (eshell-command-result-equal "*echo hello"
+                                 "hello\n")))
+
+(ert-deftest esh-proc-test/synchronous-proc/pipeline/interactive ()
+  "Test that synchronous pipelines work in an interactive shell."
+  (skip-unless (and (executable-find "echo")
+                    (executable-find "rev")))
+  (let ((eshell-supports-asynchronous-processes nil))
+    (with-temp-eshell
+     (eshell-match-command-output "*echo hello | *rev"
+                                  "\\`olleh\n"))))
+
+(ert-deftest esh-proc-test/synchronous-proc/pipeline/command-result ()
+  "Test that synchronous pipelines work via `eshell-command-result'."
+  (skip-unless (and (executable-find "echo")
+                    (executable-find "rev")))
+  (let ((eshell-supports-asynchronous-processes nil))
+    (eshell-command-result-equal "*echo hello | *rev"
+                                 "olleh\n")))
+
 
 ;; Killing processes
 
-- 
2.25.1