* Failed pipeline doesn't err_exit/return if complex command last
@ 2023-07-05 23:18 Johan Grande
2023-07-05 23:45 ` Johan Grande
2023-07-06 9:44 ` Peter Stephenson
0 siblings, 2 replies; 10+ messages in thread
From: Johan Grande @ 2023-07-05 23:18 UTC (permalink / raw)
To: zsh-workers
Hi all,
I noticed a strange behavior when using a complex command in a pipeline.
With `set -eo pipefail`, a failing pipeline that ends in a complex
command will have non-zero status but not exit the script (or a function
with err_return):
false | if true; then true; fi
echo $?
=> prints "1", exits with 0
However, this only occurs if the `if` command is last. If we add a `|
true` at the end:
false | if true; then true; fi | true
echo $?
=> no output, exits with 1
Same with a while loop (I only checked these 2 complex commands):
false | while read line; do true; done
echo $?
=> prints "1", exits with 0
false | while read line; do true; done | true
echo $?
=> no output, exits with 1
It seems inconsistent to me that a complex command be treated just like
a simple command in the middle of a pipeline but not at the end, making
a "list" that has non-zero status but doesn't return/exit despite the
options.
As a sanity check, I ran the examples with bash and the result is what I
would expect in all 4 cases: no output, exit with 1.
I'm using zsh 5.8.1 (x86_64-ubuntu-linux-gnu).
--
Johan Grande
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Failed pipeline doesn't err_exit/return if complex command last
2023-07-05 23:18 Failed pipeline doesn't err_exit/return if complex command last Johan Grande
@ 2023-07-05 23:45 ` Johan Grande
2023-07-06 9:44 ` Peter Stephenson
1 sibling, 0 replies; 10+ messages in thread
From: Johan Grande @ 2023-07-05 23:45 UTC (permalink / raw)
To: zsh-workers
Le 06/07/2023 à 01:18, Johan Grande a écrit :
> With `set -eo pipefail`, a failing pipeline that ends in a complex
> command will have non-zero status but not exit the script (or a function
> with err_return):
Note: This happens even if the complex command is inside a function:
f() { { true; true } }
false | f
echo $?
=> prints "1", exits with 0
f() { true }
false | f
echo $?
=> no output, exits with 1
--
Johan
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Failed pipeline doesn't err_exit/return if complex command last
2023-07-05 23:18 Failed pipeline doesn't err_exit/return if complex command last Johan Grande
2023-07-05 23:45 ` Johan Grande
@ 2023-07-06 9:44 ` Peter Stephenson
2023-07-08 4:31 ` Bart Schaefer
` (2 more replies)
1 sibling, 3 replies; 10+ messages in thread
From: Peter Stephenson @ 2023-07-06 9:44 UTC (permalink / raw)
To: Johan Grande, zsh-workers
> On 06/07/2023 00:18 Johan Grande <nahoj@crans.org> wrote:
> I noticed a strange behavior when using a complex command in a pipeline.
>
> With `set -eo pipefail`, a failing pipeline that ends in a complex
> command will have non-zero status but not exit the script (or a function
> with err_return):
So in a nutshell
fn() {
emulate -L zsh
setopt errreturn pipefail
false | { true }
print "Shouldn't get here, status $?"
}
The final part of the pipeline is run in the current shell, so the
job control is a bit fiddly. It looks like the relevant code,
including the function storepipestats(), is run too late in this
case.
pws
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Failed pipeline doesn't err_exit/return if complex command last
2023-07-06 9:44 ` Peter Stephenson
@ 2023-07-08 4:31 ` Bart Schaefer
[not found] ` <cf275400-d63a-d991-3aa5-1543d20a2f42@crans.org>
2023-07-17 23:34 ` Bart Schaefer
2 siblings, 0 replies; 10+ messages in thread
From: Bart Schaefer @ 2023-07-08 4:31 UTC (permalink / raw)
To: Peter Stephenson; +Cc: Johan Grande, zsh-workers
On Thu, Jul 6, 2023 at 2:44 AM Peter Stephenson
<p.w.stephenson@ntlworld.com> wrote:
>
> The final part of the pipeline is run in the current shell, so the
> job control is a bit fiddly. It looks like the relevant code,
> including the function storepipestats(), is run too late in this
> case.
Possibly relevant, I'm getting random failures of the expected output
status for E01 "Was testing: PIPE_FAIL option" on MacOS Catalina. The
test line
true | false | true
sometimes returns 147 instead of 1.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Failed pipeline doesn't err_exit/return if complex command last
[not found] ` <cf275400-d63a-d991-3aa5-1543d20a2f42@crans.org>
@ 2023-07-16 14:36 ` Peter Stephenson
2023-07-17 19:25 ` Johan Grande
0 siblings, 1 reply; 10+ messages in thread
From: Peter Stephenson @ 2023-07-16 14:36 UTC (permalink / raw)
To: Zsh hackers list
On Fri, 2023-07-14 at 20:31 +0200, Johan Grande wrote:
> Le 06/07/2023 à 11:44, Peter Stephenson a écrit :
> > > On 06/07/2023 00:18 Johan Grande <nahoj@crans.org> wrote:
> > > I noticed a strange behavior when using a complex command in a pipeline.
> > So in a nutshell
>
> I'm curious about the process: whether the bug is likely going to be
> fixed, and whether it's tracked somewhere public.
>
> No urgency or pressure in my request, I'm just happy to follow up.
There's nowhere to look, other than this list --- if someone here doesn't
pick it up, no one is going to.
This area of the code has given a number of us sleepless nights over the
years, with many hours spent trying to resolve subtle job control issues,
so unless someone happens to spot some key clue, I doubt it's going anywhere
very fast. Every now and then there are bursts of activity, however.
Cheers
pws
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Failed pipeline doesn't err_exit/return if complex command last
2023-07-16 14:36 ` Peter Stephenson
@ 2023-07-17 19:25 ` Johan Grande
0 siblings, 0 replies; 10+ messages in thread
From: Johan Grande @ 2023-07-17 19:25 UTC (permalink / raw)
To: zsh-workers
Le 16/07/2023 à 16:36, Peter Stephenson a écrit :
> On Fri, 2023-07-14 at 20:31 +0200, Johan Grande wrote:
>> Le 06/07/2023 à 11:44, Peter Stephenson a écrit :
>>>> On 06/07/2023 00:18 Johan Grande <nahoj@crans.org> wrote:
>>>> I noticed a strange behavior when using a complex command in a pipeline.
>>> So in a nutshell
>>
>> I'm curious about the process: whether the bug is likely going to be
>> fixed, and whether it's tracked somewhere public.
>>
>> No urgency or pressure in my request, I'm just happy to follow up.
>
> There's nowhere to look, other than this list --- if someone here doesn't
> pick it up, no one is going to.
>
> This area of the code has given a number of us sleepless nights over the
> years, with many hours spent trying to resolve subtle job control issues,
> so unless someone happens to spot some key clue, I doubt it's going anywhere
> very fast. Every now and then there are bursts of activity, however.
Understood, thank you for your answer.
In the meantime, the problem is easily worked around by suffixing every
pipeline with || return $?.
--
Johan
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Failed pipeline doesn't err_exit/return if complex command last
2023-07-06 9:44 ` Peter Stephenson
2023-07-08 4:31 ` Bart Schaefer
[not found] ` <cf275400-d63a-d991-3aa5-1543d20a2f42@crans.org>
@ 2023-07-17 23:34 ` Bart Schaefer
2023-07-18 1:51 ` Bart Schaefer
2 siblings, 1 reply; 10+ messages in thread
From: Bart Schaefer @ 2023-07-17 23:34 UTC (permalink / raw)
To: Peter Stephenson; +Cc: Johan Grande, zsh-workers
On Thu, Jul 6, 2023 at 2:44 AM Peter Stephenson
<p.w.stephenson@ntlworld.com> wrote:
>
\> fn() {
> emulate -L zsh
> setopt errreturn pipefail
> false | { true }
> print "Shouldn't get here, status $?"
> }
>
> The final part of the pipeline is run in the current shell, so the
> job control is a bit fiddly. It looks like the relevant code,
> including the function storepipestats(), is run too late in this
> case.
It's not that it's run too late, it's that it's run too SOON:
pipestatus has already been updated and lastval set, but then we get
here:
4186 if (!errflag) {
4187 int ret = execbuiltin(args, assigns, (Builtin) hn);
4188 /*
4189 * In case of interruption assume builtin status
4190 * is less useful than what interrupt set.
4191 */
4192 if (!(errflag & ERRFLAG_INT))
4193 lastval = ret;
4194 }
and clobber lastval with the result of execbuiltin(). Or that's how
it currently appears from pass through gdb. Per "fiddly" it's a bit
hard to be sure.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Failed pipeline doesn't err_exit/return if complex command last
2023-07-17 23:34 ` Bart Schaefer
@ 2023-07-18 1:51 ` Bart Schaefer
2023-07-18 13:57 ` Peter Stephenson
0 siblings, 1 reply; 10+ messages in thread
From: Bart Schaefer @ 2023-07-18 1:51 UTC (permalink / raw)
To: Peter Stephenson; +Cc: Johan Grande, zsh-workers
On Mon, Jul 17, 2023 at 4:34 PM Bart Schaefer <schaefer@brasslantern.com> wrote:
>
> It's not that it's run too late, it's that it's run too SOON:
> pipestatus has already been updated and lastval set, but then
No, I'm wrong. Sorry for the noise.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Failed pipeline doesn't err_exit/return if complex command last
2023-07-18 1:51 ` Bart Schaefer
@ 2023-07-18 13:57 ` Peter Stephenson
2023-07-20 8:47 ` Peter Stephenson
0 siblings, 1 reply; 10+ messages in thread
From: Peter Stephenson @ 2023-07-18 13:57 UTC (permalink / raw)
To: Johan Grande, zsh-workers
> fn () {
> emulate -L zsh
> setopt errreturn pipefail
> false | {
> true
> }
> print "Shouldn't get here, status $?"
> }
A bit more prodding reveals it's not actually timing that
has this effect --- the problem is this_noerrexit being
set to indicate something like "we're doing that complicated
internal shell stuff and we've discovered the status is OK".
This time it isn['t.
Not sure if I need the second case or not, certainly
the first case is the one we're hitting here (the
STAT_CURSH is the giveaway). My guess is that it's
simply irrelevant because if there's no STAT_CURSH
we aren't going to hit those cases where this_noerrexit
becomes non-zero.
Obviously, there could be even more complicated cases that
still cause problems, but I think we'll just have to discover
those. I'll write some tests that might help.
Note I fixed some indentation.
pws
diff --git a/Src/jobs.c b/Src/jobs.c
index dd7bba405..a3b9f667a 100644
--- a/Src/jobs.c
+++ b/Src/jobs.c
@@ -427,11 +427,17 @@ storepipestats(Job jn, int inforeground, int fixlastval)
}
if (fixlastval) {
- if (jn->stat & STAT_CURSH) {
- if (!lastval && isset(PIPEFAIL))
- lastval = pipefail;
- } else if (isset(PIPEFAIL))
- lastval = pipefail;
+ if (jn->stat & STAT_CURSH) {
+ if (!lastval && isset(PIPEFAIL)) {
+ if (inforeground)
+ this_noerrexit = 0;
+ lastval = pipefail;
+ }
+ } else if (isset(PIPEFAIL)) {
+ if (inforeground)
+ this_noerrexit = 0;
+ lastval = pipefail;
+ }
}
}
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Failed pipeline doesn't err_exit/return if complex command last
2023-07-18 13:57 ` Peter Stephenson
@ 2023-07-20 8:47 ` Peter Stephenson
0 siblings, 0 replies; 10+ messages in thread
From: Peter Stephenson @ 2023-07-20 8:47 UTC (permalink / raw)
To: Johan Grande, zsh-workers
> On 18/07/2023 14:57 Peter Stephenson <p.w.stephenson@ntlworld.com> wrote:
> > fn () {
> > emulate -L zsh
> > setopt errreturn pipefail
> > false | {
> > true
> > }
> > print "Shouldn't get here, status $?"
> > }
>
> A bit more prodding reveals it's not actually timing that
> has this effect --- the problem is this_noerrexit being
> set to indicate something like "we're doing that complicated
> internal shell stuff and we've discovered the status is OK".
> This time it isn['t.
>
> Obviously, there could be even more complicated cases that
> still cause problems, but I think we'll just have to discover
> those. I'll write some tests that might help.
Here's the patch with some tests.
pws
diff --git a/Src/jobs.c b/Src/jobs.c
index dd7bba405..a3b9f667a 100644
--- a/Src/jobs.c
+++ b/Src/jobs.c
@@ -427,11 +427,17 @@ storepipestats(Job jn, int inforeground, int fixlastval)
}
if (fixlastval) {
- if (jn->stat & STAT_CURSH) {
- if (!lastval && isset(PIPEFAIL))
- lastval = pipefail;
- } else if (isset(PIPEFAIL))
- lastval = pipefail;
+ if (jn->stat & STAT_CURSH) {
+ if (!lastval && isset(PIPEFAIL)) {
+ if (inforeground)
+ this_noerrexit = 0;
+ lastval = pipefail;
+ }
+ } else if (isset(PIPEFAIL)) {
+ if (inforeground)
+ this_noerrexit = 0;
+ lastval = pipefail;
+ }
}
}
diff --git a/Test/E01options.ztst b/Test/E01options.ztst
index 533e08773..83f0371a1 100644
--- a/Test/E01options.ztst
+++ b/Test/E01options.ztst
@@ -1379,6 +1379,64 @@ F:Regression test for workers/41811
>1
>2
+ pipefailfn1() {
+ emulate -L zsh
+ setopt errreturn pipefail
+ false | { true; }
+ print "Shouldn't get here, status $?"
+ }
+ pipefailfn1
+1:PIPE_FAIL causes ERR_RETURN with complex end of pipeline: braces
+
+ pipefailfn2() {
+ emulate -L zsh
+ setopt errreturn pipefail
+ false | if true; then true; fi
+ print "Shouldn't get here, status $?"
+ }
+ pipefailfn2 || print Function failed, as expected
+0:PIPE_FAIL causes ERR_RETURN with complex end of pipeline: if
+>Function failed, as expected
+
+ pipefailfn3() {
+ emulate -L zsh
+ setopt errreturn pipefail
+ false | while true; do break; done
+ print "Shouldn't get here, status $?"
+ }
+ pipefailfn3 || print Function failed, as expected
+0:PIPE_FAIL causes ERR_RETURN with complex end of pipeline: while
+>Function failed, as expected
+
+ pipefailfn4() {
+ emulate -L zsh
+ setopt errreturn pipefail
+ false | true
+ print "Shouldn't get here, status $?"
+ }
+ pipefailfn4
+1:PIPE_FAIL causes ERR_RETURN in simple case
+
+ pipefailfn5() {
+ emulate -L zsh
+ setopt errreturn pipefail
+ false | { true | true; }
+ print "Shouldn't get here, status $?"
+ }
+ pipefailfn5 || print Function failed as expected
+0:PIPE_FAIL causes ERR_RETURN with nested successful pipe
+>Function failed as expected
+
+ pipefailfn6() {
+ emulate -L zsh
+ setopt errreturn pipefail
+ false | { false | true; }
+ print "Shouldn't get here, status $?"
+ }
+ pipefailfn6 || print Function failed as expected
+0:PIPE_FAIL causes ERR_RETURN with nested failed pipe
+>Function failed as expected
+
for (( i = 0; i < 10; i++ )); do
() {
print $i
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2023-07-20 8:48 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-07-05 23:18 Failed pipeline doesn't err_exit/return if complex command last Johan Grande
2023-07-05 23:45 ` Johan Grande
2023-07-06 9:44 ` Peter Stephenson
2023-07-08 4:31 ` Bart Schaefer
[not found] ` <cf275400-d63a-d991-3aa5-1543d20a2f42@crans.org>
2023-07-16 14:36 ` Peter Stephenson
2023-07-17 19:25 ` Johan Grande
2023-07-17 23:34 ` Bart Schaefer
2023-07-18 1:51 ` Bart Schaefer
2023-07-18 13:57 ` Peter Stephenson
2023-07-20 8:47 ` Peter Stephenson
Code repositories for project(s) associated with this public inbox
https://git.vuxu.org/mirror/zsh/
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).