GNU bug report logs - #55590
29.0.50; Eshell subcommands clobber pipelines and produce incorrect output

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 55590 in the body.
You can then email your comments to 55590 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Jim Porter <jporterbugs <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 29.0.50; Eshell subcommands clobber pipelines and produce incorrect
 output
Date: Sun, 22 May 2022 20:34:57 -0700

Starting from `emacs -Q -f eshell', enter the following command:

  echo ${*echo hi} | rev

The output is:

  ~ $ ih

That is, the output of the command ended up *after* the prompt, when it 
should be before. This usually happens in cases where Eshell thinks all 
the subprocesses in a command have finished, but there's actually one 
still running. In this case, it's because the ${SUBCOMMAND} clobbers the 
headproc/tailproc values being recorded for the (outer) pipeline. Both 
the top-level command and the subcommand call `eshell-execute-pipeline', 
and the function isn't properly reentrant.

Patch forthcoming momentarily. I'm just getting a bug number first.

Message #8 received at 55590 <at> debbugs.gnu.org (full text, mbox):

From: Jim Porter <jporterbugs <at> gmail.com>
To: 55590 <at> debbugs.gnu.org
Subject: Re: bug#55590: [PATCH] 29.0.50; Eshell subcommands clobber pipelines
 and produce incorrect output
Date: Sun, 22 May 2022 20:43:28 -0700

[Message part 1 (text/plain, inline)]

On 5/22/2022 8:34 PM, Jim Porter wrote:
> Starting from `emacs -Q -f eshell', enter the following command:
> 
>    echo ${*echo hi} | rev
> 
> The output is:
> 
>    ~ $ ih
> 
> That is, the output of the command ended up *after* the prompt, when it 
> should be before.

The attached patch fixes this issue. It might not be the *best* way to 
do it, but it's the least-invasive I could come up with. The 
`make-symbol' dance is a bit awkward, but Eshell already uses it for 
`eshell-command-to-value' / `eshell-do-command-to-value', so hopefully 
it's ok. I'm open to other solutions though. It's possible the bug is 
really in `eshell-do-eval', since I'd expect the original code to work, 
but `eshell-do-eval' is pretty tricky, and I don't want to poke at it 
too hard for fear that other things will break.

Long-term, the best way to fix this might be to rip out `eshell-do-eval' 
entirely, which iteratively evaluates parts of Eshell commands (so as 
not to hang Emacs) and use the generator.el machinery instead. I looked 
into that briefly, and it seems like it would be quite a bit of work. 
And I don't fully understand generator.el's implementation yet anyway...

[0001-Keep-subcommands-in-pipelines-from-clobbering-the-he.patch (text/plain, attachment)]

Message #11 received at 55590 <at> debbugs.gnu.org (full text, mbox):

From: Sean Whitton <spwhitton <at> spwhitton.name>
To: Jim Porter <jporterbugs <at> gmail.com>, 55590 <at> debbugs.gnu.org
Subject: Re: bug#55590: [PATCH] 29.0.50; Eshell subcommands clobber
 pipelines and produce incorrect output
Date: Sun, 22 May 2022 22:39:18 -0700

Hello Jim,

On Sun 22 May 2022 at 08:43pm -07, Jim Porter wrote:

> On 5/22/2022 8:34 PM, Jim Porter wrote:
>> Starting from `emacs -Q -f eshell', enter the following command:
>>
>>    echo ${*echo hi} | rev
>>
>> The output is:
>>
>>    ~ $ ih
>>
>> That is, the output of the command ended up *after* the prompt, when it
>> should be before.
>
> The attached patch fixes this issue. It might not be the *best* way to
> do it, but it's the least-invasive I could come up with. The
> `make-symbol' dance is a bit awkward, but Eshell already uses it for
> `eshell-command-to-value' / `eshell-do-command-to-value', so hopefully
> it's ok.

Couldn't you just defvar a couple of vars for this purpose?

-- 
Sean Whitton

Message #14 received at 55590 <at> debbugs.gnu.org (full text, mbox):

From: Jim Porter <jporterbugs <at> gmail.com>
To: Sean Whitton <spwhitton <at> spwhitton.name>, 55590 <at> debbugs.gnu.org
Subject: Re: bug#55590: [PATCH v2] 29.0.50; Eshell subcommands clobber
 pipelines and produce incorrect output
Date: Mon, 23 May 2022 08:41:14 -0700

[Message part 1 (text/plain, inline)]

On 5/22/2022 10:39 PM, Sean Whitton wrote:
> On Sun 22 May 2022 at 08:43pm -07, Jim Porter wrote:
>> On 5/22/2022 8:34 PM, Jim Porter wrote:
>>> Starting from `emacs -Q -f eshell', enter the following command:
>>>
>>>     echo ${*echo hi} | rev
>>>
>>> The output is:
>>>
>>>     ~ $ ih
>>>
>>> That is, the output of the command ended up *after* the prompt, when it
>>> should be before.
[snip]
> 
> Couldn't you just defvar a couple of vars for this purpose?

Do you mean just defvar-ing `headproc' and `tailproc' (and renaming 
them, of course)? That works for my example above, but falls apart for 
more complex cases such as:

  echo ${*echo hi | rev} | tr a-z A-Z

The Eshell manual also says the following command is broken (output 
comes after the prompt). It works correctly with my patch, but just 
adding defvars isn't enough:

  for i in 1 2 3 { grep -q a b && *echo hi } | wc -l

The "Command evaluation macros" comment section in 
lisp/eshell/esh-cmd.el indicates that there are some odd limitations on 
these macros, so that might be part of the issue. I'm still a bit 
confused as to why the dance in my patch is necessary, but I'm ok with 
any implementation that passes the tests.

Attached is an updated patch with an additional test for the 
more-complex case and the manual entry about this bug removed.

[0001-Keep-subcommands-in-pipelines-from-clobbering-the-he.patch (text/plain, attachment)]

Message #17 received at 55590 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Jim Porter <jporterbugs <at> gmail.com>
Cc: 55590 <at> debbugs.gnu.org, Sean Whitton <spwhitton <at> spwhitton.name>
Subject: Re: bug#55590: 29.0.50; Eshell subcommands clobber pipelines and
 produce incorrect output
Date: Tue, 24 May 2022 14:58:22 +0200

Jim Porter <jporterbugs <at> gmail.com> writes:

> Attached is an updated patch with an additional test for the
> more-complex case and the manual entry about this bug removed.

Thanks; pushed to Emacs 29.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Message #22 received at 55590 <at> debbugs.gnu.org (full text, mbox):

From: Jim Porter <jporterbugs <at> gmail.com>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: Sean Whitton <spwhitton <at> spwhitton.name>, 55590 <at> debbugs.gnu.org
Subject: Re: bug#55590: 29.0.50; Eshell subcommands clobber pipelines and
 produce incorrect output
Date: Tue, 24 May 2022 19:27:31 -0700

On 5/24/2022 5:58 AM, Lars Ingebrigtsen wrote:
> Jim Porter <jporterbugs <at> gmail.com> writes:
> 
>> Attached is an updated patch with an additional test for the
>> more-complex case and the manual entry about this bug removed.
> 
> Thanks; pushed to Emacs 29.

Thanks for merging; of course, if anyone has a cleaner implementation 
that passes the tests, I'd be happy to see that merge too. I don't quite 
understand all the intricacies of Eshell's command evaluation macros...

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.