Skip to content

Commit

Permalink
fix(exit): correct exit semantics in various compund statements (#347)
Browse files Browse the repository at this point in the history
* Adjust several cases of compound commands where the exit shell request wasn't propagated.
* Add a number of compat test cases to confirm broader behavior.
  • Loading branch information
reubeno authored Jan 21, 2025
1 parent 0cf4779 commit a36cb56
Show file tree
Hide file tree
Showing 5 changed files with 100 additions and 8 deletions.
10 changes: 9 additions & 1 deletion brush-core/src/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -547,7 +547,15 @@ pub(crate) async fn invoke_shell_function(
// Restore positional parameters.
context.shell.positional_parameters = prior_positional_params;

Ok(CommandSpawnResult::ImmediateExit(result?.exit_code))
// Get the actual execution result from the body of the function.
let result = result?;

// Report back the exit code, and honor any requests to exit the whole shell.
Ok(if result.exit_shell {
CommandSpawnResult::ExitShell(result.exit_code)
} else {
CommandSpawnResult::ImmediateExit(result.exit_code)
})
}

pub(crate) async fn invoke_command_in_subshell_and_get_output(
Expand Down
22 changes: 15 additions & 7 deletions brush-core/src/interp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ impl Execute for ast::CompoundList {
}

// Check for early return.
if result.return_from_function_or_script {
if result.exit_shell || result.return_from_function_or_script {
break;
}

Expand Down Expand Up @@ -525,7 +525,11 @@ impl Execute for ast::CompoundCommand {
ast::CompoundCommand::Subshell(ast::SubshellCommand(s)) => {
// Clone off a new subshell, and run the body of the subshell there.
let mut subshell = shell.clone();
s.execute(&mut subshell, params).await
let subshell_result = s.execute(&mut subshell, params).await?;

// Preserve the subshell's exit code, but don't honor any of its requests to exit
// the shell, break out of loops, etc.
Ok(ExecutionResult::new(subshell_result.exit_code))
}
ast::CompoundCommand::ForClause(f) => f.execute(shell, params).await,
ast::CompoundCommand::CaseClause(c) => c.execute(shell, params).await,
Expand Down Expand Up @@ -578,7 +582,7 @@ impl Execute for ast::ForClauseCommand {
)?;

result = self.body.0.execute(shell, params).await?;
if result.return_from_function_or_script {
if result.exit_shell || result.return_from_function_or_script {
break;
}

Expand Down Expand Up @@ -721,16 +725,20 @@ impl Execute for (WhileOrUntil, &ast::WhileOrUntilClauseCommand) {
loop {
let condition_result = test_condition.execute(shell, params).await?;

if condition_result.is_success() != is_while {
if condition_result.exit_shell || condition_result.return_from_function_or_script {
result.exit_code = condition_result.exit_code;
result.exit_shell = condition_result.exit_shell;
result.return_from_function_or_script =
condition_result.return_from_function_or_script;
break;
}

if condition_result.return_from_function_or_script {
if condition_result.is_success() != is_while {
break;
}

result = body.0.execute(shell, params).await?;
if result.return_from_function_or_script {
if result.exit_shell || result.return_from_function_or_script {
break;
}

Expand Down Expand Up @@ -796,7 +804,7 @@ impl Execute for ast::ArithmeticForClauseCommand {
}

result = self.body.0.execute(shell, params).await?;
if result.return_from_function_or_script {
if result.exit_shell || result.return_from_function_or_script {
break;
}

Expand Down
68 changes: 68 additions & 0 deletions brush-shell/tests/cases/builtins/exit.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,71 @@ cases:
- name: "Exit with code"
stdin: |
exit 10
- name: "Exit in for loop"
stdin: |
for i in 1 2 3; do
exit 42
echo "Got past exit in loop"
done
echo "Got past loop"
- name: "Exit in arithmetic for loop body"
stdin: |
for ((i = 0; i < 10; i++)); do
exit 42
echo "Got past exit in loop"
done
echo "Got past loop"
- name: "Exit in while loop condition"
stdin: |
while exit 42; do
echo "In loop"
done
echo "Got past loop"
- name: "Exit in while loop body"
stdin: |
while true; do
exit 42
echo "Got past exit in body"
break
done
echo "Got past loop"
- name: "Exit in sequence"
stdin: |
exit 42; echo "Should not be printed"
- name: "Exit in function"
stdin: |
myfunc() {
exit 42
echo "Got past exit in function"
}
myfunc
echo "Got past function call"
- name: "Exit in subshell"
stdin: |
(exit 42)
echo "Got past subshell"
- name: "Exit in command substitution"
stdin: |
output=$(echo hi; exit 42; echo there)
echo "Got past command substitution"
- name: "Exit in and/or"
stdin: |
exit 42 || echo "Got past exit"
- name: "Exit in brace group"
stdin: |
{
exit 42
echo "Got past exit"
}
echo "Got past brace group"
1 change: 1 addition & 0 deletions brush-shell/tests/cases/builtins/jobs.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ name: "Builtins: job-related builtins"
cases:
- name: "Basic async job"
stdin: |
set +m
echo hi &
wait
jobs
7 changes: 7 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,10 @@ cases:
- name: "Piped subshell usage"
stdin: |
(echo hi) | wc -l
- name: "Breaks in subshell"
stdin: |
for i in 1 2 3; do
echo $i
(for i in 1 2 3; do break 2; done)
done

0 comments on commit a36cb56

Please sign in to comment.