From 0504444cfd29badb5dad18059f1b9b3282c5fcf6 Mon Sep 17 00:00:00 2001 From: Luca Barbato Date: Sun, 26 Apr 2026 22:19:39 +0200 Subject: [PATCH 1/3] feat: async I/O refactor for shell file operations Introduce AsyncOpenFile and related types to replace synchronous I/O across the shell's file handling. This enables non-blocking I/O for builtins, shell execution, and interactive features. Key changes: - Add AsyncOpenFile enum with tokio-based variants (File, Stdio, Pipe) - Add AsyncStream trait for custom async I/O implementations - Add AsyncOpenFiles wrapper for async file descriptor management - Convert all builtins to use async write_all/flush patterns - Fix signal race in process.wait() by polling for stopped children before entering tokio::select! loop (handles SIGCHLD arriving between SIGCONT and signal listener creation) - Update ExecutionContext with async write helper methods - Update examples to use async I/O patterns --- brush-builtins/Cargo.toml | 3 +- brush-builtins/src/alias.rs | 22 +- brush-builtins/src/bg.rs | 15 +- brush-builtins/src/bind.rs | 59 +- brush-builtins/src/caller.rs | 17 +- brush-builtins/src/cd.rs | 29 +- brush-builtins/src/command.rs | 19 +- brush-builtins/src/complete.rs | 96 +- brush-builtins/src/declare.rs | 114 +- brush-builtins/src/dirs.rs | 17 +- brush-builtins/src/echo.rs | 8 +- brush-builtins/src/enable.rs | 20 +- brush-builtins/src/export.rs | 46 +- brush-builtins/src/fc.rs | 29 +- brush-builtins/src/fg.rs | 93 +- brush-builtins/src/getopts.rs | 40 +- brush-builtins/src/hash.rs | 37 +- brush-builtins/src/help.rs | 37 +- brush-builtins/src/history.rs | 108 +- brush-builtins/src/jobs.rs | 23 +- brush-builtins/src/kill.rs | 143 +- brush-builtins/src/let_.rs | 7 +- brush-builtins/src/mapfile.rs | 42 +- brush-builtins/src/printf.rs | 15 +- brush-builtins/src/pwd.rs | 8 +- brush-builtins/src/read.rs | 78 +- brush-builtins/src/return_.rs | 6 +- brush-builtins/src/set.rs | 37 +- brush-builtins/src/shopt.rs | 62 +- brush-builtins/src/suspend.rs | 7 +- brush-builtins/src/test.rs | 7 +- brush-builtins/src/times.rs | 11 +- brush-builtins/src/trap.rs | 42 +- brush-builtins/src/type_.rs | 52 +- brush-builtins/src/ulimit.rs | 27 +- brush-builtins/src/umask.rs | 10 +- brush-builtins/src/unalias.rs | 13 +- brush-builtins/src/wait.rs | 19 +- brush-core/Cargo.toml | 4 +- brush-core/examples/call-func.rs | 2 +- brush-core/examples/custom-builtin.rs | 15 +- brush-core/src/builtins.rs | 5 +- brush-core/src/commands.rs | 53 +- brush-core/src/expansion.rs | 12 +- brush-core/src/interp.rs | 220 ++- brush-core/src/ioutils.rs | 11 +- brush-core/src/openfiles.rs | 1578 +++++++++++++++----- brush-core/src/processes.rs | 4 + brush-core/src/shell.rs | 1 + brush-core/src/shell/execution.rs | 15 +- brush-core/src/shell/history.rs | 33 +- brush-core/src/shell/io.rs | 60 +- brush-core/src/shell/job_control.rs | 2 +- brush-experimental-builtins/src/save.rs | 8 +- brush-interactive/Cargo.toml | 2 +- brush-interactive/src/interactive_shell.rs | 4 +- brush-shell/src/brushctl.rs | 89 +- brush-shell/src/bundled.rs | 10 +- brush-shell/src/entry.rs | 2 +- 59 files changed, 2382 insertions(+), 1166 deletions(-) diff --git a/brush-builtins/Cargo.toml b/brush-builtins/Cargo.toml index 02575e6f5..25a35e5f8 100644 --- a/brush-builtins/Cargo.toml +++ b/brush-builtins/Cargo.toml @@ -142,7 +142,7 @@ thiserror = "2.0.18" tracing = "0.1.44" [target.'cfg(target_family = "wasm")'.dependencies] -tokio = { version = "1.50.0", features = ["io-util", "macros", "rt"] } +tokio = { version = "1.50.0", features = ["io-util", "macros", "rt", "sync", "time"] } [target.'cfg(any(unix, windows))'.dependencies] tokio = { version = "1.50.0", features = [ @@ -153,6 +153,7 @@ tokio = { version = "1.50.0", features = [ "rt-multi-thread", "signal", "sync", + "time", ] } uucore = { version = "0.8.0", default-features = false, features = ["format"] } diff --git a/brush-builtins/src/alias.rs b/brush-builtins/src/alias.rs index 8ba642ffd..75e02eda3 100644 --- a/brush-builtins/src/alias.rs +++ b/brush-builtins/src/alias.rs @@ -23,10 +23,12 @@ impl builtins::Command for AliasCommand { context: brush_core::ExecutionContext<'_, SE>, ) -> Result { let mut exit_code = ExecutionResult::success(); + let mut output = Vec::new(); + let mut stderr_output = Vec::new(); if self.print || self.aliases.is_empty() { for (name, value) in context.shell.aliases() { - writeln!(context.stdout(), "alias {name}='{value}'")?; + writeln!(output, "alias {name}='{value}'")?; } } else { for alias in &self.aliases { @@ -38,10 +40,10 @@ impl builtins::Command for AliasCommand { .aliases_mut() .insert(name.to_owned(), unexpanded_value.to_owned()); } else if let Some(value) = context.shell.aliases().get(alias) { - writeln!(context.stdout(), "alias {alias}='{value}'")?; + writeln!(output, "alias {alias}='{value}'")?; } else { writeln!( - context.stderr(), + stderr_output, "{}: {alias}: not found", context.command_name )?; @@ -50,6 +52,20 @@ impl builtins::Command for AliasCommand { } } + if !output.is_empty() { + if let Some(mut stdout) = context.stdout() { + stdout.write_all(&output).await?; + stdout.flush().await?; + } + } + + if !stderr_output.is_empty() { + if let Some(mut stderr) = context.stderr() { + stderr.write_all(&stderr_output).await?; + stderr.flush().await?; + } + } + Ok(exit_code) } } diff --git a/brush-builtins/src/bg.rs b/brush-builtins/src/bg.rs index 97d834223..ff9c57cb5 100644 --- a/brush-builtins/src/bg.rs +++ b/brush-builtins/src/bg.rs @@ -18,6 +18,7 @@ impl builtins::Command for BgCommand { context: brush_core::ExecutionContext<'_, SE>, ) -> Result { let mut exit_code = ExecutionResult::success(); + let mut stderr_output = Vec::new(); if !self.job_specs.is_empty() { for job_spec in &self.job_specs { @@ -25,10 +26,9 @@ impl builtins::Command for BgCommand { job.move_to_background()?; } else { writeln!( - context.stderr(), + stderr_output, "{}: {}: no such job", - context.command_name, - job_spec + context.command_name, job_spec )?; exit_code = ExecutionResult::general_error(); } @@ -37,11 +37,18 @@ impl builtins::Command for BgCommand { if let Some(job) = context.shell.jobs_mut().current_job_mut() { job.move_to_background()?; } else { - writeln!(context.stderr(), "{}: no current job", context.command_name)?; + writeln!(stderr_output, "{}: no current job", context.command_name)?; exit_code = ExecutionResult::general_error(); } } + if !stderr_output.is_empty() { + if let Some(mut stderr) = context.stderr() { + stderr.write_all(&stderr_output).await?; + stderr.flush().await?; + } + } + Ok(exit_code) } } diff --git a/brush-builtins/src/bind.rs b/brush-builtins/src/bind.rs index 67b3f1541..316932c2a 100644 --- a/brush-builtins/src/bind.rs +++ b/brush-builtins/src/bind.rs @@ -148,40 +148,40 @@ impl BindCommand { context: &brush_core::ExecutionContext<'_, impl brush_core::ShellExtensions>, ) -> Result { let mut bindings = bindings.lock().await; + let mut output = Vec::new(); if self.list_funcs { for func in interfaces::InputFunction::iter() { - writeln!(context.stdout(), "{func}")?; + writeln!(output, "{func}")?; } } if self.list_funcs_and_bindings { - display_funcs_and_bindings(&*bindings, context, false /* reusable? */)?; + display_funcs_and_bindings(&*bindings, &mut output, false)?; } if self.list_funcs_and_bindings_reusable { - display_funcs_and_bindings(&*bindings, context, true /* reusable? */)?; + display_funcs_and_bindings(&*bindings, &mut output, true)?; } if self.list_key_seqs_that_invoke_macros { - display_macros(&*bindings, context, false /* reusable? */)?; + display_macros(&*bindings, &mut output, false)?; } if self.list_key_seqs_that_invoke_macros_reusable { - display_macros(&*bindings, context, true /* reusable? */)?; + display_macros(&*bindings, &mut output, true)?; } if self.list_vars { let options = &context.shell.completion_config().fallback_options; - // For now we'll just display a few items and show defaults. writeln!( - context.stdout(), + output, "mark-directories is set to `{}'", to_onoff(options.mark_directories) )?; writeln!( - context.stdout(), + output, "mark-symlinked-directories is set to `{}'", to_onoff(options.mark_symlinked_directories) )?; @@ -190,14 +190,13 @@ impl BindCommand { if self.list_vars_reusable { let options = &context.shell.completion_config().fallback_options; - // For now we'll just display a few items and show defaults. writeln!( - context.stdout(), + output, "set mark-directories {}", to_onoff(options.mark_directories) )?; writeln!( - context.stdout(), + output, "set mark-symlinked-directories {}", to_onoff(options.mark_symlinked_directories) )?; @@ -208,12 +207,19 @@ impl BindCommand { if !seqs.is_empty() { writeln!( - context.stdout(), + output, "{func_str} can be invoked via {}.", seqs.iter().map(|seq| std::format!("\"{seq}\"")).join(", ") )?; } else { - writeln!(context.stdout(), "{func_str} is not bound to any keys.")?; + writeln!(output, "{func_str} is not bound to any keys.")?; + drop(bindings); + if !output.is_empty() { + if let Some(mut stdout) = context.stdout() { + stdout.write_all(&output).await?; + stdout.flush().await?; + } + } return Ok(ExecutionResult::general_error()); } } @@ -241,13 +247,12 @@ impl BindCommand { continue; }; - writeln!(context.stdout(), "\"{seq}\" \"{cmd}\"")?; + writeln!(output, "\"{seq}\" \"{cmd}\"")?; } } if !self.key_seq_bindings.is_empty() { if self.keymap.as_ref().is_some_and(|k| k.is_vi()) { - // NOTE(vi): Quietly ignore since we don't support vi mode. return Ok(ExecutionResult::success()); } @@ -259,7 +264,6 @@ impl BindCommand { if let Some(key_sequence) = &self.key_sequence { if self.keymap.as_ref().is_some_and(|k| k.is_vi()) { - // NOTE(vi): Quietly ignore since we don't support vi mode. return Ok(ExecutionResult::success()); } @@ -269,6 +273,13 @@ impl BindCommand { drop(bindings); + if !output.is_empty() { + if let Some(mut stdout) = context.stdout() { + stdout.write_all(&output).await?; + stdout.flush().await?; + } + } + Ok(ExecutionResult::success()) } } @@ -443,7 +454,7 @@ const fn to_onoff(value: bool) -> &'static str { fn display_funcs_and_bindings( bindings: &dyn interfaces::KeyBindings, - context: &brush_core::ExecutionContext<'_, impl brush_core::ShellExtensions>, + output: &mut Vec, reusable: bool, ) -> Result<(), BindError> { let mut sequences_by_func: HashMap> = HashMap::new(); @@ -464,20 +475,20 @@ fn display_funcs_and_bindings( if let Some(seqs) = sequences_by_func.get(&func) { if reusable { for seq in seqs { - writeln!(context.stdout(), "\"{seq}\": {func}")?; + writeln!(output, "\"{seq}\": {func}")?; } } else { writeln!( - context.stdout(), + output, "{func} can be found on {}.", seqs.iter().map(|seq| std::format!("\"{seq}\"")).join(", ") )?; } } else { if reusable { - writeln!(context.stdout(), "# {func} (not bound)")?; + writeln!(output, "# {func} (not bound)")?; } else { - writeln!(context.stdout(), "{func} is not bound to any keys")?; + writeln!(output, "{func} is not bound to any keys")?; } } } @@ -487,14 +498,14 @@ fn display_funcs_and_bindings( fn display_macros( bindings: &dyn interfaces::KeyBindings, - context: &brush_core::ExecutionContext<'_, impl brush_core::ShellExtensions>, + output: &mut Vec, reusable: bool, ) -> Result<(), BindError> { for (left, right) in bindings.get_macros() { if reusable { - writeln!(context.stdout(), "\"{left}\": \"{right}\"")?; + writeln!(output, "\"{left}\": \"{right}\"")?; } else { - writeln!(context.stdout(), "{left} outputs {right}")?; + writeln!(output, "{left} outputs {right}")?; } } diff --git a/brush-builtins/src/caller.rs b/brush-builtins/src/caller.rs index 58c7b3a84..1f18f3ab0 100644 --- a/brush-builtins/src/caller.rs +++ b/brush-builtins/src/caller.rs @@ -18,17 +18,13 @@ impl builtins::Command for CallerCommand { ) -> Result { let stack = context.shell.call_stack(); - // See how far back we need to look. Frame N represents the Nth caller - // (e.g., 0 = immediate caller, 1 = caller's caller, etc.). let expr = self.expr.unwrap_or(0); - // Get all frames into a vector we can easily index into. let frames: Vec<_> = stack .iter() .filter(|frame| frame.frame_type.is_function() || frame.frame_type.is_script()) .collect(); - // Look for the last-known location in the parent of frame N. let Some(calling_frame) = frames.get(expr + 1) else { return Ok(ExecutionResult::general_error()); }; @@ -36,8 +32,8 @@ impl builtins::Command for CallerCommand { let line = calling_frame.current_line().unwrap_or(1); let filename = &calling_frame.source_info.source; - // When the expr is provided, we display "LINE FUNCTION_NAME FILENAME" - // When the expr is omitted, we only display "LINE FILENAME" + let mut output = Vec::new(); + if self.expr.is_some() { let function_name = match &calling_frame.frame_type { callstack::FrameType::Function(func_call) => func_call.name(), @@ -45,9 +41,14 @@ impl builtins::Command for CallerCommand { _ => "".into(), }; - writeln!(context.stdout(), "{line} {function_name} {filename}")?; + writeln!(output, "{line} {function_name} {filename}")?; } else { - writeln!(context.stdout(), "{line} {filename}")?; + writeln!(output, "{line} {filename}")?; + } + + if let Some(mut stdout) = context.stdout() { + stdout.write_all(&output).await?; + stdout.flush().await?; } Ok(ExecutionResult::success()) diff --git a/brush-builtins/src/cd.rs b/brush-builtins/src/cd.rs index fdae4ca69..4016a9af3 100644 --- a/brush-builtins/src/cd.rs +++ b/brush-builtins/src/cd.rs @@ -37,32 +37,36 @@ impl builtins::Command for CdCommand { &self, context: brush_core::ExecutionContext<'_, SE>, ) -> Result { - // TODO(cd): implement 'cd -@' if self.file_with_xattr_as_dir { return error::unimp("cd -@"); } let mut should_print = false; let mut target_dir = if let Some(target_dir) = &self.target_dir { - // `cd -', equivalent to `cd $OLDPWD' if target_dir.as_os_str() == "-" { should_print = true; if let Some(oldpwd) = context.shell.env_str("OLDPWD") { PathBuf::from(oldpwd.to_string()) } else { - writeln!(context.stderr(), "OLDPWD not set")?; + let mut stderr_output = Vec::new(); + writeln!(stderr_output, "OLDPWD not set")?; + if let Some(mut stderr) = context.stderr() { + stderr.write_all(&stderr_output).await?; + } return Ok(ExecutionResult::general_error()); } } else { - // TODO(cd): remove clone, and use temporary lifetime extension after rust 1.75 target_dir.clone() } - // `cd' without arguments is equivalent to `cd $HOME' } else { if let Some(home_var) = context.shell.env_str("HOME") { PathBuf::from(home_var.to_string()) } else { - writeln!(context.stderr(), "HOME not set")?; + let mut stderr_output = Vec::new(); + writeln!(stderr_output, "HOME not set")?; + if let Some(mut stderr) = context.stderr() { + stderr.write_all(&stderr_output).await?; + } return Ok(ExecutionResult::general_error()); } }; @@ -73,7 +77,6 @@ impl builtins::Command for CdCommand { .options() .do_not_resolve_symlinks_when_changing_dir { - // -e is only relevant in physical mode. if self.exit_on_failed_cwd_resolution { return error::unimp("cd -e"); } @@ -83,13 +86,13 @@ impl builtins::Command for CdCommand { context.shell.set_working_dir(&target_dir)?; - // Bash compatibility - // https://www.gnu.org/software/bash/manual/bash.html#index-cd - // If a non-empty directory name from CDPATH is used, or if '-' is the first argument, and - // the directory change is successful, the absolute pathname of the new working - // directory is written to the standard output. if should_print { - writeln!(context.stdout(), "{}", target_dir.display())?; + let mut output = Vec::new(); + writeln!(output, "{}", target_dir.display())?; + if let Some(mut stdout) = context.stdout() { + stdout.write_all(&output).await?; + stdout.flush().await?; + } } Ok(ExecutionResult::success()) diff --git a/brush-builtins/src/command.rs b/brush-builtins/src/command.rs index a87e705ef..a49302508 100644 --- a/brush-builtins/src/command.rs +++ b/brush-builtins/src/command.rs @@ -39,7 +39,6 @@ impl builtins::Command for CommandCommand { &self, context: brush_core::ExecutionContext<'_, SE>, ) -> Result { - // Silently exit if no command was provided. if let Some(command_name) = self.command() { if self.print_description || self.print_verbose_description { if let Some(found_cmd) = Self::try_find_command( @@ -47,22 +46,32 @@ impl builtins::Command for CommandCommand { command_name.as_str(), self.use_default_path, ) { + let mut output = Vec::new(); if self.print_description { - writeln!(context.stdout(), "{found_cmd}")?; + writeln!(output, "{found_cmd}")?; } else { match found_cmd { FoundCommand::Builtin(_name) => { - writeln!(context.stdout(), "{command_name} is a shell builtin")?; + writeln!(output, "{command_name} is a shell builtin")?; } FoundCommand::External(path) => { - writeln!(context.stdout(), "{command_name} is {path}")?; + writeln!(output, "{command_name} is {path}")?; } } } + if let Some(mut stdout) = context.stdout() { + stdout.write_all(&output).await?; + stdout.flush().await?; + } Ok(ExecutionResult::success()) } else { if self.print_verbose_description { - writeln!(context.stderr(), "command: {command_name}: not found")?; + let mut stderr_output = Vec::new(); + writeln!(stderr_output, "command: {command_name}: not found")?; + if let Some(mut stderr) = context.stderr() { + stderr.write_all(&stderr_output).await?; + stderr.flush().await?; + } } Ok(ExecutionResult::general_error()) } diff --git a/brush-builtins/src/complete.rs b/brush-builtins/src/complete.rs index eecb2fd7d..65fc22001 100644 --- a/brush-builtins/src/complete.rs +++ b/brush-builtins/src/complete.rs @@ -226,22 +226,42 @@ impl builtins::Command for CompleteCommand { mut context: brush_core::ExecutionContext<'_, SE>, ) -> Result { let mut result = ExecutionResult::success(); + let mut output = Vec::new(); + let mut stderr_output = Vec::new(); - // If -D, -E, or -I are specified, then any names provided are ignored. if self.use_as_default || self.use_for_empty_line || self.use_for_initial_word || self.names.is_empty() { - self.process_global(&mut context)?; + self.process_global(&mut context, &mut output, &mut stderr_output)?; } else { for name in &self.names { - if !self.try_process_for_command(&mut context, name.as_str())? { + if !self.try_process_for_command( + &mut context, + name.as_str(), + &mut output, + &mut stderr_output, + )? { result = ExecutionResult::general_error(); } } } + if !output.is_empty() { + if let Some(mut stdout) = context.stdout() { + stdout.write_all(&output).await?; + stdout.flush().await?; + } + } + + if !stderr_output.is_empty() { + if let Some(mut stderr) = context.stderr() { + stderr.write_all(&stderr_output).await?; + stderr.flush().await?; + } + } + Ok(result) } } @@ -250,11 +270,11 @@ impl CompleteCommand { fn process_global( &self, context: &mut brush_core::ExecutionContext<'_, impl brush_core::ShellExtensions>, + output: &mut Vec, + _stderr_output: &mut Vec, ) -> Result<(), brush_core::Error> { - // Read options before taking mutable borrow on completion_config let extended_globbing = context.shell.options().extended_globbing; - // These are processed in an intentional order. let special_option_name; let target_spec = if self.use_as_default { special_option_name = "-D"; @@ -270,18 +290,17 @@ impl CompleteCommand { None }; - // Treat 'complete' with no options the same as 'complete -p'. if self.print || (!self.remove && target_spec.is_none()) { if let Some(target_spec) = target_spec { if let Some(existing_spec) = target_spec { let existing_spec = existing_spec.clone(); - Self::display_spec(context, Some(special_option_name), None, &existing_spec)?; + Self::display_spec(output, Some(special_option_name), None, &existing_spec)?; } else { return error::unimp("special spec not found"); } } else { for (command_name, spec) in context.shell.completion_config().iter() { - Self::display_spec(context, None, Some(command_name.as_str()), spec)?; + Self::display_spec(output, None, Some(command_name.as_str()), spec)?; } } } else if self.remove { @@ -304,21 +323,18 @@ impl CompleteCommand { } fn try_display_spec_for_command( - context: &brush_core::ExecutionContext<'_, impl brush_core::ShellExtensions>, name: &str, + spec: &Spec, + output: &mut Vec, + _stderr_output: &mut Vec, ) -> Result { - if let Some(spec) = context.shell.completion_config().get(name) { - Self::display_spec(context, None, Some(name), spec)?; - Ok(true) - } else { - writeln!(context.stderr(), "no completion found for command")?; - Ok(false) - } + Self::display_spec(output, None, Some(name), spec)?; + Ok(true) } #[expect(clippy::too_many_lines)] fn display_spec( - context: &brush_core::ExecutionContext<'_, impl brush_core::ShellExtensions>, + output: &mut Vec, special_name: Option<&str>, command_name: Option<&str>, spec: &Spec, @@ -439,7 +455,7 @@ impl CompleteCommand { s.push_str(command_name); } - writeln!(context.stdout(), "{s}")?; + writeln!(output, "{s}")?; Ok(()) } @@ -448,32 +464,38 @@ impl CompleteCommand { &self, context: &mut brush_core::ExecutionContext<'_, impl brush_core::ShellExtensions>, name: &str, + output: &mut Vec, + stderr_output: &mut Vec, ) -> Result { if self.print { - return Self::try_display_spec_for_command(context, name); + if let Some(spec) = context.shell.completion_config().get(name) { + Self::try_display_spec_for_command(name, spec, output, stderr_output)?; + Ok(true) + } else { + writeln!(stderr_output, "no completion found for command")?; + Ok(false) + } } else if self.remove { let mut result = context.shell.completion_config_mut().remove(name); if !result { if context.shell.options().interactive { - writeln!(context.stderr(), "complete: {name}: not found")?; + writeln!(stderr_output, "complete: {name}: not found")?; } else { - // For some reason, this is not supposed to be treated as a failure - // in non-interactive execution. result = true; } } - return Ok(result); - } - - let config = self - .common_args - .create_spec(context.shell.options().extended_globbing); + Ok(result) + } else { + let config = self + .common_args + .create_spec(context.shell.options().extended_globbing); - context.shell.completion_config_mut().set(name, config); + context.shell.completion_config_mut().set(name, config); - Ok(true) + Ok(true) + } } } @@ -530,8 +552,13 @@ impl builtins::Command for CompGenCommand { return Ok(ExecutionResult::general_error()); } + let mut output = Vec::new(); for candidate in candidates { - writeln!(context.stdout(), "{candidate}")?; + writeln!(output, "{candidate}")?; + } + if let Some(mut stdout) = context.stdout() { + stdout.write_all(&output).await?; + stdout.flush().await?; } } completion::Answer::RestartCompletionProcess => { @@ -585,10 +612,15 @@ impl builtins::Command for CompOptCommand { if !self.names.is_empty() { if self.update_default || self.update_empty || self.update_initial_word { + let mut stderr_output = Vec::new(); writeln!( - context.stderr(), + stderr_output, "compopt: cannot specify names with -D, -E, or -I" )?; + if let Some(mut stderr) = context.stderr() { + stderr.write_all(&stderr_output).await?; + stderr.flush().await?; + } return Ok(ExecutionExitCode::InvalidUsage.into()); } diff --git a/brush-builtins/src/declare.rs b/brush-builtins/src/declare.rs index 11ee10427..178e54d58 100644 --- a/brush-builtins/src/declare.rs +++ b/brush-builtins/src/declare.rs @@ -135,7 +135,12 @@ impl builtins::Command for DeclareCommand { }; if matches!(verb, DeclareVerb::Local) && !context.shell.in_function() { - writeln!(context.stderr(), "can only be used in a function")?; + let mut stderr_output = Vec::new(); + writeln!(stderr_output, "can only be used in a function")?; + if let Some(mut stderr) = context.stderr() { + stderr.write_all(&stderr_output).await?; + stderr.flush().await?; + } return Ok(ExecutionResult::general_error()); } @@ -143,30 +148,54 @@ impl builtins::Command for DeclareCommand { return error::unimp("declare -I"); } + let mut output = Vec::new(); + let mut stderr_output = Vec::new(); let mut result = ExecutionResult::success(); + if !self.declarations.is_empty() { for declaration in &self.declarations { if self.print && !matches!(verb, DeclareVerb::Readonly) { - if !self.try_display_declaration(&context, declaration, verb)? { + if !self.try_display_declaration( + &context, + declaration, + verb, + &mut output, + &mut stderr_output, + )? { result = ExecutionResult::general_error(); } } else { - if !self.process_declaration(&mut context, declaration, verb)? { + if !self + .process_declaration(&mut context, declaration, verb) + .await? + { result = ExecutionResult::general_error(); } } } } else { - // Display matching declarations from the variable environment. if !self.function_names_only && !self.function_names_or_defs_only { - self.display_matching_env_declarations(&context, verb)?; + self.display_matching_env_declarations(&context, verb, &mut output)?; } - // Do the same for functions. if !matches!(verb, DeclareVerb::Local | DeclareVerb::Readonly) && (!self.print || self.function_names_only || self.function_names_or_defs_only) { - self.display_matching_functions(&context)?; + self.display_matching_functions(&context, &mut output)?; + } + } + + if !output.is_empty() { + if let Some(mut stdout) = context.stdout() { + stdout.write_all(&output).await?; + stdout.flush().await?; + } + } + + if !stderr_output.is_empty() { + if let Some(mut stderr) = context.stderr() { + stderr.write_all(&stderr_output).await?; + stderr.flush().await?; } } @@ -180,11 +209,13 @@ impl DeclareCommand { context: &brush_core::ExecutionContext<'_, impl brush_core::ShellExtensions>, declaration: &brush_core::CommandArg, verb: DeclareVerb, + output: &mut Vec, + stderr_output: &mut Vec, ) -> Result { let name = match declaration { brush_core::CommandArg::String(s) => s, brush_core::CommandArg::Assignment(_) => { - writeln!(context.stderr(), "declare: {declaration}: not found")?; + writeln!(stderr_output, "declare: {declaration}: not found")?; return Ok(false); } }; @@ -199,16 +230,15 @@ impl DeclareCommand { if let Some(func_registration) = context.shell.funcs().get(name) { if self.function_names_only { if self.print { - writeln!(context.stdout(), "declare -f {name}")?; + writeln!(output, "declare -f {name}")?; } else { - writeln!(context.stdout(), "{name}")?; + writeln!(output, "{name}")?; } } else { - writeln!(context.stdout(), "{}", func_registration.definition())?; + writeln!(output, "{}", func_registration.definition())?; } Ok(true) } else { - // For some reason, bash does not print an error message in this case. Ok(false) } } else if let Some(variable) = context.shell.env().get_using_policy(name, lookup) { @@ -225,19 +255,19 @@ impl DeclareCommand { }; writeln!( - context.stdout(), + output, "declare -{cs} {name}{separator_str}{}", resolved_value.format(variables::FormatStyle::DeclarePrint, context.shell)? )?; Ok(true) } else { - writeln!(context.stderr(), "declare: {name}: not found")?; + writeln!(stderr_output, "declare: {name}: not found")?; Ok(false) } } - fn process_declaration( + async fn process_declaration( &self, context: &mut brush_core::ExecutionContext<'_, impl brush_core::ShellExtensions>, declaration: &brush_core::CommandArg, @@ -249,28 +279,49 @@ impl DeclareCommand { && !self.create_global); if self.function_names_or_defs_only || self.function_names_only { - return self.try_display_declaration(context, declaration, verb); + let mut output = Vec::new(); + let mut stderr_output = Vec::new(); + let result = self.try_display_declaration( + context, + declaration, + verb, + &mut output, + &mut stderr_output, + )?; + if !output.is_empty() { + if let Some(mut stdout) = context.stdout() { + stdout.write_all(&output).await?; + stdout.flush().await?; + } + } + if !stderr_output.is_empty() { + if let Some(mut stderr) = context.stderr() { + stderr.write_all(&stderr_output).await?; + stderr.flush().await?; + } + } + return Ok(result); } - // Extract the variable name and the initial value being assigned (if any). let (name, assigned_index, initial_value, name_is_array) = Self::declaration_to_name_and_value(declaration)?; - // Special-case: `local -` if name == "-" && matches!(verb, DeclareVerb::Local) { - // TODO(local): `local -` allows shadowing the current `set` options (i.e., $-), with - // subsequent updates getting discarded when the current local scope is popped. tracing::warn!("not yet implemented: local -"); return Ok(true); } - // Make sure it's a valid name. if !env::valid_variable_name(name.as_str()) { + let mut stderr_output = Vec::new(); writeln!( - context.stderr(), + stderr_output, "{}: {name}: not a valid variable name", context.command_name )?; + if let Some(mut stderr) = context.stderr() { + stderr.write_all(&stderr_output).await?; + stderr.flush().await?; + } return Ok(false); } @@ -428,22 +479,16 @@ impl DeclareCommand { &self, context: &brush_core::ExecutionContext<'_, impl brush_core::ShellExtensions>, verb: DeclareVerb, + output: &mut Vec, ) -> Result<(), brush_core::Error> { - // - // Dump all declarations. Use attribute flags to filter which variables are dumped. - // - - // We start by excluding all variables that are not enumerable. #[expect(clippy::type_complexity)] let mut filters: Vec bool>> = vec![Box::new(|(_, v)| v.is_enumerable())]; - // Add filters depending on verb. if matches!(verb, DeclareVerb::Readonly) { filters.push(Box::new(|(_, v)| v.is_readonly())); } - // Add filters depending on attribute flags. if let Some(value) = self.make_indexed_array.to_bool() { filters.push(Box::new(move |(_, v)| { matches!(v.value(), ShellValue::IndexedArray(_)) == value @@ -500,8 +545,6 @@ impl DeclareCommand { EnvironmentLookup::Anywhere }; - // Iterate through an ordered list of all matching declarations tracked in the - // environment. for (name, variable) in context .shell .env() @@ -522,7 +565,7 @@ impl DeclareCommand { }; writeln!( - context.stdout(), + output, "declare -{cs} {name}{separator_str}{}", variable .value() @@ -530,7 +573,7 @@ impl DeclareCommand { )?; } else { writeln!( - context.stdout(), + output, "{name}={}", variable .value() @@ -545,12 +588,13 @@ impl DeclareCommand { fn display_matching_functions( &self, context: &brush_core::ExecutionContext<'_, impl brush_core::ShellExtensions>, + output: &mut Vec, ) -> Result<(), brush_core::Error> { for (name, registration) in context.shell.funcs().iter().sorted_by_key(|v| v.0) { if self.function_names_only { - writeln!(context.stdout(), "declare -f {name}")?; + writeln!(output, "declare -f {name}")?; } else { - writeln!(context.stdout(), "{}", registration.definition())?; + writeln!(output, "{}", registration.definition())?; } } diff --git a/brush-builtins/src/dirs.rs b/brush-builtins/src/dirs.rs index 790310226..821d6f83f 100644 --- a/brush-builtins/src/dirs.rs +++ b/brush-builtins/src/dirs.rs @@ -71,13 +71,15 @@ impl builtins::Command for DirsCommand { let one_per_line = self.print_one_per_line || self.print_one_per_line_with_index; + let mut output = Vec::new(); + for (i, dir) in dirs.iter().enumerate() { if !one_per_line && i > 0 { - write!(context.stdout(), " ")?; + write!(output, " ")?; } if self.print_one_per_line_with_index { - write!(context.stdout(), "{i:2} ")?; + write!(output, "{i:2} ")?; } let mut dir_str = dir.to_string_lossy().to_string(); @@ -86,10 +88,17 @@ impl builtins::Command for DirsCommand { dir_str = context.shell.tilde_shorten(dir_str); } - write!(context.stdout(), "{dir_str}")?; + write!(output, "{dir_str}")?; if one_per_line || i == dirs.len() - 1 { - writeln!(context.stdout())?; + writeln!(output)?; + } + } + + if !output.is_empty() { + if let Some(mut stdout) = context.stdout() { + stdout.write_all(&output).await?; + stdout.flush().await?; } } diff --git a/brush-builtins/src/echo.rs b/brush-builtins/src/echo.rs index 03da65a5a..2db521354 100644 --- a/brush-builtins/src/echo.rs +++ b/brush-builtins/src/echo.rs @@ -1,5 +1,4 @@ use clap::Parser; -use std::io::Write; use brush_core::{ExecutionResult, builtins, escape}; @@ -73,8 +72,11 @@ impl builtins::Command for EchoCommand { s.push('\n'); } - write!(context.stdout(), "{s}")?; - context.stdout().flush()?; + // Use async I/O for writing + if let Some(mut stdout) = context.stdout() { + stdout.write_all(s.as_bytes()).await?; + stdout.flush().await?; + } Ok(ExecutionResult::success()) } diff --git a/brush-builtins/src/enable.rs b/brush-builtins/src/enable.rs index 48dc7d14f..14f131a3c 100644 --- a/brush-builtins/src/enable.rs +++ b/brush-builtins/src/enable.rs @@ -45,6 +45,8 @@ impl builtins::Command for EnableCommand { context: brush_core::ExecutionContext<'_, SE>, ) -> Result { let mut result = ExecutionResult::success(); + let mut output = Vec::new(); + let mut stderr_output = Vec::new(); if self.shared_object_path.is_some() { return error::unimp("enable -f"); @@ -58,7 +60,7 @@ impl builtins::Command for EnableCommand { if let Some(builtin) = context.shell.builtin_mut(name) { builtin.disabled = self.disable; } else { - writeln!(context.stderr(), "{name}: not a shell builtin")?; + writeln!(stderr_output, "{name}: not a shell builtin")?; result = ExecutionResult::general_error(); } } @@ -87,7 +89,21 @@ impl builtins::Command for EnableCommand { let prefix = if builtin.disabled { "-n " } else { "" }; - writeln!(context.stdout(), "enable {prefix}{builtin_name}")?; + writeln!(output, "enable {prefix}{builtin_name}")?; + } + } + + if !output.is_empty() { + if let Some(mut stdout) = context.stdout() { + stdout.write_all(&output).await?; + stdout.flush().await?; + } + } + + if !stderr_output.is_empty() { + if let Some(mut stderr) = context.stderr() { + stderr.write_all(&stderr_output).await?; + stderr.flush().await?; } } diff --git a/brush-builtins/src/export.rs b/brush-builtins/src/export.rs index af7870cd1..f9117df1c 100644 --- a/brush-builtins/src/export.rs +++ b/brush-builtins/src/export.rs @@ -46,13 +46,19 @@ impl builtins::Command for ExportCommand { mut context: brush_core::ExecutionContext<'_, SE>, ) -> Result { if self.declarations.is_empty() { - display_all_exported_vars(&context)?; + let output = display_all_exported_vars(&context)?; + if !output.is_empty() { + if let Some(mut stdout) = context.stdout() { + stdout.write_all(&output).await?; + stdout.flush().await?; + } + } return Ok(ExecutionResult::success()); } let mut result = ExecutionResult::success(); for decl in &self.declarations { - let current_result = self.process_decl(&mut context, decl)?; + let current_result = self.process_decl(&mut context, decl).await?; if !current_result.is_success() { result = current_result; } @@ -63,17 +69,14 @@ impl builtins::Command for ExportCommand { } impl ExportCommand { - fn process_decl( + async fn process_decl( &self, context: &mut brush_core::ExecutionContext<'_, impl brush_core::ShellExtensions>, decl: &brush_core::CommandArg, ) -> Result { match decl { brush_core::CommandArg::String(s) => { - // See if this is supposed to be a function name. if self.names_are_functions { - // Try to find the function already present; if we find it, then mark it - // exported. if let Some(func) = context.shell.func_mut(s) { if self.unexport { func.unexport(); @@ -81,13 +84,15 @@ impl ExportCommand { func.export(); } } else { - writeln!(context.stderr(), "{s}: not a function")?; + let mut stderr_output = Vec::new(); + writeln!(stderr_output, "{s}: not a function")?; + if let Some(mut stderr) = context.stderr() { + stderr.write_all(&stderr_output).await?; + stderr.flush().await?; + } return Ok(ExecutionExitCode::InvalidUsage.into()); } - } - // Try to find the variable already present; if we find it, then mark it - // exported. - else if let Some((_, variable)) = context.shell.env_mut().get_mut(s) { + } else if let Some((_, variable)) = context.shell.env_mut().get_mut(s) { if self.unexport { variable.unexport(); } else { @@ -99,7 +104,12 @@ impl ExportCommand { let name = match &assignment.name { ast::AssignmentName::VariableName(name) => name, ast::AssignmentName::ArrayElementName(_, _) => { - writeln!(context.stderr(), "not a valid variable name")?; + let mut stderr_output = Vec::new(); + writeln!(stderr_output, "not a valid variable name")?; + if let Some(mut stderr) = context.stderr() { + stderr.write_all(&stderr_output).await?; + stderr.flush().await?; + } return Ok(ExecutionExitCode::InvalidUsage.into()); } }; @@ -117,7 +127,6 @@ impl ExportCommand { } }; - // Update the variable with the provided value and then mark it exported. context.shell.env_mut().update_or_add( name, value, @@ -141,18 +150,19 @@ impl ExportCommand { fn display_all_exported_vars( context: &brush_core::ExecutionContext<'_, impl brush_core::ShellExtensions>, -) -> Result<(), brush_core::Error> { - // Enumerate variables, sorted by key. +) -> Result, brush_core::Error> { + let mut output = Vec::new(); + for (name, variable) in context.shell.env().iter().sorted_by_key(|v| v.0) { if variable.is_exported() { let value = variable.value().try_get_cow_str(context.shell); if let Some(value) = value { - writeln!(context.stdout(), "declare -x {name}=\"{value}\"")?; + writeln!(output, "declare -x {name}=\"{value}\"")?; } else { - writeln!(context.stdout(), "declare -x {name}")?; + writeln!(output, "declare -x {name}")?; } } } - Ok(()) + Ok(output) } diff --git a/brush-builtins/src/fc.rs b/brush-builtins/src/fc.rs index 9f4695efd..1dd3973b2 100644 --- a/brush-builtins/src/fc.rs +++ b/brush-builtins/src/fc.rs @@ -46,7 +46,7 @@ impl builtins::Command for FcCommand { } if self.list { - return self.do_list(&context); + return self.do_list(&context).await; } error::unimp("fc editor mode is not yet implemented") @@ -54,7 +54,7 @@ impl builtins::Command for FcCommand { } impl FcCommand { - fn do_list( + async fn do_list( &self, context: &brush_core::ExecutionContext<'_, impl brush_core::ShellExtensions>, ) -> Result { @@ -65,7 +65,9 @@ impl FcCommand { let (first_idx, last_idx, reverse) = self.resolve_range(history)?; - // Determine the order of iteration + // Buffer output + let mut output = Vec::new(); + let indices: Vec = if reverse { (first_idx..=last_idx).rev().collect() } else { @@ -75,15 +77,21 @@ impl FcCommand { for idx in indices { if let Some(item) = history.get(idx) { if self.no_line_numbers { - // With -n, bash still outputs a tab before the command - writeln!(context.stdout(), "\t {}", item.command_line)?; + writeln!(output, "\t {}", item.command_line)?; } else { - // Match bash's fc format: number, tab, command - writeln!(context.stdout(), "{}\t {}", idx + 1, item.command_line)?; + writeln!(output, "{}\t {}", idx + 1, item.command_line)?; } } } + // Write output async + if !output.is_empty() { + if let Some(mut stdout) = context.stdout() { + stdout.write_all(&output).await?; + stdout.flush().await?; + } + } + Ok(ExecutionResult::success()) } @@ -132,7 +140,12 @@ impl FcCommand { }; // Echo the command to stderr. - writeln!(context.stderr(), "{final_cmd}")?; + let mut stderr_output = Vec::new(); + writeln!(stderr_output, "{final_cmd}")?; + if let Some(mut stderr) = context.stderr() { + stderr.write_all(&stderr_output).await?; + stderr.flush().await?; + } // Remove the fc command from history before executing the substituted command // This matches bash behavior where the fc command is replaced by the executed command diff --git a/brush-builtins/src/fg.rs b/brush-builtins/src/fg.rs index ec5796fe3..b84294b93 100644 --- a/brush-builtins/src/fg.rs +++ b/brush-builtins/src/fg.rs @@ -1,7 +1,8 @@ use clap::Parser; + use std::io::Write; -use brush_core::{ExecutionResult, builtins, jobs, sys}; +use brush_core::{ExecutionResult, builtins, jobs, openfiles, sys}; /// Move a specified job to the foreground. #[derive(Parser)] @@ -17,57 +18,61 @@ impl builtins::Command for FgCommand { &self, context: brush_core::ExecutionContext<'_, SE>, ) -> Result { - let mut stderr = context.stdout(); - - // Read interactive option before taking mutable borrow on jobs let is_interactive = context.shell.options().interactive; + let mut stdout = context.stdout(); + let mut stderr = context.stderr(); - if let Some(job_spec) = &self.job_spec { + let result = if let Some(job_spec) = &self.job_spec { if let Some(job) = context.shell.jobs_mut().resolve_job_spec(job_spec) { - job.move_to_foreground()?; - writeln!(stderr, "{}", job.command_line)?; - - let result = job.wait().await?; - if is_interactive { - sys::terminal::move_self_to_foreground()?; - } - - if matches!(job.state, jobs::JobState::Stopped) { - // N.B. We use the '\r' to overwrite any ^Z output. - let formatted = job.to_string(); - writeln!(context.stderr(), "\r{formatted}")?; - } - - Ok(result) + run_job(job, is_interactive, &mut stdout, &mut stderr).await? } else { - writeln!( - stderr, - "{}: {}: no such job", - job_spec, context.command_name - )?; - Ok(ExecutionResult::general_error()) + let mut buf = Vec::new(); + writeln!(buf, "{}: {}: no such job", job_spec, context.command_name)?; + write_to(&mut stderr, &buf).await; + ExecutionResult::general_error() } + } else if let Some(job) = context.shell.jobs_mut().current_job_mut() { + run_job(job, is_interactive, &mut stdout, &mut stderr).await? } else { - if let Some(job) = context.shell.jobs_mut().current_job_mut() { - job.move_to_foreground()?; - writeln!(stderr, "{}", job.command_line)?; + let mut buf = Vec::new(); + writeln!(buf, "{}: no current job", context.command_name)?; + write_to(&mut stderr, &buf).await; + ExecutionResult::general_error() + }; + + Ok(result) + } +} - let result = job.wait().await?; - if is_interactive { - sys::terminal::move_self_to_foreground()?; - } +async fn write_to(file: &mut Option, buf: &[u8]) { + if let Some(f) = file.as_mut() { + let _ = f.write_all(buf).await; + let _ = f.flush().await; + } +} - if matches!(job.state, jobs::JobState::Stopped) { - // N.B. We use the '\r' to overwrite any ^Z output. - let formatted = job.to_string(); - writeln!(context.stderr(), "\r{formatted}")?; - } +async fn run_job( + job: &mut brush_core::jobs::Job, + is_interactive: bool, + stdout: &mut Option, + stderr: &mut Option, +) -> Result { + job.move_to_foreground()?; - Ok(result) - } else { - writeln!(stderr, "{}: no current job", context.command_name)?; - Ok(ExecutionResult::general_error()) - } - } + let mut buf = Vec::new(); + writeln!(buf, "{}", job.command_line)?; + write_to(stdout, &buf).await; + + let result = job.wait().await?; + if is_interactive { + sys::terminal::move_self_to_foreground()?; + } + + if matches!(job.state, jobs::JobState::Stopped) { + let mut buf = Vec::new(); + writeln!(buf, "\r{job}")?; + write_to(stderr, &buf).await; } + + Ok(result) } diff --git a/brush-builtins/src/getopts.rs b/brush-builtins/src/getopts.rs index e57247fa5..29caba1de 100644 --- a/brush-builtins/src/getopts.rs +++ b/brush-builtins/src/getopts.rs @@ -103,12 +103,16 @@ impl builtins::Command for GetOptsCommand { ) -> Result { // Validate the target variable name. if !env::valid_variable_name(&self.variable_name) { + let mut stderr_output = Vec::new(); writeln!( - context.stderr(), + stderr_output, "{}: `{}': not a valid identifier", - context.command_name, - self.variable_name + context.command_name, self.variable_name )?; + if let Some(mut stderr) = context.stderr() { + stderr.write_all(&stderr_output).await?; + stderr.flush().await?; + } return Ok(ExecutionResult::new(1)); } @@ -147,7 +151,7 @@ impl builtins::Command for GetOptsCommand { &owned_args }; - let result = parse_next_option(&mut context, &spec, args_to_parse, next_index)?; + let result = parse_next_option(&mut context, &spec, args_to_parse, next_index).await?; update_variables(&mut context, &self.variable_name, result) } @@ -158,7 +162,7 @@ impl builtins::Command for GetOptsCommand { /// `-pVALUE` and `-p VALUE` forms), and error reporting for unknown options or /// missing arguments. Tracks position within combined flags via the /// `__GETOPTS_NEXT_CHAR` shell variable. -fn parse_next_option( +async fn parse_next_option( context: &mut brush_core::ExecutionContext<'_, SE>, spec: &OptionSpec, args_to_parse: &[String], @@ -237,12 +241,13 @@ fn parse_next_option( next_char_index, is_last_char_in_option, next_index, - )?; + ) + .await?; } else { optarg = None; } } else { - (variable_value, optarg) = report_unknown_option(context, spec, c)?; + (variable_value, optarg) = report_unknown_option(context, spec, c).await?; } let optind = if is_last_char_in_option { @@ -277,7 +282,7 @@ fn parse_next_option( /// Resolves the argument for an option that takes a value. Returns the updated /// `(variable_value, optarg, is_last_char, next_index)` tuple. #[allow(clippy::too_many_arguments)] -fn resolve_option_argument( +async fn resolve_option_argument( context: &brush_core::ExecutionContext<'_, SE>, spec: &OptionSpec, c: char, @@ -296,10 +301,12 @@ fn resolve_option_argument( (String::from(":"), Some(String::from(c))) } else { if is_opterr_enabled(context) { - writeln!( - context.stderr(), - "getopts: option requires an argument -- {c}" - )?; + let mut stderr_output = Vec::new(); + writeln!(stderr_output, "getopts: option requires an argument -- {c}")?; + if let Some(mut stderr) = context.stderr() { + stderr.write_all(&stderr_output).await?; + stderr.flush().await?; + } } (String::from("?"), None) }; @@ -320,13 +327,18 @@ fn resolve_option_argument( } /// Handles an unknown option character, reporting an error if appropriate. -fn report_unknown_option( +async fn report_unknown_option( context: &brush_core::ExecutionContext<'_, SE>, spec: &OptionSpec, c: char, ) -> Result<(String, Option), brush_core::Error> { if !spec.silent_errors && is_opterr_enabled(context) { - writeln!(context.stderr(), "getopts: illegal option -- {c}")?; + let mut stderr_output = Vec::new(); + writeln!(stderr_output, "getopts: illegal option -- {c}")?; + if let Some(mut stderr) = context.stderr() { + stderr.write_all(&stderr_output).await?; + stderr.flush().await?; + } } let optarg = if spec.silent_errors { diff --git a/brush-builtins/src/hash.rs b/brush-builtins/src/hash.rs index b8dda4289..44b79f5c8 100644 --- a/brush-builtins/src/hash.rs +++ b/brush-builtins/src/hash.rs @@ -37,13 +37,15 @@ impl builtins::Command for HashCommand { context: brush_core::ExecutionContext<'_, SE>, ) -> Result { let mut result = ExecutionResult::success(); + let mut output = Vec::new(); + let mut stderr_output = Vec::new(); if self.remove_all { context.shell.program_location_cache_mut().reset(); } else if self.remove { for name in &self.names { if !context.shell.program_location_cache_mut().unset(name) { - writeln!(context.stderr(), "{name}: not found")?; + writeln!(stderr_output, "{name}: not found")?; result = ExecutionResult::general_error(); } } @@ -51,11 +53,7 @@ impl builtins::Command for HashCommand { for name in &self.names { if let Some(path) = context.shell.program_location_cache().get(name) { if self.display_as_usable_input { - writeln!( - context.stdout(), - "builtin hash -p {} {name}", - path.to_string_lossy() - )?; + writeln!(output, "builtin hash -p {} {name}", path.to_string_lossy())?; } else { let mut prefix = String::new(); @@ -64,14 +62,10 @@ impl builtins::Command for HashCommand { prefix.push('\t'); } - writeln!( - context.stdout(), - "{prefix}{}", - path.to_string_lossy().as_ref() - )?; + writeln!(output, "{prefix}{}", path.to_string_lossy().as_ref())?; } } else { - writeln!(context.stderr(), "{name}: not found")?; + writeln!(stderr_output, "{name}: not found")?; result = ExecutionResult::general_error(); } } @@ -84,26 +78,37 @@ impl builtins::Command for HashCommand { } } else { for name in &self.names { - // Remove from the cache if already hashed. let _ = context.shell.program_location_cache_mut().unset(name); - // Names with slashes are accepted silently if name.contains('/') { continue; } - // Hash the path if context .shell .find_first_executable_in_path_using_cache(name) .is_none() { - writeln!(context.stderr(), "{name}: not found")?; + writeln!(stderr_output, "{name}: not found")?; result = ExecutionResult::general_error(); } } } + if !output.is_empty() { + if let Some(mut stdout) = context.stdout() { + stdout.write_all(&output).await?; + stdout.flush().await?; + } + } + + if !stderr_output.is_empty() { + if let Some(mut stderr) = context.stderr() { + stderr.write_all(&stderr_output).await?; + stderr.flush().await?; + } + } + Ok(result) } } diff --git a/brush-builtins/src/help.rs b/brush-builtins/src/help.rs index faccdca4b..ad68fc0cb 100644 --- a/brush-builtins/src/help.rs +++ b/brush-builtins/src/help.rs @@ -29,11 +29,22 @@ impl builtins::Command for HelpCommand { &self, context: brush_core::ExecutionContext<'_, SE>, ) -> Result { + // Buffer output for async write + let mut output = Vec::new(); + if self.topic_patterns.is_empty() { - Self::display_general_help(&context)?; + Self::display_general_help(&context, &mut output)?; } else { for topic_pattern in &self.topic_patterns { - self.display_help_for_topic_pattern(&context, topic_pattern)?; + self.display_help_for_topic_pattern(&context, topic_pattern, &mut output)?; + } + } + + // Write output async + if !output.is_empty() { + if let Some(mut stdout) = context.stdout() { + stdout.write_all(&output).await?; + stdout.flush().await?; } } @@ -44,15 +55,16 @@ impl builtins::Command for HelpCommand { impl HelpCommand { fn display_general_help( context: &brush_core::ExecutionContext<'_, impl brush_core::ShellExtensions>, + output: &mut Vec, ) -> Result<(), brush_core::Error> { const COLUMN_COUNT: usize = 3; if let Some(display_str) = context.shell.product_display_str() { - writeln!(context.stdout(), "{display_str}\n")?; + writeln!(output, "{display_str}\n")?; } writeln!( - context.stdout(), + output, "The following commands are implemented as shell built-ins:" )?; @@ -63,11 +75,10 @@ impl HelpCommand { for j in 0..COLUMN_COUNT { if let Some((name, builtin)) = builtins.get(i + j * items_per_column) { let prefix = if builtin.disabled { "*" } else { " " }; - write!(context.stdout(), " {prefix}{name:<20}")?; // adjust 20 to the desired - // column width + write!(output, " {prefix}{name:<20}")?; } } - writeln!(context.stdout())?; + writeln!(output)?; } Ok(()) @@ -77,6 +88,7 @@ impl HelpCommand { &self, context: &brush_core::ExecutionContext<'_, impl brush_core::ShellExtensions>, topic_pattern: &str, + output: &mut Vec, ) -> Result<(), brush_core::Error> { let pattern = brush_core::patterns::Pattern::from(topic_pattern) .set_extended_globbing(context.shell.options().extended_globbing) @@ -89,13 +101,14 @@ impl HelpCommand { context, builtin_name.as_str(), builtin_registration, + output, )?; found_count += 1; } } if found_count == 0 { - writeln!(context.stderr(), "No help topics match '{topic_pattern}'")?; + writeln!(output, "No help topics match '{topic_pattern}'")?; } Ok(()) @@ -106,6 +119,7 @@ impl HelpCommand { context: &brush_core::ExecutionContext<'_, SE>, name: &str, registration: &builtins::Registration, + output: &mut Vec, ) -> Result<(), brush_core::Error> { let content_type = if self.short_description { builtins::ContentType::ShortDescription @@ -117,20 +131,17 @@ impl HelpCommand { builtins::ContentType::DetailedHelp }; - let Some(mut stdout) = context.try_fd(brush_core::openfiles::OpenFiles::STDOUT_FD) else { - // If there's no stdout, nothing to do. + let Some(stdout) = context.try_fd(brush_core::openfiles::OpenFiles::STDOUT_FD) else { return Ok(()); }; - // For now, we assume colorized output if stdout is a terminal. let options = builtins::ContentOptions { colorized: stdout.is_terminal(), }; let content = (registration.content_func)(name, content_type, &options)?; - write!(stdout, "{content}")?; - stdout.flush()?; + write!(output, "{content}")?; Ok(()) } diff --git a/brush-builtins/src/history.rs b/brush-builtins/src/history.rs index ce891db08..fc8f3514a 100644 --- a/brush-builtins/src/history.rs +++ b/brush-builtins/src/history.rs @@ -1,4 +1,4 @@ -use brush_core::{ExecutionExitCode, ExecutionResult, builtins, error, history}; +use brush_core::{ExecutionResult, builtins, error, history}; use clap::Parser; use std::{io::Write, path::PathBuf}; @@ -57,17 +57,29 @@ impl builtins::Command for HistoryCommand { &self, context: brush_core::ExecutionContext<'_, SE>, ) -> Result { - // Retrieve the shell's history config while we still can. let config = HistoryConfig { default_history_file_path: context.shell.history_file_path(), time_format: context.shell.history_time_format(), }; - let stdout = context.stdout(); - let stderr = context.stderr(); - if let Some(history) = context.shell.history_mut() { - self.execute_with_history(history, config, stdout, stderr) + let (output, stderr_output) = self.execute_with_history(history, &config)?; + + if !output.is_empty() { + if let Some(mut stdout) = context.stdout() { + stdout.write_all(&output).await?; + stdout.flush().await?; + } + } + + if !stderr_output.is_empty() { + if let Some(mut stderr) = context.stderr() { + stderr.write_all(&stderr_output).await?; + stderr.flush().await?; + } + } + + Ok(ExecutionResult::success()) } else { Err(brush_core::ErrorKind::HistoryNotEnabled.into()) } @@ -81,55 +93,47 @@ impl HistoryCommand { fn execute_with_history( &self, history: &mut history::History, - config: HistoryConfig, - stdout: impl Write, - mut stderr: impl Write, - ) -> Result { + config: &HistoryConfig, + ) -> Result<(Vec, Vec), brush_core::Error> { + let mut stderr_output = Vec::new(); + if self.clear_history { history.clear()?; } if let Some(offset) = self.delete_offset { if offset == 0 { - writeln!(stderr, "cannot delete history item at offset 0")?; - return Ok(ExecutionExitCode::InvalidUsage.into()); + writeln!(stderr_output, "cannot delete history item at offset 0")?; + return Ok((Vec::new(), stderr_output)); } if offset > 0 { - // Convert to 0-based index. let index = (offset - 1) as usize; if !history.remove_nth_item(index) { - writeln!(stderr, "index past end of history")?; - return Ok(ExecutionExitCode::InvalidUsage.into()); + writeln!(stderr_output, "index past end of history")?; } } else { let count = history.count() as i64; let index = count + offset; if index < 0 { - writeln!(stderr, "index before beginning of history")?; - return Ok(ExecutionExitCode::InvalidUsage.into()); + writeln!(stderr_output, "index before beginning of history")?; } let _ = history.remove_nth_item(index as usize); } - return Ok(ExecutionResult::success()); + return Ok((Vec::new(), stderr_output)); } if let Some(append_option) = &self.append_session_to_file { if let Some(file_path) = get_effective_history_file_path( - config.default_history_file_path, + config.default_history_file_path.clone(), append_option.as_ref(), ) { - history.flush( - file_path, - true, /* append? */ - true, /* unsaved items only */ - config.time_format.is_some(), /* write timestamps? */ - )?; + history.flush(file_path, true, true, config.time_format.is_some())?; } - return Ok(ExecutionResult::success()); + return Ok((Vec::new(), Vec::new())); } if self.append_rest_of_file_to_session.is_some() { @@ -142,18 +146,13 @@ impl HistoryCommand { if let Some(write_option) = &self.write_session_to_file { if let Some(file_path) = get_effective_history_file_path( - config.default_history_file_path, + config.default_history_file_path.clone(), write_option.as_ref(), ) { - history.flush( - file_path, - false, /* append? */ - false, /* unsaved items only? */ - config.time_format.is_some(), /* write timestamps? */ - )?; + history.flush(file_path, false, false, config.time_format.is_some())?; } - return Ok(ExecutionResult::success()); + return Ok((Vec::new(), Vec::new())); } if self.expand_args.is_some() { @@ -162,7 +161,7 @@ impl HistoryCommand { if let Some(args) = &self.append_args_to_session { history.add(history::Item::new(args.join(" ")))?; - return Ok(ExecutionResult::success()); + return Ok((Vec::new(), Vec::new())); } let max_entries: Option = if let Some(arg) = self.args.first() { @@ -171,9 +170,9 @@ impl HistoryCommand { None }; - display_history(history, &config, max_entries, stdout, stderr)?; + let output = display_history(history, config, max_entries)?; - Ok(ExecutionResult::success()) + Ok((output, stderr_output)) } } @@ -181,9 +180,8 @@ fn display_history( history: &history::History, config: &HistoryConfig, max_entries: Option, - mut stdout: impl Write, - _stderr: impl Write, -) -> Result<(), brush_core::Error> { +) -> Result, brush_core::Error> { + let mut output = Vec::new(); let item_count = history.count(); let skip_count = item_count - max_entries.unwrap_or(item_count); @@ -198,17 +196,15 @@ fn display_history( } } - // Output format is something like: - // 1 echo hello world std::writeln!( - stdout, + output, "{:>5} {formatted_timestamp}{}", skip_count + i + 1, item.command_line )?; } - Ok(()) + Ok(output) } fn get_effective_history_file_path( @@ -220,27 +216,3 @@ fn get_effective_history_file_path( |file_path| Some(PathBuf::from(file_path)), ) } - -#[cfg(test)] -mod tests { - use super::*; - use anyhow::Result; - use pretty_assertions::{assert_eq, assert_matches}; - - #[test] - fn test_parse_dash_a() -> Result<()> { - let cmd = HistoryCommand::try_parse_from(["history", "5"])?; - assert_matches!(cmd.append_session_to_file, None); - - let cmd = HistoryCommand::try_parse_from(["history", "-a"])?; - assert_matches!(cmd.append_session_to_file, Some(None)); - - let cmd = HistoryCommand::try_parse_from(["history", "-a", "token"])?; - assert_eq!( - cmd.append_session_to_file, - Some(Some(String::from("token"))) - ); - - Ok(()) - } -} diff --git a/brush-builtins/src/jobs.rs b/brush-builtins/src/jobs.rs index c3686a96a..ee78da147 100644 --- a/brush-builtins/src/jobs.rs +++ b/brush-builtins/src/jobs.rs @@ -45,24 +45,31 @@ impl builtins::Command for JobsCommand { return error::unimp("jobs -n"); } + // Buffer output + let mut output = Vec::new(); + if self.job_specs.is_empty() { for job in &context.shell.jobs().jobs { - self.display_job(&context, job)?; + self.format_job(&mut output, job)?; } } else { return error::unimp("jobs with job specs"); } + // Write output async + if !output.is_empty() { + if let Some(mut stdout) = context.stdout() { + stdout.write_all(&output).await?; + stdout.flush().await?; + } + } + Ok(ExecutionResult::success()) } } impl JobsCommand { - fn display_job( - &self, - context: &brush_core::ExecutionContext<'_, impl brush_core::ShellExtensions>, - job: &jobs::Job, - ) -> Result<(), brush_core::Error> { + fn format_job(&self, output: &mut Vec, job: &jobs::Job) -> Result<(), brush_core::Error> { if self.running_jobs_only && !matches!(job.state, jobs::JobState::Running) { return Ok(()); } @@ -72,10 +79,10 @@ impl JobsCommand { if self.show_pids_only { if let Some(pid) = job.representative_pid() { - writeln!(context.stdout(), "{pid}")?; + writeln!(output, "{pid}")?; } } else { - writeln!(context.stdout(), "{job}")?; + writeln!(output, "{job}")?; } Ok(()) diff --git a/brush-builtins/src/kill.rs b/brush-builtins/src/kill.rs index 78b1a0a9f..51acdb276 100644 --- a/brush-builtins/src/kill.rs +++ b/brush-builtins/src/kill.rs @@ -33,94 +33,95 @@ impl builtins::Command for KillCommand { &self, context: brush_core::ExecutionContext<'_, SE>, ) -> Result { - // Default signal is SIGKILL. let mut trap_signal = TrapSignal::Signal(nix::sys::signal::Signal::SIGKILL); - // Try parsing the signal name (if specified). if let Some(signal_name) = &self.signal_name { if let Ok(parsed_trap_signal) = TrapSignal::try_from(signal_name.as_str()) { trap_signal = parsed_trap_signal; } else { - writeln!( - context.stderr(), - "{}: invalid signal name: {}", - context.command_name, - signal_name - )?; - return Ok(ExecutionExitCode::InvalidUsage.into()); + return write_error( + &context, + &format!( + "{}: invalid signal name: {}", + context.command_name, signal_name + ), + ExecutionExitCode::InvalidUsage, + ) + .await; } } - // Try parsing the signal number (if specified). if let Some(signal_number) = &self.signal_number { #[expect(clippy::cast_possible_truncation)] #[expect(clippy::cast_possible_wrap)] if let Ok(parsed_trap_signal) = TrapSignal::try_from(*signal_number as i32) { trap_signal = parsed_trap_signal; } else { - writeln!( - context.stderr(), - "{}: invalid signal number: {}", - context.command_name, - signal_number - )?; - return Ok(ExecutionExitCode::InvalidUsage.into()); + return write_error( + &context, + &format!( + "{}: invalid signal number: {}", + context.command_name, signal_number + ), + ExecutionExitCode::InvalidUsage, + ) + .await; } } - // Look through the remaining args for a pid/job spec or a -sigspec style option. let mut pid_or_job_spec = None; for arg in &self.args { - // See if this is -sigspec syntax. if let Some(possible_sigspec) = arg.strip_prefix("-") { - // See if this is -sigspec syntax. if let Ok(parsed_trap_signal) = TrapSignal::try_from(possible_sigspec) { trap_signal = parsed_trap_signal; } else { - writeln!( - context.stderr(), - "{}: invalid signal name", - context.command_name - )?; - return Ok(ExecutionExitCode::InvalidUsage.into()); + return write_error( + &context, + &format!("{}: invalid signal name", context.command_name), + ExecutionExitCode::InvalidUsage, + ) + .await; } } else if pid_or_job_spec.is_none() { pid_or_job_spec = Some(arg); } else { - writeln!( - context.stderr(), - "{}: too many jobs or processes specified", - context.command_name - )?; - return Ok(ExecutionExitCode::InvalidUsage.into()); + return write_error( + &context, + &format!( + "{}: too many jobs or processes specified", + context.command_name + ), + ExecutionExitCode::InvalidUsage, + ) + .await; } } if self.list_signals { - return print_signals(&context, self.args.as_ref()); + return print_signals(&context, self.args.as_ref()).await; } else { let Some(pid_or_job_spec) = pid_or_job_spec else { - writeln!(context.stderr(), "{}: invalid usage", context.command_name)?; - return Ok(ExecutionExitCode::InvalidUsage.into()); + return write_error( + &context, + &format!("{}: invalid usage", context.command_name), + ExecutionExitCode::InvalidUsage, + ) + .await; }; if pid_or_job_spec.starts_with('%') { - // It's a job spec. if let Some(job) = context.shell.jobs_mut().resolve_job_spec(pid_or_job_spec) { job.kill(trap_signal)?; } else { - writeln!( - context.stderr(), - "{}: {}: no such job", - context.command_name, - pid_or_job_spec - )?; - return Ok(ExecutionResult::general_error()); + return write_error( + &context, + &format!("{}: {}: no such job", context.command_name, pid_or_job_spec), + ExecutionExitCode::GeneralError, + ) + .await; } } else { let pid = brush_core::int_utils::parse(pid_or_job_spec.as_str(), 10)?; - - // It's a pid. sys::signal::kill_process(pid, trap_signal)?; } } @@ -128,22 +129,36 @@ impl builtins::Command for KillCommand { } } -fn print_signals( +async fn write_error( + context: &brush_core::ExecutionContext<'_, SE>, + message: &str, + exit_code: ExecutionExitCode, +) -> Result { + let mut stderr_output = Vec::new(); + writeln!(stderr_output, "{message}")?; + if let Some(mut stderr) = context.stderr() { + stderr.write_all(&stderr_output).await?; + stderr.flush().await?; + } + Ok(exit_code.into()) +} + +async fn print_signals( context: &brush_core::ExecutionContext<'_, impl brush_core::ShellExtensions>, signals: &[String], ) -> Result { let mut exit_code = ExecutionResult::success(); + let mut output = Vec::new(); + let mut stderr_output = Vec::new(); + if !signals.is_empty() { for s in signals { - // If the user gives us a code, we print the name; if they give a name, we print its - // code. enum PrintSignal { Name(&'static str), Num(i32), } let signal = if let Ok(n) = s.parse::() { - // bash compatibility. `SIGHUP` -> `HUP` TrapSignal::try_from(n).map(|s| { PrintSignal::Name(s.as_str().strip_prefix("SIG").unwrap_or(s.as_str())) }) @@ -155,23 +170,39 @@ fn print_signals( match signal { Ok(PrintSignal::Num(n)) => { - writeln!(context.stdout(), "{n}")?; + writeln!(output, "{n}")?; } Ok(PrintSignal::Name(s)) => { - writeln!(context.stdout(), "{s}")?; + writeln!(output, "{s}")?; } Err(e) => { - writeln!(context.stderr(), "{e}")?; + writeln!(stderr_output, "{e}")?; exit_code = ExecutionResult::general_error(); } } } } else { - return brush_core::traps::format_signals( - context.stdout(), + let result = brush_core::traps::format_signals( + &mut output, TrapSignal::iterator().filter(|s| !matches!(s, TrapSignal::Exit)), - ) - .map(|()| ExecutionResult::success()); + ); + if result.is_err() { + return result.map(|()| ExecutionResult::success()); + } + } + + if !output.is_empty() { + if let Some(mut stdout) = context.stdout() { + stdout.write_all(&output).await?; + stdout.flush().await?; + } + } + + if !stderr_output.is_empty() { + if let Some(mut stderr) = context.stderr() { + stderr.write_all(&stderr_output).await?; + stderr.flush().await?; + } } Ok(exit_code) diff --git a/brush-builtins/src/let_.rs b/brush-builtins/src/let_.rs index b87a705ec..01b831658 100644 --- a/brush-builtins/src/let_.rs +++ b/brush-builtins/src/let_.rs @@ -21,7 +21,12 @@ impl builtins::Command for LetCommand { let mut result = ExecutionExitCode::InvalidUsage.into(); if self.exprs.is_empty() { - writeln!(context.stderr(), "missing expression")?; + let mut stderr_output = Vec::new(); + writeln!(stderr_output, "missing expression")?; + if let Some(mut stderr) = context.stderr() { + stderr.write_all(&stderr_output).await?; + stderr.flush().await?; + } return Ok(result); } diff --git a/brush-builtins/src/mapfile.rs b/brush-builtins/src/mapfile.rs index 4d7225864..02eb10d58 100644 --- a/brush-builtins/src/mapfile.rs +++ b/brush-builtins/src/mapfile.rs @@ -1,4 +1,4 @@ -use std::io::{Read, Write}; +use std::io::Write; use clap::Parser; @@ -57,11 +57,15 @@ impl builtins::Command for MapFileCommand { if let Some(origin) = self.origin { if origin < 0 { + let mut stderr_output = Vec::new(); writeln!( - context.stderr(), + stderr_output, "{}: {origin}: invalid array origin", context.command_name )?; + if let Some(mut stderr) = context.stderr() { + stderr.write_all(&stderr_output).await?; + } return Ok(ExecutionExitCode::GeneralError.into()); } } @@ -74,12 +78,15 @@ impl builtins::Command for MapFileCommand { variables::ShellValueUnsetType::AssociativeArray ) ) { + let mut stderr_output = Vec::new(); writeln!( - context.stderr(), + stderr_output, "{}: {}: not an indexed array", - context.command_name, - self.array_var_name + context.command_name, self.array_var_name )?; + if let Some(mut stderr) = context.stderr() { + stderr.write_all(&stderr_output).await?; + } return Ok(ExecutionExitCode::GeneralError.into()); } } @@ -88,8 +95,7 @@ impl builtins::Command for MapFileCommand { .try_fd(self.fd) .ok_or_else(|| ErrorKind::BadFileDescriptor(self.fd))?; - // Read! - let results = self.read_entries(input_file)?; + let results = self.read_entries(input_file).await?; if let Some(origin) = self.origin { // -O: preserve existing array, assign at offset. @@ -122,12 +128,10 @@ impl builtins::Command for MapFileCommand { } impl MapFileCommand { - fn read_entries( + async fn read_entries( &self, mut input_file: brush_core::openfiles::OpenFile, ) -> Result { - let _term_mode = setup_terminal_settings(&input_file)?; - let mut entries = vec![]; let mut read_count = 0; let max_count = self.max_count.try_into()?; @@ -144,7 +148,7 @@ impl MapFileCommand { let mut saw_delimiter = false; loop { - match input_file.read(&mut buf) { + match input_file.read(&mut buf).await { Ok(0) => break, // End of input Ok(1) if buf[0] == b'\x03' => break, // Ctrl+C Ok(1) if buf[0] == b'\x04' && line.is_empty() => break, // Ctrl+D @@ -182,19 +186,3 @@ impl MapFileCommand { Ok(variables::ArrayLiteral(entries)) } } - -fn setup_terminal_settings( - file: &brush_core::openfiles::OpenFile, -) -> Result, brush_core::Error> { - let mode = brush_core::terminal::AutoModeGuard::new(file.to_owned()).ok(); - if let Some(mode) = &mode { - let config = brush_core::terminal::Settings::builder() - .line_input(false) - .interrupt_signals(false) - .build(); - - mode.apply_settings(&config)?; - } - - Ok(mode) -} diff --git a/brush-builtins/src/printf.rs b/brush-builtins/src/printf.rs index 0d4f523cd..b6b494874 100644 --- a/brush-builtins/src/printf.rs +++ b/brush-builtins/src/printf.rs @@ -24,17 +24,14 @@ impl builtins::Command for PrintfCommand { &self, context: brush_core::ExecutionContext<'_, SE>, ) -> Result { - if let Some(variable_name) = &self.output_variable { - // Format to a u8 vector. - let mut result: Vec = vec![]; - format(self.format_and_args.as_slice(), &mut result)?; + let mut result: Vec = vec![]; + format(self.format_and_args.as_slice(), &mut result)?; - // Convert to a string. + if let Some(variable_name) = &self.output_variable { let result_str = String::from_utf8(result).map_err(|_| { brush_core::ErrorKind::PrintfInvalidUsage("invalid UTF-8 output".into()) })?; - // Assign to the selected variable. expansion::assign_to_named_parameter( context.shell, &context.params, @@ -42,9 +39,9 @@ impl builtins::Command for PrintfCommand { result_str, ) .await?; - } else { - format(self.format_and_args.as_slice(), context.stdout())?; - context.stdout().flush()?; + } else if let Some(mut stdout) = context.stdout() { + stdout.write_all(&result).await?; + stdout.flush().await?; } Ok(ExecutionResult::success()) diff --git a/brush-builtins/src/pwd.rs b/brush-builtins/src/pwd.rs index 09f9cadc5..a6694205c 100644 --- a/brush-builtins/src/pwd.rs +++ b/brush-builtins/src/pwd.rs @@ -33,7 +33,13 @@ impl builtins::Command for PwdCommand { cwd = cwd.canonicalize()?.into(); } - writeln!(context.stdout(), "{}", cwd.to_string_lossy())?; + let mut output = Vec::new(); + writeln!(output, "{}", cwd.to_string_lossy())?; + + if let Some(mut stdout) = context.stdout() { + stdout.write_all(&output).await?; + stdout.flush().await?; + } Ok(ExecutionResult::success()) } diff --git a/brush-builtins/src/read.rs b/brush-builtins/src/read.rs index b239203d6..4282320d1 100644 --- a/brush-builtins/src/read.rs +++ b/brush-builtins/src/read.rs @@ -1,12 +1,11 @@ use clap::Parser; use itertools::Itertools; use std::collections::VecDeque; +use std::io::Write; use std::time::{Duration, Instant}; use brush_core::{ErrorKind, builtins, env, error, variables}; -use std::io::{Read, Write}; - /// Exit code returned when `read` times out. /// This is 128 + SIGALRM (14) = 142, matching bash behavior. const TIMEOUT_EXIT_CODE: u8 = 142; @@ -91,7 +90,7 @@ impl builtins::Command for ReadCommand { } // Validate timeout value if provided. - if let Some(result) = self.validate_timeout(&context)? { + if let Some(result) = self.validate_timeout(&context).await? { return Ok(result); } @@ -101,10 +100,10 @@ impl builtins::Command for ReadCommand { brush_core::ShellFd::from, ); - // Retrieve the file. - let input_stream = context + // Check if input is a terminal + let is_terminal = context .try_fd(fd_num) - .ok_or_else(|| ErrorKind::BadFileDescriptor(fd_num))?; + .is_some_and(|f: brush_core::openfiles::OpenFile| f.is_terminal()); // Retrieve effective value of IFS for splitting. // We convert to owned String to release the borrow before the mutable borrow @@ -115,7 +114,9 @@ impl builtins::Command for ReadCommand { let timeout = self.timeout_in_seconds.map(Duration::from_secs_f64); // Perform the read operation (potentially with timeout). - let read_result = self.read_line(input_stream, context.stderr(), timeout)?; + let read_result = self + .read_line(&context, fd_num, is_terminal, timeout) + .await?; // Determine whether to skip IFS splitting (for -N option). let skip_ifs_splitting = self.return_after_n_chars_no_delimiter.is_some(); @@ -348,24 +349,22 @@ impl InputReader { brush_core::sys::poll::poll_for_input(&self.input, Duration::ZERO).unwrap_or(false) } - /// Reads the next input event, handling timeout and control characters. - fn read_event(&mut self) -> Result { - // Check timeout before attempting read. - if let Some(deadline) = self.deadline { + /// Reads the next input event asynchronously, handling timeout and control characters. + async fn read_event(&mut self) -> Result { + let n = if let Some(deadline) = self.deadline { let remaining = deadline.saturating_duration_since(Instant::now()); if remaining.is_zero() { return Ok(InputEvent::Timeout); } - // Poll for input with remaining timeout. - match brush_core::sys::poll::poll_for_input(&self.input, remaining) { - Ok(true) => { /* Data available, proceed. */ } - Ok(false) => return Ok(InputEvent::Timeout), - Err(e) => return Err(e.into()), + match tokio::time::timeout(remaining, self.input.read(&mut self.buffer)).await { + Ok(result) => result?, + Err(_elapsed) => return Ok(InputEvent::Timeout), } - } + } else { + self.input.read(&mut self.buffer).await? + }; - let n = self.input.read(&mut self.buffer)?; if n == 0 { return Ok(InputEvent::Eof); } @@ -400,7 +399,7 @@ struct LineReaderConfig { /// For example, with `-n 3` and input `a\bc` (4 bytes): /// - Bash processes: 'a' (output 1), '\b' → 'b' (output 2), 'c' (output 3) → "abc" /// - The backslash is consumed but doesn't count toward the limit -fn read_line_with_reader( +async fn read_line_with_reader( reader: &mut InputReader, config: &LineReaderConfig, ) -> Result { @@ -408,7 +407,7 @@ fn read_line_with_reader( let mut pending_backslash = false; loop { - let event = reader.read_event()?; + let event = reader.read_event().await?; match event { InputEvent::Eof => { @@ -503,25 +502,35 @@ fn read_line_with_reader( } impl ReadCommand { + /// Reads a line of input, optionally with a timeout. /// Reads a line of input, optionally with a timeout. /// /// Handles backslash escape processing: /// - Without `-r`: backslash-newline is line continuation, other backslashes escape the next /// char /// - With `-r`: backslash is treated as a literal character - fn read_line( + async fn read_line( &self, - input_file: brush_core::openfiles::OpenFile, - mut stderr_file: impl std::io::Write, + context: &brush_core::ExecutionContext<'_, SE>, + fd_num: brush_core::ShellFd, + is_terminal: bool, timeout: Option, ) -> Result { + let input_file = context + .try_fd(fd_num) + .ok_or_else(|| ErrorKind::BadFileDescriptor(fd_num))?; + let term_mode = self.setup_terminal_settings(&input_file)?; // Display prompt on stderr, but only if input is from a terminal (per bash behavior). if let Some(prompt) = &self.prompt { - if input_file.is_terminal() { - write!(stderr_file, "{prompt}")?; - stderr_file.flush()?; + if is_terminal { + let mut stderr_output = Vec::new(); + write!(stderr_output, "{prompt}")?; + if let Some(mut stderr) = context.stderr() { + stderr.write_all(&stderr_output).await?; + stderr.flush().await?; + } } } @@ -561,7 +570,7 @@ impl ReadCommand { process_escapes: !self.raw_mode, }; - read_line_with_reader(&mut reader, &config) + read_line_with_reader(&mut reader, &config).await } fn setup_terminal_settings( @@ -583,22 +592,21 @@ impl ReadCommand { } /// Validates the timeout value and returns an error result if invalid. - /// - /// Returns `Ok(Some(result))` if the timeout is invalid (caller should return early), - /// `Ok(None)` if the timeout is valid or not specified. - /// - /// TODO(read): Bash uses $TMOUT as a default timeout for `read` when -t is not specified. - fn validate_timeout( + async fn validate_timeout( &self, - context: &brush_core::ExecutionContext<'_, impl brush_core::ShellExtensions>, + context: &brush_core::ExecutionContext<'_, SE>, ) -> Result, brush_core::Error> { if let Some(timeout) = self.timeout_in_seconds { if timeout < 0.0 { + let mut stderr_output = Vec::new(); writeln!( - context.stderr(), + stderr_output, "{}: -t: invalid timeout specification", context.command_name )?; + if let Some(mut stderr) = context.stderr() { + stderr.write_all(&stderr_output).await?; + } return Ok(Some(brush_core::ExecutionResult::general_error())); } } diff --git a/brush-builtins/src/return_.rs b/brush-builtins/src/return_.rs index f6040a56d..737a74ce7 100644 --- a/brush-builtins/src/return_.rs +++ b/brush-builtins/src/return_.rs @@ -30,10 +30,14 @@ impl builtins::Command for ReturnCommand { Ok(result) } else { + let mut stderr_output = Vec::new(); let _ = writeln!( - context.stderr(), + stderr_output, "return: can only be used in a function or sourced script" ); + if let Some(mut stderr) = context.stderr() { + stderr.write_all(&stderr_output).await?; + } Ok(ExecutionExitCode::InvalidUsage.into()) } } diff --git a/brush-builtins/src/set.rs b/brush-builtins/src/set.rs index 95cf752ee..9d5ade05e 100644 --- a/brush-builtins/src/set.rs +++ b/brush-builtins/src/set.rs @@ -322,6 +322,8 @@ impl builtins::Command for SetCommand { } let mut named_options: HashMap = HashMap::new(); + let mut output = Vec::new(); + if let Some(option_names) = &self.set_option.disable { saw_option = true; if option_names.is_empty() { @@ -333,7 +335,7 @@ impl builtins::Command for SetCommand { { let option_value = option.definition.get(context.shell.options()); let option_value_str = if option_value { "-o" } else { "+o" }; - writeln!(context.stdout(), "set {option_value_str} {}", option.name)?; + writeln!(output, "set {option_value_str} {}", option.name)?; } } else { for option_name in option_names { @@ -352,7 +354,7 @@ impl builtins::Command for SetCommand { { let option_value = option.definition.get(context.shell.options()); let option_value_str = if option_value { "on" } else { "off" }; - writeln!(context.stdout(), "{:15}\t{option_value_str}", option.name)?; + writeln!(output, "{:15}\t{option_value_str}", option.name)?; } } else { for option_name in option_names { @@ -398,10 +400,18 @@ impl builtins::Command for SetCommand { saw_option = saw_option || !self.positional_args.is_empty(); - // If we *still* haven't seen any options, then we need to display all variables and - // functions. if !saw_option { - display_all(&context)?; + let all_output = display_all(&context)?; + if !all_output.is_empty() { + output.extend(all_output); + } + } + + if !output.is_empty() { + if let Some(mut stdout) = context.stdout() { + stdout.write_all(&output).await?; + stdout.flush().await?; + } } Ok(result) @@ -410,40 +420,35 @@ impl builtins::Command for SetCommand { fn display_all( context: &brush_core::ExecutionContext<'_, impl brush_core::ShellExtensions>, -) -> Result<(), brush_core::Error> { - // Display variables. +) -> Result, brush_core::Error> { + let mut output = Vec::new(); + for (name, var) in context.shell.env().iter().sorted_by_key(|v| v.0) { if !var.is_enumerable() { continue; } - // TODO(set): For now, skip all dynamic variables. The current behavior - // of bash is not quite clear. We've empirically found that some - // special variables don't get displayed until they're observed - // at least once. if matches!(var.value(), variables::ShellValue::Dynamic { .. }) { continue; } - // Skip variables that have been declared but are unset. if !var.value().is_set() { continue; } writeln!( - context.stdout(), + output, "{name}={}", var.value() .format(variables::FormatStyle::Basic, context.shell)?, )?; } - // Display functions... unless we're in posix compliance mode. if !context.shell.options().posix_mode { for (_name, registration) in context.shell.funcs().iter().sorted_by_key(|v| v.0) { - writeln!(context.stdout(), "{}", registration.definition())?; + writeln!(output, "{}", registration.definition())?; } } - Ok(()) + Ok(output) } diff --git a/brush-builtins/src/shopt.rs b/brush-builtins/src/shopt.rs index 8069e3c6e..b78a16260 100644 --- a/brush-builtins/src/shopt.rs +++ b/brush-builtins/src/shopt.rs @@ -40,19 +40,26 @@ impl builtins::Command for ShoptCommand { context: brush_core::ExecutionContext<'_, SE>, ) -> Result { if self.set && self.unset { + let mut stderr_output = Vec::new(); writeln!( - context.stderr(), + stderr_output, "cannot set and unset shell options simultaneously" )?; + if let Some(mut stderr) = context.stderr() { + stderr.write_all(&stderr_output).await?; + stderr.flush().await?; + } return Ok(ExecutionExitCode::InvalidUsage.into()); } + let mut output = Vec::new(); + let mut stderr_output = Vec::new(); + if self.options.is_empty() { if self.quiet { return Ok(ExecutionResult::success()); } - // Enumerate all options of the selected type. let options = if self.set_o_names_only { brush_core::namedoptions::options(brush_core::namedoptions::ShellOptionKind::SetO) .iter() @@ -75,22 +82,19 @@ impl builtins::Command for ShoptCommand { if self.print { if self.set_o_names_only { let option_value_str = if option_value { "-o" } else { "+o" }; - writeln!(context.stdout(), "set {option_value_str} {}", option.name)?; + writeln!(output, "set {option_value_str} {}", option.name)?; } else { let option_value_str = if option_value { "-s" } else { "-u" }; - writeln!(context.stdout(), "shopt {option_value_str} {}", option.name)?; + writeln!(output, "shopt {option_value_str} {}", option.name)?; } } else { let option_value_str = if option_value { "on" } else { "off" }; - writeln!(context.stdout(), "{:20}\t{option_value_str}", option.name)?; + writeln!(output, "{:20}\t{option_value_str}", option.name)?; } } - - Ok(ExecutionResult::success()) } else { let mut return_value = ExecutionResult::success(); - // Enumerate only the specified options. for option_name in &self.options { let option_definition = if self.set_o_names_only { brush_core::namedoptions::options( @@ -119,35 +123,51 @@ impl builtins::Command for ShoptCommand { if self.print { if self.set_o_names_only { let option_value_str = if option_value { "-o" } else { "+o" }; - writeln!( - context.stdout(), - "set {option_value_str} {option_name}" - )?; + writeln!(output, "set {option_value_str} {option_name}")?; } else { let option_value_str = if option_value { "-s" } else { "-u" }; - writeln!( - context.stdout(), - "shopt {option_value_str} {option_name}" - )?; + writeln!(output, "shopt {option_value_str} {option_name}")?; } } else { let option_value_str = if option_value { "on" } else { "off" }; - writeln!(context.stdout(), "{option_name:20}\t{option_value_str}")?; + writeln!(output, "{option_name:20}\t{option_value_str}")?; } } } } else { writeln!( - context.stderr(), + stderr_output, "{}: {}: invalid shell option name", - context.command_name, - option_name + context.command_name, option_name )?; return_value = ExecutionResult::general_error(); } } - Ok(return_value) + if !stderr_output.is_empty() { + if let Some(mut stderr) = context.stderr() { + stderr.write_all(&stderr_output).await?; + stderr.flush().await?; + } + } + + if !output.is_empty() { + if let Some(mut stdout) = context.stdout() { + stdout.write_all(&output).await?; + stdout.flush().await?; + } + } + + return Ok(return_value); + } + + if !output.is_empty() { + if let Some(mut stdout) = context.stdout() { + stdout.write_all(&output).await?; + stdout.flush().await?; + } } + + Ok(ExecutionResult::success()) } } diff --git a/brush-builtins/src/suspend.rs b/brush-builtins/src/suspend.rs index 513408599..7e1ba902a 100644 --- a/brush-builtins/src/suspend.rs +++ b/brush-builtins/src/suspend.rs @@ -19,7 +19,12 @@ impl builtins::Command for SuspendCommand { context: brush_core::ExecutionContext<'_, SE>, ) -> Result { if context.shell.options().login_shell && !self.force { - writeln!(context.stderr(), "login shell cannot be suspended")?; + let mut stderr_output = Vec::new(); + writeln!(stderr_output, "login shell cannot be suspended")?; + if let Some(mut stderr) = context.stderr() { + stderr.write_all(&stderr_output).await?; + stderr.flush().await?; + } return Ok(ExecutionExitCode::InvalidUsage.into()); } diff --git a/brush-builtins/src/test.rs b/brush-builtins/src/test.rs index e9846b24f..956edc28e 100644 --- a/brush-builtins/src/test.rs +++ b/brush-builtins/src/test.rs @@ -40,7 +40,12 @@ impl builtins::Command for TestCommand { match args.last() { Some(s) if s == "]" => (), None | Some(_) => { - writeln!(context.stderr(), "[: missing ']'")?; + let mut stderr_output = Vec::new(); + writeln!(stderr_output, "[: missing ']'")?; + if let Some(mut stderr) = context.stderr() { + stderr.write_all(&stderr_output).await?; + stderr.flush().await?; + } return Ok(ExecutionExitCode::InvalidUsage.into()); } } diff --git a/brush-builtins/src/times.rs b/brush-builtins/src/times.rs index f6e39648e..4758f34a2 100644 --- a/brush-builtins/src/times.rs +++ b/brush-builtins/src/times.rs @@ -14,9 +14,11 @@ impl builtins::Command for TimesCommand { &self, context: brush_core::ExecutionContext<'_, SE>, ) -> Result { + let mut output = Vec::new(); + let (self_user, self_system) = brush_core::sys::resource::get_self_user_and_system_time()?; writeln!( - context.stdout(), + output, "{} {}", timing::format_duration_non_posixly(&self_user), timing::format_duration_non_posixly(&self_system), @@ -25,12 +27,17 @@ impl builtins::Command for TimesCommand { let (children_user, children_system) = brush_core::sys::resource::get_children_user_and_system_time()?; writeln!( - context.stdout(), + output, "{} {}", timing::format_duration_non_posixly(&children_user), timing::format_duration_non_posixly(&children_system), )?; + if let Some(mut stdout) = context.stdout() { + stdout.write_all(&output).await?; + stdout.flush().await?; + } + Ok(ExecutionResult::success()) } } diff --git a/brush-builtins/src/trap.rs b/brush-builtins/src/trap.rs index ac1743fe5..166c07799 100644 --- a/brush-builtins/src/trap.rs +++ b/brush-builtins/src/trap.rs @@ -26,30 +26,34 @@ impl builtins::Command for TrapCommand { mut context: brush_core::ExecutionContext<'_, SE>, ) -> Result { if self.list_signals { - brush_core::traps::format_signals(context.stdout(), TrapSignal::iterator()) - .map(|()| ExecutionResult::success()) + let mut output = Vec::new(); + brush_core::traps::format_signals(&mut output, TrapSignal::iterator())?; + if let Some(mut stdout) = context.stdout() { + stdout.write_all(&output).await?; + stdout.flush().await?; + } } else if self.print_trap_commands || self.args.is_empty() { + let mut output = Vec::new(); if !self.args.is_empty() { for signal_type in &self.args { - Self::display_handlers_for(&context, signal_type.parse()?)?; + Self::display_handlers_for(&context, signal_type.parse()?, &mut output)?; } } else { - Self::display_all_handlers(&context)?; + Self::display_all_handlers(&context, &mut output)?; + } + if !output.is_empty() { + if let Some(mut stdout) = context.stdout() { + stdout.write_all(&output).await?; + stdout.flush().await?; + } } - Ok(ExecutionResult::success()) } else if self.args.len() == 1 { - // When only a single argument is given, it is assumed to be a signal name - // and an indication to remove the handlers for that signal. let signal = self.args[0].as_str(); Self::remove_all_handlers(&mut context, signal.parse()?); - Ok(ExecutionResult::success()) } else if self.args[0] == "-" { - // "-" as the first argument indicates that the remaining - // arguments are signal names and we need to remove the handlers for them. for signal in &self.args[1..] { Self::remove_all_handlers(&mut context, signal.parse()?); } - Ok(ExecutionResult::success()) } else { let handler = &self.args[0]; @@ -59,17 +63,19 @@ impl builtins::Command for TrapCommand { } Self::register_handler(&mut context, signal_types, handler.as_str()); - Ok(ExecutionResult::success()) } + + Ok(ExecutionResult::success()) } } impl TrapCommand { fn display_all_handlers( context: &brush_core::ExecutionContext<'_, impl brush_core::ShellExtensions>, + output: &mut Vec, ) -> Result<(), brush_core::Error> { for (signal, _) in context.shell.traps().iter_handlers() { - Self::display_handlers_for(context, signal)?; + Self::display_handlers_for(context, signal, output)?; } Ok(()) } @@ -77,13 +83,10 @@ impl TrapCommand { fn display_handlers_for( context: &brush_core::ExecutionContext<'_, impl brush_core::ShellExtensions>, signal_type: TrapSignal, + output: &mut Vec, ) -> Result<(), brush_core::Error> { if let Some(handler) = context.shell.traps().get_handler(signal_type) { - writeln!( - context.stdout(), - "trap -- '{}' {signal_type}", - &handler.command - )?; + writeln!(output, "trap -- '{}' {signal_type}", &handler.command)?; } Ok(()) } @@ -100,9 +103,6 @@ impl TrapCommand { signals: Vec, handler: &str, ) { - // Our new source context is relative to the current position. - // TODO(source-info): Provide the location of the specific token that makes up - // `self.args[0]`. let source_info = context.shell.call_stack().current_pos_as_source_info(); for signal in signals { diff --git a/brush-builtins/src/type_.rs b/brush-builtins/src/type_.rs index bcf9840a2..fc95789ca 100644 --- a/brush-builtins/src/type_.rs +++ b/brush-builtins/src/type_.rs @@ -50,13 +50,15 @@ impl builtins::Command for TypeCommand { context: brush_core::ExecutionContext<'_, SE>, ) -> Result { let mut result = ExecutionResult::success(); + let mut output = Vec::new(); + let mut stderr_output = Vec::new(); for name in &self.names { let resolved_types = self.resolve_types(context.shell, name); if resolved_types.is_empty() { if !self.type_only && !self.force_path_search && !self.show_path_only { - writeln!(context.stderr(), "type: {name} not found")?; + writeln!(stderr_output, "type: {name} not found")?; } result = ExecutionResult::general_error(); @@ -69,56 +71,55 @@ impl builtins::Command for TypeCommand { } else if self.type_only { match resolved_type { ResolvedType::Alias(_) => { - writeln!(context.stdout(), "alias")?; + writeln!(output, "alias")?; } ResolvedType::Keyword => { - writeln!(context.stdout(), "keyword")?; + writeln!(output, "keyword")?; } ResolvedType::Function(_) => { - writeln!(context.stdout(), "function")?; + writeln!(output, "function")?; } ResolvedType::Builtin => { - writeln!(context.stdout(), "builtin")?; + writeln!(output, "builtin")?; } ResolvedType::File { path, .. } => { if self.show_path_only || self.force_path_search { - writeln!(context.stdout(), "{}", path.to_string_lossy())?; + writeln!(output, "{}", path.to_string_lossy())?; } else { - writeln!(context.stdout(), "file")?; + writeln!(output, "file")?; } } } } else { match resolved_type { ResolvedType::Alias(target) => { - writeln!(context.stdout(), "{name} is aliased to `{target}'")?; + writeln!(output, "{name} is aliased to `{target}'")?; } ResolvedType::Keyword => { - writeln!(context.stdout(), "{name} is a shell keyword")?; + writeln!(output, "{name} is a shell keyword")?; } ResolvedType::Function(def) => { - writeln!(context.stdout(), "{name} is a function")?; - writeln!(context.stdout(), "{def}")?; + writeln!(output, "{name} is a function")?; + writeln!(output, "{def}")?; } ResolvedType::Builtin => { - writeln!(context.stdout(), "{name} is a shell builtin")?; + writeln!(output, "{name} is a shell builtin")?; } ResolvedType::File { path, hashed } => { if hashed && self.all_locations && !self.force_path_search { - // Do nothing. When we're displaying all locations, then - // we don't show hashed paths. + // Do nothing. } else if self.show_path_only || self.force_path_search { - writeln!(context.stdout(), "{}", path.to_string_lossy())?; + writeln!(output, "{}", path.to_string_lossy())?; } else if hashed { writeln!( - context.stdout(), + output, "{name} is hashed ({path})", name = name, path = path.to_string_lossy() )?; } else { writeln!( - context.stdout(), + output, "{name} is {path}", name = name, path = path.to_string_lossy() @@ -128,13 +129,28 @@ impl builtins::Command for TypeCommand { } } - // If we only want the first, then break after the first. if !self.all_locations { break; } } } + // Write output async + if !output.is_empty() { + if let Some(mut stdout) = context.stdout() { + stdout.write_all(&output).await?; + stdout.flush().await?; + } + } + + // Write stderr + if !stderr_output.is_empty() { + if let Some(mut stderr) = context.stderr() { + stderr.write_all(&stderr_output).await?; + stderr.flush().await?; + } + } + Ok(result) } } diff --git a/brush-builtins/src/ulimit.rs b/brush-builtins/src/ulimit.rs index 7dd5f4ff2..445136ec5 100644 --- a/brush-builtins/src/ulimit.rs +++ b/brush-builtins/src/ulimit.rs @@ -280,11 +280,7 @@ impl ResourceDescription { } /// Print either soft or hard limit - fn print( - &self, - context: &brush_core::ExecutionContext<'_, impl brush_core::ShellExtensions>, - hard: bool, - ) -> io::Result<()> { + fn print_to(&self, output: &mut Vec, hard: bool) -> io::Result<()> { if !self.resource.is_supported() { return Ok(()); } @@ -298,13 +294,7 @@ impl ResourceDescription { Unit::Seconds => format!("(seconds, -{})", self.short), }; let resource = self.get(hard).unwrap_or_else(|e| format!("{e}")); - writeln!( - context.stdout(), - "{:<26}{:>16} {}", - self.description, - unit, - resource - ) + writeln!(output, "{:<26}{:>16} {}", self.description, unit, resource) } /// Provide the matching help String @@ -491,11 +481,20 @@ impl builtins::Command for ULimitCommand { resource.set(self.hard, value)?; } + let mut output = Vec::new(); + if resources_to_get.len() == 1 { - writeln!(context.stdout(), "{}", resources_to_get[0].get(self.hard)?)?; + writeln!(output, "{}", resources_to_get[0].get(self.hard)?)?; } else { for resource in resources_to_get { - resource.print(&context, self.hard)?; + resource.print_to(&mut output, self.hard)?; + } + } + + if !output.is_empty() { + if let Some(mut stdout) = context.stdout() { + stdout.write_all(&output).await?; + stdout.flush().await?; } } diff --git a/brush-builtins/src/umask.rs b/brush-builtins/src/umask.rs index 08a7c368a..350f5122c 100644 --- a/brush-builtins/src/umask.rs +++ b/brush-builtins/src/umask.rs @@ -46,10 +46,16 @@ impl builtins::Command for UmaskCommand { std::format!("{umask:04o}") }; + let mut output = Vec::new(); if self.print_roundtrippable { - writeln!(context.stdout(), "umask {formatted}")?; + writeln!(output, "umask {formatted}")?; } else { - writeln!(context.stdout(), "{formatted}")?; + writeln!(output, "{formatted}")?; + } + + if let Some(mut stdout) = context.stdout() { + stdout.write_all(&output).await?; + stdout.flush().await?; } } diff --git a/brush-builtins/src/unalias.rs b/brush-builtins/src/unalias.rs index c2cd283fa..aef65d92f 100644 --- a/brush-builtins/src/unalias.rs +++ b/brush-builtins/src/unalias.rs @@ -22,6 +22,7 @@ impl builtins::Command for UnaliasCommand { context: brush_core::ExecutionContext<'_, SE>, ) -> Result { let mut exit_code = ExecutionResult::success(); + let mut stderr_output = Vec::new(); if self.remove_all { context.shell.aliases_mut().clear(); @@ -29,16 +30,22 @@ impl builtins::Command for UnaliasCommand { for alias in &self.aliases { if context.shell.aliases_mut().remove(alias).is_none() { writeln!( - context.stderr(), + stderr_output, "{}: {}: not found", - context.command_name, - alias + context.command_name, alias )?; exit_code = ExecutionResult::general_error(); } } } + if !stderr_output.is_empty() { + if let Some(mut stderr) = context.stderr() { + stderr.write_all(&stderr_output).await?; + stderr.flush().await?; + } + } + Ok(exit_code) } } diff --git a/brush-builtins/src/wait.rs b/brush-builtins/src/wait.rs index 05b200975..727882008 100644 --- a/brush-builtins/src/wait.rs +++ b/brush-builtins/src/wait.rs @@ -1,5 +1,4 @@ use clap::Parser; -use std::io::Write; use brush_core::{ExecutionExitCode, ExecutionResult, builtins, error}; @@ -49,12 +48,14 @@ impl builtins::Command for WaitCommand { if let Some(job) = context.shell.jobs_mut().resolve_job_spec(id) { job.wait().await?; } else { - writeln!( - context.stderr(), - "{}: no such job: {}", - context.command_name, - id - )?; + if let Some(mut stderr) = context.stderr() { + let _ = stderr + .write_all( + format!("{}: no such job: {}\n", context.command_name, id) + .as_bytes(), + ) + .await; + } result = ExecutionExitCode::GeneralError.into(); } @@ -69,7 +70,9 @@ impl builtins::Command for WaitCommand { if context.shell.options().enable_job_control { for job in jobs { - writeln!(context.stdout(), "{job}")?; + if let Some(mut stdout) = context.stdout() { + let _ = stdout.write_all(format!("{job}\n").as_bytes()).await; + } } } } diff --git a/brush-core/Cargo.toml b/brush-core/Cargo.toml index ed7d9285c..57b11e51c 100644 --- a/brush-core/Cargo.toml +++ b/brush-core/Cargo.toml @@ -45,11 +45,13 @@ thiserror = "2.0.18" tracing = "0.1.44" [target.'cfg(target_family = "wasm")'.dependencies] -tokio = { version = "1.50.0", features = ["io-util", "macros", "rt", "sync"] } +tokio = { version = "1.50.0", features = ["io-util", "macros", "rt", "sync", "time"] } [target.'cfg(any(unix, windows))'.dependencies] hostname = "0.4.2" tokio = { version = "1.50.0", features = [ + "fs", + "io-std", "io-util", "macros", "net", diff --git a/brush-core/examples/call-func.rs b/brush-core/examples/call-func.rs index 38c225bc3..b0d0a5fba 100644 --- a/brush-core/examples/call-func.rs +++ b/brush-core/examples/call-func.rs @@ -30,7 +30,7 @@ async fn run_func(shell: &mut brush_core::Shell, suppress_stdout: bool) -> Resul if suppress_stdout { params.set_fd( brush_core::openfiles::OpenFiles::STDOUT_FD, - brush_core::openfiles::null()?, + brush_core::openfiles::null()?.into(), ); } diff --git a/brush-core/examples/custom-builtin.rs b/brush-core/examples/custom-builtin.rs index c32de4041..631325751 100644 --- a/brush-core/examples/custom-builtin.rs +++ b/brush-core/examples/custom-builtin.rs @@ -13,10 +13,8 @@ //! ``` use anyhow::Result; -use clap::Parser; -use std::io::Write; - use brush_core::{ExecutionResult, builtins}; +use clap::Parser; // // Step 1 (optional): Define a custom error type for your builtin @@ -104,7 +102,16 @@ impl builtins::Command for GreetCommand { // Execute the greeting. for _ in 0..self.repeat_count { - writeln!(context.stdout(), "{greeting}")?; + let mut stdout = context + .stdout() + .ok_or(GreetError::IoError(std::io::Error::new( + std::io::ErrorKind::BrokenPipe, + "stdout not available", + )))?; + stdout + .write_all(format!("{greeting}\n").as_bytes()) + .await + .map_err(GreetError::IoError)?; } // Return success diff --git a/brush-core/src/builtins.rs b/brush-core/src/builtins.rs index 180b1bff0..9f7cf4a31 100644 --- a/brush-core/src/builtins.rs +++ b/brush-core/src/builtins.rs @@ -2,7 +2,6 @@ use clap::builder::styling; pub use futures::future::BoxFuture; -use std::io::Write; use crate::{BuiltinError, CommandArg, commands, error, extensions, results}; @@ -480,7 +479,7 @@ async fn exec_builtin_impl command, Err(e) => { - let _ = writeln!(context.stderr(), "{e}"); + context.write_error(e).await; return Ok(results::ExecutionExitCode::InvalidUsage.into()); } }; @@ -523,7 +522,7 @@ async fn exec_declaration_builtin_impl< let mut command = match result { Ok(command) => command, Err(e) => { - let _ = writeln!(context.stderr(), "{e}"); + context.write_error(e).await; return Ok(results::ExecutionExitCode::InvalidUsage.into()); } }; diff --git a/brush-core/src/commands.rs b/brush-core/src/commands.rs index 077c4ac2c..8f4d40113 100644 --- a/brush-core/src/commands.rs +++ b/brush-core/src/commands.rs @@ -43,27 +43,23 @@ pub struct ExecutionContext<'a, SE: ShellExtensions = extensions::DefaultShellEx } impl ExecutionContext<'_, SE> { - /// Returns the standard input file; usable with `write!` et al. - pub fn stdin(&self) -> impl std::io::Read + 'static { - self.params.stdin(self.shell) + /// Returns the standard input file. + pub fn stdin(&self) -> Option { + self.params.try_stdin(self.shell) } - /// Returns the standard output file; usable with `write!` et al. - pub fn stdout(&self) -> impl std::io::Write + 'static { - self.params.stdout(self.shell) + /// Returns the standard output file. + pub fn stdout(&self) -> Option { + self.params.try_stdout(self.shell) } - /// Returns the standard error file; usable with `write!` et al. - pub fn stderr(&self) -> impl std::io::Write + 'static { - self.params.stderr(self.shell) + /// Returns the standard error file. + pub fn stderr(&self) -> Option { + self.params.try_stderr(self.shell) } /// Returns the file descriptor with the given number. Returns `None` /// if the file descriptor is not open. - /// - /// # Arguments - /// - /// * `fd` - The file descriptor number to retrieve. pub fn try_fd(&self, fd: ShellFd) -> Option { self.params.try_fd(self.shell, fd) } @@ -72,6 +68,31 @@ impl ExecutionContext<'_, SE> { pub fn iter_fds(&self) -> impl Iterator { self.params.iter_fds(self.shell) } + + /// Writes an error message to stderr. + pub async fn write_error(&self, msg: impl std::fmt::Display) { + if let Some(mut stderr) = self.stderr() { + let _ = stderr.write_all(format!("{msg}\n").as_bytes()).await; + } + } + + /// Writes a formatted line to stderr if available. + pub async fn write_stderr(&self, msg: impl std::fmt::Display) -> std::io::Result<()> { + if let Some(mut stderr) = self.stderr() { + stderr.write_all(format!("{msg}\n").as_bytes()).await?; + stderr.flush().await?; + } + Ok(()) + } + + /// Writes a formatted line to stdout if available. + pub async fn write_stdout(&self, msg: impl std::fmt::Display) -> std::io::Result<()> { + if let Some(mut stdout) = self.stdout() { + stdout.write_all(format!("{msg}\n").as_bytes()).await?; + stdout.flush().await?; + } + Ok(()) + } } /// An argument to a command. @@ -805,7 +826,11 @@ async fn run_substitution_command( if let Ok(program) = &parse_result { if let Some(redir) = try_unwrap_bare_input_redir_program(program) { interp::setup_redirect(&mut shell, &mut params, redir).await?; - std::io::copy(&mut params.stdin(&shell), &mut params.stdout(&shell))?; + if let (Some(mut stdin), Some(mut stdout)) = + (params.try_stdin(&shell), params.try_stdout(&shell)) + { + tokio::io::copy(&mut stdin, &mut stdout).await?; + } return Ok(ExecutionResult::new(0)); } } diff --git a/brush-core/src/expansion.rs b/brush-core/src/expansion.rs index abcd2d879..b2649ac1e 100644 --- a/brush-core/src/expansion.rs +++ b/brush-core/src/expansion.rs @@ -2,7 +2,6 @@ use std::borrow::Cow; use std::cmp::min; -use std::io::Write as _; use brush_parser::word::{ParameterTransformOp, SubstringMatchKind}; use itertools::Itertools; @@ -916,10 +915,13 @@ impl<'a, SE: extensions::ShellExtensions> WordExpander<'a, SE> { // Strips null bytes from command substitution output for compatibility. if cmd_output.contains('\0') { - writeln!( - self.params.stderr(self.shell), - "warning: command substitution: ignored null byte in input", - )?; + if let Some(mut stderr) = self.params.try_stderr(self.shell) { + let _ = stderr + .write_all( + b"warning: command substitution: ignored null byte in input\n", + ) + .await; + } cmd_output.retain(|c| c != '\0'); } diff --git a/brush-core/src/interp.rs b/brush-core/src/interp.rs index 212e3059d..e894ffba4 100644 --- a/brush-core/src/interp.rs +++ b/brush-core/src/interp.rs @@ -15,9 +15,7 @@ use crate::shell::Shell; use crate::variables::{ ArrayLiteral, ShellValue, ShellValueLiteral, ShellValueUnsetType, ShellVariable, }; -use crate::{ - ShellFd, error, expansion, extendedtests, extensions, ioutils, jobs, openfiles, sys, timing, -}; +use crate::{ShellFd, error, expansion, extendedtests, extensions, jobs, openfiles, sys, timing}; /// Encapsulates the context of execution in a command pipeline. struct PipelineExecutionContext<'a, SE: extensions::ShellExtensions> { @@ -40,87 +38,23 @@ pub struct ExecutionParameters { } impl ExecutionParameters { - /// Returns the standard input file; usable with `write!` et al. - /// - /// # Arguments - /// - /// * `shell` - The shell context. - pub fn stdin( - &self, - shell: &Shell, - ) -> impl std::io::Read + 'static { - self.try_stdin(shell).unwrap_or_else(|| { - ioutils::FailingReaderWriter::new("standard input not available").into() - }) - } - /// Tries to retrieve the standard input file. Returns `None` if not set. - /// - /// # Arguments - /// - /// * `shell` - The shell context. pub fn try_stdin(&self, shell: &Shell) -> Option { self.try_fd(shell, openfiles::OpenFiles::STDIN_FD) } - /// Returns the standard output file; usable with `write!` et al. In the event that - /// no such file is available, returns a valid implementation of `std::io::Write` - /// that fails all I/O requests. - /// - /// - /// # Arguments - /// - /// * `shell` - The shell context. - pub fn stdout( - &self, - shell: &Shell, - ) -> impl std::io::Write + 'static { - self.try_stdout(shell).unwrap_or_else(|| { - ioutils::FailingReaderWriter::new("standard output not available").into() - }) - } - /// Tries to retrieve the standard output file. Returns `None` if not set. - /// - /// # Arguments - /// - /// * `shell` - The shell context. pub fn try_stdout(&self, shell: &Shell) -> Option { self.try_fd(shell, openfiles::OpenFiles::STDOUT_FD) } - /// Returns the standard error file; usable with `write!` et al. In the event that - /// no such file is available, returns a valid implementation of `std::io::Write` - /// that fails all I/O requests. - /// - /// # Arguments - /// - /// * `shell` - The shell context. - pub fn stderr( - &self, - shell: &Shell, - ) -> impl std::io::Write + 'static { - self.try_stderr(shell).unwrap_or_else(|| { - ioutils::FailingReaderWriter::new("standard error not available").into() - }) - } - /// Tries to retrieve the standard error file. Returns `None` if not set. - /// - /// # Arguments - /// - /// * `shell` - The shell context. pub fn try_stderr(&self, shell: &Shell) -> Option { self.try_fd(shell, openfiles::OpenFiles::STDERR_FD) } /// Returns the file descriptor with the given number. Returns `None` /// if the file descriptor is not open. - /// - /// # Arguments - /// - /// * `shell` - The shell context. - /// * `fd` - The file descriptor number to retrieve. pub fn try_fd( &self, shell: &Shell, @@ -130,45 +64,31 @@ impl ExecutionParameters { openfiles::OpenFileEntry::Open(f) => Some(f.clone()), openfiles::OpenFileEntry::NotPresent => None, openfiles::OpenFileEntry::NotSpecified => { - // We didn't have this fd specified one way or the other; we fallback - // to what's represented in the shell's open files. shell.persistent_open_files().try_fd(fd).cloned() } } } /// Sets the given file descriptor to the provided open file. - /// - /// # Arguments - /// - /// * `fd` - The file descriptor number to set. - /// * `file` - The open file to set. pub fn set_fd(&mut self, fd: ShellFd, file: openfiles::OpenFile) { self.open_files.set_fd(fd, file); } /// Iterates over all open file descriptors in this context. - /// - /// # Arguments - /// - /// * `shell` - The shell context. pub fn iter_fds( &self, shell: &Shell, ) -> impl Iterator { - let our_fds = self.open_files.iter_fds(); - let shell_fds = shell - .persistent_open_files() + self.open_files .iter_fds() - .filter(|(fd, _)| !self.open_files.contains_fd(*fd)); - - #[allow(clippy::needless_collect)] - let all_fds: Vec<_> = our_fds - .chain(shell_fds) - .map(|(fd, file)| (fd, file.clone())) - .collect(); - - all_fds.into_iter() + .map(|(fd, f)| (fd, f.clone())) + .chain( + shell + .persistent_open_files() + .iter_fds() + .filter(|(fd, _)| !self.open_files.contains_fd(*fd)) + .map(|(fd, f)| (fd, f.clone())), + ) } } @@ -216,8 +136,9 @@ impl Execute for ast::Program { match command.execute(shell, params).await { Ok(exec_result) => result = exec_result, Err(err) => { - // Display the error and convert to an execution result. - let _ = shell.display_error(&mut params.stderr(shell), &err); + if let Some(mut stderr) = params.try_stderr(shell) { + let _ = shell.display_error(&err, &mut stderr).await; + } result = err.into_result(shell); } } @@ -252,7 +173,11 @@ impl Execute for ast::CompoundList { let job_formatted = job.to_pid_style_string(); if shell.options().interactive && !shell.is_subshell() { - writeln!(params.stderr(shell), "{job_formatted}")?; + if let Some(mut stderr) = params.try_stderr(shell) { + let _ = stderr + .write_all(format!("{job_formatted}\n").as_bytes()) + .await; + } } result = ExecutionResult::success(); @@ -287,7 +212,7 @@ fn spawn_async_ao_list_in_task<'a, SE: extensions::ShellExtensions>( // Redirect stdin to null, per spec. if let Ok(null) = openfiles::null() { - cloned_params.set_fd(openfiles::OpenFiles::STDIN_FD, null); + cloned_params.set_fd(openfiles::OpenFiles::STDIN_FD, null.into()); } let join_handle = tokio::spawn(async move { @@ -419,23 +344,22 @@ impl Execute for ast::Pipeline { && let Some(mut stderr) = params.try_fd(shell, openfiles::OpenFiles::STDERR_FD) { let timing = stopwatch.stop()?; - if timed.is_posix_output() { - std::write!( - stderr, + let output = if timed.is_posix_output() { + format!( "real {}\nuser {}\nsys {}\n", timing::format_duration_posixly(&timing.wall), timing::format_duration_posixly(&timing.user), timing::format_duration_posixly(&timing.system), - )?; + ) } else { - std::write!( - stderr, + format!( "\nreal\t{}\nuser\t{}\nsys\t{}\n", timing::format_duration_non_posixly(&timing.wall), timing::format_duration_non_posixly(&timing.user), timing::format_duration_non_posixly(&timing.system), - )?; - } + ) + }; + let _ = stderr.write_all(output.as_bytes()).await; } Ok(result) @@ -596,7 +520,11 @@ async fn wait_for_pipeline_processes_and_update_status( let formatted = job.to_string(); // N.B. We use the '\r' to overwrite any ^Z output. - writeln!(params.stderr(shell), "\r{formatted}")?; + if let Some(mut stderr) = params.try_stderr(shell) { + let _ = stderr + .write_all(format!("\r{formatted}\n").as_bytes()) + .await; + } } Ok(result) @@ -666,6 +594,64 @@ enum WhileOrUntil { Until, } +#[async_trait::async_trait] +impl Execute for ast::Command { + async fn execute( + &self, + shell: &mut Shell, + params: &ExecutionParameters, + ) -> Result { + match self { + Self::Simple(simple) => { + let context = PipelineExecutionContext { + shell: commands::ShellForCommand::ParentShell(shell), + process_group_id: None, + }; + match simple.execute_in_pipeline(context, params.clone()).await? { + ExecutionSpawnResult::Completed(result) => Ok(result), + ExecutionSpawnResult::StartedProcess(mut child) => { + let wait_result = child.wait().await?; + match wait_result { + crate::processes::ProcessWaitResult::Completed(output) => { + Ok(ExecutionResult::from(output)) + } + crate::processes::ProcessWaitResult::Stopped => { + Ok(ExecutionResult::stopped()) + } + } + } + ExecutionSpawnResult::StartedTask(handle) => handle.await?, + } + } + Self::Compound(compound, redirects) => { + let mut params = params.clone(); + if let Some(redirects) = redirects { + for redirect in &redirects.0 { + setup_redirect(shell, &mut params, redirect).await?; + } + } + compound.execute(shell, ¶ms).await + } + Self::Function(func) => func.execute(shell, params).await, + Self::ExtendedTest(e, redirects) => { + let mut params = params.clone(); + if let Some(redirects) = redirects { + for redirect in &redirects.0 { + setup_redirect(shell, &mut params, redirect).await?; + } + } + let result = + if extendedtests::eval_extended_test_expr(&e.expr, shell, ¶ms).await? { + 0 + } else { + 1 + }; + Ok(ExecutionResult::new(result)) + } + } + } +} + #[async_trait::async_trait] impl Execute for ast::CompoundCommand { async fn execute( @@ -687,11 +673,9 @@ impl Execute for ast::CompoundCommand { let subshell_result = match list.execute(&mut subshell, params).await { Ok(result) => result, Err(error) => { - // Display the error to stderr, but prevent fatal error propagation - let mut stderr = params.stderr(shell); - let _ = shell.display_error(&mut stderr, &error); - - // Convert error to result in subshell context + if let Some(mut stderr) = params.try_stderr(shell) { + let _ = shell.display_error(&error, &mut stderr).await; + } error.into_result(&subshell) } }; @@ -730,10 +714,11 @@ impl Execute for ast::CoprocessCommand { .map_or_else(|| "COPROC".to_string(), |w| w.to_string()); if !valid_variable_name(&name) { - writeln!( - params.stderr(shell), - "coproc {name}: not a valid identifier" - )?; + if let Some(mut stderr) = params.try_stderr(shell) { + let _ = stderr + .write_all(format!("coproc {name}: not a valid identifier\n").as_bytes()) + .await; + } return Ok(ExecutionExitCode::GeneralError.into()); } @@ -1178,7 +1163,9 @@ impl ExecuteInPipeline for ast::SimpleComma CommandPrefixOrSuffixItem::IoRedirect(redirect) => { if let Err(e) = setup_redirect(&mut context.shell, &mut params, redirect).await { - writeln!(params.stderr(&context.shell), "error: {e}")?; + if let Some(mut stderr) = params.try_stderr(&context.shell) { + let _ = stderr.write_all(format!("error: {e}\n").as_bytes()).await; + } return Ok(ExecutionResult::general_error().into()); } } @@ -1276,8 +1263,6 @@ impl ExecuteInPipeline for ast::SimpleComma // If we have a command, then execute it. if let Some(CommandArg::String(cmd_name)) = args.first().cloned() { - let mut stderr = params.stderr(&context.shell); - let (owned_shell, parent_shell) = match context.shell { commands::ShellForCommand::ParentShell(shell) => (None, shell), commands::ShellForCommand::OwnedShell { target, parent } => (Some(target), parent), @@ -1297,10 +1282,13 @@ impl ExecuteInPipeline for ast::SimpleComma process_group_id: context.process_group_id, }; + let params_clone = params.clone(); match execute_command(context, params, cmd_name, assignments, args).await { Ok(result) => Ok(result), Err(err) => { - let _ = parent_shell.display_error(&mut stderr, &err); + if let Some(mut stderr) = params_clone.try_stderr(parent_shell) { + let _ = parent_shell.display_error(&err, &mut stderr).await; + } let result = err.into_result(parent_shell); Ok(result.into()) diff --git a/brush-core/src/ioutils.rs b/brush-core/src/ioutils.rs index 8cc6e6ff4..7a950a6f8 100644 --- a/brush-core/src/ioutils.rs +++ b/brush-core/src/ioutils.rs @@ -35,10 +35,15 @@ impl FailingReaderWriter { pub const fn new(message: &'static str) -> Self { Self { message } } + + /// Returns the error message. + pub const fn message(&self) -> &'static str { + self.message + } } -impl openfiles::Stream for FailingReaderWriter { - fn clone_box(&self) -> Box { +impl openfiles::BlockingStream for FailingReaderWriter { + fn clone_box(&self) -> Box { Box::new(self.clone()) } @@ -73,7 +78,7 @@ impl std::io::Write for FailingReaderWriter { } } -impl From for openfiles::OpenFile { +impl From for openfiles::blocking::File { fn from(frw: FailingReaderWriter) -> Self { Self::Stream(Box::new(frw)) } diff --git a/brush-core/src/openfiles.rs b/brush-core/src/openfiles.rs index c34e7941b..7bf1ea07a 100644 --- a/brush-core/src/openfiles.rs +++ b/brush-core/src/openfiles.rs @@ -1,457 +1,1307 @@ //! Managing files open within a shell instance. -use std::collections::HashMap; -use std::io::IsTerminal; -use std::process::Stdio; - -use crate::ShellFd; use crate::error; -use crate::ioutils; use crate::sys; -/// A trait representing a stream that can be read from and written to. -/// This is used for custom stream implementations in `OpenFile`. +/// Blocking file types for child process spawning. /// -/// Types that implement this trait are expected to be cloneable via the -/// `clone_box` function. -pub trait Stream: std::io::Read + std::io::Write + Send + Sync { - /// Clones the stream into a boxed trait object. - fn clone_box(&self) -> Box; - - /// Converts the stream into an `OwnedFd`. Returns an error if the operation - /// is not supported or if it fails. - #[cfg(unix)] - fn try_clone_to_owned(&self) -> Result; +/// These types are used internally for converting to `std::process::Stdio` +/// when spawning child processes. +pub mod blocking { + use std::io::IsTerminal; + use std::process::Stdio; - /// Borrows the stream as a `BorrowedFd`. Returns an error if the operation - /// is not supported or if it fails. - #[cfg(unix)] - fn try_borrow_as_fd(&self) -> Result, error::Error>; -} + use crate::ShellFd; + use crate::error; + use crate::ioutils; -/// Represents a file open in a shell context. -pub enum OpenFile { - /// The original standard input this process was started with. - Stdin(std::io::Stdin), - /// The original standard output this process was started with. - Stdout(std::io::Stdout), - /// The original standard error this process was started with. - Stderr(std::io::Stderr), - /// A file open for reading or writing. - File(std::fs::File), - /// A read end of a pipe. - PipeReader(std::io::PipeReader), - /// A write end of a pipe. - PipeWriter(std::io::PipeWriter), - /// A custom stream. - Stream(Box), -} + /// A trait representing a blocking stream that can be read from and written to. + pub trait BlockingStream: std::io::Read + std::io::Write + Send + Sync { + /// Clones the stream into a boxed trait object. + fn clone_box(&self) -> Box; -#[cfg(feature = "serde")] -impl serde::Serialize for OpenFile { - fn serialize(&self, serializer: S) -> Result - where - S: serde::Serializer, - { - match self { - Self::Stdin(_) => serializer.serialize_str("stdin"), - Self::Stdout(_) => serializer.serialize_str("stdout"), - Self::Stderr(_) => serializer.serialize_str("stderr"), - Self::File(_) => serializer.serialize_str("file"), - Self::PipeReader(_) => serializer.serialize_str("pipe_reader"), - Self::PipeWriter(_) => serializer.serialize_str("pipe_writer"), - Self::Stream(_) => serializer.serialize_str("stream"), + /// Converts the stream into an `OwnedFd`. + #[cfg(unix)] + fn try_clone_to_owned(&self) -> Result; + + /// Borrows the stream as a `BorrowedFd`. + #[cfg(unix)] + fn try_borrow_as_fd(&self) -> Result, error::Error>; + } + + /// Represents a blocking file open in a shell context. + pub enum File { + /// The original standard input this process was started with. + Stdin(std::io::Stdin), + /// The original standard output this process was started with. + Stdout(std::io::Stdout), + /// The original standard error this process was started with. + Stderr(std::io::Stderr), + /// A file open for reading or writing. + File(std::fs::File), + /// A read end of a pipe. + PipeReader(std::io::PipeReader), + /// A write end of a pipe. + PipeWriter(std::io::PipeWriter), + /// A custom stream. + Stream(Box), + } + + impl Clone for File { + fn clone(&self) -> Self { + self.try_clone().unwrap_or_else(|_err| { + ioutils::FailingReaderWriter::new("failed to duplicate open file").into() + }) } } -} -#[cfg(feature = "serde")] -impl<'de> serde::Deserialize<'de> for OpenFile { - fn deserialize(deserializer: D) -> Result - where - D: serde::Deserializer<'de>, - { - match String::deserialize(deserializer)?.as_str() { - "stdin" => return Ok(std::io::stdin().into()), - "stdout" => return Ok(std::io::stdout().into()), - "stderr" => return Ok(std::io::stderr().into()), - "file" => (), - "pipe_reader" => (), - "pipe_writer" => (), - "stream" => (), - _ => return Err(serde::de::Error::custom("invalid open file")), + impl std::fmt::Display for File { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::Stdin(_) => write!(f, "stdin"), + Self::Stdout(_) => write!(f, "stdout"), + Self::Stderr(_) => write!(f, "stderr"), + Self::File(_) => write!(f, "file"), + Self::PipeReader(_) => write!(f, "pipe reader"), + Self::PipeWriter(_) => write!(f, "pipe writer"), + Self::Stream(_) => write!(f, "stream"), + } } + } - // TODO(serde): Figure out something better to do with open pipes and files. - null().map_err(serde::de::Error::custom) + impl File { + /// Tries to duplicate the open file. + pub fn try_clone(&self) -> Result { + let result = match self { + Self::Stdin(_) => std::io::stdin().into(), + Self::Stdout(_) => std::io::stdout().into(), + Self::Stderr(_) => std::io::stderr().into(), + Self::File(f) => f.try_clone()?.into(), + Self::PipeReader(f) => f.try_clone()?.into(), + Self::PipeWriter(f) => f.try_clone()?.into(), + Self::Stream(s) => Self::Stream(s.clone_box()), + }; + + Ok(result) + } + + /// Converts the open file into an `OwnedFd`. + #[cfg(unix)] + #[allow(dead_code)] + pub(crate) fn try_clone_to_owned(self) -> Result { + use std::os::fd::AsFd as _; + + match self { + Self::Stdin(f) => Ok(f.as_fd().try_clone_to_owned()?), + Self::Stdout(f) => Ok(f.as_fd().try_clone_to_owned()?), + Self::Stderr(f) => Ok(f.as_fd().try_clone_to_owned()?), + Self::File(f) => Ok(f.into()), + Self::PipeReader(r) => Ok(std::os::fd::OwnedFd::from(r)), + Self::PipeWriter(w) => Ok(std::os::fd::OwnedFd::from(w)), + Self::Stream(s) => s.try_clone_to_owned(), + } + } + + /// Borrows the open file as a `BorrowedFd`. + #[cfg(unix)] + pub fn try_borrow_as_fd(&self) -> Result, error::Error> { + use std::os::fd::AsFd as _; + + match self { + Self::Stdin(f) => Ok(f.as_fd()), + Self::Stdout(f) => Ok(f.as_fd()), + Self::Stderr(f) => Ok(f.as_fd()), + Self::File(f) => Ok(f.as_fd()), + Self::PipeReader(r) => Ok(r.as_fd()), + Self::PipeWriter(w) => Ok(w.as_fd()), + Self::Stream(s) => s.try_borrow_as_fd(), + } + } + + #[allow(dead_code)] + pub(crate) fn is_dir(&self) -> bool { + match self { + Self::Stdin(_) | Self::Stdout(_) | Self::Stderr(_) => false, + Self::File(file) => file.metadata().is_ok_and(|m| m.is_dir()), + Self::PipeReader(_) | Self::PipeWriter(_) | Self::Stream(_) => false, + } + } + + /// Checks if the open file is associated with a terminal. + pub fn is_terminal(&self) -> bool { + match self { + Self::Stdin(f) => f.is_terminal(), + Self::Stdout(f) => f.is_terminal(), + Self::Stderr(f) => f.is_terminal(), + Self::File(f) => f.is_terminal(), + Self::PipeReader(_) | Self::PipeWriter(_) | Self::Stream(_) => false, + } + } } -} -/// Returns an open file that will discard all I/O. -pub fn null() -> Result { - let file = sys::fs::open_null_file()?; - Ok(OpenFile::File(file)) -} + impl From for File { + fn from(stdin: std::io::Stdin) -> Self { + Self::Stdin(stdin) + } + } -impl Clone for OpenFile { - fn clone(&self) -> Self { - // If we fail to clone the open file for any reason, we return a special file - // that discards all I/O. This allows us to avoid fatally erroring out. - self.try_clone().unwrap_or_else(|_err| { - ioutils::FailingReaderWriter::new("failed to duplicate open file").into() - }) + impl From for File { + fn from(stdout: std::io::Stdout) -> Self { + Self::Stdout(stdout) + } } -} -impl std::fmt::Display for OpenFile { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - Self::Stdin(_) => write!(f, "stdin"), - Self::Stdout(_) => write!(f, "stdout"), - Self::Stderr(_) => write!(f, "stderr"), - Self::File(_) => write!(f, "file"), - Self::PipeReader(_) => write!(f, "pipe reader"), - Self::PipeWriter(_) => write!(f, "pipe writer"), - Self::Stream(_) => write!(f, "stream"), + impl From for File { + fn from(stderr: std::io::Stderr) -> Self { + Self::Stderr(stderr) } } -} -impl OpenFile { - /// Tries to duplicate the open file. - pub fn try_clone(&self) -> Result { - let result = match self { - Self::Stdin(_) => std::io::stdin().into(), - Self::Stdout(_) => std::io::stdout().into(), - Self::Stderr(_) => std::io::stderr().into(), - Self::File(f) => f.try_clone()?.into(), - Self::PipeReader(f) => f.try_clone()?.into(), - Self::PipeWriter(f) => f.try_clone()?.into(), - Self::Stream(s) => Self::Stream(s.clone_box()), - }; + impl From for File { + fn from(file: std::fs::File) -> Self { + Self::File(file) + } + } - Ok(result) + impl From for File { + fn from(reader: std::io::PipeReader) -> Self { + Self::PipeReader(reader) + } } - /// Converts the open file into an `OwnedFd`. - #[cfg(unix)] - pub(crate) fn try_clone_to_owned(self) -> Result { - use std::os::fd::AsFd as _; + impl From for File { + fn from(writer: std::io::PipeWriter) -> Self { + Self::PipeWriter(writer) + } + } - match self { - Self::Stdin(f) => Ok(f.as_fd().try_clone_to_owned()?), - Self::Stdout(f) => Ok(f.as_fd().try_clone_to_owned()?), - Self::Stderr(f) => Ok(f.as_fd().try_clone_to_owned()?), - Self::File(f) => Ok(f.into()), - Self::PipeReader(r) => Ok(std::os::fd::OwnedFd::from(r)), - Self::PipeWriter(w) => Ok(std::os::fd::OwnedFd::from(w)), - Self::Stream(s) => s.try_clone_to_owned(), + impl From for Stdio { + fn from(open_file: File) -> Self { + match open_file { + File::Stdin(_) => Self::inherit(), + File::Stdout(_) => Self::inherit(), + File::Stderr(_) => Self::inherit(), + File::File(f) => f.into(), + File::PipeReader(f) => f.into(), + File::PipeWriter(f) => f.into(), + File::Stream(_) => Self::null(), + } } } - /// Borrows the open file as a `BorrowedFd`. - /// - /// # Errors - /// - /// Returns an error if the operation is not supported for the underlying file type. - #[cfg(unix)] - pub fn try_borrow_as_fd(&self) -> Result, error::Error> { - use std::os::fd::AsFd as _; + impl std::io::Read for File { + fn read(&mut self, buf: &mut [u8]) -> std::io::Result { + match self { + Self::Stdin(f) => f.read(buf), + Self::Stdout(_) => Err(std::io::Error::other( + error::ErrorKind::OpenFileNotReadable("stdout"), + )), + Self::Stderr(_) => Err(std::io::Error::other( + error::ErrorKind::OpenFileNotReadable("stderr"), + )), + Self::File(f) => f.read(buf), + Self::PipeReader(reader) => reader.read(buf), + Self::PipeWriter(_) => Err(std::io::Error::other( + error::ErrorKind::OpenFileNotReadable("pipe writer"), + )), + Self::Stream(s) => s.read(buf), + } + } + } + + impl std::io::Write for File { + fn write(&mut self, buf: &[u8]) -> std::io::Result { + match self { + Self::Stdin(_) => Err(std::io::Error::other( + error::ErrorKind::OpenFileNotWritable("stdin"), + )), + Self::Stdout(f) => f.write(buf), + Self::Stderr(f) => f.write(buf), + Self::File(f) => f.write(buf), + Self::PipeReader(_) => Err(std::io::Error::other( + error::ErrorKind::OpenFileNotWritable("pipe reader"), + )), + Self::PipeWriter(writer) => writer.write(buf), + Self::Stream(s) => s.write(buf), + } + } - match self { - Self::Stdin(f) => Ok(f.as_fd()), - Self::Stdout(f) => Ok(f.as_fd()), - Self::Stderr(f) => Ok(f.as_fd()), - Self::File(f) => Ok(f.as_fd()), - Self::PipeReader(r) => Ok(r.as_fd()), - Self::PipeWriter(w) => Ok(w.as_fd()), - Self::Stream(s) => s.try_borrow_as_fd(), + fn flush(&mut self) -> std::io::Result<()> { + match self { + Self::Stdin(_) => Ok(()), + Self::Stdout(f) => f.flush(), + Self::Stderr(f) => f.flush(), + Self::File(f) => f.flush(), + Self::PipeReader(_) => Ok(()), + Self::PipeWriter(writer) => writer.flush(), + Self::Stream(s) => s.flush(), + } } } - pub(crate) fn is_dir(&self) -> bool { - match self { - Self::Stdin(_) | Self::Stdout(_) | Self::Stderr(_) => false, - Self::File(file) => file.metadata().is_ok_and(|m| m.is_dir()), - Self::PipeReader(_) | Self::PipeWriter(_) | Self::Stream(_) => false, + /// Represents the blocking open files in a shell context. + #[derive(Clone, Default)] + pub struct Files { + pub(crate) files: std::collections::HashMap>, + } + + impl Files { + /// File descriptor for standard input. + pub const STDIN_FD: ShellFd = 0; + /// File descriptor for standard output. + pub const STDOUT_FD: ShellFd = 1; + /// File descriptor for standard error. + pub const STDERR_FD: ShellFd = 2; + + #[allow(dead_code)] + pub(crate) fn new() -> Self { + Self { + files: std::collections::HashMap::from([ + (Self::STDIN_FD, Some(std::io::stdin().into())), + (Self::STDOUT_FD, Some(std::io::stdout().into())), + (Self::STDERR_FD, Some(std::io::stderr().into())), + ]), + } + } + + /// Returns a reference to the standard input file, if available. + pub fn try_stdin(&self) -> Option<&File> { + self.files.get(&Self::STDIN_FD).and_then(|f| f.as_ref()) + } + + /// Returns a reference to the standard output file, if available. + pub fn try_stdout(&self) -> Option<&File> { + self.files.get(&Self::STDOUT_FD).and_then(|f| f.as_ref()) + } + + /// Returns a reference to the standard error file, if available. + pub fn try_stderr(&self) -> Option<&File> { + self.files.get(&Self::STDERR_FD).and_then(|f| f.as_ref()) + } + + /// Returns a reference to the file at the given file descriptor, if available. + pub fn try_fd(&self, fd: ShellFd) -> Option<&File> { + self.files.get(&fd).and_then(|f| f.as_ref()) + } + + /// Sets the file at the given file descriptor, returning the previous file if any. + pub fn set_fd(&mut self, fd: ShellFd, file: File) -> Option { + self.files.insert(fd, Some(file)).and_then(|f| f) + } + + /// Adds a new file, returning the assigned file descriptor. + pub fn add(&mut self, file: File) -> Result { + let mut fd = 3; + while self.files.contains_key(&fd) { + if fd >= 1024 { + return Err(error::ErrorKind::TooManyOpenFiles.into()); + } + fd += 1; + } + self.files.insert(fd, Some(file)); + Ok(fd) + } + + /// Removes the file at the given file descriptor, returning it if it existed. + pub fn remove_fd(&mut self, fd: ShellFd) -> Option { + self.files.insert(fd, None).and_then(|f| f) + } + + /// Returns an iterator over all file descriptors and their associated files. + pub fn iter_fds(&self) -> impl Iterator { + self.files + .iter() + .filter_map(|(fd, file)| file.as_ref().map(|f| (*fd, f))) } } - /// Checks if the open file is associated with a terminal. - pub fn is_terminal(&self) -> bool { - match self { - Self::Stdin(f) => f.is_terminal(), - Self::Stdout(f) => f.is_terminal(), - Self::Stderr(f) => f.is_terminal(), - Self::File(f) => f.is_terminal(), - Self::PipeReader(_) | Self::PipeWriter(_) | Self::Stream(_) => false, + impl From for Files + where + I: Iterator, + { + fn from(iter: I) -> Self { + let files = iter.map(|(fd, file)| (fd, Some(file))).collect(); + Self { files } } } } -impl From for OpenFile { - /// Creates an `OpenFile` from standard input. - fn from(stdin: std::io::Stdin) -> Self { - Self::Stdin(stdin) - } +/// Returns an open file that will discard all I/O. +pub fn null() -> Result { + let file = sys::fs::open_null_file()?; + Ok(File::File(file)) } -impl From for OpenFile { - /// Creates an `OpenFile` from standard output. - fn from(stdout: std::io::Stdout) -> Self { - Self::Stdout(stdout) +pub use blocking::{BlockingStream, File}; + +/// Async file abstractions for non-blocking I/O operations. +pub mod async_file { + use std::io::{self, IsTerminal}; + use std::pin::Pin; + use std::task::{Context, Poll}; + + use tokio::io::{AsyncRead, AsyncWrite, ReadBuf}; + + use crate::error; + + /// Polyfill for tokio's stdio and file types on wasm targets. + /// + /// Since wasm targets don't support tokio's `io-std` and `fs` features, + /// we provide blocking wrappers that implement the async traits. + #[cfg(target_family = "wasm")] + pub mod stdio_polyfill { + use std::io::{self, IsTerminal, Read as _, Write as _}; + use std::pin::Pin; + use std::task::{Context, Poll}; + + use tokio::io::{AsyncRead, AsyncWrite, ReadBuf}; + + /// Async wrapper for standard input on wasm. + pub struct Stdin(io::Stdin); + + /// Async wrapper for standard output on wasm. + pub struct Stdout(io::Stdout); + + /// Async wrapper for standard error on wasm. + pub struct Stderr(io::Stderr); + + /// Async wrapper for a file on wasm. + pub struct File(std::fs::File); + + impl File { + /// Creates a new async file from a standard file. + pub fn from_std(file: std::fs::File) -> Self { + Self(file) + } + + /// Tries to convert the async file back to a standard file. + pub fn try_into_std(self) -> Result { + Ok(self.0) + } + + /// Returns metadata about the file. + pub async fn metadata(&self) -> io::Result { + self.0.metadata() + } + + /// Creates a new independently owned handle to the underlying file. + pub fn try_clone(&self) -> io::Result { + self.0.try_clone().map(Self) + } + } + + impl AsyncRead for File { + fn poll_read( + mut self: Pin<&mut Self>, + _cx: &mut Context<'_>, + buf: &mut ReadBuf<'_>, + ) -> Poll> { + let n = self.0.read(buf.initialize_unfilled())?; + buf.advance(n); + Poll::Ready(Ok(())) + } + } + + impl AsyncWrite for File { + fn poll_write( + mut self: Pin<&mut Self>, + _cx: &mut Context<'_>, + buf: &[u8], + ) -> Poll> { + Poll::Ready(self.0.write(buf)) + } + + fn poll_flush(mut self: Pin<&mut Self>, _cx: &mut Context<'_>) -> Poll> { + Poll::Ready(self.0.flush()) + } + + fn poll_shutdown(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll> { + self.poll_flush(cx) + } + } + + impl Stdin { + /// Creates a new async stdin wrapper. + pub fn new() -> Self { + Self(io::stdin()) + } + } + + impl Stdout { + /// Creates a new async stdout wrapper. + pub fn new() -> Self { + Self(io::stdout()) + } + } + + impl Stderr { + /// Creates a new async stderr wrapper. + pub fn new() -> Self { + Self(io::stderr()) + } + } + + impl AsyncRead for Stdin { + fn poll_read( + mut self: Pin<&mut Self>, + _cx: &mut Context<'_>, + buf: &mut ReadBuf<'_>, + ) -> Poll> { + let n = self.0.read(buf.initialize_unfilled())?; + buf.advance(n); + Poll::Ready(Ok(())) + } + } + + impl AsyncWrite for Stdout { + fn poll_write( + mut self: Pin<&mut Self>, + _cx: &mut Context<'_>, + buf: &[u8], + ) -> Poll> { + Poll::Ready(self.0.write(buf)) + } + + fn poll_flush(mut self: Pin<&mut Self>, _cx: &mut Context<'_>) -> Poll> { + Poll::Ready(self.0.flush()) + } + + fn poll_shutdown(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll> { + self.poll_flush(cx) + } + } + + impl AsyncWrite for Stderr { + fn poll_write( + mut self: Pin<&mut Self>, + _cx: &mut Context<'_>, + buf: &[u8], + ) -> Poll> { + Poll::Ready(self.0.write(buf)) + } + + fn poll_flush(mut self: Pin<&mut Self>, _cx: &mut Context<'_>) -> Poll> { + Poll::Ready(self.0.flush()) + } + + fn poll_shutdown(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll> { + self.poll_flush(cx) + } + } + + impl Stdin { + /// Returns true if this is a terminal. + pub fn is_terminal(&self) -> bool { + self.0.is_terminal() + } + } + + impl Stdout { + /// Returns true if this is a terminal. + pub fn is_terminal(&self) -> bool { + self.0.is_terminal() + } + } + + impl Stderr { + /// Returns true if this is a terminal. + pub fn is_terminal(&self) -> bool { + self.0.is_terminal() + } + } } -} -impl From for OpenFile { - /// Creates an `OpenFile` from standard error. - fn from(stderr: std::io::Stderr) -> Self { - Self::Stderr(stderr) + #[cfg(target_family = "wasm")] + use stdio_polyfill::{File, Stderr, Stdin, Stdout}; + + #[cfg(not(target_family = "wasm"))] + use tokio::fs::File; + + #[cfg(not(target_family = "wasm"))] + use tokio::io::{Stderr, Stdin, Stdout}; + + #[cfg(target_family = "wasm")] + fn stdin() -> Stdin { + Stdin::new() } -} -impl From for OpenFile { - fn from(file: std::fs::File) -> Self { - Self::File(file) + #[cfg(target_family = "wasm")] + fn stdout() -> Stdout { + Stdout::new() } -} -impl From for OpenFile { - fn from(reader: std::io::PipeReader) -> Self { - Self::PipeReader(reader) + #[cfg(target_family = "wasm")] + fn stderr() -> Stderr { + Stderr::new() } -} -impl From for OpenFile { - fn from(writer: std::io::PipeWriter) -> Self { - Self::PipeWriter(writer) + #[cfg(not(target_family = "wasm"))] + fn stdin() -> Stdin { + tokio::io::stdin() } -} -impl From for Stdio { - fn from(open_file: OpenFile) -> Self { - match open_file { - OpenFile::Stdin(_) => Self::inherit(), - OpenFile::Stdout(_) => Self::inherit(), - OpenFile::Stderr(_) => Self::inherit(), - OpenFile::File(f) => f.into(), - OpenFile::PipeReader(f) => f.into(), - OpenFile::PipeWriter(f) => f.into(), - // NOTE: Custom streams cannot be converted to `Stdio`; we do our best here - // and return a null device instead. - OpenFile::Stream(_) => Self::null(), + #[cfg(not(target_family = "wasm"))] + fn stdout() -> Stdout { + tokio::io::stdout() + } + + #[cfg(not(target_family = "wasm"))] + fn stderr() -> Stderr { + tokio::io::stderr() + } + + /// Sets a file descriptor to blocking mode. + #[cfg(unix)] + fn set_fd_blocking(fd: &impl std::os::fd::AsFd) -> std::io::Result<()> { + use std::os::fd::AsRawFd; + let borrowed_fd = fd.as_fd(); + let raw_fd = borrowed_fd.as_raw_fd(); + // SAFETY: fcntl with F_GETFL is safe to call on a valid file descriptor. + let flags = unsafe { libc::fcntl(raw_fd, libc::F_GETFL) }; + if flags < 0 { + return Err(std::io::Error::last_os_error()); } + // SAFETY: fcntl with F_SETFL is safe to call on a valid file descriptor with valid flags. + let result = unsafe { libc::fcntl(raw_fd, libc::F_SETFL, flags & !libc::O_NONBLOCK) }; + if result < 0 { + return Err(std::io::Error::last_os_error()); + } + Ok(()) + } + + /// A trait representing an async stream that can be read from and written to. + pub trait AsyncStream: AsyncRead + AsyncWrite + Send + Sync + Unpin { + /// Clones the stream into a boxed trait object. + fn clone_box(&self) -> Box; + + /// Converts the stream into an `OwnedFd`. + #[cfg(unix)] + fn try_clone_to_owned(&self) -> Result; + + /// Borrows the stream as a `BorrowedFd`. + #[cfg(unix)] + fn try_borrow_as_fd(&self) -> Result, error::Error>; + } + + /// Represents an async file open in a shell context. + #[cfg(unix)] + pub enum AsyncOpenFile { + /// The original standard input. + Stdin(Stdin), + /// The original standard output. + Stdout(Stdout), + /// The original standard error. + Stderr(Stderr), + /// A file open for reading or writing. + File(File), + /// The read end of a pipe. + PipeReader(tokio::net::unix::pipe::Receiver), + /// The write end of a pipe. + PipeWriter(tokio::net::unix::pipe::Sender), + /// A custom async stream. + Stream(Box), + /// A broken file that always returns an error; used as a safe fallback when a + /// file cannot be cloned or converted instead of silently misdirecting I/O. + Broken(String), } -} -impl std::io::Read for OpenFile { - fn read(&mut self, buf: &mut [u8]) -> std::io::Result { - match self { - Self::Stdin(f) => f.read(buf), - Self::Stdout(_) => Err(std::io::Error::other( - error::ErrorKind::OpenFileNotReadable("stdout"), - )), - Self::Stderr(_) => Err(std::io::Error::other( - error::ErrorKind::OpenFileNotReadable("stderr"), - )), - Self::File(f) => f.read(buf), - Self::PipeReader(reader) => reader.read(buf), - Self::PipeWriter(_) => Err(std::io::Error::other( - error::ErrorKind::OpenFileNotReadable("pipe writer"), - )), - Self::Stream(s) => s.read(buf), + /// Represents an async file open in a shell context. + #[cfg(not(unix))] + pub enum AsyncOpenFile { + /// The original standard input. + Stdin(Stdin), + /// The original standard output. + Stdout(Stdout), + /// The original standard error. + Stderr(Stderr), + /// A file open for reading or writing. + File(File), + /// The read end of a pipe. + PipeReader(tokio::io::DuplexStream), + /// The write end of a pipe. + PipeWriter(tokio::io::DuplexStream), + /// A custom async stream. + Stream(Box), + /// A broken file that always returns an error; used as a safe fallback when a + /// file cannot be cloned or converted instead of silently misdirecting I/O. + Broken(String), + } + + impl AsyncRead for AsyncOpenFile { + fn poll_read( + self: Pin<&mut Self>, + cx: &mut Context<'_>, + buf: &mut ReadBuf<'_>, + ) -> Poll> { + match self.get_mut() { + #[cfg(unix)] + Self::Stdin(f) => Pin::new(f).poll_read(cx, buf), + #[cfg(unix)] + Self::PipeReader(r) => Pin::new(r).poll_read(cx, buf), + #[cfg(unix)] + Self::PipeWriter(_) => Poll::Ready(Err(io::Error::other( + error::ErrorKind::OpenFileNotReadable("pipe writer"), + ))), + #[cfg(not(unix))] + Self::Stdin(f) => Pin::new(f).poll_read(cx, buf), + #[cfg(not(unix))] + Self::PipeReader(r) => Pin::new(r).poll_read(cx, buf), + #[cfg(not(unix))] + Self::PipeWriter(_) => Poll::Ready(Err(io::Error::other( + error::ErrorKind::OpenFileNotReadable("pipe writer"), + ))), + Self::Stdout(_) => Poll::Ready(Err(io::Error::other( + error::ErrorKind::OpenFileNotReadable("stdout"), + ))), + Self::Stderr(_) => Poll::Ready(Err(io::Error::other( + error::ErrorKind::OpenFileNotReadable("stderr"), + ))), + Self::File(f) => Pin::new(f).poll_read(cx, buf), + Self::Stream(s) => Pin::new(s.as_mut()).poll_read(cx, buf), + Self::Broken(msg) => Poll::Ready(Err(io::Error::other(msg.clone()))), + } } } -} -impl std::io::Write for OpenFile { - fn write(&mut self, buf: &[u8]) -> std::io::Result { - match self { - Self::Stdin(_) => Err(std::io::Error::other( - error::ErrorKind::OpenFileNotWritable("stdin"), - )), - Self::Stdout(f) => f.write(buf), - Self::Stderr(f) => f.write(buf), - Self::File(f) => f.write(buf), - Self::PipeReader(_) => Err(std::io::Error::other( - error::ErrorKind::OpenFileNotWritable("pipe reader"), - )), - Self::PipeWriter(writer) => writer.write(buf), - Self::Stream(s) => s.write(buf), - } - } - - fn flush(&mut self) -> std::io::Result<()> { - match self { - Self::Stdin(_) => Ok(()), - Self::Stdout(f) => f.flush(), - Self::Stderr(f) => f.flush(), - Self::File(f) => f.flush(), - Self::PipeReader(_) => Ok(()), - Self::PipeWriter(writer) => writer.flush(), - Self::Stream(s) => s.flush(), + impl AsyncWrite for AsyncOpenFile { + fn poll_write( + self: Pin<&mut Self>, + cx: &mut Context<'_>, + buf: &[u8], + ) -> Poll> { + match self.get_mut() { + #[cfg(unix)] + Self::Stdin(_) => Poll::Ready(Err(io::Error::other( + error::ErrorKind::OpenFileNotWritable("stdin"), + ))), + #[cfg(unix)] + Self::Stdout(f) => Pin::new(f).poll_write(cx, buf), + #[cfg(unix)] + Self::Stderr(f) => Pin::new(f).poll_write(cx, buf), + #[cfg(unix)] + Self::PipeReader(_) => Poll::Ready(Err(io::Error::other( + error::ErrorKind::OpenFileNotWritable("pipe reader"), + ))), + #[cfg(unix)] + Self::PipeWriter(w) => Pin::new(w).poll_write(cx, buf), + #[cfg(not(unix))] + Self::Stdin(_) => Poll::Ready(Err(io::Error::other( + error::ErrorKind::OpenFileNotWritable("stdin"), + ))), + #[cfg(not(unix))] + Self::Stdout(f) => Pin::new(f).poll_write(cx, buf), + #[cfg(not(unix))] + Self::Stderr(f) => Pin::new(f).poll_write(cx, buf), + #[cfg(not(unix))] + Self::PipeReader(_) => Poll::Ready(Err(io::Error::other( + error::ErrorKind::OpenFileNotWritable("pipe reader"), + ))), + #[cfg(not(unix))] + Self::PipeWriter(w) => Pin::new(w).poll_write(cx, buf), + Self::File(f) => Pin::new(f).poll_write(cx, buf), + Self::Stream(s) => Pin::new(s.as_mut()).poll_write(cx, buf), + Self::Broken(msg) => Poll::Ready(Err(io::Error::other(msg.clone()))), + } + } + + fn poll_flush(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll> { + match self.get_mut() { + Self::Stdin(_) => Poll::Ready(Ok(())), + Self::Stdout(f) => Pin::new(f).poll_flush(cx), + Self::Stderr(f) => Pin::new(f).poll_flush(cx), + Self::File(f) => Pin::new(f).poll_flush(cx), + Self::PipeReader(_) => Poll::Ready(Ok(())), + #[cfg(unix)] + Self::PipeWriter(w) => Pin::new(w).poll_flush(cx), + #[cfg(not(unix))] + Self::PipeWriter(w) => Pin::new(w).poll_flush(cx), + Self::Stream(s) => Pin::new(s.as_mut()).poll_flush(cx), + Self::Broken(msg) => Poll::Ready(Err(io::Error::other(msg.clone()))), + } + } + + fn poll_shutdown(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll> { + match self.get_mut() { + Self::Stdin(_) => Poll::Ready(Ok(())), + Self::Stdout(f) => Pin::new(f).poll_shutdown(cx), + Self::Stderr(f) => Pin::new(f).poll_shutdown(cx), + Self::File(f) => Pin::new(f).poll_shutdown(cx), + Self::PipeReader(_) => Poll::Ready(Ok(())), + #[cfg(unix)] + Self::PipeWriter(w) => Pin::new(w).poll_shutdown(cx), + #[cfg(not(unix))] + Self::PipeWriter(w) => Pin::new(w).poll_shutdown(cx), + Self::Stream(s) => Pin::new(s.as_mut()).poll_shutdown(cx), + Self::Broken(msg) => Poll::Ready(Err(io::Error::other(msg.clone()))), + } } } -} -/// Tristate representing the an `OpenFile` entry in an `OpenFiles` structure. -pub enum OpenFileEntry<'a> { - /// File descriptor is present and has a valid associated `OpenFile`. - Open(&'a OpenFile), - /// File descriptor is explicitly marked as not being mapped to any `OpenFile`. - NotPresent, - /// File descriptor is not specified in any way; it may be provided by a - /// parent context of some kind. - NotSpecified, -} + #[cfg(unix)] + impl AsyncOpenFile { + /// Creates an async file from a standard file. + pub fn from_std_file(file: std::fs::File) -> Self { + Self::File(tokio::fs::File::from_std(file)) + } -/// Represents the open files in a shell context. -#[derive(Clone, Default)] -#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] -pub struct OpenFiles { - /// Maps shell file descriptors to open files. - files: HashMap>, -} + /// Creates an async pipe reader from a blocking pipe reader. + pub fn from_pipe_reader(reader: std::io::PipeReader) -> io::Result { + use std::os::fd::OwnedFd; + let owned_fd = OwnedFd::from(reader); + let receiver = + tokio::net::unix::pipe::Receiver::from_file(std::fs::File::from(owned_fd))?; + Ok(Self::PipeReader(receiver)) + } -impl OpenFiles { - /// File descriptor used for standard input. - pub const STDIN_FD: ShellFd = 0; - /// File descriptor used for standard output. - pub const STDOUT_FD: ShellFd = 1; - /// File descriptor used for standard error. - pub const STDERR_FD: ShellFd = 2; - - /// First file descriptor available for non-stdio files. - const FIRST_NON_STDIO_FD: ShellFd = 3; - /// Maximum file descriptor number allowed. - const MAX_FD: ShellFd = 1024; - - /// Creates a new `OpenFiles` instance populated with stdin, stdout, and stderr - /// from the host environment. - pub(crate) fn new() -> Self { - Self { - files: HashMap::from([ - (Self::STDIN_FD, Some(std::io::stdin().into())), - (Self::STDOUT_FD, Some(std::io::stdout().into())), - (Self::STDERR_FD, Some(std::io::stderr().into())), - ]), - } - } - - /// Updates the open files from the provided iterator of (fd number, `OpenFile`) pairs. - /// Any existing entries for the provided file descriptors will be overwritten. - /// - /// # Arguments - /// - /// * `files`: An iterator of (fd number, `OpenFile`) pairs to update the open files with. - pub fn update_from(&mut self, files: impl Iterator) { - for (fd, file) in files { - let _ = self.files.insert(fd, Some(file)); + /// Creates an async pipe writer from a blocking pipe writer. + pub fn from_pipe_writer(writer: std::io::PipeWriter) -> io::Result { + use std::os::fd::OwnedFd; + let owned_fd = OwnedFd::from(writer); + let sender = tokio::net::unix::pipe::Sender::from_file(std::fs::File::from(owned_fd))?; + Ok(Self::PipeWriter(sender)) + } + + /// Tries to clone the open file. + pub fn try_clone(&self) -> io::Result { + match self { + Self::Stdin(_) => Ok(Self::Stdin(stdin())), + Self::Stdout(_) => Ok(Self::Stdout(stdout())), + Self::Stderr(_) => Ok(Self::Stderr(stderr())), + Self::File(f) => { + use std::os::fd::{AsFd, FromRawFd, OwnedFd}; + let raw = + nix::fcntl::fcntl(f.as_fd(), nix::fcntl::FcntlArg::F_DUPFD_CLOEXEC(0))?; + // SAFETY: fcntl(F_DUPFD_CLOEXEC) returns a valid, new file descriptor on success. + let new_fd = unsafe { OwnedFd::from_raw_fd(raw) }; + let std_file = std::fs::File::from(new_fd); + Ok(Self::File(File::from_std(std_file))) + } + Self::PipeReader(r) => { + use std::os::fd::{AsFd, FromRawFd, OwnedFd}; + let raw = + nix::fcntl::fcntl(r.as_fd(), nix::fcntl::FcntlArg::F_DUPFD_CLOEXEC(0))?; + // SAFETY: fcntl(F_DUPFD_CLOEXEC) returns a valid, new file descriptor on success. + let new_fd = unsafe { OwnedFd::from_raw_fd(raw) }; + Ok(Self::PipeReader( + tokio::net::unix::pipe::Receiver::from_owned_fd_unchecked(new_fd)?, + )) + } + Self::PipeWriter(w) => { + use std::os::fd::{AsFd, FromRawFd, OwnedFd}; + let raw = + nix::fcntl::fcntl(w.as_fd(), nix::fcntl::FcntlArg::F_DUPFD_CLOEXEC(0))?; + // SAFETY: fcntl(F_DUPFD_CLOEXEC) returns a valid, new file descriptor on success. + let new_fd = unsafe { OwnedFd::from_raw_fd(raw) }; + Ok(Self::PipeWriter( + tokio::net::unix::pipe::Sender::from_owned_fd_unchecked(new_fd)?, + )) + } + Self::Stream(s) => Ok(Self::Stream(s.clone_box())), + Self::Broken(msg) => Ok(Self::Broken(msg.clone())), + } } } - /// Retrieves the file backing standard input in this context. - pub fn try_stdin(&self) -> Option<&OpenFile> { - self.files.get(&Self::STDIN_FD).and_then(|f| f.as_ref()) + #[cfg(not(unix))] + impl AsyncOpenFile { + /// Creates an async file from a standard file. + pub fn from_std_file(file: std::fs::File) -> Self { + Self::File(File::from_std(file)) + } + + /// Creates an async pipe reader from a blocking pipe reader. + pub fn from_pipe_reader(_reader: std::io::PipeReader) -> io::Result { + Err(io::Error::new( + io::ErrorKind::Unsupported, + "async pipes not supported on non-unix", + )) + } + + /// Creates an async pipe writer from a blocking pipe writer. + pub fn from_pipe_writer(_writer: std::io::PipeWriter) -> io::Result { + Err(io::Error::new( + io::ErrorKind::Unsupported, + "async pipes not supported on non-unix", + )) + } + + /// Tries to clone the open file. + #[cfg(not(target_family = "wasm"))] + pub fn try_clone(&self) -> io::Result { + match self { + Self::Stdin(_) => Ok(Self::Stdin(stdin())), + Self::Stdout(_) => Ok(Self::Stdout(stdout())), + Self::Stderr(_) => Ok(Self::Stderr(stderr())), + Self::File(f) => { + let handle = tokio::runtime::Handle::current(); + tokio::task::block_in_place(|| handle.block_on(f.try_clone())).map(Self::File) + } + Self::PipeReader(_) | Self::PipeWriter(_) => Err(io::Error::new( + io::ErrorKind::Unsupported, + "cannot clone pipes", + )), + Self::Stream(s) => Ok(Self::Stream(s.clone_box())), + Self::Broken(msg) => Ok(Self::Broken(msg.clone())), + } + } + + /// Tries to clone the open file. + #[cfg(target_family = "wasm")] + pub fn try_clone(&self) -> io::Result { + match self { + Self::Stdin(_) => Ok(Self::Stdin(stdin())), + Self::Stdout(_) => Ok(Self::Stdout(stdout())), + Self::Stderr(_) => Ok(Self::Stderr(stderr())), + Self::File(f) => f.try_clone().map(Self::File), + Self::PipeReader(_) | Self::PipeWriter(_) => Err(io::Error::new( + io::ErrorKind::Unsupported, + "cannot clone pipes", + )), + Self::Stream(s) => Ok(Self::Stream(s.clone_box())), + Self::Broken(msg) => Ok(Self::Broken(msg.clone())), + } + } + } + + impl AsyncOpenFile { + /// Reads bytes asynchronously into the provided buffer. + /// + /// Returns the number of bytes read, or 0 on EOF. + /// Reads bytes into the provided buffer. + pub async fn read(&mut self, buf: &mut [u8]) -> io::Result { + use tokio::io::AsyncReadExt; + match self { + Self::Stdin(f) => f.read(buf).await, + Self::Stdout(_) => Err(io::Error::other(error::ErrorKind::OpenFileNotReadable( + "stdout", + ))), + Self::Stderr(_) => Err(io::Error::other(error::ErrorKind::OpenFileNotReadable( + "stderr", + ))), + Self::File(f) => f.read(buf).await, + Self::PipeReader(r) => r.read(buf).await, + Self::PipeWriter(_) => Err(io::Error::other( + error::ErrorKind::OpenFileNotReadable("pipe writer"), + )), + Self::Stream(s) => Pin::new(s.as_mut()).read(buf).await, + Self::Broken(msg) => Err(io::Error::other(msg.clone())), + } + } + + /// Reads all bytes until EOF into a new String. + pub async fn read_to_string(&mut self) -> io::Result { + use tokio::io::AsyncReadExt; + let mut s = String::new(); + match self { + Self::Stdin(f) => f.read_to_string(&mut s).await?, + Self::Stdout(_) => { + return Err(io::Error::other(error::ErrorKind::OpenFileNotReadable( + "stdout", + ))); + } + Self::Stderr(_) => { + return Err(io::Error::other(error::ErrorKind::OpenFileNotReadable( + "stderr", + ))); + } + Self::File(f) => f.read_to_string(&mut s).await?, + Self::PipeReader(r) => r.read_to_string(&mut s).await?, + Self::PipeWriter(_) => { + return Err(io::Error::other(error::ErrorKind::OpenFileNotReadable( + "pipe writer", + ))); + } + Self::Stream(s_) => Pin::new(s_.as_mut()).read_to_string(&mut s).await?, + Self::Broken(msg) => return Err(io::Error::other(msg.clone())), + }; + Ok(s) + } + + /// Writes bytes from the provided buffer. + pub async fn write(&mut self, buf: &[u8]) -> io::Result { + use tokio::io::AsyncWriteExt; + match self { + Self::Stdin(_) => Err(io::Error::other(error::ErrorKind::OpenFileNotWritable( + "stdin", + ))), + Self::Stdout(f) => f.write(buf).await, + Self::Stderr(f) => f.write(buf).await, + Self::File(f) => f.write(buf).await, + Self::PipeReader(_) => Err(io::Error::other( + error::ErrorKind::OpenFileNotWritable("pipe reader"), + )), + Self::PipeWriter(w) => w.write(buf).await, + Self::Stream(s) => Pin::new(s.as_mut()).write(buf).await, + Self::Broken(msg) => Err(io::Error::other(msg.clone())), + } + } + + /// Writes all bytes from the provided buffer. + pub async fn write_all(&mut self, buf: &[u8]) -> io::Result<()> { + use tokio::io::AsyncWriteExt; + match self { + Self::Stdin(_) => Err(io::Error::other(error::ErrorKind::OpenFileNotWritable( + "stdin", + ))), + Self::Stdout(f) => f.write_all(buf).await, + Self::Stderr(f) => f.write_all(buf).await, + Self::File(f) => f.write_all(buf).await, + Self::PipeReader(_) => Err(io::Error::other( + error::ErrorKind::OpenFileNotWritable("pipe reader"), + )), + Self::PipeWriter(w) => w.write_all(buf).await, + Self::Stream(s) => Pin::new(s.as_mut()).write_all(buf).await, + Self::Broken(msg) => Err(io::Error::other(msg.clone())), + } + } + + /// Flushes the output stream. + pub async fn flush(&mut self) -> io::Result<()> { + use tokio::io::AsyncWriteExt; + match self { + Self::Stdin(_) => Ok(()), + Self::Stdout(f) => f.flush().await, + Self::Stderr(f) => f.flush().await, + Self::File(f) => f.flush().await, + Self::PipeReader(_) => Ok(()), + Self::PipeWriter(w) => w.flush().await, + Self::Stream(s) => Pin::new(s.as_mut()).flush().await, + Self::Broken(msg) => Err(io::Error::other(msg.clone())), + } + } + + /// Checks if this file represents a terminal. + pub fn is_terminal(&self) -> bool { + match self { + Self::Stdin(_) => std::io::stdin().is_terminal(), + Self::Stdout(_) => std::io::stdout().is_terminal(), + Self::Stderr(_) => std::io::stderr().is_terminal(), + Self::File(_) + | Self::PipeReader(_) + | Self::PipeWriter(_) + | Self::Stream(_) + | Self::Broken(_) => false, + } + } + + /// Checks if this file represents a directory. + pub async fn is_dir(&self) -> bool { + match self { + Self::File(f) => f.metadata().await.is_ok_and(|m| m.is_dir()), + _ => false, + } + } + + /// Tries to convert the open file into an `OwnedFd`. + #[cfg(unix)] + pub fn try_clone_to_owned(&self) -> Result { + use std::os::fd::AsFd; + match self { + Self::Stdin(_) => Ok(std::io::stdin().as_fd().try_clone_to_owned()?), + Self::Stdout(_) => Ok(std::io::stdout().as_fd().try_clone_to_owned()?), + Self::Stderr(_) => Ok(std::io::stderr().as_fd().try_clone_to_owned()?), + Self::File(f) => Ok(f.as_fd().try_clone_to_owned()?), + Self::PipeReader(r) => { + let fd = r.as_fd().try_clone_to_owned()?; + set_fd_blocking(&fd)?; + Ok(fd) + } + Self::PipeWriter(w) => { + let fd = w.as_fd().try_clone_to_owned()?; + set_fd_blocking(&fd)?; + Ok(fd) + } + Self::Stream(s) => s.try_clone_to_owned(), + Self::Broken(_) => Err(error::ErrorKind::CannotConvertToNativeFd.into()), + } + } + + /// Borrows the open file as a `BorrowedFd`. + #[cfg(unix)] + pub fn try_borrow_as_fd(&self) -> Result, error::Error> { + use std::os::fd::AsFd; + match self { + Self::Stdin(f) => Ok(f.as_fd()), + Self::Stdout(f) => Ok(f.as_fd()), + Self::Stderr(f) => Ok(f.as_fd()), + Self::File(f) => Ok(f.as_fd()), + Self::PipeReader(r) => Ok(r.as_fd()), + Self::PipeWriter(w) => Ok(w.as_fd()), + Self::Stream(s) => s.try_borrow_as_fd(), + Self::Broken(_) => Err(crate::error::ErrorKind::CannotConvertToNativeFd.into()), + } + } } - /// Retrieves the file backing standard output in this context. - pub fn try_stdout(&self) -> Option<&OpenFile> { - self.files.get(&Self::STDOUT_FD).and_then(|f| f.as_ref()) + impl Clone for AsyncOpenFile { + fn clone(&self) -> Self { + self.try_clone().unwrap_or_else(|err| { + tracing::warn!("failed to clone open file ({err})"); + Self::Broken(err.to_string()) + }) + } } - /// Retrieves the file backing standard error in this context. - pub fn try_stderr(&self) -> Option<&OpenFile> { - self.files.get(&Self::STDERR_FD).and_then(|f| f.as_ref()) + impl From for AsyncOpenFile { + fn from(frw: crate::ioutils::FailingReaderWriter) -> Self { + Self::Broken(frw.message().to_owned()) + } } - /// Tries to remove an open file by its file descriptor. If the file descriptor - /// is not used, `None` will be returned; otherwise, the removed file will - /// be returned. - /// - /// Arguments: - /// - /// * `fd`: The file descriptor to remove. - pub fn remove_fd(&mut self, fd: ShellFd) -> Option { - self.files.insert(fd, None).and_then(|f| f) + impl From for AsyncOpenFile { + fn from(file: super::blocking::File) -> Self { + match file { + super::blocking::File::Stdin(_) => Self::Stdin(stdin()), + super::blocking::File::Stdout(_) => Self::Stdout(stdout()), + super::blocking::File::Stderr(_) => Self::Stderr(stderr()), + super::blocking::File::File(f) => Self::File(File::from_std(f)), + super::blocking::File::PipeReader(r) => { + Self::from_pipe_reader(r).unwrap_or_else(|err| { + tracing::warn!("failed to convert pipe reader to async ({err})"); + Self::Broken(err.to_string()) + }) + } + super::blocking::File::PipeWriter(w) => { + Self::from_pipe_writer(w).unwrap_or_else(|err| { + tracing::warn!("failed to convert pipe writer to async ({err})"); + Self::Broken(err.to_string()) + }) + } + super::blocking::File::Stream(_) => { + Self::Broken("cannot convert blocking stream to async".to_owned()) + } + } + } } - /// Tries to lookup the `OpenFile` associated with a file descriptor. - /// Returns `None` if the file descriptor is not present. - /// - /// Arguments: - /// - /// * `fd`: The file descriptor to lookup. - pub fn try_fd(&self, fd: ShellFd) -> Option<&OpenFile> { - self.files.get(&fd).and_then(|f| f.as_ref()) + impl From for AsyncOpenFile { + fn from(reader: std::io::PipeReader) -> Self { + Self::from_pipe_reader(reader).unwrap_or_else(|err| { + tracing::warn!("failed to convert pipe reader to async ({err})"); + Self::Broken(err.to_string()) + }) + } } - /// Tries to lookup the `OpenFile` associated with a file descriptor. Returns - /// an `OpenFileEntry` representing the state of the file descriptor. - /// - /// Arguments: - /// - /// * `fd`: The file descriptor to lookup. - pub fn fd_entry(&self, fd: ShellFd) -> OpenFileEntry<'_> { - self.files - .get(&fd) - .map_or(OpenFileEntry::NotSpecified, |opt_file| match opt_file { - Some(f) => OpenFileEntry::Open(f), - None => OpenFileEntry::NotPresent, + impl From for AsyncOpenFile { + fn from(writer: std::io::PipeWriter) -> Self { + Self::from_pipe_writer(writer).unwrap_or_else(|err| { + tracing::warn!("failed to convert pipe writer to async ({err})"); + Self::Broken(err.to_string()) }) + } } - /// Checks if the given file descriptor is in use. - pub fn contains_fd(&self, fd: ShellFd) -> bool { - self.files.contains_key(&fd) + impl From for AsyncOpenFile { + fn from(file: std::fs::File) -> Self { + Self::File(File::from_std(file)) + } } - /// Associates the given file descriptor with the provided file. If the file descriptor - /// is already in use, the previous file will be returned; otherwise, `None` - /// will be returned. - /// - /// Arguments: - /// - /// * `fd`: The file descriptor to associate with the file. - /// * `file`: The file to associate with the file descriptor. - pub fn set_fd(&mut self, fd: ShellFd, file: OpenFile) -> Option { - self.files.insert(fd, Some(file)).and_then(|f| f) + impl From for AsyncOpenFile { + fn from(_stdin: std::io::Stdin) -> Self { + Self::Stdin(stdin()) + } } - /// Iterates over all file descriptors. - pub fn iter_fds(&self) -> impl Iterator { - self.files - .iter() - .filter_map(|(fd, file)| file.as_ref().map(|f| (*fd, f))) + impl From for AsyncOpenFile { + fn from(_stdout: std::io::Stdout) -> Self { + Self::Stdout(stdout()) + } } - /// Adds a new open file, returning the assigned file descriptor. - /// - /// # Arguments - /// - /// * `file`: The open file to add. - pub fn add(&mut self, file: OpenFile) -> Result { - // Start searching for free file descriptors after the standard ones. - let mut fd = Self::FIRST_NON_STDIO_FD; - while self.files.contains_key(&fd) { - if fd >= Self::MAX_FD { - return Err(error::ErrorKind::TooManyOpenFiles.into()); + impl From for AsyncOpenFile { + fn from(_stderr: std::io::Stderr) -> Self { + Self::Stderr(stderr()) + } + } + + #[cfg(unix)] + impl From for std::process::Stdio { + fn from(file: AsyncOpenFile) -> Self { + match file { + AsyncOpenFile::Stdin(_) => Self::inherit(), + AsyncOpenFile::Stdout(_) => Self::inherit(), + AsyncOpenFile::Stderr(_) => Self::inherit(), + AsyncOpenFile::File(f) => f + .try_into_std() + .map_or_else(|_| Self::inherit(), Self::from), + AsyncOpenFile::PipeReader(r) => r + .into_blocking_fd() + .map(std::fs::File::from) + .map_or_else(|_| Self::null(), Self::from), + AsyncOpenFile::PipeWriter(w) => w + .into_blocking_fd() + .map(std::fs::File::from) + .map_or_else(|_| Self::null(), Self::from), + AsyncOpenFile::Stream(_) | AsyncOpenFile::Broken(_) => Self::null(), } + } + } - fd += 1; + #[cfg(not(unix))] + impl From for std::process::Stdio { + fn from(file: AsyncOpenFile) -> Self { + match file { + AsyncOpenFile::Stdin(_) => Self::inherit(), + AsyncOpenFile::Stdout(_) => Self::inherit(), + AsyncOpenFile::Stderr(_) => Self::inherit(), + AsyncOpenFile::File(f) => f + .try_into_std() + .map_or_else(|_| Self::inherit(), Self::from), + AsyncOpenFile::PipeReader(_) | AsyncOpenFile::PipeWriter(_) => Self::null(), + AsyncOpenFile::Stream(_) | AsyncOpenFile::Broken(_) => Self::null(), + } } + } + + use std::collections::HashMap; + + use crate::ShellFd; - self.files.insert(fd, Some(file)); - Ok(fd) + /// Tristate representing an `AsyncOpenFile` entry in an `AsyncOpenFiles` structure. + pub enum AsyncOpenFileEntry<'a> { + /// File descriptor is present and has a valid associated `AsyncOpenFile`. + Open(&'a AsyncOpenFile), + /// File descriptor is explicitly marked as not being mapped to any `AsyncOpenFile`. + NotPresent, + /// File descriptor is not specified in any way; it may be provided by a + /// parent context of some kind. + NotSpecified, } -} -impl From for OpenFiles -where - I: Iterator, -{ - fn from(iter: I) -> Self { - let files = iter.map(|(fd, file)| (fd, Some(file))).collect(); - Self { files } + /// Represents the open files in an async shell context. + #[derive(Clone, Default)] + pub struct AsyncOpenFiles { + /// Maps shell file descriptors to async open files. + files: HashMap>, + } + + impl AsyncOpenFiles { + /// File descriptor used for standard input. + pub const STDIN_FD: ShellFd = 0; + /// File descriptor used for standard output. + pub const STDOUT_FD: ShellFd = 1; + /// File descriptor used for standard error. + pub const STDERR_FD: ShellFd = 2; + + /// First file descriptor available for non-stdio files. + const FIRST_NON_STDIO_FD: ShellFd = 3; + /// Maximum file descriptor number allowed. + const MAX_FD: ShellFd = 1024; + + /// Creates a new `AsyncOpenFiles` instance populated with stdin, stdout, and stderr + /// from the host environment. + pub fn new() -> Self { + Self { + files: HashMap::from([ + (Self::STDIN_FD, Some(AsyncOpenFile::Stdin(stdin()))), + (Self::STDOUT_FD, Some(AsyncOpenFile::Stdout(stdout()))), + (Self::STDERR_FD, Some(AsyncOpenFile::Stderr(stderr()))), + ]), + } + } + + /// Retrieves the file backing standard input in this context. + pub fn try_stdin(&self) -> Option<&AsyncOpenFile> { + self.files.get(&Self::STDIN_FD).and_then(|f| f.as_ref()) + } + + /// Retrieves the file backing standard output in this context. + pub fn try_stdout(&self) -> Option<&AsyncOpenFile> { + self.files.get(&Self::STDOUT_FD).and_then(|f| f.as_ref()) + } + + /// Retrieves the file backing standard error in this context. + pub fn try_stderr(&self) -> Option<&AsyncOpenFile> { + self.files.get(&Self::STDERR_FD).and_then(|f| f.as_ref()) + } + + /// Tries to remove an async open file by its file descriptor. + pub fn remove_fd(&mut self, fd: ShellFd) -> Option { + self.files.insert(fd, None).and_then(|f| f) + } + + /// Tries to lookup the `AsyncOpenFile` associated with a file descriptor. + pub fn try_fd(&self, fd: ShellFd) -> Option<&AsyncOpenFile> { + self.files.get(&fd).and_then(|f| f.as_ref()) + } + + /// Tries to lookup the `AsyncOpenFile` associated with a file descriptor. + pub fn fd_entry(&self, fd: ShellFd) -> AsyncOpenFileEntry<'_> { + self.files.get(&fd).map_or( + AsyncOpenFileEntry::NotSpecified, + |opt_file| match opt_file { + Some(f) => AsyncOpenFileEntry::Open(f), + None => AsyncOpenFileEntry::NotPresent, + }, + ) + } + + /// Checks if the given file descriptor is in use. + pub fn contains_fd(&self, fd: ShellFd) -> bool { + self.files.contains_key(&fd) + } + + /// Associates the given file descriptor with the provided file. + pub fn set_fd(&mut self, fd: ShellFd, file: AsyncOpenFile) -> Option { + self.files.insert(fd, Some(file)).and_then(|f| f) + } + + /// Updates the open files from the provided iterator. + pub fn update_from(&mut self, files: impl Iterator) { + for (fd, file) in files { + self.files.insert(fd, Some(file)); + } + } + + /// Adds a new async open file, returning the assigned file descriptor. + pub fn add(&mut self, file: AsyncOpenFile) -> Result { + let mut fd = Self::FIRST_NON_STDIO_FD; + while self.files.contains_key(&fd) { + if fd >= Self::MAX_FD { + return Err(error::ErrorKind::TooManyOpenFiles.into()); + } + fd += 1; + } + self.files.insert(fd, Some(file)); + Ok(fd) + } + + /// Iterates over all file descriptors. + pub fn iter_fds(&self) -> impl Iterator { + self.files + .iter() + .filter_map(|(fd, file)| file.as_ref().map(|f| (*fd, f))) + } + } + + impl From for AsyncOpenFiles { + fn from(open_files: super::blocking::Files) -> Self { + Self { + files: open_files + .files + .into_iter() + .map(|(fd, opt_file)| (fd, opt_file.map(AsyncOpenFile::from))) + .collect(), + } + } + } + + impl From for AsyncOpenFiles + where + I: Iterator, + { + fn from(iter: I) -> Self { + Self { + files: iter.map(|(fd, file)| (fd, Some(file))).collect(), + } + } } } + +/// Re-export async types as the main public API. +pub use async_file::{ + AsyncOpenFile as OpenFile, AsyncOpenFileEntry as OpenFileEntry, AsyncOpenFiles as OpenFiles, + AsyncStream as Stream, +}; diff --git a/brush-core/src/processes.rs b/brush-core/src/processes.rs index 4b5535632..c09b19af4 100644 --- a/brush-core/src/processes.rs +++ b/brush-core/src/processes.rs @@ -52,6 +52,10 @@ impl ChildProcess { #[allow(clippy::ignored_unit_patterns)] loop { + if sys::signal::poll_for_stopped_children()? { + break Ok(ProcessWaitResult::Stopped); + } + tokio::select! { output = &mut self.exec_future => { break Ok(ProcessWaitResult::Completed(output?)) diff --git a/brush-core/src/shell.rs b/brush-core/src/shell.rs index 82dbca460..dd66f12a7 100644 --- a/brush-core/src/shell.rs +++ b/brush-core/src/shell.rs @@ -65,6 +65,7 @@ pub struct Shell crate::Shell { .open_file(&options, path, params) .map_err(|e| error::ErrorKind::FailedSourcingFile(path.to_owned(), e))?; - if opened_file.is_dir() { + if opened_file.is_dir().await { return Err(error::ErrorKind::FailedSourcingFile( path.to_owned(), std::io::Error::from(std::io::ErrorKind::IsADirectory), @@ -125,15 +125,16 @@ impl crate::Shell { /// * `args` - The arguments to pass to the script as positional parameters. /// * `params` - Execution parameters. /// * `call_type` - The type of script call being made. - async fn source_file, I: Iterator>( + async fn source_file, I: Iterator>( &mut self, - file: F, + mut file: openfiles::OpenFile, source_info: &crate::SourceInfo, args: I, params: &ExecutionParameters, call_type: callstack::ScriptCallType, ) -> Result { - let mut reader = std::io::BufReader::new(file); + let content = file.read_to_string().await?; + let mut reader = std::io::Cursor::new(content); let mut parser = brush_parser::Parser::new(&mut reader, &self.parser_options()); tracing::debug!(target: trace_categories::PARSE, "Parsing sourced file: {}", source_info.source); @@ -251,7 +252,9 @@ impl crate::Shell { match result { Ok(result) => Ok(result), Err(err) => { - let _ = self.display_error(&mut params.stderr(self), &err); + if let Some(mut stderr) = params.try_stderr(self) { + let _ = self.display_error(&err, &mut stderr).await; + } let result = err.into_result(self); self.set_last_exit_status(result.exit_code.into()); diff --git a/brush-core/src/shell/history.rs b/brush-core/src/shell/history.rs index 54be623bf..d04737da6 100644 --- a/brush-core/src/shell/history.rs +++ b/brush-core/src/shell/history.rs @@ -2,41 +2,30 @@ use std::path::PathBuf; -use crate::{error, openfiles}; +use crate::{error, history}; impl crate::Shell { - pub(super) fn load_history(&self) -> Result, error::Error> { + pub(super) fn load_history(&self) -> Result, error::Error> { const MAX_FILE_SIZE_FOR_HISTORY_IMPORT: u64 = 1024 * 1024 * 1024; // 1 GiB let Some(history_path) = self.history_file_path() else { return Ok(None); }; - let mut options = std::fs::File::options(); - options.read(true); + let history_file = std::fs::File::open(&history_path)?; - let mut history_file = - self.open_file(&options, history_path, &self.default_exec_params())?; + let file_metadata = history_file.metadata()?; + let file_size = file_metadata.len(); - // Check on the file's size. - if let openfiles::OpenFile::File(file) = &mut history_file { - let file_metadata = file.metadata()?; - let file_size = file_metadata.len(); - - // If the file is empty, no reason to try reading it. Note that this will also - // end up excluding non-regular files that report a 0 file size but appear - // to have contents when read. - if file_size == 0 { - return Ok(None); - } + if file_size == 0 { + return Ok(None); + } - // Bail if the file is unrealistically large. For now we just refuse to import it. - if file_size > MAX_FILE_SIZE_FOR_HISTORY_IMPORT { - return Err(error::ErrorKind::HistoryFileTooLargeToImport.into()); - } + if file_size > MAX_FILE_SIZE_FOR_HISTORY_IMPORT { + return Err(error::ErrorKind::HistoryFileTooLargeToImport.into()); } - Ok(Some(crate::history::History::import(history_file)?)) + Ok(Some(history::History::import(history_file)?)) } /// Returns the path to the history file used by the shell, if one is set. diff --git a/brush-core/src/shell/io.rs b/brush-core/src/shell/io.rs index 2c74387a0..d3fb076e5 100644 --- a/brush-core/src/shell/io.rs +++ b/brush-core/src/shell/io.rs @@ -1,47 +1,38 @@ //! I/O support for shell instances. -use std::io::Write; - -use crate::{error, extensions, ioutils}; +use crate::{error, extensions, ioutils, openfiles}; impl crate::Shell { - /// Returns a value that can be used to write to the shell's currently configured - /// standard output stream using `write!` et al. - pub fn stdout(&self) -> impl std::io::Write + 'static { + /// Returns the standard output file. + pub fn stdout(&self) -> openfiles::OpenFile { self.open_files.try_stdout().cloned().unwrap_or_else(|| { - ioutils::FailingReaderWriter::new("standard output not available").into() + openfiles::OpenFile::from(openfiles::blocking::File::from( + ioutils::FailingReaderWriter::new("standard output not available"), + )) }) } - /// Returns a value that can be used to write to the shell's currently configured - /// standard error stream using `write!` et al. - pub fn stderr(&self) -> impl std::io::Write + 'static { + /// Returns the standard error file. + pub fn stderr(&self) -> openfiles::OpenFile { self.open_files.try_stderr().cloned().unwrap_or_else(|| { - ioutils::FailingReaderWriter::new("standard error not available").into() + openfiles::OpenFile::from(openfiles::blocking::File::from( + ioutils::FailingReaderWriter::new("standard error not available"), + )) }) } - /// Outputs `set -x` style trace output for a command. Intentionally does not return - /// a result or error to avoid risk that a caller treats an error as fatal. Tracing - /// failure should generally always be ignored to avoid interfering with execution - /// flows. - /// - /// # Arguments - /// - /// * `command` - The command to trace. + /// Outputs `set -x` style trace output for a command. pub(crate) async fn trace_command>( &mut self, params: &crate::interp::ExecutionParameters, command: S, ) { - // Expand the PS4 prompt variable to get our prefix. let mut prefix = self .as_mut() .expand_prompt_var("PS4", "") .await .unwrap_or_default(); - // Add additional depth-based prefixes using the first character of PS4. let additional_depth = self.call_stack.script_source_depth() + self.depth; if let Some(c) = prefix.chars().next() { for _ in 0..additional_depth { @@ -49,9 +40,7 @@ impl crate::Shell { } } - // Resolve which file descriptor to use for tracing. We default to stderr, - // but if BASH_XTRACEFD is set and refers to a valid file descriptor, use that instead. - let trace_file = if let Some((_, xtracefd_var)) = self.env.get("BASH_XTRACEFD") + let mut trace_file = if let Some((_, xtracefd_var)) = self.env.get("BASH_XTRACEFD") && let Ok(fd) = xtracefd_var .value() .to_cow_str(self) @@ -63,28 +52,23 @@ impl crate::Shell { params.try_stderr(self) }; - // If we have a valid trace file, write to it. - if let Some(trace_file) = trace_file - && let Ok(mut trace_file) = trace_file.try_clone() - { - let _ = writeln!(trace_file, "{prefix}{}", command.as_ref()); + if let Some(ref mut trace_file) = trace_file { + let _ = trace_file + .write_all(format!("{prefix}{}\n", command.as_ref()).as_bytes()) + .await; } } - /// Displays the given error to the user, using the shell's error display mechanisms. - /// - /// # Arguments - /// - /// * `file_table` - The open file table to use for any file descriptor references. - /// * `err` - The error to display. - pub fn display_error( + /// Displays the given error to the user. + pub async fn display_error( &self, - file: &mut impl std::io::Write, err: &error::Error, + stderr: &mut openfiles::OpenFile, ) -> Result<(), error::Error> { use crate::extensions::ErrorFormatter as _; + let str = self.error_formatter.format_error(err, self); - write!(file, "{str}")?; + stderr.write_all(str.as_bytes()).await?; Ok(()) } diff --git a/brush-core/src/shell/job_control.rs b/brush-core/src/shell/job_control.rs index 2ddb350ae..92618656f 100644 --- a/brush-core/src/shell/job_control.rs +++ b/brush-core/src/shell/job_control.rs @@ -11,7 +11,7 @@ impl crate::Shell { if self.options.enable_job_control { for (job, _result) in results { - writeln!(self.stderr(), "{job}")?; + let _ = writeln!(std::io::stderr(), "{job}"); } } diff --git a/brush-experimental-builtins/src/save.rs b/brush-experimental-builtins/src/save.rs index 004eecca8..cee2c725a 100644 --- a/brush-experimental-builtins/src/save.rs +++ b/brush-experimental-builtins/src/save.rs @@ -1,6 +1,5 @@ use brush_core::{ExecutionResult, builtins}; use clap::Parser; -use std::io::Write; /// (*EXPERIMENTAL*) Serializes the current shell state to JSON and writes it to stdout. /// Beware that the serialized state may include sensitive information, such as any @@ -19,7 +18,12 @@ impl builtins::Command for SaveCommand { brush_core::Error::from(brush_core::ErrorKind::InternalError(e.to_string())) })?; - writeln!(context.stdout(), "{serialized_str}")?; + let mut stdout = context.stdout().ok_or_else(|| { + brush_core::Error::from(brush_core::ErrorKind::OpenFileNotWritable("stdout")) + })?; + stdout + .write_all(format!("{serialized_str}\n").as_bytes()) + .await?; Ok(ExecutionResult::success()) } diff --git a/brush-interactive/Cargo.toml b/brush-interactive/Cargo.toml index f78274bae..4c04fe82d 100644 --- a/brush-interactive/Cargo.toml +++ b/brush-interactive/Cargo.toml @@ -35,7 +35,7 @@ nu-ansi-term = { version = "0.50.3", optional = true } radix_trie = "0.3.0" reedline = { version = "0.46.0", optional = true } thiserror = "2.0.18" -tokio = { version = "1.50.0", features = ["rt", "sync"] } +tokio = { version = "1.50.0", features = ["io-util", "rt", "sync"] } tracing = "0.1.44" [dev-dependencies] diff --git a/brush-interactive/src/interactive_shell.rs b/brush-interactive/src/interactive_shell.rs index 10002fe0a..cc1b5f310 100644 --- a/brush-interactive/src/interactive_shell.rs +++ b/brush-interactive/src/interactive_shell.rs @@ -135,7 +135,7 @@ impl<'a, IB: InputBackend, SE: brush_core::ShellExtensions> InteractiveShell<'a, // Report the error, but continue to execute. let shell = self.shell.lock().await; let mut stderr = shell.stderr(); - let _ = shell.display_error(&mut stderr, &err); + let _ = shell.display_error(&err, &mut stderr).await; drop(shell); } @@ -155,7 +155,7 @@ impl<'a, IB: InputBackend, SE: brush_core::ShellExtensions> InteractiveShell<'a, shell.end_interactive_session()?; if announce_exit { - writeln!(shell.stderr(), "exit")?; + shell.stderr().write_all(b"exit\n").await?; } if let Err(e) = shell.save_history() { diff --git a/brush-shell/src/brushctl.rs b/brush-shell/src/brushctl.rs index 509e04e87..f5163dc21 100644 --- a/brush-shell/src/brushctl.rs +++ b/brush-shell/src/brushctl.rs @@ -1,6 +1,5 @@ use brush_core::{ExecutionResult, sys}; use clap::{Parser, Subcommand}; -use std::io::Write; use crate::events; @@ -119,16 +118,16 @@ impl brush_core::builtins::Command for BrushCtlCommand { mut context: brush_core::ExecutionContext<'_, SE>, ) -> Result { match &self.command_group { - CommandGroup::Call(call) => call.execute(&context), + CommandGroup::Call(call) => call.execute(&context).await, CommandGroup::Complete(complete) => complete.execute(&mut context).await, - CommandGroup::Events(events) => events.execute(&context), - CommandGroup::Process(process) => process.execute(&context), + CommandGroup::Events(events) => events.execute(&context).await, + CommandGroup::Process(process) => process.execute(&context).await, } } } impl CallCommand { - fn execute( + async fn execute( &self, context: &brush_core::ExecutionContext<'_, impl brush_core::ShellExtensions>, ) -> Result { @@ -140,7 +139,12 @@ impl CallCommand { show_entry_points: *detailed, }; - write!(context.stdout(), "{}", stack.format(&format_options))?; + let mut stdout = context.stdout().ok_or_else(|| { + brush_core::Error::from(brush_core::ErrorKind::OpenFileNotWritable("stdout")) + })?; + stdout + .write_all(stack.format(&format_options).to_string().as_bytes()) + .await?; Ok(ExecutionResult::success()) } @@ -159,8 +163,13 @@ impl CompleteCommand { .shell .complete(line, cursor_index.unwrap_or(line.len())) .await?; + let mut stdout = context.stdout().ok_or_else(|| { + brush_core::Error::from(brush_core::ErrorKind::OpenFileNotWritable("stdout")) + })?; for candidate in completions.candidates { - writeln!(context.stdout(), "{candidate}")?; + stdout + .write_all(format!("{candidate}\n").as_bytes()) + .await?; } Ok(ExecutionResult::success()) } @@ -169,7 +178,7 @@ impl CompleteCommand { } impl EventsCommand { - fn execute( + async fn execute( &self, context: &brush_core::ExecutionContext<'_, impl brush_core::ShellExtensions>, ) -> Result { @@ -185,8 +194,13 @@ impl EventsCommand { match self { Self::Status => { let enabled_events = event_config.get_enabled_events(); + let mut stdout = context.stdout().ok_or_else(|| { + brush_core::Error::from(brush_core::ErrorKind::OpenFileNotWritable( + "stdout", + )) + })?; for event in enabled_events { - writeln!(context.stdout(), "{event}")?; + stdout.write_all(format!("{event}\n").as_bytes()).await?; } } Self::Enable { event } => event_config.enable(*event)?, @@ -201,39 +215,80 @@ impl EventsCommand { } impl ProcessCommand { - fn execute( + async fn execute( &self, context: &brush_core::ExecutionContext<'_, impl brush_core::ShellExtensions>, ) -> Result { match self { Self::ShowProcessId => { - writeln!(context.stdout(), "{}", std::process::id())?; + let mut stdout = context.stdout().ok_or_else(|| { + brush_core::Error::from(brush_core::ErrorKind::OpenFileNotWritable("stdout")) + })?; + stdout + .write_all(format!("{}\n", std::process::id()).as_bytes()) + .await?; Ok(ExecutionResult::success()) } Self::ShowProcessGroupId => { if let Some(pgid) = sys::terminal::get_process_group_id() { - writeln!(context.stdout(), "{pgid}")?; + let mut stdout = context.stdout().ok_or_else(|| { + brush_core::Error::from(brush_core::ErrorKind::OpenFileNotWritable( + "stdout", + )) + })?; + stdout.write_all(format!("{pgid}\n").as_bytes()).await?; Ok(ExecutionResult::success()) } else { - writeln!(context.stderr(), "failed to get process group ID")?; + let mut stderr = context.stderr().ok_or_else(|| { + brush_core::Error::from(brush_core::ErrorKind::OpenFileNotWritable( + "stderr", + )) + })?; + stderr + .write_all(b"failed to get process group ID\n") + .await?; Ok(ExecutionResult::general_error()) } } Self::ShowForegroundProcessId => { if let Some(pid) = sys::terminal::get_foreground_pid() { - writeln!(context.stdout(), "{pid}")?; + let mut stdout = context.stdout().ok_or_else(|| { + brush_core::Error::from(brush_core::ErrorKind::OpenFileNotWritable( + "stdout", + )) + })?; + stdout.write_all(format!("{pid}\n").as_bytes()).await?; Ok(ExecutionResult::success()) } else { - writeln!(context.stderr(), "failed to get foreground process ID")?; + let mut stderr = context.stderr().ok_or_else(|| { + brush_core::Error::from(brush_core::ErrorKind::OpenFileNotWritable( + "stderr", + )) + })?; + stderr + .write_all(b"failed to get foreground process ID\n") + .await?; Ok(ExecutionResult::general_error()) } } Self::ShowParentProcessId => { if let Some(pid) = sys::terminal::get_parent_process_id() { - writeln!(context.stdout(), "{pid}")?; + let mut stdout = context.stdout().ok_or_else(|| { + brush_core::Error::from(brush_core::ErrorKind::OpenFileNotWritable( + "stdout", + )) + })?; + stdout.write_all(format!("{pid}\n").as_bytes()).await?; Ok(ExecutionResult::success()) } else { - writeln!(context.stderr(), "failed to get parent process ID")?; + let mut stderr = context.stderr().ok_or_else(|| { + brush_core::Error::from(brush_core::ErrorKind::OpenFileNotWritable( + "stderr", + )) + })?; + stderr + .write_all(b"failed to get parent process ID\n") + .await?; Ok(ExecutionResult::general_error()) } } diff --git a/brush-shell/src/bundled.rs b/brush-shell/src/bundled.rs index d410bd248..2ed928047 100644 --- a/brush-shell/src/bundled.rs +++ b/brush-shell/src/bundled.rs @@ -28,7 +28,6 @@ use std::collections::HashMap; use std::ffi::OsString; -use std::io::Write; use std::path::PathBuf; use std::sync::OnceLock; @@ -220,10 +219,11 @@ fn shim_execute( let exe_path = if let Some(p) = self_exe() { p.to_string_lossy().into_owned() } else { - let _ = writeln!( - context.stderr(), - "brush: cannot determine path to running executable" - ); + if let Some(mut stderr) = context.stderr() { + let _ = stderr + .write_all(b"brush: cannot determine path to running executable\n") + .await; + } return Ok(ExecutionExitCode::CannotExecute.into()); }; diff --git a/brush-shell/src/entry.rs b/brush-shell/src/entry.rs index 946fbb01b..e72934dd2 100644 --- a/brush-shell/src/entry.rs +++ b/brush-shell/src/entry.rs @@ -303,7 +303,7 @@ async fn run_async( Err(brush_interactive::ShellError::ShellError(e)) => { let shell = shell.lock().await; let mut stderr = shell.stderr(); - let _ = shell.display_error(&mut stderr, &e); + let _ = shell.display_error(&e, &mut stderr).await; drop(shell); 1 } From be8ba71a8a8e9a841b9209e71d8759d34eaeddb2 Mon Sep 17 00:00:00 2001 From: Luca Barbato Date: Sun, 26 Apr 2026 23:21:15 +0200 Subject: [PATCH 2/3] fix: use synchronous writes for xtrace output to preserve ordering The async I/O refactor caused xtrace (set -x) output lines to appear in a different order than bash on Linux. This happened because each trace_command call cloned AsyncOpenFile::Stderr into a fresh tokio::io::stderr() handle, and Linux's async I/O scheduling could reorder small writes between handles. Fix by using block_in_place + synchronous write on a dup'd FD, matching the original behavior. This ensures trace output ordering is deterministic while keeping the surrounding code async. --- brush-core/src/shell/io.rs | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/brush-core/src/shell/io.rs b/brush-core/src/shell/io.rs index d3fb076e5..784df455a 100644 --- a/brush-core/src/shell/io.rs +++ b/brush-core/src/shell/io.rs @@ -21,7 +21,8 @@ impl crate::Shell { }) } - /// Outputs `set -x` style trace output for a command. + /// Outputs `set -x` style trace output for a command. Intentionally does not return + /// a result or error to avoid risk that a caller treats an error as fatal. pub(crate) async fn trace_command>( &mut self, params: &crate::interp::ExecutionParameters, @@ -40,7 +41,7 @@ impl crate::Shell { } } - let mut trace_file = if let Some((_, xtracefd_var)) = self.env.get("BASH_XTRACEFD") + let trace_file = if let Some((_, xtracefd_var)) = self.env.get("BASH_XTRACEFD") && let Ok(fd) = xtracefd_var .value() .to_cow_str(self) @@ -52,10 +53,15 @@ impl crate::Shell { params.try_stderr(self) }; - if let Some(ref mut trace_file) = trace_file { - let _ = trace_file - .write_all(format!("{prefix}{}\n", command.as_ref()).as_bytes()) - .await; + if let Some(trace_file) = trace_file + && let Ok(owned_fd) = trace_file.try_clone_to_owned() + { + let output = format!("{prefix}{}\n", command.as_ref()); + tokio::task::block_in_place(|| { + use std::io::Write; + let mut file = std::fs::File::from(owned_fd); + let _ = file.write_all(output.as_bytes()); + }); } } From 7d4c613a4181d6823cec8b8bc648119817955ef1 Mon Sep 17 00:00:00 2001 From: Luca Barbato Date: Mon, 27 Apr 2026 10:38:17 +0200 Subject: [PATCH 3/3] fix: use async fallback for trace writes on non-unix platforms block_in_place and try_clone_to_owned are not available on WASM and Windows. Fall back to async write_all + flush on non-unix platforms. --- brush-core/src/shell/io.rs | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/brush-core/src/shell/io.rs b/brush-core/src/shell/io.rs index 784df455a..cc4a403b8 100644 --- a/brush-core/src/shell/io.rs +++ b/brush-core/src/shell/io.rs @@ -53,15 +53,21 @@ impl crate::Shell { params.try_stderr(self) }; - if let Some(trace_file) = trace_file - && let Ok(owned_fd) = trace_file.try_clone_to_owned() - { + if let Some(mut trace_file) = trace_file { let output = format!("{prefix}{}\n", command.as_ref()); - tokio::task::block_in_place(|| { - use std::io::Write; - let mut file = std::fs::File::from(owned_fd); - let _ = file.write_all(output.as_bytes()); - }); + #[cfg(unix)] + { + if let Ok(owned_fd) = trace_file.try_clone_to_owned() { + tokio::task::block_in_place(|| { + use std::io::Write; + let mut file = std::fs::File::from(owned_fd); + let _ = file.write_all(output.as_bytes()); + }); + return; + } + } + let _ = trace_file.write_all(output.as_bytes()).await; + let _ = trace_file.flush().await; } }