Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

fix: propagate execution parameters more thoroughly #374

Merged
merged 2 commits into from
Feb 2, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion brush-core/benches/shell.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@ mod unix {
}

async fn expand_string(shell: &mut brush_core::Shell, s: &str) {
let _ = shell.basic_expand_string(s).await.unwrap();
let params = shell.default_exec_params();
let _ = shell.basic_expand_string(&params, s).await.unwrap();
}

fn eval_arithmetic_expr(shell: &mut brush_core::Shell, expr: &str) {
Expand Down
21 changes: 16 additions & 5 deletions brush-core/src/arithmetic.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use std::borrow::Cow;

use crate::{env, expansion, variables, Shell};
use crate::{env, expansion, variables, ExecutionParameters, Shell};
use brush_parser::ast;

/// Represents an error that occurs during evaluation of an arithmetic expression.
Expand Down Expand Up @@ -47,12 +47,22 @@ pub trait ExpandAndEvaluate {
///
/// * `shell` - The shell to use for evaluation.
/// * `trace_if_needed` - Whether to trace the evaluation.
async fn eval(&self, shell: &mut Shell, trace_if_needed: bool) -> Result<i64, EvalError>;
async fn eval(
&self,
shell: &mut Shell,
params: &ExecutionParameters,
trace_if_needed: bool,
) -> Result<i64, EvalError>;
}

impl ExpandAndEvaluate for ast::UnexpandedArithmeticExpr {
async fn eval(&self, shell: &mut Shell, trace_if_needed: bool) -> Result<i64, EvalError> {
expand_and_eval(shell, self.value.as_str(), trace_if_needed).await
async fn eval(
&self,
shell: &mut Shell,
params: &ExecutionParameters,
trace_if_needed: bool,
) -> Result<i64, EvalError> {
expand_and_eval(shell, params, self.value.as_str(), trace_if_needed).await
}
}

Expand All @@ -65,11 +75,12 @@ impl ExpandAndEvaluate for ast::UnexpandedArithmeticExpr {
/// * `trace_if_needed` - Whether to trace the evaluation.
pub(crate) async fn expand_and_eval(
shell: &mut Shell,
params: &ExecutionParameters,
expr: &str,
trace_if_needed: bool,
) -> Result<i64, EvalError> {
// Per documentation, first shell-expand it.
let expanded_self = expansion::basic_expand_str_without_tilde(shell, expr)
let expanded_self = expansion::basic_expand_str_without_tilde(shell, params, expr)
.await
.map_err(|_e| EvalError::FailedToExpandExpression(expr.to_owned()))?;

Expand Down
8 changes: 7 additions & 1 deletion brush-core/src/builtins/printf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,13 @@ impl builtins::Command for PrintfCommand {
let result = self.evaluate(&context)?;

if let Some(variable_name) = &self.output_variable {
expansion::assign_to_named_parameter(context.shell, variable_name, result).await?;
expansion::assign_to_named_parameter(
context.shell,
&context.params,
variable_name,
result,
)
.await?;
} else {
write!(context.stdout(), "{result}")?;
context.stdout().flush()?;
Expand Down
12 changes: 8 additions & 4 deletions brush-core/src/builtins/test.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use clap::Parser;
use std::io::Write;

use crate::{builtins, commands, error, tests, Shell};
use crate::{builtins, commands, error, tests, ExecutionParameters, Shell};

/// Evaluate test expression.
#[derive(Parser)]
Expand Down Expand Up @@ -30,16 +30,20 @@ impl builtins::Command for TestCommand {
args = &args[0..args.len() - 1];
}

if execute_test(context.shell, args)? {
if execute_test(context.shell, &context.params, args)? {
Ok(builtins::ExitCode::Success)
} else {
Ok(builtins::ExitCode::Custom(1))
}
}
}

fn execute_test(shell: &mut Shell, args: &[String]) -> Result<bool, error::Error> {
fn execute_test(
shell: &mut Shell,
params: &ExecutionParameters,
args: &[String],
) -> Result<bool, error::Error> {
let test_command =
brush_parser::test_command::parse(args).map_err(error::Error::TestCommandParseError)?;
tests::eval_test_expr(&test_command, shell)
tests::eval_test_expr(&test_command, shell, params)
}
12 changes: 7 additions & 5 deletions brush-core/src/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -521,7 +521,7 @@ pub(crate) async fn invoke_shell_function(
// Apply any redirects specified at function definition-time.
if let Some(redirects) = redirects {
for redirect in &redirects.0 {
interp::setup_redirect(&mut context.params.open_files, context.shell, redirect).await?;
interp::setup_redirect(context.shell, &mut context.params, redirect).await?;
}
}

Expand Down Expand Up @@ -563,21 +563,23 @@ pub(crate) async fn invoke_shell_function(

pub(crate) async fn invoke_command_in_subshell_and_get_output(
shell: &mut Shell,
params: &ExecutionParameters,
s: String,
) -> Result<String, error::Error> {
// Instantiate a subshell to run the command in.
let mut subshell = shell.clone();

// Get our own set of parameters we can customize and use.
let mut params = params.clone();
params.process_group_policy = ProcessGroupPolicy::SameProcessGroup;

// Set up pipe so we can read the output.
let (reader, writer) = sys::pipes::pipe()?;
subshell
params
.open_files
.files
.insert(1, openfiles::OpenFile::PipeWriter(writer));

let mut params = subshell.default_exec_params();
params.process_group_policy = ProcessGroupPolicy::SameProcessGroup;

// Run the command.
let result = subshell.run_string(s, &params).await?;

Expand Down
11 changes: 8 additions & 3 deletions brush-core/src/completion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,9 @@ impl Spec {
// Generate completions based on any provided actions (and on words).
let mut candidates = self.generate_action_completions(shell, context).await?;
if let Some(word_list) = &self.word_list {
let words = crate::expansion::full_expand_and_split_str(shell, word_list).await?;
let params = shell.default_exec_params();
let words =
crate::expansion::full_expand_and_split_str(shell, &params, word_list).await?;
for word in words {
if word.starts_with(context.token_to_complete) {
candidates.insert(word);
Expand Down Expand Up @@ -607,8 +609,10 @@ impl Spec {
}

// Run the command.
let params = shell.default_exec_params();
let output =
commands::invoke_command_in_subshell_and_get_output(&mut shell, command_line).await?;
commands::invoke_command_in_subshell_and_get_output(&mut shell, &params, command_line)
.await?;

// Split results.
let mut candidates = IndexSet::new();
Expand Down Expand Up @@ -1052,8 +1056,9 @@ async fn get_file_completions(
) -> IndexSet<String> {
// Basic-expand the token-to-be-completed; it won't have been expanded to this point.
let mut throwaway_shell = shell.clone();
let params = throwaway_shell.default_exec_params();
let expanded_token = throwaway_shell
.basic_expand_string(token_to_complete)
.basic_expand_string(&params, token_to_complete)
.await
.unwrap_or_else(|_err| token_to_complete.to_owned());

Expand Down
67 changes: 42 additions & 25 deletions brush-core/src/expansion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ use crate::trace_categories;
use crate::variables::ShellValueUnsetType;
use crate::variables::ShellVariable;
use crate::variables::{self, ShellValue};
use crate::ExecutionParameters;

#[derive(Debug)]
struct Expansion {
Expand Down Expand Up @@ -294,17 +295,19 @@ enum ParameterState {

pub(crate) async fn basic_expand_pattern(
shell: &mut Shell,
params: &ExecutionParameters,
word: &ast::Word,
) -> Result<patterns::Pattern, error::Error> {
let mut expander = WordExpander::new(shell);
let mut expander = WordExpander::new(shell, params);
expander.basic_expand_pattern(&word.flatten()).await
}

pub(crate) async fn basic_expand_regex(
shell: &mut Shell,
params: &ExecutionParameters,
word: &ast::Word,
) -> Result<crate::regex::Regex, error::Error> {
let mut expander = WordExpander::new(shell);
let mut expander = WordExpander::new(shell, params);

// Brace expansion does not appear to be used in regexes.
expander.force_disable_brace_expansion = true;
Expand All @@ -314,63 +317,73 @@ pub(crate) async fn basic_expand_regex(

pub(crate) async fn basic_expand_word(
shell: &mut Shell,
params: &ExecutionParameters,
word: &ast::Word,
) -> Result<String, error::Error> {
basic_expand_str(shell, word.flatten().as_str()).await
basic_expand_str(shell, params, word.flatten().as_str()).await
}

pub(crate) async fn basic_expand_str(shell: &mut Shell, s: &str) -> Result<String, error::Error> {
let mut expander = WordExpander::new(shell);
pub(crate) async fn basic_expand_str(
shell: &mut Shell,
params: &ExecutionParameters,
s: &str,
) -> Result<String, error::Error> {
let mut expander = WordExpander::new(shell, params);
expander.basic_expand_to_str(s).await
}

pub(crate) async fn basic_expand_str_without_tilde(
shell: &mut Shell,
params: &ExecutionParameters,
s: &str,
) -> Result<String, error::Error> {
let mut expander = WordExpander::new(shell);
let mut expander = WordExpander::new(shell, params);
expander.parser_options.tilde_expansion = false;
expander.basic_expand_to_str(s).await
}

pub(crate) async fn full_expand_and_split_word(
shell: &mut Shell,
params: &ExecutionParameters,
word: &ast::Word,
) -> Result<Vec<String>, error::Error> {
full_expand_and_split_str(shell, word.flatten().as_str()).await
full_expand_and_split_str(shell, params, word.flatten().as_str()).await
}

pub(crate) async fn full_expand_and_split_str(
shell: &mut Shell,
params: &ExecutionParameters,
s: &str,
) -> Result<Vec<String>, error::Error> {
let mut expander = WordExpander::new(shell);
let mut expander = WordExpander::new(shell, params);
expander.full_expand_with_splitting(s).await
}

pub(crate) async fn assign_to_named_parameter(
shell: &mut Shell,
params: &ExecutionParameters,
name: &str,
value: String,
) -> Result<(), error::Error> {
let parser_options = shell.parser_options();
let mut expander = WordExpander::new(shell);
let mut expander = WordExpander::new(shell, params);
let parameter = brush_parser::word::parse_parameter(name, &parser_options)?;
expander.assign_to_parameter(&parameter, value).await
}

struct WordExpander<'a> {
shell: &'a mut Shell,
params: &'a ExecutionParameters,
parser_options: brush_parser::ParserOptions,
force_disable_brace_expansion: bool,
}

impl<'a> WordExpander<'a> {
pub fn new(shell: &'a mut Shell) -> Self {
pub fn new(shell: &'a mut Shell, params: &'a ExecutionParameters) -> Self {
let parser_options = shell.parser_options();

Self {
shell,
params,
parser_options,
force_disable_brace_expansion: false,
}
Expand Down Expand Up @@ -710,7 +723,8 @@ impl<'a> WordExpander<'a> {
brush_parser::word::WordPiece::BackquotedCommandSubstitution(s)
| brush_parser::word::WordPiece::CommandSubstitution(s) => {
let output_str =
commands::invoke_command_in_subshell_and_get_output(self.shell, s).await?;
commands::invoke_command_in_subshell_and_get_output(self.shell, self.params, s)
.await?;

// We trim trailing newlines, per spec.
let trimmed = output_str.trim_end_matches('\n');
Expand Down Expand Up @@ -923,7 +937,7 @@ impl<'a> WordExpander<'a> {
);
}

let expanded_offset = offset.eval(self.shell, false).await?;
let expanded_offset = offset.eval(self.shell, self.params, false).await?;
let expanded_offset = if expanded_offset < 0 {
0
} else {
Expand All @@ -934,7 +948,7 @@ impl<'a> WordExpander<'a> {
let expanded_offset = min(expanded_offset, expanded_parameter_len);

let end_offset = if let Some(length) = length {
let mut expanded_length = length.eval(self.shell, false).await?;
let mut expanded_length = length.eval(self.shell, self.params, false).await?;
if expanded_length < 0 {
let param_length: i64 = i64::try_from(expanded_parameter_len)?;
expanded_length += param_length;
Expand Down Expand Up @@ -1357,7 +1371,7 @@ impl<'a> WordExpander<'a> {
let index_to_use = if for_set_associative_array {
self.basic_expand_to_str(index).await?
} else {
arithmetic::expand_and_eval(self.shell, index, false)
arithmetic::expand_and_eval(self.shell, self.params, index, false)
.await?
.to_string()
};
Expand Down Expand Up @@ -1416,7 +1430,7 @@ impl<'a> WordExpander<'a> {
&mut self,
expr: brush_parser::ast::UnexpandedArithmeticExpr,
) -> Result<String, error::Error> {
let value = expr.eval(self.shell, false).await?;
let value = expr.eval(self.shell, self.params, false).await?;
Ok(value.to_string())
}

Expand Down Expand Up @@ -1691,33 +1705,34 @@ mod tests {
async fn test_full_expansion() -> Result<()> {
let options = crate::shell::CreateOptions::default();
let mut shell = crate::shell::Shell::new(&options).await?;
let params = shell.default_exec_params();

assert_eq!(
full_expand_and_split_str(&mut shell, "\"\"").await?,
full_expand_and_split_str(&mut shell, &params, "\"\"").await?,
vec![""]
);
assert_eq!(
full_expand_and_split_str(&mut shell, "a b").await?,
full_expand_and_split_str(&mut shell, &params, "a b").await?,
vec!["a", "b"]
);
assert_eq!(
full_expand_and_split_str(&mut shell, "ab").await?,
full_expand_and_split_str(&mut shell, &params, "ab").await?,
vec!["ab"]
);
assert_eq!(
full_expand_and_split_str(&mut shell, r#""a b""#).await?,
full_expand_and_split_str(&mut shell, &params, r#""a b""#).await?,
vec!["a b"]
);
assert_eq!(
full_expand_and_split_str(&mut shell, "").await?,
full_expand_and_split_str(&mut shell, &params, "").await?,
Vec::<String>::new()
);
assert_eq!(
full_expand_and_split_str(&mut shell, "$@").await?,
full_expand_and_split_str(&mut shell, &params, "$@").await?,
Vec::<String>::new()
);
assert_eq!(
full_expand_and_split_str(&mut shell, "$*").await?,
full_expand_and_split_str(&mut shell, &params, "$*").await?,
Vec::<String>::new()
);

Expand All @@ -1728,7 +1743,8 @@ mod tests {
async fn test_brace_expansion() -> Result<()> {
let options = crate::shell::CreateOptions::default();
let mut shell = crate::shell::Shell::new(&options).await?;
let expander = WordExpander::new(&mut shell);
let params = shell.default_exec_params();
let expander = WordExpander::new(&mut shell, &params);

assert_eq!(expander.brace_expand_if_needed("abc")?, ["abc"]);
assert_eq!(expander.brace_expand_if_needed("a{,b}d")?, ["ad", "abd"]);
Expand All @@ -1751,7 +1767,8 @@ mod tests {
async fn test_field_splitting() -> Result<()> {
let options = crate::shell::CreateOptions::default();
let mut shell = crate::shell::Shell::new(&options).await?;
let expander = WordExpander::new(&mut shell);
let params = shell.default_exec_params();
let expander = WordExpander::new(&mut shell, &params);

let expansion = Expansion {
fields: vec![
Expand Down
Loading
Loading