Support errexit option in multi-command pipelines

yash-cli/tests/scripted_test/errexit-p.sh

@@ -73,7 +73,6 @@ __IN__
 reached
 __OUT__
 
-: TODO yash is broken <<\__IN__
 test_O -e n 'errexit: last of pipeline' -e
 true | true | false
 echo not reached

yash-env/CHANGELOG.md

@@ -5,7 +5,15 @@ All notable changes to `yash-env` will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
-## [0.1.1] - Unreleased
+## [0.2.0] - Unreleased
+
+### Added
+
+- `Env::errexit_is_applicable`
+
+### Removed
+
+- `semantics::apply_errexit`
 
 ### Fixed
 

yash-env/src/lib.rs

@@ -400,16 +400,23 @@ impl Env {
         variable
     }
 
-    pub(crate) fn errexit_is_applicable(&self) -> bool {
+    /// Tests whether the [`ErrExit`] option is applicable in the current context.
+    ///
+    /// This function returns true if and only if:
+    /// - the [`ErrExit`] option is on, and
+    /// - the current stack has no [`Condition`] frame.
+    ///
+    /// [`Condition`]: Frame::Condition
+    pub fn errexit_is_applicable(&self) -> bool {
         self.options.get(ErrExit) == On && !self.stack.contains(&Frame::Condition)
     }
 
-    /// Returns a `Divert` if the shell should exit because of the `ErrExit`
-    /// [shell option](self::option::Option).
+    /// Returns a `Divert` if the shell should exit because of the [`ErrExit`]
+    /// shell option.
     ///
-    /// The function returns `Break(Divert::Exit)` if the `ErrExit` option is
-    /// on, the current `self.exit_status` is non-zero, and the current stack
-    /// has no `Condition` [frame](Frame); otherwise, `Continue(())`.
+    /// The function returns `Break(Divert::Exit(None))` if the [`errexit`
+    /// option is applicable](Self::errexit_is_applicable) and the current
+    /// `self.exit_status` is non-zero. Otherwise, it returns `Continue(())`.
     pub fn apply_errexit(&self) -> ControlFlow<Divert> {
         if !self.exit_status.is_successful() && self.errexit_is_applicable() {
             Break(Divert::Exit(None))

yash-env/src/semantics.rs

@@ -17,10 +17,9 @@
 //! Type definitions for command execution.
 
 use crate::system::Errno;
-use crate::Env;
 use nix::sys::signal::Signal;
 use std::ffi::c_int;
-use std::ops::ControlFlow::{self, Break};
+use std::ops::ControlFlow;
 use std::process::ExitCode;
 use std::process::Termination;
 use yash_syntax::source::Location;
@@ -230,63 +229,9 @@ impl Divert {
 /// next.
 pub type Result<T = ()> = ControlFlow<Divert, T>;
 
-/// Applies the `ErrExit` shell option to the result.
-///
-/// If the `ErrExit` option is on in `env.options`, `env.stack` contains no
-/// `Frame::Condition`, and the `result` is `Divert::Interrupt`, then the result
-/// is converted to `Divert::Exit`.
-pub fn apply_errexit<T>(result: Result<T>, env: &Env) -> Result<T> {
-    match result {
-        Break(Divert::Interrupt(exit_status)) if env.errexit_is_applicable() => {
-            Break(Divert::Exit(exit_status))
-        }
-
-        other => other,
-    }
-}
-
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::option::Option::ErrExit;
-    use crate::option::State::On;
-    use crate::stack::Frame;
-
-    #[test]
-    fn apply_errexit_applicable() {
-        let mut env = Env::new_virtual();
-        env.options.set(ErrExit, On);
-        let subject: Result = Break(Divert::Interrupt(Some(ExitStatus(42))));
-        let result = apply_errexit(subject, &env);
-        assert_eq!(result, Break(Divert::Exit(Some(ExitStatus(42)))));
-    }
-
-    #[test]
-    fn apply_errexit_to_non_interrupt() {
-        let mut env = Env::new_virtual();
-        env.options.set(ErrExit, On);
-        let subject: Result = Break(Divert::Return(None));
-        let result = apply_errexit(subject, &env);
-        assert_eq!(result, subject);
-    }
-
-    #[test]
-    fn apply_errexit_in_condition_context() {
-        let mut env = Env::new_virtual();
-        env.options.set(ErrExit, On);
-        let env = env.push_frame(Frame::Condition);
-        let subject: Result = Break(Divert::Interrupt(Some(ExitStatus(42))));
-        let result = apply_errexit(subject, &env);
-        assert_eq!(result, subject);
-    }
-
-    #[test]
-    fn apply_errexit_with_disabled_option() {
-        let env = Env::new_virtual();
-        let subject: Result = Break(Divert::Interrupt(Some(ExitStatus(42))));
-        let result = apply_errexit(subject, &env);
-        assert_eq!(result, subject);
-    }
 
     #[test]
     fn signal_try_from_exit_status() {

yash-semantics/CHANGELOG.md

@@ -5,6 +5,17 @@ All notable changes to `yash-semantics` will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [0.2.0] - Unreleased
+
+### Added
+
+- Support for the `ErrExit` shell option in multi-command pipelines
+
+### Changed
+
+- `<expansion::Error as handle::Handle>::handle` now returns `Divert::Exit`
+  instead of `Divert::Interrupt` when the `ErrExit` shell option is applicable.
+
 ## [0.1.0] - 2024-04-13
 
 ### Added

yash-semantics/src/command/compound_command/case.rs

@@ -26,7 +26,6 @@ use crate::xtrace::XTrace;
 use crate::Handle;
 use std::fmt::Write;
 use std::ops::ControlFlow::Continue;
-use yash_env::semantics::apply_errexit;
 use yash_env::semantics::ExitStatus;
 use yash_env::semantics::Result;
 use yash_env::Env;
@@ -56,15 +55,15 @@ fn config() -> Config {
 pub async fn execute(env: &mut Env, subject: &Word, items: &[CaseItem]) -> Result {
     let subject = match expand_word(env, subject).await {
         Ok((expansion, _exit_status)) => expansion,
-        Err(error) => return apply_errexit(error.handle(env).await, env),
+        Err(error) => return error.handle(env).await,
     };
     trace_subject(env, &subject.value).await;
 
     'outer: for item in items {
         for pattern in &item.patterns {
             let mut pattern = match expand_word_attr(env, pattern).await {
                 Ok((expansion, _exit_status)) => expansion,
-                Err(error) => return apply_errexit(error.handle(env).await, env),
+                Err(error) => return error.handle(env).await,
             };
 
             // Unquoted backslashes should act as quoting, as required by POSIX XCU 2.13.1

yash-semantics/src/command/compound_command/for_loop.rs

@@ -28,7 +28,6 @@ use crate::xtrace::XTrace;
 use crate::Handle;
 use std::fmt::Write;
 use std::ops::ControlFlow::{Break, Continue};
-use yash_env::semantics::apply_errexit;
 use yash_env::semantics::Divert;
 use yash_env::semantics::Field;
 use yash_env::semantics::Result;
@@ -48,13 +47,13 @@ pub async fn execute(
 ) -> Result {
     let (name, _) = match expand_word(env, name).await {
         Ok(word) => word,
-        Err(error) => return apply_errexit(error.handle(env).await, env),
+        Err(error) => return error.handle(env).await,
     };
 
     let values = if let Some(words) = values {
         match expand_words(env, words).await {
             Ok((fields, _)) => fields,
-            Err(error) => return apply_errexit(error.handle(env).await, env),
+            Err(error) => return error.handle(env).await,
         }
     } else {
         env.variables
@@ -92,7 +91,7 @@ pub async fn execute(
                 });
                 let location = name.origin;
                 let error = Error { cause, location };
-                return apply_errexit(error.handle(env).await, env);
+                return error.handle(env).await;
             }
         };
     }

yash-semantics/src/command/pipeline.rs

@@ -63,7 +63,7 @@ use yash_syntax::syntax;
 /// In POSIX, the expected exit status is unclear when an inverted pipeline
 /// performs a jump as in `! return 42`. The behavior disagrees among existing
 /// shells. This implementation does not invert the exit status when the return
-/// value is `Err(Divert::...)`, which is different from yash 2.
+/// value is `Err(Divert::...)`.
 ///
 /// # `noexec` option
 ///
@@ -112,9 +112,17 @@ async fn execute_commands_in_pipeline(env: &mut Env, commands: &[Rc<syntax::Comm
             env.exit_status = ExitStatus::SUCCESS;
             Continue(())
         }
+
         1 => commands[0].execute(env).await,
-        _ if env.controls_jobs() => execute_job_controlled_pipeline(env, commands).await,
-        _ => execute_multi_command_pipeline(env, commands).await,
+
+        _ => {
+            if env.controls_jobs() {
+                execute_job_controlled_pipeline(env, commands).await?
+            } else {
+                execute_multi_command_pipeline(env, commands).await?
+            }
+            env.apply_errexit()
+        }
     }
 }
 
@@ -322,6 +330,7 @@ mod tests {
     use yash_env::builtin::Builtin;
     use yash_env::builtin::Type::Special;
     use yash_env::job::ProcessState;
+    use yash_env::option::Option::ErrExit;
     use yash_env::option::Option::Monitor;
     use yash_env::option::State::On;
     use yash_env::semantics::Field;
@@ -495,6 +504,20 @@ mod tests {
         assert_eq!(env.exit_status, ExitStatus::SUCCESS);
     }
 
+    #[test]
+    fn errexit_option() {
+        in_virtual_system(|mut env, _state| async move {
+            env.builtins.insert("return", return_builtin());
+            env.options.set(ErrExit, On);
+
+            let pipeline: syntax::Pipeline = "return -n 0 | return -n 93".parse().unwrap();
+            let result = pipeline.execute(&mut env).await;
+
+            assert_eq!(result, Break(Divert::Exit(None)));
+            assert_eq!(env.exit_status, ExitStatus(93));
+        });
+    }
+
     #[test]
     fn stack_without_inversion() {
         fn stub_builtin(

yash-semantics/src/handle.rs

@@ -46,16 +46,27 @@ impl Handle for yash_syntax::parser::Error {
     }
 }
 
-/// Prints an error message and sets the exit status to non-zero.
+/// Prints an error message and returns a divert result indicating a non-zero
+/// exit status.
 ///
 /// This implementation handles the error by printing an error message to the
 /// standard error and returning `Divert::Interrupt(Some(ExitStatus::ERROR))`.
+/// If the [`ErrExit`] option is set, `Divert::Exit(Some(ExitStatus::ERROR))` is
+/// returned instead.
+///
 /// Note that other POSIX-compliant implementations may use different non-zero
 /// exit statuses.
+///
+/// [`ErrExit`]: yash_env::option::Option::ErrExit
 impl Handle for crate::expansion::Error {
     async fn handle(&self, env: &mut Env) -> super::Result {
         print_message(env, self).await;
-        Break(Divert::Interrupt(Some(ExitStatus::ERROR)))
+
+        if env.errexit_is_applicable() {
+            Break(Divert::Exit(Some(ExitStatus::ERROR)))
+        } else {
+            Break(Divert::Interrupt(Some(ExitStatus::ERROR)))
+        }
     }
 }