* [bug] $SHLVL decremented for the last command of a subshell @ 2016-08-22 6:17 Stephane Chazelas 2016-08-29 15:51 ` [PATCH] " Stephane Chazelas 0 siblings, 1 reply; 13+ messages in thread From: Stephane Chazelas @ 2016-08-22 6:17 UTC (permalink / raw) To: Zsh hackers list (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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH] [bug] $SHLVL decremented for the last command of a subshell 2016-08-22 6:17 [bug] $SHLVL decremented for the last command of a subshell Stephane Chazelas @ 2016-08-29 15:51 ` Stephane Chazelas 2016-08-30 1:55 ` Bart Schaefer 0 siblings, 1 reply; 13+ messages in thread From: Stephane Chazelas @ 2016-08-29 15:51 UTC (permalink / raw) To: Zsh hackers list 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; ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] [bug] $SHLVL decremented for the last command of a subshell 2016-08-29 15:51 ` [PATCH] " Stephane Chazelas @ 2016-08-30 1:55 ` Bart Schaefer 2016-08-30 12:44 ` [PATCHv2] " Stephane Chazelas 0 siblings, 1 reply; 13+ messages in thread From: Bart Schaefer @ 2016-08-30 1:55 UTC (permalink / raw) To: Zsh hackers list 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". ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCHv2] [bug] $SHLVL decremented for the last command of a subshell 2016-08-30 1:55 ` Bart Schaefer @ 2016-08-30 12:44 ` Stephane Chazelas 2016-08-30 13:04 ` Peter Stephenson 2016-08-31 10:06 ` Peter Stephenson 0 siblings, 2 replies; 13+ messages in thread From: Stephane Chazelas @ 2016-08-30 12:44 UTC (permalink / raw) To: Bart Schaefer; +Cc: Zsh hackers list 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)' ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCHv2] [bug] $SHLVL decremented for the last command of a subshell 2016-08-30 12:44 ` [PATCHv2] " Stephane Chazelas @ 2016-08-30 13:04 ` Peter Stephenson 2016-08-30 16:46 ` Stephane Chazelas 2016-08-31 10:06 ` Peter Stephenson 1 sibling, 1 reply; 13+ messages in thread From: Peter Stephenson @ 2016-08-30 13:04 UTC (permalink / raw) To: Zsh hackers list 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCHv2] [bug] $SHLVL decremented for the last command of a subshell 2016-08-30 13:04 ` Peter Stephenson @ 2016-08-30 16:46 ` Stephane Chazelas 2016-08-30 17:00 ` Peter Stephenson 0 siblings, 1 reply; 13+ messages in thread From: Stephane Chazelas @ 2016-08-30 16:46 UTC (permalink / raw) To: Peter Stephenson; +Cc: Zsh hackers list 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCHv2] [bug] $SHLVL decremented for the last command of a subshell 2016-08-30 16:46 ` Stephane Chazelas @ 2016-08-30 17:00 ` Peter Stephenson 2016-08-30 18:34 ` Mikael Magnusson 2016-08-30 19:54 ` Stephane Chazelas 0 siblings, 2 replies; 13+ messages in thread From: Peter Stephenson @ 2016-08-30 17:00 UTC (permalink / raw) To: Zsh hackers list 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCHv2] [bug] $SHLVL decremented for the last command of a subshell 2016-08-30 17:00 ` Peter Stephenson @ 2016-08-30 18:34 ` Mikael Magnusson 2016-08-31 9:02 ` Peter Stephenson 2016-08-30 19:54 ` Stephane Chazelas 1 sibling, 1 reply; 13+ messages in thread From: Mikael Magnusson @ 2016-08-30 18:34 UTC (permalink / raw) To: Peter Stephenson; +Cc: Zsh hackers list 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCHv2] [bug] $SHLVL decremented for the last command of a subshell 2016-08-30 18:34 ` Mikael Magnusson @ 2016-08-31 9:02 ` Peter Stephenson 0 siblings, 0 replies; 13+ messages in thread From: Peter Stephenson @ 2016-08-31 9:02 UTC (permalink / raw) To: Zsh hackers list 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCHv2] [bug] $SHLVL decremented for the last command of a subshell 2016-08-30 17:00 ` Peter Stephenson 2016-08-30 18:34 ` Mikael Magnusson @ 2016-08-30 19:54 ` Stephane Chazelas 2016-08-30 20:10 ` Stephane Chazelas 1 sibling, 1 reply; 13+ messages in thread From: Stephane Chazelas @ 2016-08-30 19:54 UTC (permalink / raw) To: Peter Stephenson; +Cc: Zsh hackers list 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCHv2] [bug] $SHLVL decremented for the last command of a subshell 2016-08-30 19:54 ` Stephane Chazelas @ 2016-08-30 20:10 ` Stephane Chazelas 0 siblings, 0 replies; 13+ messages in thread From: Stephane Chazelas @ 2016-08-30 20:10 UTC (permalink / raw) To: Peter Stephenson, Zsh hackers list 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCHv2] [bug] $SHLVL decremented for the last command of a subshell 2016-08-30 12:44 ` [PATCHv2] " Stephane Chazelas 2016-08-30 13:04 ` Peter Stephenson @ 2016-08-31 10:06 ` Peter Stephenson 2016-09-04 5:32 ` Stephane Chazelas 1 sibling, 1 reply; 13+ messages in thread From: Peter Stephenson @ 2016-08-31 10:06 UTC (permalink / raw) To: Zsh hackers list 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)' ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCHv2] [bug] $SHLVL decremented for the last command of a subshell 2016-08-31 10:06 ` Peter Stephenson @ 2016-09-04 5:32 ` Stephane Chazelas 0 siblings, 0 replies; 13+ messages in thread From: Stephane Chazelas @ 2016-09-04 5:32 UTC (permalink / raw) To: zsh-workers 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2016-09-04 5:35 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-08-22 6:17 [bug] $SHLVL decremented for the last command of a subshell Stephane Chazelas 2016-08-29 15:51 ` [PATCH] " Stephane Chazelas 2016-08-30 1:55 ` Bart Schaefer 2016-08-30 12:44 ` [PATCHv2] " Stephane Chazelas 2016-08-30 13:04 ` Peter Stephenson 2016-08-30 16:46 ` Stephane Chazelas 2016-08-30 17:00 ` Peter Stephenson 2016-08-30 18:34 ` Mikael Magnusson 2016-08-31 9:02 ` Peter Stephenson 2016-08-30 19:54 ` Stephane Chazelas 2016-08-30 20:10 ` Stephane Chazelas 2016-08-31 10:06 ` Peter Stephenson 2016-09-04 5:32 ` Stephane Chazelas
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).