Skip to content

Commit

Permalink
Refactor process spawn for more granular stdio options
Browse files Browse the repository at this point in the history
  • Loading branch information
filiptibell committed Oct 11, 2023
1 parent 1aa6aef commit e16c28f
Show file tree
Hide file tree
Showing 8 changed files with 263 additions and 105 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
part:TestMethod("Hello", "world!")
```

### Changed

- Stdio options when using `process.spawn` can now be set with more granularity, allowing stderr & stdout to be disabled individually and completely to improve memory usage when they are not being used.

## `0.7.8` - October 5th, 2023

### Added
Expand Down
48 changes: 24 additions & 24 deletions src/lune/builtins/process/mod.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use std::{
env::{self, consts},
path,
process::{ExitStatus, Stdio},
process::Stdio,
};

use dunce::canonicalize;
Expand All @@ -13,12 +13,12 @@ use crate::lune::{scheduler::Scheduler, util::TableBuilder};

mod tee_writer;

mod pipe_inherit;
use pipe_inherit::pipe_and_inherit_child_process_stdio;

mod options;
use options::ProcessSpawnOptions;

mod wait_for_child;
use wait_for_child::{wait_for_child, WaitForChildResult};

const PROCESS_EXIT_IMPL_LUA: &str = r#"
exit(...)
yield()
Expand Down Expand Up @@ -169,21 +169,26 @@ async fn process_spawn(
runtime place it on a different thread if possible / necessary
Note that we have to use our scheduler here, we can't
use anything like tokio::task::spawn because our lua
scheduler will not drive those futures to completion
be using tokio::task::spawn directly because our lua
scheduler would not drive those futures to completion
*/
let sched = lua
.app_data_ref::<&Scheduler>()
.expect("Lua struct is missing scheduler");

let (status, stdout, stderr) = sched
let res = sched
.spawn(spawn_command(program, args, options))
.await
.expect("Failed to receive result of spawned process")?;

// NOTE: If an exit code was not given by the child process,
// we default to 1 if it yielded any error output, otherwise 0
let code = status.code().unwrap_or(match stderr.is_empty() {
/*
NOTE: If an exit code was not given by the child process,
we default to 1 if it yielded any error output, otherwise 0
An exit code may be missing if the process was terminated by
some external signal, which is the only time we use this default
*/
let code = res.status.code().unwrap_or(match res.stderr.is_empty() {
true => 0,
false => 1,
});
Expand All @@ -192,39 +197,34 @@ async fn process_spawn(
TableBuilder::new(lua)?
.with_value("ok", code == 0)?
.with_value("code", code)?
.with_value("stdout", lua.create_string(&stdout)?)?
.with_value("stderr", lua.create_string(&stderr)?)?
.with_value("stdout", lua.create_string(&res.stdout)?)?
.with_value("stderr", lua.create_string(&res.stderr)?)?
.build_readonly()
}

async fn spawn_command(
program: String,
args: Option<Vec<String>>,
mut options: ProcessSpawnOptions,
) -> LuaResult<(ExitStatus, Vec<u8>, Vec<u8>)> {
let inherit_stdio = options.inherit_stdio;
let stdin = options.stdin.take();
) -> LuaResult<WaitForChildResult> {
let stdout = options.stdio.stdout;
let stderr = options.stdio.stderr;
let stdin = options.stdio.stdin.take();

let mut child = options
.into_command(program, args)
.stdin(match stdin.is_some() {
true => Stdio::piped(),
false => Stdio::null(),
})
.stdout(Stdio::piped())
.stderr(Stdio::piped())
.stdout(stdout.as_stdio())
.stderr(stderr.as_stdio())
.spawn()?;

// If the stdin option was provided, we write that to the child
if let Some(stdin) = stdin {
let mut child_stdin = child.stdin.take().unwrap();
child_stdin.write_all(&stdin).await.into_lua_err()?;
}

if inherit_stdio {
pipe_and_inherit_child_process_stdio(child).await
} else {
let output = child.wait_with_output().await?;
Ok((output.status, output.stdout, output.stderr))
}
wait_for_child(child, stdout, stderr).await
}
80 changes: 80 additions & 0 deletions src/lune/builtins/process/options/kind.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
use std::{fmt, process::Stdio, str::FromStr};

use itertools::Itertools;
use mlua::prelude::*;

#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)]
pub enum ProcessSpawnOptionsStdioKind {
// TODO: We need better more obvious names
// for these, but that is a breaking change
#[default]
Default,
Forward,
Inherit,
None,
}

impl ProcessSpawnOptionsStdioKind {
pub fn all() -> &'static [Self] {
&[Self::Default, Self::Forward, Self::Inherit, Self::None]
}

pub fn as_stdio(self) -> Stdio {
match self {
Self::None => Stdio::null(),
Self::Forward => Stdio::inherit(),
_ => Stdio::piped(),
}
}
}

impl fmt::Display for ProcessSpawnOptionsStdioKind {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
let s = match *self {
Self::Default => "default",
Self::Forward => "forward",
Self::Inherit => "inherit",
Self::None => "none",
};
f.write_str(s)
}
}

impl FromStr for ProcessSpawnOptionsStdioKind {
type Err = LuaError;
fn from_str(s: &str) -> Result<Self, Self::Err> {
Ok(match s.trim().to_ascii_lowercase().as_str() {
"default" => Self::Default,
"forward" => Self::Forward,
"inherit" => Self::Inherit,
"none" => Self::None,
_ => {
return Err(LuaError::RuntimeError(format!(
"Invalid spawn options stdio kind - got '{}', expected one of {}",
s,
ProcessSpawnOptionsStdioKind::all()
.iter()
.map(|k| format!("'{k}'"))
.join(", ")
)))
}
})
}
}

impl<'lua> FromLua<'lua> for ProcessSpawnOptionsStdioKind {
fn from_lua(value: LuaValue<'lua>, _: &'lua Lua) -> LuaResult<Self> {
match value {
LuaValue::Nil => Ok(Self::default()),
LuaValue::String(s) => s.to_str()?.parse(),
_ => Err(LuaError::FromLuaConversionError {
from: value.type_name(),
to: "ProcessSpawnOptionsStdioKind",
message: Some(format!(
"Invalid spawn options stdio kind - expected string, got {}",
value.type_name()
)),
}),
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,18 @@ use directories::UserDirs;
use mlua::prelude::*;
use tokio::process::Command;

mod kind;
mod stdio;

pub(super) use kind::*;
pub(super) use stdio::*;

#[derive(Debug, Clone, Default)]
pub struct ProcessSpawnOptions {
pub(crate) cwd: Option<PathBuf>,
pub(crate) envs: HashMap<String, String>,
pub(crate) shell: Option<String>,
pub(crate) inherit_stdio: bool,
pub(crate) stdin: Option<Vec<u8>>,
pub(super) struct ProcessSpawnOptions {
pub cwd: Option<PathBuf>,
pub envs: HashMap<String, String>,
pub shell: Option<String>,
pub stdio: ProcessSpawnOptionsStdio,
}

impl<'lua> FromLua<'lua> for ProcessSpawnOptions {
Expand Down Expand Up @@ -112,34 +117,14 @@ impl<'lua> FromLua<'lua> for ProcessSpawnOptions {
}

/*
If we got options for stdio handling, make sure its one of the constant values
*/
match value.get("stdio")? {
LuaValue::Nil => {}
LuaValue::String(s) => match s.to_str()? {
"inherit" => this.inherit_stdio = true,
"default" => this.inherit_stdio = false,
_ => {
return Err(LuaError::RuntimeError(format!(
"Invalid value for option 'stdio' - expected 'inherit' or 'default', got '{}'",
s.to_string_lossy()
)))
}
},
value => {
return Err(LuaError::RuntimeError(format!(
"Invalid type for option 'stdio' - expected 'string', got '{}'",
value.type_name()
)))
}
}

/*
If we have stdin contents, we need to pass those to the child process
If we got options for stdio handling, parse those as well - note that
we accept a separate "stdin" value here for compatibility with older
scripts, but the user should preferrably pass it in the stdio table
*/
this.stdio = value.get("stdio")?;
match value.get("stdin")? {
LuaValue::Nil => {}
LuaValue::String(s) => this.stdin = Some(s.as_bytes().to_vec()),
LuaValue::String(s) => this.stdio.stdin = Some(s.as_bytes().to_vec()),
value => {
return Err(LuaError::RuntimeError(format!(
"Invalid type for option 'stdin' - expected 'string', got '{}'",
Expand Down
56 changes: 56 additions & 0 deletions src/lune/builtins/process/options/stdio.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
use mlua::prelude::*;

use super::kind::ProcessSpawnOptionsStdioKind;

#[derive(Debug, Clone, Default)]
pub struct ProcessSpawnOptionsStdio {
pub stdout: ProcessSpawnOptionsStdioKind,
pub stderr: ProcessSpawnOptionsStdioKind,
pub stdin: Option<Vec<u8>>,
}

impl From<ProcessSpawnOptionsStdioKind> for ProcessSpawnOptionsStdio {
fn from(value: ProcessSpawnOptionsStdioKind) -> Self {
Self {
stdout: value,
stderr: value,
..Default::default()
}
}
}

impl<'lua> FromLua<'lua> for ProcessSpawnOptionsStdio {
fn from_lua(value: LuaValue<'lua>, lua: &'lua Lua) -> LuaResult<Self> {
match value {
LuaValue::Nil => Ok(Self::default()),
LuaValue::String(s) => {
Ok(ProcessSpawnOptionsStdioKind::from_lua(LuaValue::String(s), lua)?.into())
}
LuaValue::Table(t) => {
let mut this = Self::default();

if let Some(stdin) = t.get("stdin")? {
this.stdin = stdin;
}

if let Some(stdout) = t.get("stdout")? {
this.stdout = stdout;
}

if let Some(stderr) = t.get("stderr")? {
this.stderr = stderr;
}

Ok(this)
}
_ => Err(LuaError::FromLuaConversionError {
from: value.type_name(),
to: "ProcessSpawnOptionsStdio",
message: Some(format!(
"Invalid spawn options stdio - expected string or table, got {}",
value.type_name()
)),
}),
}
}
}
46 changes: 0 additions & 46 deletions src/lune/builtins/process/pipe_inherit.rs

This file was deleted.

Loading

0 comments on commit e16c28f

Please sign in to comment.