* Re: master 7f631a3e2ac: Allow using multiple buffers in 'eshell-command'
2024-07-06 19:47 ` Jim Porter
@ 2024-07-06 21:17 ` Jim Porter
2024-07-10 0:32 ` Jim Porter
0 siblings, 1 reply; 4+ messages in thread
From: Jim Porter @ 2024-07-06 21:17 UTC (permalink / raw)
To: Stefan Kangas, Thierry Volpiatto; +Cc: emacs-devel
[-- Attachment #1: Type: text/plain, Size: 737 bytes --]
On 7/6/2024 12:47 PM, Jim Porter wrote:
> On 7/6/2024 10:15 AM, Stefan Kangas wrote:
>> Jim Porter <jporterbugs@gmail.com> writes:
>>
>>> branch: master
>>> commit 7f631a3e2aca97e95b8659c902c25ab21f084e08
>>> Author: Thierry Volpiatto <thievol@posteo.net>
>>> Commit: Jim Porter <jporterbugs@gmail.com>
>>>
>>> Allow using multiple buffers in 'eshell-command'
> [snip]
>> This new test is failing here:
>
> Hm, I can't reproduce that, but I'm guessing it's a race condition with
> the process sentinel. Does this make the test pass for you?
Scratch that patch. I looked into this code more, and the following
should (hopefully) fix the test, as well as improving Eshell's behavior
when waiting for processes more generally.
[-- Attachment #2: 0001-Improve-Eshell-s-behavior-when-waiting-for-processes.patch --]
[-- Type: text/plain, Size: 8006 bytes --]
From a7ace1e286333dde0b20b1a6cdff80c5751bcca4 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Sat, 6 Jul 2024 14:09:08 -0700
Subject: [PATCH] Improve Eshell's behavior when waiting for processes
This has a few benefits. First, it fixes a race condition when killing
old processes in 'eshell-command'. Second, the "wait" built-in command
is now more useful. Finally, killing processes when exiting Eshell (via
'eshell-round-robin-kill') should be much faster.
* lisp/eshell/esh-proc.el (esh-opt): Require.
(eshell-wait-for-process): Make obsolete in favor of...
(eshell-wait-for-processes): ... this. Accept a timeout and support
PIDs. Update callers.
(eshell/wait): New implementation accepting -t/--timeout.
(eshell-round-robin-kill): Use 'eshell-wait-for-processes'.
* lisp/eshell/eshell.el (eshell-command): Use 'eshell-round-robin-kill'.
* doc/misc/eshell.texi (List of Built-ins): Document the new "wait"
behavior.
* etc/NEWS: Announce this change.
---
doc/misc/eshell.texi | 7 +++--
etc/NEWS | 6 ++++
lisp/eshell/esh-cmd.el | 2 +-
lisp/eshell/esh-proc.el | 65 +++++++++++++++++++++++++++++------------
lisp/eshell/eshell.el | 7 ++---
5 files changed, 62 insertions(+), 25 deletions(-)
diff --git a/doc/misc/eshell.texi b/doc/misc/eshell.texi
index 69f94fab469..8547131194e 100644
--- a/doc/misc/eshell.texi
+++ b/doc/misc/eshell.texi
@@ -1201,8 +1201,11 @@ List of Built-ins
@cmindex wait
@cindex processes, waiting for
-@item wait [@var{process}]@dots{}
-Wait until each specified @var{process} has exited.
+@item wait [-t @var{timeout}] [@var{process}]@dots{}
+Wait until each specified @var{process} has exited. Processes can
+either be process objects (@pxref{Processes, , , elisp, GNU Emacs Lisp
+Reference Manual}) or integer PIDs. If you pass @code{-t} or
+@code{--timeout}, wait at most that many seconds before exiting.
@cmindex which
@item which @var{command}@dots{}
diff --git a/etc/NEWS b/etc/NEWS
index ba58fa7b319..720c194e094 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -54,6 +54,12 @@ this will prompt for confirmation before creating a new buffer when
necessary. To restore the previous behavior, set this option to
'confirm-kill-process'.
++++
+*** Eshell's built-in "wait" command now accepts a timeout.
+By passing "-t" or "--timeout", you can specify a maximum time to wait
+for the processes to exit. Additionally, you can now wait for external
+processes by passing their PIDs.
+
** SHR
+++
diff --git a/lisp/eshell/esh-cmd.el b/lisp/eshell/esh-cmd.el
index aaae19df5d6..f086aa20181 100644
--- a/lisp/eshell/esh-cmd.el
+++ b/lisp/eshell/esh-cmd.el
@@ -1300,7 +1300,7 @@ eshell-do-eval
(if-let (((memq (car form) eshell-deferrable-commands))
(procs (eshell-make-process-list result)))
(if synchronous-p
- (apply #'eshell/wait procs)
+ (funcall #'eshell-wait-for-processes procs)
(eshell-manipulate form "inserting ignore form"
(setcar form 'ignore)
(setcdr form nil))
diff --git a/lisp/eshell/esh-proc.el b/lisp/eshell/esh-proc.el
index a5e9de79907..94be0ee1d3c 100644
--- a/lisp/eshell/esh-proc.el
+++ b/lisp/eshell/esh-proc.el
@@ -25,6 +25,7 @@
(require 'esh-arg)
(require 'esh-io)
+(require 'esh-opt)
(require 'esh-util)
(require 'pcomplete)
@@ -184,16 +185,46 @@ eshell-process-active-p
;; cleared out the handles (see `eshell-sentinel').
(process-get process :eshell-handles)))
-(defun eshell-wait-for-process (&rest procs)
- "Wait until PROCS have successfully completed."
- (dolist (proc procs)
- (when (eshell-processp proc)
- (while (eshell-process-active-p proc)
- (when (input-pending-p)
- (discard-input))
- (sit-for eshell-process-wait-time)))))
+(defun eshell-wait-for-processes (&optional procs timeout)
+ "Wait until PROCS have completed execution.
+If TIMEOUT is non-nil, wait at most that many seconds. Return non-nil
+if all the processes finished executing before the timeout expired."
+ (let ((expiration (when timeout (time-add (current-time) timeout))))
+ (dolist (proc procs)
+ (unless (or (processp proc) (natnump proc))
+ (error "Invalid process %S" proc)))
+ (catch 'timeout
+ (dolist (proc procs)
+ (while (if (processp proc)
+ (eshell-process-active-p proc)
+ (process-attributes proc))
+ (when (input-pending-p)
+ (discard-input))
+ (when (and expiration
+ (not (time-less-p (current-time) expiration)))
+ (throw 'timeout nil))
+ (sit-for eshell-process-wait-time)))
+ t)))
-(defalias 'eshell/wait #'eshell-wait-for-process)
+(defun eshell-wait-for-process (&rest procs)
+ "Wait until PROCS have completed execution."
+ (declare (obsolete 'eshell-wait-for-processes "31.1"))
+ (eshell-wait-for-processes procs))
+
+(defun eshell/wait (&rest args)
+ "Wait until processes have completed execution."
+ (eshell-eval-using-options
+ "wait" args
+ '((?h "help" nil nil "show this usage screen")
+ (?t "timeout" t timeout "timeout in seconds")
+ :preserve-args
+ :show-usage
+ :usage "[OPTION] PROCESS...
+Wait until PROCESS(es) have completed execution.")
+ (when (stringp timeout)
+ (setq timeout (string-to-number timeout)))
+ (unless (eshell-wait-for-processes args timeout)
+ (error "wait: timed out after %s seconds" timeout))))
(defun eshell/jobs ()
"List processes, if there are any."
@@ -621,16 +652,14 @@ eshell-kill-processes-on-exit
(defun eshell-round-robin-kill (&optional query)
"Kill current process by trying various signals in sequence.
See the variable `eshell-kill-processes-on-exit'."
- (let ((sigs eshell-kill-process-signals))
- (while sigs
+ (catch 'done
+ (dolist (sig eshell-kill-process-signals)
(eshell-process-interact
- (lambda (proc)
- (signal-process (process-id proc) (car sigs))) t query)
- (setq query nil)
- (if (not eshell-process-list)
- (setq sigs nil)
- (sleep-for eshell-kill-process-wait-time)
- (setq sigs (cdr sigs))))))
+ (lambda (proc) (signal-process proc sig)) t query)
+ (when (eshell-wait-for-processes (mapcar #'car eshell-process-list)
+ eshell-kill-process-wait-time)
+ (throw 'done nil))
+ (setq query nil))))
(defun eshell-query-kill-processes ()
"Kill processes belonging to the current Eshell buffer, possibly with query."
diff --git a/lisp/eshell/eshell.el b/lisp/eshell/eshell.el
index 568f6745067..b7be3dd1643 100644
--- a/lisp/eshell/eshell.el
+++ b/lisp/eshell/eshell.el
@@ -176,7 +176,7 @@
(require 'cl-lib))
(require 'esh-util)
(require 'esh-module) ;For eshell-using-module
-(require 'esh-proc) ;For eshell-wait-for-process
+(require 'esh-proc) ;For eshell-wait-for-processes
(require 'esh-io) ;For eshell-last-command-status
(require 'esh-cmd)
@@ -357,8 +357,7 @@ eshell-command
(with-current-buffer bufname
;; Stop all the processes in the old buffer (there may
;; be several).
- (eshell-process-interact #'interrupt-process t))
- (accept-process-output)
+ (eshell-round-robin-kill))
(kill-buffer bufname))
((eq eshell-command-async-buffer 'confirm-new-buffer)
(shell-command--same-buffer-confirm "Use a new buffer")
@@ -377,7 +376,7 @@ eshell-command
;; make the output as attractive as possible, with no
;; extraneous newlines
(unless async
- (apply #'eshell-wait-for-process (cadr eshell-foreground-command))
+ (funcall #'eshell-wait-for-processes (cadr eshell-foreground-command))
(cl-assert (not eshell-foreground-command))
(goto-char (point-max))
(while (and (bolp) (not (bobp)))
--
2.25.1
^ permalink raw reply related [flat|nested] 4+ messages in thread