Skip to content

Commit bd16863

Browse files
chaliyclaude
andauthored
fix(builtins): make xargs execute commands instead of echoing (#364)
## Summary - Fix xargs to execute commands through the interpreter instead of echoing them as text - Intercept `xargs` at the interpreter level (alongside `timeout`, `eval`, `source`) and dispatch constructed `SimpleCommand`s via `execute_command()` - Add 5 integration tests covering: basic execution, default echo, newline splitting, `-n 1` per-item execution, and `-I {}` replacement Closes #346 ## Test plan - [x] `test_xargs_executes_command` - verifies `echo file.txt | xargs cat` reads file contents - [x] `test_xargs_default_echo` - verifies `echo 'a b c' | xargs` defaults to echo - [x] `test_xargs_splits_newlines` - verifies multi-line input is split into separate args - [x] `test_xargs_n1_executes_per_item` - verifies `-n 1` executes once per argument - [x] `test_xargs_replace_str` - verifies `-I {}` substitution with actual execution - [x] All existing xargs unit tests still pass (option parsing, `-d`, `-0`, etc.) - [x] `cargo fmt --check` clean - [x] `cargo clippy -p bashkit --all-targets --all-features -- -D warnings` clean - [x] `cargo test -p bashkit --lib --all-features` - 1109 tests pass - [x] `cargo test -p bashkit --all-features --tests` - 118 integration tests pass Co-authored-by: Claude <noreply@anthropic.com>
1 parent f4dc103 commit bd16863

File tree

2 files changed

+259
-2
lines changed

2 files changed

+259
-2
lines changed

crates/bashkit/src/builtins/pipeline.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,8 @@ use crate::interpreter::ExecResult;
1616
/// -d DELIM Use DELIM as delimiter instead of whitespace
1717
/// -0 Use NUL as delimiter (same as -d '\0')
1818
///
19-
/// Note: In virtual mode, xargs outputs the commands that would be run
20-
/// instead of executing them, unless the command is a builtin.
19+
/// Note: xargs is intercepted at the interpreter level for actual command
20+
/// execution. This builtin fallback only handles option parsing/validation.
2121
pub struct Xargs;
2222

2323
#[async_trait]

crates/bashkit/src/interpreter/mod.rs

Lines changed: 257 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1847,6 +1847,159 @@ impl Interpreter {
18471847
Some(Duration::from_secs_f64(total_secs_f64))
18481848
}
18491849

1850+
/// Execute `xargs` - build and execute command lines from stdin.
1851+
///
1852+
/// Parses xargs options, splits stdin into arguments, and executes the
1853+
/// target command via the interpreter for each batch.
1854+
async fn execute_xargs(
1855+
&mut self,
1856+
args: &[String],
1857+
stdin: Option<String>,
1858+
redirects: &[Redirect],
1859+
) -> Result<ExecResult> {
1860+
let mut replace_str: Option<String> = None;
1861+
let mut max_args: Option<usize> = None;
1862+
let mut delimiter: Option<char> = None;
1863+
let mut command: Vec<String> = Vec::new();
1864+
1865+
// Parse xargs options
1866+
let mut i = 0;
1867+
while i < args.len() {
1868+
let arg = &args[i];
1869+
match arg.as_str() {
1870+
"-I" => {
1871+
i += 1;
1872+
if i >= args.len() {
1873+
return Ok(ExecResult::err(
1874+
"xargs: option requires an argument -- 'I'\n".to_string(),
1875+
1,
1876+
));
1877+
}
1878+
replace_str = Some(args[i].clone());
1879+
max_args = Some(1); // -I implies -n 1
1880+
}
1881+
"-n" => {
1882+
i += 1;
1883+
if i >= args.len() {
1884+
return Ok(ExecResult::err(
1885+
"xargs: option requires an argument -- 'n'\n".to_string(),
1886+
1,
1887+
));
1888+
}
1889+
match args[i].parse::<usize>() {
1890+
Ok(n) if n > 0 => max_args = Some(n),
1891+
_ => {
1892+
return Ok(ExecResult::err(
1893+
format!("xargs: invalid number: '{}'\n", args[i]),
1894+
1,
1895+
));
1896+
}
1897+
}
1898+
}
1899+
"-d" => {
1900+
i += 1;
1901+
if i >= args.len() {
1902+
return Ok(ExecResult::err(
1903+
"xargs: option requires an argument -- 'd'\n".to_string(),
1904+
1,
1905+
));
1906+
}
1907+
delimiter = args[i].chars().next();
1908+
}
1909+
"-0" => {
1910+
delimiter = Some('\0');
1911+
}
1912+
s if s.starts_with('-') && s != "-" => {
1913+
return Ok(ExecResult::err(
1914+
format!("xargs: invalid option -- '{}'\n", &s[1..]),
1915+
1,
1916+
));
1917+
}
1918+
_ => {
1919+
// Rest is the command
1920+
command.extend(args[i..].iter().cloned());
1921+
break;
1922+
}
1923+
}
1924+
i += 1;
1925+
}
1926+
1927+
// Default command is echo
1928+
if command.is_empty() {
1929+
command.push("echo".to_string());
1930+
}
1931+
1932+
// Read input
1933+
let input = stdin.as_deref().unwrap_or("");
1934+
if input.is_empty() {
1935+
let result = ExecResult::ok(String::new());
1936+
return self.apply_redirections(result, redirects).await;
1937+
}
1938+
1939+
// Split input by delimiter
1940+
let items: Vec<&str> = if let Some(delim) = delimiter {
1941+
input.split(delim).filter(|s| !s.is_empty()).collect()
1942+
} else {
1943+
input.split_whitespace().collect()
1944+
};
1945+
1946+
if items.is_empty() {
1947+
let result = ExecResult::ok(String::new());
1948+
return self.apply_redirections(result, redirects).await;
1949+
}
1950+
1951+
let mut combined_stdout = String::new();
1952+
let mut combined_stderr = String::new();
1953+
let mut last_exit_code = 0;
1954+
1955+
// Group items based on max_args
1956+
let chunk_size = max_args.unwrap_or(items.len());
1957+
let chunks: Vec<Vec<&str>> = items.chunks(chunk_size).map(|c| c.to_vec()).collect();
1958+
1959+
for chunk in chunks {
1960+
let cmd_args: Vec<String> = if let Some(ref repl) = replace_str {
1961+
// With -I, substitute REPLACE string in all command args
1962+
let item = chunk.first().unwrap_or(&"");
1963+
command.iter().map(|arg| arg.replace(repl, item)).collect()
1964+
} else {
1965+
// Append chunk items as arguments after the command
1966+
let mut full = command.clone();
1967+
full.extend(chunk.iter().map(|s| s.to_string()));
1968+
full
1969+
};
1970+
1971+
// Build a SimpleCommand and execute it through the interpreter
1972+
let cmd_name = cmd_args[0].clone();
1973+
let cmd_rest: Vec<Word> = cmd_args[1..]
1974+
.iter()
1975+
.map(|s| Word::literal(s.clone()))
1976+
.collect();
1977+
1978+
let inner_cmd = Command::Simple(SimpleCommand {
1979+
name: Word::literal(cmd_name),
1980+
args: cmd_rest,
1981+
redirects: Vec::new(),
1982+
assignments: Vec::new(),
1983+
span: Span::new(),
1984+
});
1985+
1986+
let result = self.execute_command(&inner_cmd).await?;
1987+
combined_stdout.push_str(&result.stdout);
1988+
combined_stderr.push_str(&result.stderr);
1989+
last_exit_code = result.exit_code;
1990+
}
1991+
1992+
let mut result = ExecResult {
1993+
stdout: combined_stdout,
1994+
stderr: combined_stderr,
1995+
exit_code: last_exit_code,
1996+
control_flow: ControlFlow::None,
1997+
};
1998+
1999+
result = self.apply_redirections(result, redirects).await?;
2000+
Ok(result)
2001+
}
2002+
18502003
/// Execute `bash` or `sh` command - interpret scripts using this interpreter.
18512004
///
18522005
/// Supports:
@@ -3159,6 +3312,11 @@ impl Interpreter {
31593312
return self.execute_timeout(&args, stdin, &command.redirects).await;
31603313
}
31613314

3315+
// Handle `xargs` specially - needs interpreter-level command execution
3316+
if name == "xargs" {
3317+
return self.execute_xargs(&args, stdin, &command.redirects).await;
3318+
}
3319+
31623320
// Handle `bash` and `sh` specially - execute scripts using the interpreter
31633321
if name == "bash" || name == "sh" {
31643322
return self
@@ -7582,4 +7740,103 @@ mod tests {
75827740
result.stderr
75837741
);
75847742
}
7743+
7744+
// ==================== xargs execution tests ====================
7745+
7746+
#[tokio::test]
7747+
async fn test_xargs_executes_command() {
7748+
// xargs should execute the command, not echo it
7749+
let fs = Arc::new(InMemoryFs::new());
7750+
fs.mkdir(std::path::Path::new("/workspace"), true)
7751+
.await
7752+
.unwrap();
7753+
fs.write_file(std::path::Path::new("/workspace/file.txt"), b"hello world")
7754+
.await
7755+
.unwrap();
7756+
7757+
let mut interp = Interpreter::new(fs.clone());
7758+
let parser = Parser::new("echo /workspace/file.txt | xargs cat");
7759+
let ast = parser.parse().unwrap();
7760+
let result = interp.execute(&ast).await.unwrap();
7761+
7762+
assert_eq!(result.exit_code, 0);
7763+
assert_eq!(
7764+
result.stdout.trim(),
7765+
"hello world",
7766+
"xargs should execute cat, not echo it. Got: {:?}",
7767+
result.stdout
7768+
);
7769+
}
7770+
7771+
#[tokio::test]
7772+
async fn test_xargs_default_echo() {
7773+
// With no command, xargs defaults to echo
7774+
let result = run_script("echo 'a b c' | xargs").await;
7775+
assert_eq!(result.exit_code, 0);
7776+
assert_eq!(result.stdout.trim(), "a b c");
7777+
}
7778+
7779+
#[tokio::test]
7780+
async fn test_xargs_splits_newlines() {
7781+
// xargs should split input on whitespace/newlines into separate args
7782+
let fs = Arc::new(InMemoryFs::new());
7783+
fs.mkdir(std::path::Path::new("/workspace"), true)
7784+
.await
7785+
.unwrap();
7786+
fs.write_file(std::path::Path::new("/workspace/a.txt"), b"AAA")
7787+
.await
7788+
.unwrap();
7789+
fs.write_file(std::path::Path::new("/workspace/b.txt"), b"BBB")
7790+
.await
7791+
.unwrap();
7792+
7793+
let mut interp = Interpreter::new(fs.clone());
7794+
let script = "printf '/workspace/a.txt\\n/workspace/b.txt' | xargs cat";
7795+
let parser = Parser::new(script);
7796+
let ast = parser.parse().unwrap();
7797+
let result = interp.execute(&ast).await.unwrap();
7798+
7799+
assert_eq!(result.exit_code, 0);
7800+
assert!(
7801+
result.stdout.contains("AAA"),
7802+
"should contain contents of a.txt"
7803+
);
7804+
assert!(
7805+
result.stdout.contains("BBB"),
7806+
"should contain contents of b.txt"
7807+
);
7808+
}
7809+
7810+
#[tokio::test]
7811+
async fn test_xargs_n1_executes_per_item() {
7812+
// xargs -n 1 should execute once per argument
7813+
let result = run_script("echo 'a b c' | xargs -n 1 echo item:").await;
7814+
assert_eq!(result.exit_code, 0);
7815+
let lines: Vec<&str> = result.stdout.trim().lines().collect();
7816+
assert_eq!(lines.len(), 3);
7817+
assert_eq!(lines[0], "item: a");
7818+
assert_eq!(lines[1], "item: b");
7819+
assert_eq!(lines[2], "item: c");
7820+
}
7821+
7822+
#[tokio::test]
7823+
async fn test_xargs_replace_str() {
7824+
// xargs -I {} should substitute {} with each input line
7825+
let fs = Arc::new(InMemoryFs::new());
7826+
fs.mkdir(std::path::Path::new("/workspace"), true)
7827+
.await
7828+
.unwrap();
7829+
fs.write_file(std::path::Path::new("/workspace/hello.txt"), b"Hello!")
7830+
.await
7831+
.unwrap();
7832+
7833+
let mut interp = Interpreter::new(fs.clone());
7834+
let script = "echo /workspace/hello.txt | xargs -I {} cat {}";
7835+
let parser = Parser::new(script);
7836+
let ast = parser.parse().unwrap();
7837+
let result = interp.execute(&ast).await.unwrap();
7838+
7839+
assert_eq!(result.exit_code, 0);
7840+
assert_eq!(result.stdout.trim(), "Hello!");
7841+
}
75857842
}

0 commit comments

Comments
 (0)