diff --git a/src/runtime/shell/builtin/exit.rs b/src/runtime/shell/builtin/exit.rs index 767cbe4b2bd7..05e53473e259 100644 --- a/src/runtime/shell/builtin/exit.rs +++ b/src/runtime/shell/builtin/exit.rs @@ -33,16 +33,24 @@ impl Exit { return Self::fail(interp, cmd, b"exit: too many arguments\n"); } }; - // Intentional divergence from bash: this completes only the current - // Cmd rather than unwinding the whole script. + Self::request_exit(interp, cmd, code); Builtin::done(interp, cmd, code) } + /// Like bash, a bad argument still ends the script, with status 1. fn fail(interp: &Interpreter, cmd: NodeId, msg: &[u8]) -> Yield { Self::state_mut(interp, cmd).state = State::WaitingIo; + Self::request_exit(interp, cmd, 1); Builtin::write_failing_error(interp, cmd, msg, 1) } + /// End the enclosing execution context with `code`. A subshell, command + /// substitution, or pipeline element owns its own `ShellExecEnv`, so + /// `exit` never escapes the context that ran it. + fn request_exit(interp: &Interpreter, cmd: NodeId, code: crate::shell::ExitCode) { + interp.as_cmd_mut(cmd).base.shell_mut().exit_requested = Some(code); + } + pub(crate) fn on_io_writer_chunk( interp: &Interpreter, cmd: NodeId, diff --git a/src/runtime/shell/interpreter.rs b/src/runtime/shell/interpreter.rs index 693b33fa798c..5cb6ae670090 100644 --- a/src/runtime/shell/interpreter.rs +++ b/src/runtime/shell/interpreter.rs @@ -588,6 +588,7 @@ impl Interpreter { __cwd: cwd_arr, cwd_fd, async_pids: SmolList::default(), + exit_requested: None, }), root_io: JsCell::new(IO { stdin: crate::shell::io::InKind::Fd(stdin_reader), @@ -1793,6 +1794,10 @@ pub struct ShellExecEnv { pub __cwd: Vec, pub cwd_fd: Fd, pub async_pids: SmolList, + /// Status the `exit` builtin asked this execution context to end with. + /// Nodes sharing this env stop running children. A subshell, command + /// substitution, or pipeline element gets its own env, which scopes `exit`. + pub exit_requested: Option, } pub enum Bufio { @@ -1958,6 +1963,8 @@ impl ShellExecEnv { __cwd: self.__cwd.clone(), cwd_fd: dupedfd, async_pids: SmolList::default(), + // Fresh execution context: `exit` neither carries in nor escapes. + exit_requested: None, }); Ok(bun_core::heap::into_raw(duped)) } diff --git a/src/runtime/shell/states/Base.rs b/src/runtime/shell/states/Base.rs index 0c33e0f4eb5f..3afff722df17 100644 --- a/src/runtime/shell/states/Base.rs +++ b/src/runtime/shell/states/Base.rs @@ -4,7 +4,7 @@ //! `parent: NodeId` and the `*mut ShellExecEnv` (which may be owned or //! borrowed — see field doc) are stored here. -use crate::shell::interpreter::{NodeId, ShellExecEnv, StateKind}; +use crate::shell::interpreter::{ExitCode, NodeId, ShellExecEnv, StateKind}; pub struct Base { pub kind: StateKind, @@ -49,6 +49,13 @@ impl Base { // time. unsafe { &mut *self.shell } } + + /// `Some(status)` once `exit` ran in this node's execution context: stop + /// walking children and unwind with it. See `ShellExecEnv::exit_requested`. + #[inline] + pub fn exit_requested(&self) -> Option { + self.shell().exit_requested + } } /// `error{Sys}` — see `Interpreter::try_`. diff --git a/src/runtime/shell/states/Binary.rs b/src/runtime/shell/states/Binary.rs index 3da8ab94e8ee..9c428ae1ba0f 100644 --- a/src/runtime/shell/states/Binary.rs +++ b/src/runtime/shell/states/Binary.rs @@ -45,6 +45,12 @@ impl Binary { }; let n = node.get(); + // Checked before the short-circuit below: `exit 0 && echo hi` must not + // run the right-hand side even though the left side succeeded. + if let Some(code) = interp.as_binary(this).base.exit_requested() { + return interp.child_done(parent, this, code); + } + if let Some(right) = right_exit { return interp.child_done(parent, this, right); } diff --git a/src/runtime/shell/states/If.rs b/src/runtime/shell/states/If.rs index 8129bd800399..8f2951968b59 100644 --- a/src/runtime/shell/states/If.rs +++ b/src/runtime/shell/states/If.rs @@ -78,6 +78,12 @@ impl If { pub fn next(interp: &Interpreter, this: NodeId) -> Yield { let parent = interp.as_if(this).base.parent; + // A Pipeline spawns an If directly into its own env, with no Stmt in + // between to report the status, so the `Action::Done(0)` arms below + // would swallow it: `echo hi | if exit 5; then echo t; fi` exits 5. + if let Some(code) = interp.as_if(this).base.exit_requested() { + return interp.child_done(parent, this, code); + } loop { // Read/mutate `state` via a short-lived borrow, decide an action, // then drop the borrow before calling back into `interp`. diff --git a/src/runtime/shell/states/Stmt.rs b/src/runtime/shell/states/Stmt.rs index aff6c0d6281e..83dcb318ceb5 100644 --- a/src/runtime/shell/states/Stmt.rs +++ b/src/runtime/shell/states/Stmt.rs @@ -58,6 +58,12 @@ impl Stmt { me.base.shell, ) }; + // Every command in a script body or an `if` branch is spawned as a + // Stmt, so refusing to run one here stops the enclosing list. `Binary` + // and `If` do not go through a Stmt, and check for themselves. + if let Some(code) = interp.as_stmt(this).base.exit_requested() { + return interp.child_done(parent, this, code); + } if idx >= len { return interp.child_done(parent, this, last.unwrap_or(0)); } diff --git a/test/js/bun/shell/bunshell.test.ts b/test/js/bun/shell/bunshell.test.ts index 66697850d8bd..b939e3d29f9d 100644 --- a/test/js/bun/shell/bunshell.test.ts +++ b/test/js/bun/shell/bunshell.test.ts @@ -2644,12 +2644,10 @@ describe("subshell", () => { // test_oE 'effect of subshell' TestBuilder.command /* sh */ ` a=1 - # (a=2; echo $a; exit; echo not reached) - # NOTE: We actually implemented exit wrong so changing this for now until we fix it - (a=2; echo $a; exit; echo reached) + (a=2; echo $a; exit; echo not reached) echo $a ` - .stdout("2\nreached\n1\n") + .stdout("2\n1\n") .runAsTest("effect of subshell"); // test_x -e 23 'exit status of subshell' diff --git a/test/js/bun/shell/commands/exit.test.ts b/test/js/bun/shell/commands/exit.test.ts index 5a3239d0f49d..5f591be98e93 100644 --- a/test/js/bun/shell/commands/exit.test.ts +++ b/test/js/bun/shell/commands/exit.test.ts @@ -17,4 +17,124 @@ describe("exit", async () => { // prettier-ignore TestBuilder.command`exit abc`.exitCode(1).stderr("exit: numeric argument required\n").runAsTest("numeric argument required"); + + describe("ends the script", async () => { + TestBuilder.command`echo start; exit 5; echo never` + .exitCode(5) + .stdout("start\n") + .runAsTest("skips the statements after it"); + + TestBuilder.command`exit 1; exit 2`.exitCode(1).runAsTest("the first exit wins"); + + // https://github.com/oven-sh/bun/issues/20368 + TestBuilder.command /* sh */ ` +echo "Good Bun!" +exit +exit 0 +exit 1 +echo "Bad Bun!" +` + .exitCode(0) + .stdout("Good Bun!\n") + .runAsTest("a bare exit on its own line"); + + // Bare `exit` should report the last command's status. That needs the + // shell to track it, which it does not do yet, so this exits 0 today. + TestBuilder.command`false; exit` + .exitCode(1) + .todo("the shell does not track the last command's status yet") + .runAsTest("a bare exit reports the last command's status"); + + // Not short-circuiting on a status: `exit 0` ends an && chain and + // `exit 5` ends an || chain, where the status alone would keep going. + TestBuilder.command`exit 0 && echo never`.exitCode(0).stdout("").runAsTest("exit 0 ends an && chain"); + + TestBuilder.command`exit 5 || echo never`.exitCode(5).stdout("").runAsTest("exit 5 ends an || chain"); + + TestBuilder.command`false || exit 3; echo never`.exitCode(3).stdout("").runAsTest("from the right side of ||"); + + TestBuilder.command`if true; then exit 7; echo never; fi; echo never2` + .exitCode(7) + .stdout("") + .runAsTest("from an if body"); + + TestBuilder.command`if exit 5; then echo t; else echo f; fi` + .exitCode(5) + .stdout("") + .runAsTest("from an if condition"); + + // A failed condition would normally pick the else arm, and a failed elif + // condition the next one. No command in any arm may run after `exit`. + TestBuilder.command`if exit 5; then echo t1; echo t2; else echo f1; echo f2; fi` + .exitCode(5) + .stdout("") + .runAsTest("from an if condition, with multi-statement arms"); + + TestBuilder.command`if exit 5; then echo t; elif echo e; then echo t2; else echo f; fi` + .exitCode(5) + .stdout("") + .runAsTest("from an if condition, skipping the elif condition"); + + // A compound command may be followed by another expression in the same + // statement (`fi` is not a statement terminator). + TestBuilder.command`if true; then exit 5; fi echo never` + .exitCode(5) + .stdout("") + .runAsTest("from a compound command sharing a statement"); + + TestBuilder.command`exit abc; echo never` + .exitCode(1) + .stdout("") + .stderr("exit: numeric argument required\n") + .runAsTest("on a numeric argument error"); + + TestBuilder.command`exit 3 5; echo never` + .exitCode(1) + .stdout("") + .stderr("exit: too many arguments\n") + .runAsTest("on too many arguments"); + }); + + // `exit` ends the execution context that ran it, not the whole interpreter: + // a subshell, command substitution, or pipeline element is its own context. + describe("stays inside its execution context", async () => { + TestBuilder.command`(echo sub; exit 6; echo never); echo after` + .exitCode(0) + .stdout("sub\nafter\n") + .runAsTest("subshell"); + + TestBuilder.command`(echo sub; exit 6; echo never) && echo never2` + .exitCode(6) + .stdout("sub\n") + .runAsTest("subshell status reaches the parent"); + + TestBuilder.command`echo cs=$(echo sub; exit 4; echo never); echo after` + .exitCode(0) + .stdout("cs=sub\nafter\n") + .runAsTest("command substitution"); + + TestBuilder.command`echo a; exit 5 | cat; echo b`.exitCode(0).stdout("a\nb\n").runAsTest("pipeline element"); + + // A pipeline spawns an if-clause straight into its own env, with no + // statement in between, so the status has to come from the if itself. + TestBuilder.command`echo hi | if exit 5; then echo t; fi` + .exitCode(5) + .stdout("") + .runAsTest("if-clause as a pipeline element"); + + TestBuilder.command`echo hi | if false; then echo x; elif exit 5; then echo y; fi` + .exitCode(5) + .stdout("") + .runAsTest("if-clause with an elif as a pipeline element"); + + TestBuilder.command`(if true; then exit 2; fi; echo never); echo after` + .exitCode(0) + .stdout("after\n") + .runAsTest("if body nested in a subshell"); + + TestBuilder.command`if true; then (exit 2); fi; echo after` + .exitCode(0) + .stdout("after\n") + .runAsTest("subshell nested in an if body"); + }); });