Don't touch $SHLVL if not interactive

It's not super clear what $SHLVL is useful for, but the current
definition is essentially
"number of shells in the parent processes + 1"

which isn't *super useful*?

Bash's behavior here is a bit weird in that it increments $SHLVL
basically always, but since it auto-execs the last process it will
decrement it again, so in practice it's often not incremented.

E.g.

```
> echo $SHLVL
1
> bash -c 'echo $SHLVL; bash'
2
>> echo $SHLVL
2
```

Both bashes here end up having the same $SHLVL because this is
equivalent to `echo $SHLVL; exec bash`. Running `echo $SHLVL` and then
`bash -c 'echo $SHLVL'` in an interactive bash will have a different
result (1 and 2) because that doesn't *exec* the inner bash.

That's not something we want to get into, so what we do is increment
$SHLVL in every interactive fish. Non-interactive fish will simply
import the existing value.

That means if you had e.g. a bash that runs a fish script that ends up
opening a new fish session, you would have a $SHLVL of *2* - one for the
bash, and one for the inner fish.

We key this off is_interactive_session() (which can also be enabled
via `fish -i`) because it's easy and because `fish -i` is asking for
fish to be, in some form, "interactive".

That means most of the time $SHLVL will be "how many shells am I deep,
how often do I have to `exit`", except for when you specifically asked
for a fish to be "interactive". If that's a problem, we can rethink it.

Fixes #7864.
This commit is contained in:
Fabian Homborg 2021-03-29 17:35:55 +02:00
parent f1c93a99f9
commit e1d19cf571
4 changed files with 40 additions and 24 deletions

View File

@ -1248,7 +1248,7 @@ Fish also provides additional information through the values of certain environm
- ``pipestatus``, a list of exit statuses of all processes that made up the last executed pipe.
- ``SHLVL``, the level of nesting of shells.
- ``SHLVL``, the level of nesting of shells. Fish increments this in interactive shells, otherwise it simply passes it along.
- ``status``, the `exit status <#variables-status>`_ of the last foreground job to exit. If the job was terminated through a signal, the exit status will be 128 plus the signal number.

View File

@ -351,16 +351,23 @@ void env_init(const struct config_paths_t *paths /* or NULL */) {
// Set up SHLVL variable. Not we can't use vars.get() because SHLVL is read-only, and therefore
// was not inherited from the environment.
wcstring nshlvl_str = L"1";
if (const char *shlvl_var = getenv("SHLVL")) {
const wchar_t *end;
// TODO: Figure out how to handle invalid numbers better. Shouldn't we issue a diagnostic?
long shlvl_i = fish_wcstol(str2wcstring(shlvl_var).c_str(), &end);
if (!errno && shlvl_i >= 0) {
nshlvl_str = to_string(shlvl_i + 1);
if (is_interactive_session()) {
wcstring nshlvl_str = L"1";
if (const char *shlvl_var = getenv("SHLVL")) {
const wchar_t *end;
// TODO: Figure out how to handle invalid numbers better. Shouldn't we issue a diagnostic?
long shlvl_i = fish_wcstol(str2wcstring(shlvl_var).c_str(), &end);
if (!errno && shlvl_i >= 0) {
nshlvl_str = to_string(shlvl_i + 1);
}
}
vars.set_one(L"SHLVL", ENV_GLOBAL | ENV_EXPORT, nshlvl_str);
} else {
// If we're not interactive, simply pass the value along.
if (const char *shlvl_var = getenv("SHLVL")) {
vars.set_one(L"SHLVL", ENV_GLOBAL | ENV_EXPORT, str2wcstring(shlvl_var));
}
}
vars.set_one(L"SHLVL", ENV_GLOBAL | ENV_EXPORT, nshlvl_str);
// initialize the PWD variable if necessary
// Note we may inherit a virtual PWD that doesn't match what getcwd would return; respect that

View File

@ -240,15 +240,17 @@ static void internal_exec(env_stack_t &vars, job_t *j, const io_chain_t &block_i
dup2_list_t redirs = dup2_list_t::resolve_chain(all_ios);
if (child_setup_process(INVALID_PID, INVALID_PID, *j, false, redirs) == 0) {
// Decrement SHLVL as we're removing ourselves from the shell "stack".
auto shlvl_var = vars.get(L"SHLVL", ENV_GLOBAL | ENV_EXPORT);
wcstring shlvl_str = L"0";
if (shlvl_var) {
long shlvl = fish_wcstol(shlvl_var->as_string().c_str());
if (!errno && shlvl > 0) {
shlvl_str = to_string(shlvl - 1);
if (is_interactive_session()) {
auto shlvl_var = vars.get(L"SHLVL", ENV_GLOBAL | ENV_EXPORT);
wcstring shlvl_str = L"0";
if (shlvl_var) {
long shlvl = fish_wcstol(shlvl_var->as_string().c_str());
if (!errno && shlvl > 0) {
shlvl_str = to_string(shlvl - 1);
}
}
vars.set_one(L"SHLVL", ENV_GLOBAL | ENV_EXPORT, std::move(shlvl_str));
}
vars.set_one(L"SHLVL", ENV_GLOBAL | ENV_EXPORT, std::move(shlvl_str));
// launch_process _never_ returns.
launch_process_nofork(vars, p);

View File

@ -347,25 +347,32 @@ $FISH -c 'set -q testu; or echo testu undef in sub shell'
# test SHLVL
# use a subshell to ensure a clean slate
env SHLVL= $FISH -c 'echo SHLVL: $SHLVL; $FISH -c \'echo SHLVL: $SHLVL\''
env SHLVL= $FISH -ic 'echo SHLVL: $SHLVL; $FISH -ic \'echo SHLVL: $SHLVL\''
# CHECK: SHLVL: 1
# CHECK: SHLVL: 2
# exec should decrement SHLVL
env SHLVL= $FISH -c 'echo SHLVL: $SHLVL; exec $FISH -c \'echo SHLVL: $SHLVL\''
# CHECK: SHLVL: 1
# CHECK: SHLVL: 1
# exec should decrement SHLVL - outer fish increments by 1, decrements for exec,
# inner fish increments again so the value stays the same.
env SHLVL=1 $FISH -ic 'echo SHLVL: $SHLVL; exec $FISH -ic \'echo SHLVL: $SHLVL\''
# CHECK: SHLVL: 2
# CHECK: SHLVL: 2
# garbage SHLVLs should be treated as garbage
env SHLVL=3foo $FISH -c 'echo SHLVL: $SHLVL'
env SHLVL=3foo $FISH -ic 'echo SHLVL: $SHLVL'
# CHECK: SHLVL: 1
# whitespace is allowed though (for bash compatibility)
env SHLVL="3 " $FISH -c 'echo SHLVL: $SHLVL'
env SHLVL=" 3" $FISH -c 'echo SHLVL: $SHLVL'
env SHLVL="3 " $FISH -ic 'echo SHLVL: $SHLVL'
env SHLVL=" 3" $FISH -ic 'echo SHLVL: $SHLVL'
# CHECK: SHLVL: 4
# CHECK: SHLVL: 4
# Non-interactive fish doesn't touch $SHLVL
env SHLVL=2 $FISH -c 'echo SHLVL: $SHLVL'
# CHECK: SHLVL: 2
env SHLVL=banana $FISH -c 'echo SHLVL: $SHLVL'
# CHECK: SHLVL: banana
# Test transformation of inherited variables
env DISPLAY="localhost:0.0" $FISH -c 'echo Elements in DISPLAY: (count $DISPLAY)'
# CHECK: Elements in DISPLAY: 1