Re: esh-proc test failures

[Jim Porter <jporterbugs@gmail.com>]

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

On 8/14/2022 11:44 AM, Jim Porter wrote:
> On 8/14/2022 11:06 AM, Lars Ingebrigtsen wrote:
>> Some esh-proc tests seem to be failing here:
>>
>> https://hydra.nixos.org/build/187349852/nixlog/1
>>
>> And I've seen the same myself, but intermittently, so it seems like some
>> of the (new) tests there aren't quite stable.
> 
>  From Hydra, it seems that the issue is that there's a broken pipe, so 
> there's probably a timing issue with how the external process runs vs 
> Eshell's internal code. I'll try to reproduce this locally and fix it.

Ok, after stepping away for a bit and letting this bubble around in my 
head, the issue turned out to be pretty simple (I think). Attached is a 
patch that should fix this.

Here's what's going on. Imagine you have a pipeline like 'lisp-command | 
external-command'. It could operate in this order:

   * Start external-command
   * Send output of lisp-command to external-command
   * Finish external-command

That's good. However, it could also do this:

   * Start external-command
   * Finish external-command
   * Send output of lisp-command to external-command -> SIGPIPE!

The fix is to make sure that lisp commands also handle SIGPIPE (aka 
'eshell-broken-pipe'), just like how external commands do.

This is actually a bug introduced from the patch for bug#54062, but 
luckily the tests I added for bug#56025 revealed the issue.

[-- Attachment #2: 0001-Handle-eshell-pipe-broken-when-evaluating-Lisp-forms.patch --]
[-- Type: text/plain, Size: 2147 bytes --]

From 7360ecfc73781c83dc9f6c55c7cb7366e5472895 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Mon, 22 Aug 2022 09:53:24 -0700
Subject: [PATCH] Handle 'eshell-pipe-broken' when evaluating Lisp forms in
 Eshell

* lisp/eshell/esh-cmd.el (eshell-exec-lisp): Handle
'eshell-pipe-broken'.

* test/lisp/eshell/esh-proc-tests.el
(esh-proc-test/pipeline-connection-type/middle)
(esh-proc-test/pipeline-connection-type/last): Remove ':unstable'.
---
 lisp/eshell/esh-cmd.el             | 4 ++++
 test/lisp/eshell/esh-proc-tests.el | 4 ----
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/lisp/eshell/esh-cmd.el b/lisp/eshell/esh-cmd.el
index 2f77f3f497..d5cc3706fd 100644
--- a/lisp/eshell/esh-cmd.el
+++ b/lisp/eshell/esh-cmd.el
@@ -1347,6 +1347,10 @@ eshell-exec-lisp
                  (apply func-or-form args)))))
         (and result (funcall printer result))
         result)
+    (eshell-pipe-broken
+     ;; 141 is 128 + 13 (the numeric value of SIGPIPE).
+     (setq eshell-last-command-status 141)
+     nil)
     (error
      (setq eshell-last-command-status 1)
      (let ((msg (error-message-string err)))
diff --git a/test/lisp/eshell/esh-proc-tests.el b/test/lisp/eshell/esh-proc-tests.el
index 62e784e8f6..2369bb5cc0 100644
--- a/test/lisp/eshell/esh-proc-tests.el
+++ b/test/lisp/eshell/esh-proc-tests.el
@@ -74,8 +74,6 @@ esh-proc-test/pipeline-connection-type/first
 (ert-deftest esh-proc-test/pipeline-connection-type/middle ()
   "Test that all streams are pipes when a command is in the middle of a
 pipeline."
-  ;; Repeated unreproducible errors.
-  :tags '(:unstable)
   (skip-unless (and (executable-find "sh")
                     (executable-find "cat")))
   (eshell-command-result-equal
@@ -84,8 +82,6 @@ esh-proc-test/pipeline-connection-type/middle
 
 (ert-deftest esh-proc-test/pipeline-connection-type/last ()
   "Test that only output streams are PTYs when a command ends a pipeline."
-  ;; Repeated unreproducible errors.
-  :tags '(:unstable)
   (skip-unless (executable-find "sh"))
   (eshell-command-result-equal
    (concat "echo | " esh-proc-test--detect-pty-cmd)
-- 
2.25.1