[bug] $SHLVL decremented for the last command of a subshell

[bug] $SHLVL decremented for the last command of a subshell

[Stephane Chazelas]

(from https://unix.stackexchange.com/questions/304917/detect-when-zsh-is-running-within-the-subshell-of-another-shell)

$ printenv SHLVL
1
$ (printenv SHLVL)
0
$ (printenv SHLVL;:)
1
$ (:;printenv SHLVL)
0


http://www.zsh.org/mla/workers/1999/msg02472.html
sounds like a potential culprit.

-- 
Stephane

[PATCH] [bug] $SHLVL decremented for the last command of a subshell

[Stephane Chazelas]

2016-08-22 07:17:23 +0100, Stephane Chazelas:
> (from https://unix.stackexchange.com/questions/304917/detect-when-zsh-is-running-within-the-subshell-of-another-shell)
> 
> $ printenv SHLVL
> 1
> $ (printenv SHLVL)
> 0
> $ (printenv SHLVL;:)
> 1
> $ (:;printenv SHLVL)
> 0
> 
> 
> http://www.zsh.org/mla/workers/1999/msg02472.html
> sounds like a potential culprit.
[...]

I think the change below should be enough (also condensed the 2
"if"s below (unrelated)):

diff --git a/Src/exec.c b/Src/exec.c
index 9b24d38..54ececc 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -3694,14 +3694,12 @@ execcmd(Estate state, int input, int output, int how, int last1)
 		restore_params(restorelist, removelist);
 
 	} else {
-	    if (!forked)
+	    if (!forked && !subsh)
 		setiparam("SHLVL", --shlvl);
-	    if (do_exec) {
+	    if (do_exec && !subsh && isset(RCS) && interact && !nohistsave)
 		/* If we are exec'ing a command, and we are not *
 		 * in a subshell, then save the history file.   */
-		if (!subsh && isset(RCS) && interact && !nohistsave)
-		    savehistfile(NULL, 1, HFILE_USE_OPTIONS);
-	    }
+	        savehistfile(NULL, 1, HFILE_USE_OPTIONS);
 	    if (type == WC_SIMPLE || type == WC_TYPESET) {
 		if (varspc) {
 		    int addflags = ADDVAR_EXPORT|ADDVAR_RESTRICT;

Re: [PATCH] [bug] $SHLVL decremented for the last command of a subshell

[Bart Schaefer]

On Aug 29,  4:51pm, Stephane Chazelas wrote:
}
} I think the change below should be enough (also condensed the 2
} "if"s below (unrelated)):

This looks fine, although I think we generally keep the braces on the
"if" body whenever there is more than one line following -- even if
some of those lines are comments.

Either that or move the comment above the "if".

[PATCHv2] [bug] $SHLVL decremented for the last command of a subshell

[Stephane Chazelas]

2016-08-29 18:55:05 -0700, Bart Schaefer:
> On Aug 29,  4:51pm, Stephane Chazelas wrote:
> }
> } I think the change below should be enough (also condensed the 2
> } "if"s below (unrelated)):
> 
> This looks fine, although I think we generally keep the braces on the
> "if" body whenever there is more than one line following -- even if
> some of those lines are comments.
> 
> Either that or move the comment above the "if".

How about this (factorised the !subsh some more comments and
added some test cases):

diff --git a/Src/exec.c b/Src/exec.c
index 9b24d38..2e251b9 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -3694,12 +3694,15 @@ execcmd(Estate state, int input, int output, int how, int last1)
 		restore_params(restorelist, removelist);
 
 	} else {
-	    if (!forked)
-		setiparam("SHLVL", --shlvl);
-	    if (do_exec) {
+	    if (!subsh) {
+	        /* for either implicit or explicit "exec", decrease $SHLVL
+		 * as we're now done as a shell */
+		if (!forked)
+		    setiparam("SHLVL", --shlvl);
+
 		/* If we are exec'ing a command, and we are not *
 		 * in a subshell, then save the history file.   */
-		if (!subsh && isset(RCS) && interact && !nohistsave)
+		if (do_exec && isset(RCS) && interact && !nohistsave)
 		    savehistfile(NULL, 1, HFILE_USE_OPTIONS);
 	    }
 	    if (type == WC_SIMPLE || type == WC_TYPESET) {
diff --git a/Test/D04parameter.ztst b/Test/D04parameter.ztst
index 0630079..75ace5a 100644
--- a/Test/D04parameter.ztst
+++ b/Test/D04parameter.ztst
@@ -1718,6 +1718,24 @@
 >2
 >2
 
+  SHLVL=1
+  $ZTST_testdir/../Src/zsh -fc 'sh -c "echo \$SHLVL"'
+  $ZTST_testdir/../Src/zsh -fc '(sh -c "echo \$SHLVL")'
+  $ZTST_testdir/../Src/zsh -fc '( (sh -c "echo \$SHLVL"))'
+0:SHLVL decremented upon implicit exec optimisation
+>1
+>1
+>1
+
+  SHLVL=1
+  $ZTST_testdir/../Src/zsh -fc '(sh -c "echo \$SHLVL"); exit'
+  $ZTST_testdir/../Src/zsh -fc '(exec sh -c "echo \$SHLVL"); exit'
+  $ZTST_testdir/../Src/zsh -fc '( (sh -c "echo \$SHLVL"); exit)'
+0:SHLVL not decremented upon exec in subshells
+>2
+>2
+>2
+
 # The following tests the return behaviour of parsestr/parsestrnoerr
   alias param-test-alias='print $'\''\x45xpanded in substitution'\' 
   param='$(param-test-alias)'

Re: [PATCHv2] [bug] $SHLVL decremented for the last command of a subshell

[Peter Stephenson]

On Tue, 30 Aug 2016 13:44:26 +0100
Stephane Chazelas <stephane.chazelas@gmail.com> wrote:
> 2016-08-29 18:55:05 -0700, Bart Schaefer:
> > On Aug 29,  4:51pm, Stephane Chazelas wrote:
> > }
> > } I think the change below should be enough (also condensed the 2
> > } "if"s below (unrelated)):
> > 
> > This looks fine, although I think we generally keep the braces on the
> > "if" body whenever there is more than one line following -- even if
> > some of those lines are comments.
> > 
> > Either that or move the comment above the "if".
> 
> How about this (factorised the !subsh some more comments and
> added some test cases):

This is probably fine.  I've been dithering over cases where for
consistency we might want to decrement SHLVL even in a subshell because
we're exec'ing for some other reason (so would need to do it at the last
minute before exiting if the exec is faked), but I suspect this is too
obscure to be worth special treatment.

pws

Re: [PATCHv2] [bug] $SHLVL decremented for the last command of a subshell

[Stephane Chazelas]

2016-08-30 14:04:57 +0100, Peter Stephenson:
[...]
> This is probably fine.  I've been dithering over cases where for
> consistency we might want to decrement SHLVL even in a subshell because
> we're exec'ing for some other reason (so would need to do it at the last
> minute before exiting if the exec is faked), but I suspect this is too
> obscure to be worth special treatment.
[...]

Note that zsh is currently correctly decrementing in

zsh -c '(printenv SHLVL)'

Where zsh optimises out the fork(s).

ksh93 seems to never decrease $SHLVL even in:

$ env SHLVL=1 ksh93 -c 'exec printenv SHLVL'
2

bash only optimises out the fork in the

bash -c 'single command'

case and gets it wrong:

$ env SHLVL=1  bash -c 'printenv SHLVL'
2
$ env SHLVL=1  bash -c 'exec printenv SHLVL'
1

tcsh doesn't opimise out any fork AFAICT.

Other shells don't seem to support $SHLVL.

-- 
Stephane

Re: [PATCHv2] [bug] $SHLVL decremented for the last command of a subshell

[Peter Stephenson]

On Tue, 30 Aug 2016 17:46:01 +0100
Stephane Chazelas <stephane.chazelas@gmail.com> wrote:

> 2016-08-30 14:04:57 +0100, Peter Stephenson:
> [...]
> > This is probably fine.  I've been dithering over cases where for
> > consistency we might want to decrement SHLVL even in a subshell because
> > we're exec'ing for some other reason (so would need to do it at the last
> > minute before exiting if the exec is faked), but I suspect this is too
> > obscure to be worth special treatment.
> [...]
> 
> Note that zsh is currently correctly decrementing in
> 
> zsh -c '(printenv SHLVL)'
> 
> Where zsh optimises out the fork(s).
> 
> ksh93 seems to never decrease $SHLVL even in:
> 
> $ env SHLVL=1 ksh93 -c 'exec printenv SHLVL'
> 2

When this was introduced in zsh, there was some controversy over whether
it was the right thing to do.  I decided it was on the basis that the
"level" is how deep you are in the process hierarchy, rather than how
many processes have previously been executed to get where you are,
seeing as the latter isn't really visible in the current context.
But I'm not aware of any requirement specification [remember those?]
saying what the purpose of SHLVL is.

pws

Re: [PATCHv2] [bug] $SHLVL decremented for the last command of a subshell

[Mikael Magnusson]

On Tue, Aug 30, 2016 at 7:00 PM, Peter Stephenson
<p.stephenson@samsung.com> wrote:
> On Tue, 30 Aug 2016 17:46:01 +0100
> Stephane Chazelas <stephane.chazelas@gmail.com> wrote:
>
>> 2016-08-30 14:04:57 +0100, Peter Stephenson:
>> [...]
>> > This is probably fine.  I've been dithering over cases where for
>> > consistency we might want to decrement SHLVL even in a subshell because
>> > we're exec'ing for some other reason (so would need to do it at the last
>> > minute before exiting if the exec is faked), but I suspect this is too
>> > obscure to be worth special treatment.
>> [...]
>>
>> Note that zsh is currently correctly decrementing in
>>
>> zsh -c '(printenv SHLVL)'
>>
>> Where zsh optimises out the fork(s).
>>
>> ksh93 seems to never decrease $SHLVL even in:
>>
>> $ env SHLVL=1 ksh93 -c 'exec printenv SHLVL'
>> 2
>
> When this was introduced in zsh, there was some controversy over whether
> it was the right thing to do.  I decided it was on the basis that the
> "level" is how deep you are in the process hierarchy, rather than how
> many processes have previously been executed to get where you are,
> seeing as the latter isn't really visible in the current context.
> But I'm not aware of any requirement specification [remember those?]
> saying what the purpose of SHLVL is.

For me personally, I just use it in my prompt to indicate whether
typing "exit" will close the terminal or not, eg if I'm inside a
nested actual shell. When I added that I wasn't even aware of any of
the weird rules regarding subshells affecting it. It's not immediately
obvious to me when that is useful... Regarding the link in the
original report, there's no guarantee that a "2" means it _is_ a
subshell either, just doing for example "xterm &" will also increment
it in the new session.

-- 
Mikael Magnusson

Re: [PATCHv2] [bug] $SHLVL decremented for the last command of a subshell

[Stephane Chazelas]

2016-08-30 18:00:32 +0100, Peter Stephenson:
[...]
> When this was introduced in zsh, there was some controversy over whether
> it was the right thing to do.  I decided it was on the basis that the
> "level" is how deep you are in the process hierarchy, rather than how
> many processes have previously been executed to get where you are,
> seeing as the latter isn't really visible in the current context.
> But I'm not aware of any requirement specification [remember those?]
> saying what the purpose of SHLVL is.
[...]

Yes, it's not POSIX. It was probably either a tcsh or bash
invention as both had it in 1990 and ksh88 didn't and still
doesn't have it.

And zsh behaves like tcsh and bash here (in that SHLVL is
decreased in zsh -c 'exec cmd').

-- 
Stephane

Re: [PATCHv2] [bug] $SHLVL decremented for the last command of a subshell

[Stephane Chazelas]

2016-08-30 20:54:29 +0100, Stephane Chazelas:
[...]
> Yes, it's not POSIX. It was probably either a tcsh or bash
> invention as both had it in 1990 and ksh88 didn't and still
> doesn't have it.
[...]

tcsh.

https://groups.google.com/forum/#!original/net.sources/WdFUCQb7vr0/o3NafIxCUV4J
https://groups.google.com/forum/#!original/net.sources/VpHdbFdEi0w/a_AgLdbJbxkJ
(1984) a patch for tcsh with SHLVL referenced.

Strangely though, tcsh 5.7 from 1987 didn't have it.

-- 
Stephane

Re: [PATCHv2] [bug] $SHLVL decremented for the last command of a subshell

[Peter Stephenson]

On Tue, 30 Aug 2016 20:34:34 +0200
Mikael Magnusson <mikachu@gmail.com> wrote:
> > When this was introduced in zsh, there was some controversy over whether
> > it was the right thing to do.  I decided it was on the basis that the
> > "level" is how deep you are in the process hierarchy, rather than how
> > many processes have previously been executed to get where you are,
> > seeing as the latter isn't really visible in the current context.
> > But I'm not aware of any requirement specification [remember those?]
> > saying what the purpose of SHLVL is.
> 
> For me personally, I just use it in my prompt to indicate whether
> typing "exit" will close the terminal or not, eg if I'm inside a
> nested actual shell. When I added that I wasn't even aware of any of
> the weird rules regarding subshells affecting it. It's not immediately
> obvious to me when that is useful...

There are no weird *rules* regarding subshells and SHLVL; the rule is
SHLVL should indicate how deeply nested the parent shell is in process
terms.  The effect with subshells was a bug because we do a fake
exec (i.e. we don't bother forking for the new last command) with the
last process in a subshell because we know the current process
will come to an end anyway.

The decrement is needed in general on a real exec because you are
creating a new shell which isn't more deeply nested, i.e. if you do
"exec zsh" from the command line you (probably) expect SHLVL to look the
same since the nesting level hasn't changed. If everything is working
this is the sole effect (but process management is complicated enough
real life isn't necessarily that simple).

The final question was whether after the fix there was any remaining
business to do with the combination of execs and subshells, but it
doesn't look like there is anything significant.

pws

Re: [PATCHv2] [bug] $SHLVL decremented for the last command of a subshell

[Peter Stephenson]

On Tue, 30 Aug 2016 13:44:26 +0100
Stephane Chazelas <stephane.chazelas@gmail.com> wrote:
> +  SHLVL=1
> +  $ZTST_testdir/../Src/zsh -fc 'sh -c "echo \$SHLVL"'
> +  $ZTST_testdir/../Src/zsh -fc '(sh -c "echo \$SHLVL")'
> +  $ZTST_testdir/../Src/zsh -fc '( (sh -c "echo \$SHLVL"))'
> +0:SHLVL decremented upon implicit exec optimisation
> +>1
> +>1
> +>1
> +
> +  SHLVL=1
> +  $ZTST_testdir/../Src/zsh -fc '(sh -c "echo \$SHLVL"); exit'
> +  $ZTST_testdir/../Src/zsh -fc '(exec sh -c "echo \$SHLVL"); exit'
> +  $ZTST_testdir/../Src/zsh -fc '( (sh -c "echo \$SHLVL"); exit)'
> +0:SHLVL not decremented upon exec in subshells
> +>2
> +>2
> +>2
> +

I should have looked harder before committing... unfortunately, we can't
rely on sh *not* incrementing SHLVL because it might be bash.

The path of least resistance seems to be use to zsh, who's behaviour we
(think we) know, and take the hit of the extra increment, so I've added
notes.

pws

diff --git a/Test/D04parameter.ztst b/Test/D04parameter.ztst
index 75ace5a..e779aab 100644
--- a/Test/D04parameter.ztst
+++ b/Test/D04parameter.ztst
@@ -1718,24 +1718,26 @@
 >2
 >2
 
+  # SHLVL is incremented twice and decremented once in between.
   SHLVL=1
-  $ZTST_testdir/../Src/zsh -fc 'sh -c "echo \$SHLVL"'
-  $ZTST_testdir/../Src/zsh -fc '(sh -c "echo \$SHLVL")'
-  $ZTST_testdir/../Src/zsh -fc '( (sh -c "echo \$SHLVL"))'
+  $ZTST_testdir/../Src/zsh -fc $ZTST_testdir/../Src/zsh' -fc "echo \$SHLVL"'
+  $ZTST_testdir/../Src/zsh -fc '('$ZTST_testdir/../Src/zsh' -fc "echo \$SHLVL")'
+  $ZTST_testdir/../Src/zsh -fc '( ('$ZTST_testdir/../Src/zsh' -fc "echo \$SHLVL"))'
 0:SHLVL decremented upon implicit exec optimisation
->1
->1
->1
-
-  SHLVL=1
-  $ZTST_testdir/../Src/zsh -fc '(sh -c "echo \$SHLVL"); exit'
-  $ZTST_testdir/../Src/zsh -fc '(exec sh -c "echo \$SHLVL"); exit'
-  $ZTST_testdir/../Src/zsh -fc '( (sh -c "echo \$SHLVL"); exit)'
-0:SHLVL not decremented upon exec in subshells
 >2
 >2
 >2
 
+  # SHLVL is incremented twice with no decrement in between.
+  SHLVL=1
+  $ZTST_testdir/../Src/zsh -fc '('$ZTST_testdir/../Src/zsh' -fc "echo \$SHLVL"); exit'
+  $ZTST_testdir/../Src/zsh -fc '(exec '$ZTST_testdir/../Src/zsh' -fc "echo \$SHLVL"); exit'
+  $ZTST_testdir/../Src/zsh -fc '( ('$ZTST_testdir/../Src/zsh' -fc "echo \$SHLVL"); exit)'
+0:SHLVL not decremented upon exec in subshells
+>3
+>3
+>3
+
 # The following tests the return behaviour of parsestr/parsestrnoerr
   alias param-test-alias='print $'\''\x45xpanded in substitution'\' 
   param='$(param-test-alias)'

Re: [PATCHv2] [bug] $SHLVL decremented for the last command of a subshell

[Stephane Chazelas]

2016-08-31 11:06:20 +0100, Peter Stephenson:
[...]
> I should have looked harder before committing... unfortunately, we can't
> rely on sh *not* incrementing SHLVL because it might be bash.
[...]
> -  $ZTST_testdir/../Src/zsh -fc 'sh -c "echo \$SHLVL"'
[...]
> +  $ZTST_testdir/../Src/zsh -fc $ZTST_testdir/../Src/zsh' -fc "echo \$SHLVL"'
[...]

Oops, sorry. That was very silly of me.

Note that it's not just bash, it's also ksh93 (as in Solaris 11
/bin/sh) or busybox sh.

Another approach could be to use "printenv" but I guess that's
not portable enough.

Note that the change also fixes the problem of SHLVL being
decremented in:

zsh -c '(exec printenv SHLVL); exit'

Which also affects bash and tcsh:

https://lists.gnu.org/archive/html/bug-bash/2016-09/msg00000.html
http://bugs.gw.com/view.php?id=572

(trying posting via gmane, as the zsh.org mta seems to
spam-blacklist some of gmail's server IP addresses)

-- 
Stephane