From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Milan Zimmermann Newsgroups: gmane.emacs.devel Subject: Re: Checking if this is a Eshell bug in emacs 29: Should eshell redirect all output in esh script to stdout Date: Wed, 23 Nov 2022 15:46:50 -0500 Message-ID: References: <82bcaf3d-29a6-c7be-46e9-edafc9c33de1@gmail.com> Mime-Version: 1.0 Content-Type: multipart/alternative; boundary="0000000000006868b805ee296519" Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="36319"; mail-complaints-to="usenet@ciao.gmane.io" Cc: emacs-devel@gnu.org To: Jim Porter Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Thu Nov 24 07:18:17 2022 Return-path: Envelope-to: ged-emacs-devel@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1oy5Yv-0009BZ-ME for ged-emacs-devel@m.gmane-mx.org; Thu, 24 Nov 2022 07:18:17 +0100 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1oy5YO-00058R-On; Thu, 24 Nov 2022 01:17:44 -0500 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1oxwea-0006zp-0l for emacs-devel@gnu.org; Wed, 23 Nov 2022 15:47:32 -0500 Original-Received: from mail-vs1-xe31.google.com ([2607:f8b0:4864:20::e31]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1oxweV-0000Lk-0K for emacs-devel@gnu.org; Wed, 23 Nov 2022 15:47:31 -0500 Original-Received: by mail-vs1-xe31.google.com with SMTP id v128so4072649vsb.13 for ; Wed, 23 Nov 2022 12:47:26 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=cnVdiHMuodE2UTB92efqniM4GDOM230/8sYEFN2vYGA=; b=Ikxnr5w+AYXblwpwloY3NrUVcxvND9Sl2E9KMTHlyg4nRCYXef9Pk1GJ+lf2tQNXhU QZhHoBl6rBUwLnOtos7XwYrfg2fBIcuuacKV1JmEJDsXltQSC1BGi+tQU5FTnIox+Dhz RXAyDdEZMEYlKUcGvQwLCvhIlgT+2xiWQ9PUOseJ3FCGDgIkMdE59+Ud2Y2JR5KLHPCm LtVHnac4KceYNlT4I4ZLotAUg1IOXnencqiUF8M7efCjgcxZLp2dk+J9J9e9OQ7vAyKB c2dxG+rlbXYp6jSkiBvEAxZNv4ULa9h8WaaRPSBKbomTkTCT9NzzGEvIB0L6XkecpQb5 +aQg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=cnVdiHMuodE2UTB92efqniM4GDOM230/8sYEFN2vYGA=; b=Oi3I/IGgtHLhVxuCN7+mOegmrUaOuvzOAsxacjGJdEsrFL5bAl/L7aq0ikK96Mzx5g 4lnFtFYrNQRyHcy/TV1Jrm7TVz5z0DetI3FPZ7dz99mgOpXWTf+vhwgaqDKu5DpGMCHQ bW+AIJZLB67Q/ZsgXnSKnejGvVxgEfA7ObIYCa/kOnmU8mHSypOWU3GDgKA6ruoA6Ozk ASqhs26Gkt10gs5UXKcDffjGTfE5EtJMwXdK6dzOEdIhCjnRQ9x46mvmKUcx5nSuLD+u ox5TT8OUhl/HY6HbsUHGlfk7BTlbaAOWDklK0Nq0VVm+cuBhn4kn1JtPng7aMNle8ZED S2LA== X-Gm-Message-State: ANoB5plulmRtufExQ7tF8gwYoVqVIrPZPhHE35peRL6PliNVhTbnp9KA XjCDUCS3l+QZjb5upbjKTUColJiuMRGXAk2pkcTrX4dZZzM= X-Google-Smtp-Source: AA0mqf7sudQiYqkeq2g0OqKdOm2yJ47EU81FQhUIqiPJ2ISBl8Lxe6kBHIj9/FZvKv339JaILN9BTKjunB7z+vwg2QA= X-Received: by 2002:a67:1a02:0:b0:3a7:ad22:1b2e with SMTP id a2-20020a671a02000000b003a7ad221b2emr6710947vsa.50.1669236445962; Wed, 23 Nov 2022 12:47:25 -0800 (PST) In-Reply-To: <82bcaf3d-29a6-c7be-46e9-edafc9c33de1@gmail.com> Received-SPF: pass client-ip=2607:f8b0:4864:20::e31; envelope-from=milan.zimmermann@gmail.com; helo=mail-vs1-xe31.google.com X-Spam_score_int: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, FREEMAIL_FROM=0.001, HTML_MESSAGE=0.001, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-Mailman-Approved-At: Thu, 24 Nov 2022 01:17:42 -0500 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Original-Sender: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.devel:300426 Archived-At: --0000000000006868b805ee296519 Content-Type: text/plain; charset="UTF-8" Jim, Thanks for the explanation. Well, it looks like I keep running against issues that would likely be gone if https://debbugs.gnu.org/cgi/bugreport.cgi?bug=57635 is addressed. Unfortunately I do not have much confidence in my ability to help ,as it would take me a long time to even get to the point of my Elisp to start attempting. On the limited scope of this output issue: Should I create a bug out of this, or do you want to go ahead and report it? (or something else) Thanks, Milan PS: I needed the stdout and stderr redirection to behave properly to be able to finish my Org-babel ob-eshell project I started a few days ago. There is already an existing ob-eshell, but it does not work for me, so I started a re-do. (I mostly want :results output to work). I see now that effort is on a backburner for now until the redirection works. On Wed, Nov 23, 2022 at 3:30 PM Jim Porter wrote: > On 11/23/2022 10:56 AM, Milan Zimmermann wrote: > > I would like to check here before reporting an Eshell bug. > > > > A script named 'redirect-echo.esh' with two echo commands. > > > > Emacs 29: Sourcing it and redirecting output to a file, results in the > > first echo string ('hello') showing in eshell, only the second > > ('there')(presumably because it is last) showing in the output file: > [snip] > > I believe this is a bug but I am not sure about Eshell's intended > > features, so I wanted to check first. > > This is definitely a bug. I think the best way to fix it though would be > to first replace 'eshell-do-eval', since it's a bit limited in what Lisp > forms it can manage correctly. > > (Warning: the following is based on my memory of looking at this a few > months ago, so I might have misremembered some details. If so, sorry > about that.) > > Currently, Eshell manages output handles in a fairly tricky way: many > internal Eshell forms automatically try to close the handles, so to get > around this, other Eshell forms increment the handles' refcount so that > closing just decrements that count instead of actually closing it. > That's a fine strategy in general, but the increment-refcount > ('eshell-protect') and decrement-refcount-and-close > ('eshell-close-handles') are called very far from each other in the > code, so the logic gets pretty hard to follow, leading to bugs like this. > > I think the best way to fix this would be to put the increment and > decrement code in the same spot, so that you'd do something like so: > > (unwind-protect > (progn > (eshell-protect) > (do-stuff)) > (eshell-close-handles)) > > Unfortunately, if 'do-stuff' calls a deferred command[1], Eshell ends up > evaluating 'eshell-close-handles' immediately, instead of when > 'do-stuff' actually finishes. That means that either a) we'd need to fix > this without using 'unwind-protect', which would be painful to get > right, or b) we'd need to make 'unwind-protect' work as expected. (b) is > effectively bug#57635[2], I think. (That bug suggests using > generator.el's internals to drive Eshell's iterative evaluation.) > > Bug#57635 is a pretty big change too, but I think it would greatly help > with the overall reliability of Eshell. > > [1] Basically, a command that should yield execution back to the rest of > Emacs while waiting on some background task (e.g. a subprocess). > > [2] https://debbugs.gnu.org/cgi/bugreport.cgi?bug=57635 > --0000000000006868b805ee296519 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Jim,

Thanks for the explanation.
<= div>


Thanks,
Milan

PS: I needed the stdout a= nd stderr redirection to behave properly to be able to finish my Org-babel = ob-eshell project I started a few days ago.=C2=A0 There is already an exist= ing ob-eshell, but it does not work for me, so I started a re-do.=C2=A0 (I = mostly want :results output to work). I see now that effort is on a backbur= ner for now until=C2=A0the redirection works.


3D""
<= /div>


On Wed, Nov 23, 2022 at 3:30 PM Jim Porter <jporterbugs@gmail.com> wrote:
<= /div>
On 11/23/2022 10:56 = AM, Milan Zimmermann wrote:
> I would like to check here before reporting an Eshell bug.
>
> A script named 'redirect-echo.esh' with two echo commands.
>
> Emacs 29: Sourcing it and redirecting output to a file, results in the=
> first echo string ('hello') showing in eshell, only the second=
> ('there')(presumably because it is last) showing in the output= file:
[snip]
>=C2=A0 =C2=A0I believe this is a bug but I am not sure about Eshell'= ;s intended
> features, so I wanted to check first.

This is definitely a bug. I think the best way to fix it though would be to first replace 'eshell-do-eval', since it's a bit limited in = what Lisp
forms it can manage correctly.

(Warning: the following is based on my memory of looking at this a few
months ago, so I might have misremembered some details. If so, sorry
about that.)

Currently, Eshell manages output handles in a fairly tricky way: many
internal Eshell forms automatically try to close the handles, so to get around this, other Eshell forms increment the handles' refcount so that=
closing just decrements that count instead of actually closing it.
That's a fine strategy in general, but the increment-refcount
('eshell-protect') and decrement-refcount-and-close
('eshell-close-handles') are called very far from each other in the=
code, so the logic gets pretty hard to follow, leading to bugs like this.
I think the best way to fix this would be to put the increment and
decrement code in the same spot, so that you'd do something like so:
=C2=A0 =C2=A0(unwind-protect
=C2=A0 =C2=A0 =C2=A0 =C2=A0(progn
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(eshell-protect)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(do-stuff))
=C2=A0 =C2=A0 =C2=A0(eshell-close-handles))

Unfortunately, if 'do-stuff' calls a deferred command[1], Eshell en= ds up
evaluating 'eshell-close-handles' immediately, instead of when
'do-stuff' actually finishes. That means that either a) we'd ne= ed to fix
this without using 'unwind-protect', which would be painful to get =
right, or b) we'd need to make 'unwind-protect' work as expecte= d. (b) is
effectively bug#57635[2], I think. (That bug suggests using
generator.el's internals to drive Eshell's iterative evaluation.)
Bug#57635 is a pretty big change too, but I think it would greatly help with the overall reliability of Eshell.

[1] Basically, a command that should yield execution back to the rest of Emacs while waiting on some background task (e.g. a subprocess).

[2] https://debbugs.gnu.org/cgi/bugreport.cgi= ?bug=3D57635
--0000000000006868b805ee296519--