Skip to content

Commit

Permalink
Fix ownership issues with runners
Browse files Browse the repository at this point in the history
  • Loading branch information
smores56 committed Aug 14, 2024
1 parent bc29782 commit 5f8ff89
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 35 deletions.
41 changes: 27 additions & 14 deletions crates/cli_utils/src/bench_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,14 @@ fn exec_bench_w_input<T: Measurement>(
expected_ending: &str,
bench_group_opt: Option<&mut BenchmarkGroup<T>>,
) {
let run = Run::new_roc().add_args(["build", BUILD_HOST_FLAG, OPTIMIZE_FLAG, file.to_str().unwrap()]);
let compile_out = run.run();
let runner = Run::new_roc().add_args([
"build",
BUILD_HOST_FLAG,
OPTIMIZE_FLAG,
file.to_str().unwrap(),
]);

let compile_out = runner.run();

if !compile_out.stderr.is_empty() && compile_out.stderr != "🔨 Rebuilding platform...\n" {
panic!("stderr was not empty:\n\t{}", compile_out.stderr);
Expand Down Expand Up @@ -43,17 +49,19 @@ fn check_cmd_output(
.unwrap()
.to_string();

let run = Run::new(&cmd_str).with_stdin_vals([stdin_str]);
let runner = Run::new(&cmd_str).with_stdin_vals([stdin_str]);

let out = if cmd_str.contains("cfold") {
let child = thread::Builder::new()
.stack_size(CFOLD_STACK_SIZE)
.spawn(move || run.run())
.unwrap();

child.join().unwrap()
thread::scope(|s| {
let child = thread::Builder::new()
.stack_size(CFOLD_STACK_SIZE)
.spawn_scoped(s, move || runner.run())
.unwrap();

child.join().unwrap()
})
} else {
run.run()
runner.run()
};

if !&out.stdout.ends_with(expected_ending) {
Expand All @@ -64,7 +72,7 @@ fn check_cmd_output(

fn bench_cmd<T: Measurement>(
file: &Path,
stdin_str: &str,
stdin_str: &'static str,
executable_filename: &str,
bench_group_opt: Option<&mut BenchmarkGroup<T>>,
) {
Expand Down Expand Up @@ -92,13 +100,18 @@ fn bench_cmd<T: Measurement>(
}
}


if let Some(bench_group) = bench_group_opt {
bench_group.bench_function(&format!("Benchmarking {executable_filename:?}"), |b| {
b.iter(|| Run::new(black_box(&cmd_str)).with_stdin_vals([stdin_str]).run())
b.iter(|| {
Run::new(black_box(&cmd_str))
.with_stdin_vals([stdin_str])
.run()
})
});
} else {
Run::new(file.with_file_name(executable_filename).to_str().unwrap()).with_stdin_vals([stdin_str]).run();
Run::new(file.with_file_name(executable_filename).to_str().unwrap())
.with_stdin_vals([stdin_str])
.run();
}
}

Expand Down
43 changes: 22 additions & 21 deletions crates/cli_utils/src/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ use serde::Deserialize;
use serde_xml_rs::from_str;
use std::env;
use std::ffi::{OsStr, OsString};
use std::io::Read;
use std::io::Write;
use std::ops::Deref;
use std::path::PathBuf;
Expand All @@ -34,16 +33,16 @@ lazy_static::lazy_static! {
static GLUE_LOCK: Mutex<()> = Mutex::new(());

#[derive(Debug)]
pub struct Out<'run> {
pub struct Out {
pub cmd_str: OsString, // command with all its arguments, for easy debugging
pub stdout: String,
pub stderr: String,
pub status: ExitStatus,
pub run: &'run Run<'run>,
pub run: Run,
pub valgrind_xml: Option<NamedTempFile>,
}

impl std::fmt::Display for Out<'_> {
impl std::fmt::Display for Out {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(
f,
Expand All @@ -56,7 +55,7 @@ impl std::fmt::Display for Out<'_> {
}
}

impl Out<'_> {
impl Out {
pub fn assert_clean_success(&self) {
self.assert_success_with_no_unexpected_errors(COMMON_STDERR.as_slice());
}
Expand Down Expand Up @@ -91,13 +90,13 @@ impl Out<'_> {
/// Unlike `std::process::Command`, this builder is clonable and provides convenience methods for
/// constructing Commands for the configured run and instrumenting the configured command with valgrind.
#[derive(Debug, Clone)]
pub struct Run<'a> {
pub struct Run {
args: Vec<OsString>,
env: Vec<(String, String)>,
stdin_vals: Vec<&'a str>,
stdin_vals: Vec<&'static str>,
}

impl<'run> Run<'run> {
impl<'run> Run {
pub fn new<S>(exe: S) -> Self
where
S: AsRef<OsStr>,
Expand Down Expand Up @@ -159,21 +158,21 @@ impl<'run> Run<'run> {
(cmd, named_tempfile)
}

pub fn arg<S>(&mut self, arg: S) -> &mut Self
pub fn arg<S>(mut self, arg: S) -> Self
where
S: Into<OsString>,
{
self.args.push(arg.into());
self
}

pub fn add_args<I, S>(&mut self, args: I) -> &mut Self
pub fn add_args<I, S>(mut self, args: I) -> Self
where
I: IntoIterator<Item = S>,
S: AsRef<OsStr>,
{
for arg in args {
self.arg(&arg);
self = self.arg(&arg);
}
self
}
Expand All @@ -186,24 +185,25 @@ impl<'run> Run<'run> {
self.env.iter().map(|(k, v)| (k.as_str(), v.as_str()))
}

pub fn with_stdin_vals<I>(&mut self, stdin_vals: I) -> &mut Self
pub fn with_stdin_vals<I>(mut self, stdin_vals: I) -> Self
where
I: IntoIterator<Item = &'run str>,
I: IntoIterator<Item = &'static str>,
{
self.stdin_vals.extend(stdin_vals.into_iter());
self
}

pub fn with_env<'a, I>(&mut self, env: I) -> &mut Self
where I: IntoIterator<Item = (&'a str, &'a str)>
where
I: IntoIterator<Item = (&'a str, &'a str)>,
{
for (k, v) in env {
self.env.push((k.into(), v.into()));
}
self
}

fn do_run(&self, mut cmd: Command, stdin_vals: &[&str]) -> Out {
fn run_with_command(self, mut cmd: Command) -> Out {
let cmd_str = pretty_command_string(&cmd);
let mut roc_cmd_child = cmd
.stdin(Stdio::piped())
Expand All @@ -215,7 +215,7 @@ impl<'run> Run<'run> {
});
let stdin = roc_cmd_child.stdin.as_mut().expect("Failed to open stdin");

for stdin_str in stdin_vals.iter() {
for stdin_str in self.stdin_vals.iter() {
stdin
.write_all(stdin_str.as_bytes())
.unwrap_or_else(|err| {
Expand All @@ -236,18 +236,19 @@ impl<'run> Run<'run> {
}
}

pub fn run(&mut self) -> Out {
self.do_run(self.command(), &self.stdin_vals)
pub fn run(self) -> Out {
let command = self.command();
self.run_with_command(command)
}

pub fn run_glue(&mut self) -> Out {
pub fn run_glue(self) -> Out {
let _guard = GLUE_LOCK.lock().unwrap();
self.run()
}

pub fn run_with_valgrind(&mut self) -> Out {
pub fn run_with_valgrind(self) -> Out {
let (valgrind_cmd, valgrind_xml) = self.valgrind_command();
let mut out = self.do_run(valgrind_cmd, &self.stdin_vals);
let mut out = self.run_with_command(valgrind_cmd);
out.valgrind_xml = Some(valgrind_xml);
out
}
Expand Down

0 comments on commit 5f8ff89

Please sign in to comment.