From 041a61f14771ec78d272953ddd9c87cd702a744e Mon Sep 17 00:00:00 2001 From: Sandijigs Date: Thu, 9 Apr 2026 21:20:03 +0100 Subject: [PATCH 1/3] Fixed split_maybe_args to split on default shell IFS (space, tab, newline) --- src/tools/run-make-support/src/run.rs | 74 +++++++++++++++++++++++++-- 1 file changed, 69 insertions(+), 5 deletions(-) diff --git a/src/tools/run-make-support/src/run.rs b/src/tools/run-make-support/src/run.rs index 43f0473ddf4a1..88a9ed4c5674f 100644 --- a/src/tools/run-make-support/src/run.rs +++ b/src/tools/run-make-support/src/run.rs @@ -90,10 +90,74 @@ pub fn cmd>(program: S) -> Command { } fn split_maybe_args(s: &str) -> Vec { - // FIXME(132599): implement proper env var/shell argument splitting. - s.split(' ') - .filter_map(|s| { - if s.chars().all(|c| c.is_whitespace()) { None } else { Some(OsString::from(s)) } - }) + // Split on default shell IFS (space, tab, newline). + s.split(|c: char| matches!(c, ' ' | '\t' | '\n')) + .filter(|s| !s.is_empty()) + .map(OsString::from) .collect() } + +#[cfg(test)] +mod tests { + use super::*; + + fn os(s: &str) -> OsString { + OsString::from(s) + } + + // --- Tests that PASS on the current (buggy) code --- + + #[test] + fn split_on_space() { + assert_eq!( + split_maybe_args("valgrind --tool=memcheck"), + vec![os("valgrind"), os("--tool=memcheck")] + ); + } + + #[test] + fn single_arg_no_whitespace() { + assert_eq!(split_maybe_args("valgrind"), vec![os("valgrind")]); + } + + #[test] + fn empty_string() { + assert_eq!(split_maybe_args(""), Vec::::new()); + } + + // --- Tests that FAIL on the current (buggy) code --- + // They prove the bug: split_maybe_args only splits on space, + // but shells split on IFS which defaults to space, tab, and newline. + + #[test] + fn split_on_tab() { + assert_eq!( + split_maybe_args("valgrind\t--tool=memcheck"), + vec![os("valgrind"), os("--tool=memcheck")] + ); + } + + #[test] + fn split_on_newline() { + assert_eq!( + split_maybe_args("valgrind\n--tool=memcheck"), + vec![os("valgrind"), os("--tool=memcheck")] + ); + } + + #[test] + fn multiple_ifs_separators() { + assert_eq!( + split_maybe_args("a b\t\tc\n\nd"), + vec![os("a"), os("b"), os("c"), os("d")] + ); + } + + #[test] + fn leading_and_trailing_whitespace() { + assert_eq!( + split_maybe_args(" valgrind\t"), + vec![os("valgrind")] + ); + } +} From 6f60fa789452ed5f65bdce223c2856412ec62d21 Mon Sep 17 00:00:00 2001 From: Sandijigs Date: Thu, 9 Apr 2026 23:30:32 +0100 Subject: [PATCH 2/3] Fixed rustfmt formatting in split_maybe_args tests --- src/tools/run-make-support/src/run.rs | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/tools/run-make-support/src/run.rs b/src/tools/run-make-support/src/run.rs index 88a9ed4c5674f..83bcaa777dea7 100644 --- a/src/tools/run-make-support/src/run.rs +++ b/src/tools/run-make-support/src/run.rs @@ -147,17 +147,11 @@ mod tests { #[test] fn multiple_ifs_separators() { - assert_eq!( - split_maybe_args("a b\t\tc\n\nd"), - vec![os("a"), os("b"), os("c"), os("d")] - ); + assert_eq!(split_maybe_args("a b\t\tc\n\nd"), vec![os("a"), os("b"), os("c"), os("d")]); } #[test] fn leading_and_trailing_whitespace() { - assert_eq!( - split_maybe_args(" valgrind\t"), - vec![os("valgrind")] - ); + assert_eq!(split_maybe_args(" valgrind\t"), vec![os("valgrind")]); } } From 7b0a01f446e4f050b17e6bdfeb1599e0d48113df Mon Sep 17 00:00:00 2001 From: Sandijigs Date: Fri, 10 Apr 2026 00:08:13 +0100 Subject: [PATCH 3/3] Moved split_maybe_args tests into separate tests.rs to satisfy tidy check --- .../src/{run.rs => run/mod.rs} | 60 +------------------ src/tools/run-make-support/src/run/tests.rs | 51 ++++++++++++++++ 2 files changed, 53 insertions(+), 58 deletions(-) rename src/tools/run-make-support/src/{run.rs => run/mod.rs} (65%) create mode 100644 src/tools/run-make-support/src/run/tests.rs diff --git a/src/tools/run-make-support/src/run.rs b/src/tools/run-make-support/src/run/mod.rs similarity index 65% rename from src/tools/run-make-support/src/run.rs rename to src/tools/run-make-support/src/run/mod.rs index 83bcaa777dea7..ea5d16248bcf9 100644 --- a/src/tools/run-make-support/src/run.rs +++ b/src/tools/run-make-support/src/run/mod.rs @@ -89,7 +89,7 @@ pub fn cmd>(program: S) -> Command { command } -fn split_maybe_args(s: &str) -> Vec { +pub(crate) fn split_maybe_args(s: &str) -> Vec { // Split on default shell IFS (space, tab, newline). s.split(|c: char| matches!(c, ' ' | '\t' | '\n')) .filter(|s| !s.is_empty()) @@ -98,60 +98,4 @@ fn split_maybe_args(s: &str) -> Vec { } #[cfg(test)] -mod tests { - use super::*; - - fn os(s: &str) -> OsString { - OsString::from(s) - } - - // --- Tests that PASS on the current (buggy) code --- - - #[test] - fn split_on_space() { - assert_eq!( - split_maybe_args("valgrind --tool=memcheck"), - vec![os("valgrind"), os("--tool=memcheck")] - ); - } - - #[test] - fn single_arg_no_whitespace() { - assert_eq!(split_maybe_args("valgrind"), vec![os("valgrind")]); - } - - #[test] - fn empty_string() { - assert_eq!(split_maybe_args(""), Vec::::new()); - } - - // --- Tests that FAIL on the current (buggy) code --- - // They prove the bug: split_maybe_args only splits on space, - // but shells split on IFS which defaults to space, tab, and newline. - - #[test] - fn split_on_tab() { - assert_eq!( - split_maybe_args("valgrind\t--tool=memcheck"), - vec![os("valgrind"), os("--tool=memcheck")] - ); - } - - #[test] - fn split_on_newline() { - assert_eq!( - split_maybe_args("valgrind\n--tool=memcheck"), - vec![os("valgrind"), os("--tool=memcheck")] - ); - } - - #[test] - fn multiple_ifs_separators() { - assert_eq!(split_maybe_args("a b\t\tc\n\nd"), vec![os("a"), os("b"), os("c"), os("d")]); - } - - #[test] - fn leading_and_trailing_whitespace() { - assert_eq!(split_maybe_args(" valgrind\t"), vec![os("valgrind")]); - } -} +mod tests; diff --git a/src/tools/run-make-support/src/run/tests.rs b/src/tools/run-make-support/src/run/tests.rs new file mode 100644 index 0000000000000..690a0c4e68bc7 --- /dev/null +++ b/src/tools/run-make-support/src/run/tests.rs @@ -0,0 +1,51 @@ +use std::ffi::OsString; + +use super::split_maybe_args; + +fn os(s: &str) -> OsString { + OsString::from(s) +} + +#[test] +fn split_on_space() { + assert_eq!( + split_maybe_args("valgrind --tool=memcheck"), + vec![os("valgrind"), os("--tool=memcheck")] + ); +} + +#[test] +fn single_arg_no_whitespace() { + assert_eq!(split_maybe_args("valgrind"), vec![os("valgrind")]); +} + +#[test] +fn empty_string() { + assert_eq!(split_maybe_args(""), Vec::::new()); +} + +#[test] +fn split_on_tab() { + assert_eq!( + split_maybe_args("valgrind\t--tool=memcheck"), + vec![os("valgrind"), os("--tool=memcheck")] + ); +} + +#[test] +fn split_on_newline() { + assert_eq!( + split_maybe_args("valgrind\n--tool=memcheck"), + vec![os("valgrind"), os("--tool=memcheck")] + ); +} + +#[test] +fn multiple_ifs_separators() { + assert_eq!(split_maybe_args("a b\t\tc\n\nd"), vec![os("a"), os("b"), os("c"), os("d")]); +} + +#[test] +fn leading_and_trailing_whitespace() { + assert_eq!(split_maybe_args(" valgrind\t"), vec![os("valgrind")]); +}