* bug#62578: 30.0.50; [PATCH] Add regression tests for synchronous processes in Eshell
@ 2023-04-01 4:41 Jim Porter
2023-04-01 6:07 ` Eli Zaretskii
0 siblings, 1 reply; 9+ messages in thread
From: Jim Porter @ 2023-04-01 4:41 UTC (permalink / raw)
To: 62578
[-- Attachment #1: Type: text/plain, Size: 522 bytes --]
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.
[-- Attachment #2: 0001-Add-tests-for-synchronous-processes-in-Eshell.patch --]
[-- Type: text/plain, Size: 5013 bytes --]
From f6a0f2d74fcac44a4faeb3c0b73184d5498ec525 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
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"))))
+\f
+;; 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")))
+
\f
;; Killing processes
--
2.25.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* bug#62578: 30.0.50; [PATCH] Add regression tests for synchronous processes in Eshell
2023-04-01 4:41 bug#62578: 30.0.50; [PATCH] Add regression tests for synchronous processes in Eshell Jim Porter
@ 2023-04-01 6:07 ` Eli Zaretskii
2023-04-01 7:16 ` Jim Porter
0 siblings, 1 reply; 9+ messages in thread
From: Eli Zaretskii @ 2023-04-01 6:07 UTC (permalink / raw)
To: Jim Porter; +Cc: 62578
> Date: Fri, 31 Mar 2023 21:41:09 -0700
> From: Jim Porter <jporterbugs@gmail.com>
>
> 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.
Thanks.
> +(ert-deftest esh-proc-test/synchronous-proc/simple/interactive ()
> + "Test that synchronous processes work in an interactive shell."
> + (skip-unless (executable-find "echo"))
This will always skip on MS-DOS, since "echo" is not available as an
external program OOTB, only if GNU Coreutils are installed. And even
if Coreutils _are_ installed, a command that invokes "echo" will most
probably invoke the shell builtin instead.
> + (skip-unless (and (executable-find "echo")
> + (executable-find "rev")))
Likewise here: "rev" is not expected to be available on MS-DOS.
I think you need to rethink these tests in a way that uses different
commands. My suggestion is to use the Emacs executable, since that is
always available when running these tests.
^ permalink raw reply [flat|nested] 9+ messages in thread
* bug#62578: 30.0.50; [PATCH] Add regression tests for synchronous processes in Eshell
2023-04-01 6:07 ` Eli Zaretskii
@ 2023-04-01 7:16 ` Jim Porter
2023-04-01 7:22 ` Eli Zaretskii
0 siblings, 1 reply; 9+ messages in thread
From: Jim Porter @ 2023-04-01 7:16 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 62578
On 3/31/2023 11:07 PM, Eli Zaretskii wrote:
>> Date: Fri, 31 Mar 2023 21:41:09 -0700
>> From: Jim Porter <jporterbugs@gmail.com>
>>
>> +(ert-deftest esh-proc-test/synchronous-proc/simple/interactive ()
>> + "Test that synchronous processes work in an interactive shell."
>> + (skip-unless (executable-find "echo"))
>
> This will always skip on MS-DOS, since "echo" is not available as an
> external program OOTB, only if GNU Coreutils are installed. And even
> if Coreutils _are_ installed, a command that invokes "echo" will most
> probably invoke the shell builtin instead.
My main goal here is to test synchronous subprocesses on platforms
*other than* MS-DOS, since these new tests aren't really necessary on
MS-DOS itself: there are plenty of existing Eshell tests that create
processes, and they'd *all* use synchronous subprocesses on MS-DOS.
As for calling a shell builtin, Eshell should translate "*echo" to
"/path/to/echo" (or "C:/path/to/echo.exe") before calling it, so it
shouldn't use a shell builtin here. You're right though that the tests
probably require GNU Coreutils to be installed (this is also true of
quite a few existing Eshell tests though).
>> + (skip-unless (and (executable-find "echo")
>> + (executable-find "rev")))
>
> Likewise here: "rev" is not expected to be available on MS-DOS.
To make this work in more places, I could switch "rev" to something
that's actually in GNU Coreutils. Maybe "wc".
> I think you need to rethink these tests in a way that uses different
> commands. My suggestion is to use the Emacs executable, since that is
> always available when running these tests.
Since these tests are meant to check the "synchronous subprocess" code
in Eshell on non-MS-DOS platforms, I'd say it's ok. However, I can
change my patch if you prefer. I could either:
1) Add a comment to the tests explaining that they're just meant to
simulate some of MS-DOS's limitations on non-MS-DOS systems, or
2) Rework these tests so they work the same on both MS-DOS and other
systems.
Personally, I lean softly towards (1), partly because the Eshell test
suite probably breaks in quite a few other places on MS-DOS anyway.
However, it shouldn't be too hard to do (2) instead.
What do you think?
^ permalink raw reply [flat|nested] 9+ messages in thread
* bug#62578: 30.0.50; [PATCH] Add regression tests for synchronous processes in Eshell
2023-04-01 7:16 ` Jim Porter
@ 2023-04-01 7:22 ` Eli Zaretskii
2023-04-01 7:40 ` Jim Porter
0 siblings, 1 reply; 9+ messages in thread
From: Eli Zaretskii @ 2023-04-01 7:22 UTC (permalink / raw)
To: Jim Porter; +Cc: 62578
> Date: Sat, 1 Apr 2023 00:16:38 -0700
> Cc: 62578@debbugs.gnu.org
> From: Jim Porter <jporterbugs@gmail.com>
>
> Since these tests are meant to check the "synchronous subprocess" code
> in Eshell on non-MS-DOS platforms, I'd say it's ok. However, I can
> change my patch if you prefer. I could either:
>
> 1) Add a comment to the tests explaining that they're just meant to
> simulate some of MS-DOS's limitations on non-MS-DOS systems, or
>
> 2) Rework these tests so they work the same on both MS-DOS and other
> systems.
>
> Personally, I lean softly towards (1), partly because the Eshell test
> suite probably breaks in quite a few other places on MS-DOS anyway.
> However, it shouldn't be too hard to do (2) instead.
>
> What do you think?
I don't understand why not use Emacs instead of all those external
commands. That solves all the problems nicely and portably, and still
allows you to do anything you want.
But it's your call, eventually.
^ permalink raw reply [flat|nested] 9+ messages in thread
* bug#62578: 30.0.50; [PATCH] Add regression tests for synchronous processes in Eshell
2023-04-01 7:22 ` Eli Zaretskii
@ 2023-04-01 7:40 ` Jim Porter
2023-04-02 0:58 ` Jim Porter
0 siblings, 1 reply; 9+ messages in thread
From: Jim Porter @ 2023-04-01 7:40 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 62578
On 4/1/2023 12:22 AM, Eli Zaretskii wrote:
> I don't understand why not use Emacs instead of all those external
> commands. That solves all the problems nicely and portably, and still
> allows you to do anything you want.
Mostly just so that the tests are easier to understand, but for these
newly-added ones, I think they should be easy enough to understand no
matter what. I'll try using Emacs.
My only concern there is making Emacs do something reasonable when
piping data into it, but I can just try a few things out, and if it
really doesn't work, the existing tests in my patch are hopefully still
better than nothing.
^ permalink raw reply [flat|nested] 9+ messages in thread
* bug#62578: 30.0.50; [PATCH] Add regression tests for synchronous processes in Eshell
2023-04-01 7:40 ` Jim Porter
@ 2023-04-02 0:58 ` Jim Porter
2023-04-02 5:29 ` Eli Zaretskii
0 siblings, 1 reply; 9+ messages in thread
From: Jim Porter @ 2023-04-02 0:58 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 62578
[-- Attachment #1: Type: text/plain, Size: 325 bytes --]
On 4/1/2023 12:40 AM, Jim Porter wrote:
> My only concern there is making Emacs do something reasonable when
> piping data into it, but I can just try a few things out, and if it
> really doesn't work, the existing tests in my patch are hopefully still
> better than nothing.
Ok, that wasn't too hard. How does this look?
[-- Attachment #2: 0001-Add-tests-for-synchronous-processes-in-Eshell.patch --]
[-- Type: text/plain, Size: 5975 bytes --]
From f5e8fad60c00acbb8cd7cd82acf1bae21e01aafb Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
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/emacs-command): New function.
(esh-proc-test/emacs-echo, esh-proc-test/emacs-upcase): New variables.
(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 | 52 ++++++++++++++++++++++++++++++
3 files changed, 57 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..6395bdf4f71 100644
--- a/test/lisp/eshell/esh-proc-tests.el
+++ b/test/lisp/eshell/esh-proc-tests.el
@@ -191,6 +191,58 @@ esh-proc-test/pipeline-connection-type/last
(unless (eq system-type 'windows-nt)
"stdout\nstderr\n"))))
+\f
+;; Synchronous processes
+
+;; These tests check that synchronous subprocesses (only used on
+;; MS-DOS by default) work correctly. To help them run on MS-DOS as
+;; well, we use the Emacs executable as our subprocess to test
+;; against; that way, users don't need to have GNU coreutils (or
+;; similar) installed.
+
+(defsubst esh-proc-test/emacs-command (command)
+ "Evaluate COMMAND in a new Emacs batch instance."
+ (mapconcat #'shell-quote-argument
+ `(,(expand-file-name invocation-name invocation-directory)
+ "-Q" "--batch" "--eval" ,(prin1-to-string command))
+ " "))
+
+(defvar esh-proc-test/emacs-echo
+ (esh-proc-test/emacs-command '(message "hello"))
+ "A command that prints \"hello\" to stdout using Emacs.")
+
+(defvar esh-proc-test/emacs-upcase
+ (esh-proc-test/emacs-command '(message "%s" (upcase (read-string ""))))
+ "A command that upcases the text from stdin using Emacs.")
+
+(ert-deftest esh-proc-test/synchronous-proc/simple/interactive ()
+ "Test that synchronous processes work in an interactive shell."
+ (let ((eshell-supports-asynchronous-processes nil))
+ (with-temp-eshell
+ (eshell-match-command-output esh-proc-test/emacs-echo
+ "\\`hello\n"))))
+
+(ert-deftest esh-proc-test/synchronous-proc/simple/command-result ()
+ "Test that synchronous processes work via `eshell-command-result'."
+ (let ((eshell-supports-asynchronous-processes nil))
+ (eshell-command-result-equal esh-proc-test/emacs-echo
+ "hello\n")))
+
+(ert-deftest esh-proc-test/synchronous-proc/pipeline/interactive ()
+ "Test that synchronous pipelines work in an interactive shell."
+ (let ((eshell-supports-asynchronous-processes nil))
+ (with-temp-eshell
+ (eshell-match-command-output (concat esh-proc-test/emacs-echo " | "
+ esh-proc-test/emacs-upcase)
+ "\\`HELLO\n"))))
+
+(ert-deftest esh-proc-test/synchronous-proc/pipeline/command-result ()
+ "Test that synchronous pipelines work via `eshell-command-result'."
+ (let ((eshell-supports-asynchronous-processes nil))
+ (eshell-command-result-equal (concat esh-proc-test/emacs-echo " | "
+ esh-proc-test/emacs-upcase)
+ "HELLO\n")))
+
\f
;; Killing processes
--
2.25.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* bug#62578: 30.0.50; [PATCH] Add regression tests for synchronous processes in Eshell
2023-04-02 0:58 ` Jim Porter
@ 2023-04-02 5:29 ` Eli Zaretskii
2023-04-02 5:51 ` Jim Porter
0 siblings, 1 reply; 9+ messages in thread
From: Eli Zaretskii @ 2023-04-02 5:29 UTC (permalink / raw)
To: Jim Porter; +Cc: 62578
> Date: Sat, 1 Apr 2023 17:58:59 -0700
> From: Jim Porter <jporterbugs@gmail.com>
> Cc: 62578@debbugs.gnu.org
>
> On 4/1/2023 12:40 AM, Jim Porter wrote:
> > My only concern there is making Emacs do something reasonable when
> > piping data into it, but I can just try a few things out, and if it
> > really doesn't work, the existing tests in my patch are hopefully still
> > better than nothing.
>
> Ok, that wasn't too hard. How does this look?
LGTM, thanks.
One minor nit: 'message' in a batch session prints to stderr, not to
stdout.
^ permalink raw reply [flat|nested] 9+ messages in thread
* bug#62578: 30.0.50; [PATCH] Add regression tests for synchronous processes in Eshell
2023-04-02 5:29 ` Eli Zaretskii
@ 2023-04-02 5:51 ` Jim Porter
2023-04-02 21:25 ` Jim Porter
0 siblings, 1 reply; 9+ messages in thread
From: Jim Porter @ 2023-04-02 5:51 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 62578
[-- Attachment #1: Type: text/plain, Size: 556 bytes --]
On 4/1/2023 10:29 PM, Eli Zaretskii wrote:
> One minor nit: 'message' in a batch session prints to stderr, not to
> stdout.
!! Thanks for catching that. That actually means there's a small bug (or
a missing feature) in Eshell: it doesn't support redirecting stdout and
stderr to separate places when using synchronous subprocesses.
I've added a note about this to esh-proc.el (fixing it would probably be
tricky), and changed 'message' in the tests to be 'princ'. See attached;
I'll probably merge this tomorrow unless you catch any remaining issues.
[-- Attachment #2: 0001-Add-tests-for-synchronous-processes-in-Eshell.patch --]
[-- Type: text/plain, Size: 6337 bytes --]
From ec1a5b3cda09945cb4b74273f9568267de5bd96f Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
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/emacs-command): New function.
(esh-proc-test/emacs-echo, esh-proc-test/emacs-upcase): New variables.
(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 | 11 ++++---
test/lisp/eshell/esh-proc-tests.el | 53 ++++++++++++++++++++++++++++++
3 files changed, 60 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..00e0c8014e1 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)
@@ -367,6 +370,8 @@ eshell-gather-process-output
(erase-buffer)
(set-buffer oldbuf)
(run-hook-with-args 'eshell-exec-hook command)
+ ;; XXX: This doesn't support sending stdout and stderr to
+ ;; separate places.
(setq exit-status
(apply #'call-process-region
(append (list eshell-last-sync-output-start (point)
@@ -392,10 +397,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..fa20efa71e1 100644
--- a/test/lisp/eshell/esh-proc-tests.el
+++ b/test/lisp/eshell/esh-proc-tests.el
@@ -191,6 +191,59 @@ esh-proc-test/pipeline-connection-type/last
(unless (eq system-type 'windows-nt)
"stdout\nstderr\n"))))
+\f
+;; Synchronous processes
+
+;; These tests check that synchronous subprocesses (only used on
+;; MS-DOS by default) work correctly. To help them run on MS-DOS as
+;; well, we use the Emacs executable as our subprocess to test
+;; against; that way, users don't need to have GNU coreutils (or
+;; similar) installed.
+
+(defsubst esh-proc-test/emacs-command (command)
+ "Evaluate COMMAND in a new Emacs batch instance."
+ (mapconcat #'shell-quote-argument
+ `(,(expand-file-name invocation-name invocation-directory)
+ "-Q" "--batch" "--eval" ,(prin1-to-string command))
+ " "))
+
+(defvar esh-proc-test/emacs-echo
+ (esh-proc-test/emacs-command '(princ "hello\n"))
+ "A command that prints \"hello\" to stdout using Emacs.")
+
+(defvar esh-proc-test/emacs-upcase
+ (esh-proc-test/emacs-command
+ '(princ (upcase (concat (read-string "") "\n"))))
+ "A command that upcases the text from stdin using Emacs.")
+
+(ert-deftest esh-proc-test/synchronous-proc/simple/interactive ()
+ "Test that synchronous processes work in an interactive shell."
+ (let ((eshell-supports-asynchronous-processes nil))
+ (with-temp-eshell
+ (eshell-match-command-output esh-proc-test/emacs-echo
+ "\\`hello\n"))))
+
+(ert-deftest esh-proc-test/synchronous-proc/simple/command-result ()
+ "Test that synchronous processes work via `eshell-command-result'."
+ (let ((eshell-supports-asynchronous-processes nil))
+ (eshell-command-result-equal esh-proc-test/emacs-echo
+ "hello\n")))
+
+(ert-deftest esh-proc-test/synchronous-proc/pipeline/interactive ()
+ "Test that synchronous pipelines work in an interactive shell."
+ (let ((eshell-supports-asynchronous-processes nil))
+ (with-temp-eshell
+ (eshell-match-command-output (concat esh-proc-test/emacs-echo " | "
+ esh-proc-test/emacs-upcase)
+ "\\`HELLO\n"))))
+
+(ert-deftest esh-proc-test/synchronous-proc/pipeline/command-result ()
+ "Test that synchronous pipelines work via `eshell-command-result'."
+ (let ((eshell-supports-asynchronous-processes nil))
+ (eshell-command-result-equal (concat esh-proc-test/emacs-echo " | "
+ esh-proc-test/emacs-upcase)
+ "HELLO\n")))
+
\f
;; Killing processes
--
2.25.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* bug#62578: 30.0.50; [PATCH] Add regression tests for synchronous processes in Eshell
2023-04-02 5:51 ` Jim Porter
@ 2023-04-02 21:25 ` Jim Porter
0 siblings, 0 replies; 9+ messages in thread
From: Jim Porter @ 2023-04-02 21:25 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 62578-done
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.
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2023-04-02 21:25 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-04-01 4:41 bug#62578: 30.0.50; [PATCH] Add regression tests for synchronous processes in Eshell Jim Porter
2023-04-01 6:07 ` Eli Zaretskii
2023-04-01 7:16 ` Jim Porter
2023-04-01 7:22 ` Eli Zaretskii
2023-04-01 7:40 ` Jim Porter
2023-04-02 0:58 ` Jim Porter
2023-04-02 5:29 ` Eli Zaretskii
2023-04-02 5:51 ` Jim Porter
2023-04-02 21:25 ` Jim Porter
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.