Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: implement $SHLVL and $BASH_SUBSHELL #251

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 31 additions & 1 deletion brush-core/src/shell.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,11 +79,17 @@ pub struct Shell {

impl Clone for Shell {
fn clone(&self) -> Self {
let mut env = self.env.clone();

if !self.options.sh_mode {
increment_level_var(&mut env, "BASH_SUBSHELL", 1, false).unwrap();
}

Self {
traps: self.traps.clone(),
open_files: self.open_files.clone(),
working_dir: self.working_dir.clone(),
env: self.env.clone(),
env,
funcs: self.funcs.clone(),
options: self.options.clone(),
jobs: jobs::JobManager::new(),
Expand Down Expand Up @@ -279,7 +285,13 @@ impl Shell {
)?;
}

increment_level_var(&mut env, "SHLVL", 1, true)?;

if !options.sh_mode {
let mut lvl = ShellVariable::new("0".into());
lvl.export();
env.set_global("BASH_SUBSHELL", lvl)?;

if let Some(shell_name) = &options.shell_name {
env.set_global("BASH", ShellVariable::new(shell_name.into()))?;
}
Expand Down Expand Up @@ -1182,3 +1194,21 @@ fn parse_string_impl(
tracing::debug!(target: trace_categories::PARSE, "Parsing string as program...");
parser.parse(true)
}

// parse a numeric variable from a string, increment it by 1, then re-export
fn increment_level_var(
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shell.rs is already, well, longer than I'd like. Could you turn this into a pub(crate) helper in variables.rs or similar?

For what it's worth, it seems more understandable to me to have the default value expressing the pre-increment default (i.e., 0 here).

env: &mut ShellEnvironment,
var: &str,
default: usize,
export: bool,
) -> Result<(), error::Error> {
let lvl = env
.get_str(var)
.and_then(|s| s.parse::<usize>().ok().map(|n| n + 1))
.unwrap_or(default);
let mut lvl = ShellVariable::new(ShellValue::String(lvl.to_string()));
if export {
lvl.export();
}
env.set_global(var, lvl)
}
34 changes: 34 additions & 0 deletions brush-shell/tests/cases/compound_cmds/subshell.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,37 @@ cases:
- name: "Piped subshell usage"
stdin: |
(echo hi) | wc -l

- name: "SHLVL subshell"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like we need some tests to ensure that SHLVL will work correctly across process invocations (i.e., nested shells and not just subshells). Thoughts on how to do that? Nested invocation of $0? Call through bash in the middle?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@reubeno I thought about it and created a test using $SHELL but is points to nowhere inside the Oracle. Now, looking at it, I learned about $0, so I will try using that instead. I'm not sure yet how interactivity works there.

$0
$0
$0 -c 'echo $SHLVL; $0'

stdin: |
initial="$SHLVL"
function rec_subshell {
if [ "$1" -ge "$2" ]; then
exit
else
(
test $initial -eq $SHLVL; echo $?
rec_subshell $(($1 + 1)) $2
)
fi
}
rec_subshell 1 10


- name: "BASH_SUBSHELL"
stdin: |
initial="$BASH_SUBSHELL"

function rec_subshell {
if [ "$1" -ge "$2" ]; then
exit
else
(
test $initial -ne $BASH_SUBSHELL; echo $?
echo $BASH_SUBSHELL
initial=$BASH_SUBSHELL
rec_subshell $(($1 + 1)) $2
)
fi
}
rec_subshell 1 10
Loading