From 900b6715b6d5cb0cefb8c71a264055ebf61f8f90 Mon Sep 17 00:00:00 2001 From: Erica Marigold Date: Sun, 9 Jun 2024 13:01:34 +0530 Subject: [PATCH 01/35] feat: initial revamped process.spawn impl --- Cargo.lock | 2 + crates/lune-std-process/Cargo.toml | 3 + crates/lune-std-process/src/lib.rs | 125 +++++++++++++++++++++++--- crates/lune-std-process/src/stream.rs | 41 +++++++++ test.luau | 7 ++ 5 files changed, 167 insertions(+), 11 deletions(-) create mode 100644 crates/lune-std-process/src/stream.rs create mode 100644 test.luau diff --git a/Cargo.lock b/Cargo.lock index e6c68e5a..434f2e60 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1625,6 +1625,8 @@ dependencies = [ name = "lune-std-process" version = "0.1.1" dependencies = [ + "bstr", + "bytes", "directories", "lune-utils", "mlua", diff --git a/crates/lune-std-process/Cargo.toml b/crates/lune-std-process/Cargo.toml index 83a792fd..7af5ad26 100644 --- a/crates/lune-std-process/Cargo.toml +++ b/crates/lune-std-process/Cargo.toml @@ -20,6 +20,8 @@ directories = "5.0" pin-project = "1.0" os_str_bytes = { version = "7.0", features = ["conversions"] } +bstr = "1.9" + tokio = { version = "1", default-features = false, features = [ "io-std", "io-util", @@ -29,3 +31,4 @@ tokio = { version = "1", default-features = false, features = [ ] } lune-utils = { version = "0.1.0", path = "../lune-utils" } +bytes = "1.6.0" diff --git a/crates/lune-std-process/src/lib.rs b/crates/lune-std-process/src/lib.rs index d3fd502c..04286ff9 100644 --- a/crates/lune-std-process/src/lib.rs +++ b/crates/lune-std-process/src/lib.rs @@ -1,12 +1,14 @@ #![allow(clippy::cargo_common_metadata)] use std::{ + cell::RefCell, env::{ self, consts::{ARCH, OS}, }, path::MAIN_SEPARATOR, process::Stdio, + rc::Rc, }; use mlua::prelude::*; @@ -14,14 +16,16 @@ use mlua::prelude::*; use lune_utils::TableBuilder; use mlua_luau_scheduler::{Functions, LuaSpawnExt}; use os_str_bytes::RawOsString; -use tokio::io::AsyncWriteExt; +use stream::{ChildProcessReader, ChildProcessWriter}; +use tokio::{io::AsyncWriteExt, process::Child}; mod options; +mod stream; mod tee_writer; mod wait_for_child; use self::options::ProcessSpawnOptions; -use self::wait_for_child::{wait_for_child, WaitForChildResult}; +use self::wait_for_child::wait_for_child; use lune_utils::path::get_current_dir; @@ -73,6 +77,7 @@ pub fn module(lua: &Lua) -> LuaResult { .with_value("cwd", cwd_str)? .with_value("env", env_tab)? .with_value("exit", process_exit)? + .with_async_function("exec", process_exec)? .with_async_function("spawn", process_spawn)? .build_readonly() } @@ -141,11 +146,16 @@ fn process_env_iter<'lua>( }) } -async fn process_spawn( +async fn process_exec( lua: &Lua, (program, args, options): (String, Option>, ProcessSpawnOptions), ) -> LuaResult { - let res = lua.spawn(spawn_command(program, args, options)).await?; + let res = lua + .spawn(async move { + let cmd = spawn_command(program, args, options.clone()).await?; + wait_for_child(cmd, options.stdio.stdout, options.stdio.stderr).await + }) + .await?; /* NOTE: If an exit code was not given by the child process, @@ -168,22 +178,115 @@ async fn process_spawn( .build_readonly() } +#[allow(clippy::await_holding_refcell_ref)] +async fn process_spawn( + lua: &Lua, + (program, args, options): (String, Option>, ProcessSpawnOptions), +) -> LuaResult { + let mut spawn_options = options.clone(); + spawn_options.stdio.stdin = None; + + let (stdin_tx, stdin_rx) = tokio::sync::oneshot::channel(); + let (stdout_tx, stdout_rx) = tokio::sync::oneshot::channel(); + let (stderr_tx, stderr_rx) = tokio::sync::oneshot::channel(); + let (code_tx, code_rx) = tokio::sync::broadcast::channel(100); + let code_rx_rc = Rc::new(RefCell::new(code_rx)); + + tokio::spawn(async move { + let mut child = spawn_command(program, args, spawn_options) + .await + .expect("Could not spawn child process"); + stdin_tx + .send(child.stdin.take()) + .expect("Stdin receiver was unexpectedly dropped"); + stdout_tx + .send(child.stdout.take()) + .expect("Stdout receiver was unexpectedly dropped"); + stderr_tx + .send(child.stderr.take()) + .expect("Stderr receiver was unexpectedly dropped"); + + let res = child + .wait_with_output() + .await + .expect("Failed to get status and output of spawned child process"); + + let code = res + .status + .code() + .unwrap_or(i32::from(!res.stderr.is_empty())); + + code_tx + .send(code) + .expect("ExitCode receiver was unexpectedly dropped"); + }); + + TableBuilder::new(lua)? + .with_value( + "stdout", + ChildProcessReader( + stdout_rx + .await + .expect("Stdout sender unexpectedly dropped") + .ok_or(LuaError::runtime( + "Cannot read from stdout when it is not piped", + ))?, + ), + )? + .with_value( + "stderr", + ChildProcessReader( + stderr_rx + .await + .expect("Stderr sender unexpectedly dropped") + .ok_or(LuaError::runtime( + "Cannot read from stderr when it is not piped", + ))?, + ), + )? + .with_value( + "stdin", + ChildProcessWriter( + stdin_rx + .await + .expect("Stdin sender unexpectedly dropped") + .unwrap(), + ), + )? + .with_async_function("status", move |lua, ()| { + let code_rx_rc_clone = Rc::clone(&code_rx_rc); + async move { + let code = code_rx_rc_clone + .borrow_mut() + .recv() + .await + .expect("Code sender unexpectedly dropped"); + + TableBuilder::new(lua)? + .with_value("code", code)? + .with_value("success", code == 0)? + .build_readonly() + } + })? + .build_readonly() +} + async fn spawn_command( program: String, args: Option>, mut options: ProcessSpawnOptions, -) -> LuaResult { +) -> LuaResult { let stdout = options.stdio.stdout; let stderr = options.stdio.stderr; let stdin = options.stdio.stdin.take(); + // TODO: Have an stdin_kind which the user can supply as piped or not + // TODO: Maybe even revamp the stdout/stderr kinds? User should only use + // piped when they are sure they want to read the stdout. Currently we default + // to piped let mut child = options .into_command(program, args) - .stdin(if stdin.is_some() { - Stdio::piped() - } else { - Stdio::null() - }) + .stdin(Stdio::piped()) .stdout(stdout.as_stdio()) .stderr(stderr.as_stdio()) .spawn()?; @@ -193,5 +296,5 @@ async fn spawn_command( child_stdin.write_all(&stdin).await.into_lua_err()?; } - wait_for_child(child, stdout, stderr).await + Ok(child) } diff --git a/crates/lune-std-process/src/stream.rs b/crates/lune-std-process/src/stream.rs new file mode 100644 index 00000000..df46d6e1 --- /dev/null +++ b/crates/lune-std-process/src/stream.rs @@ -0,0 +1,41 @@ +use bstr::BString; +use bytes::BytesMut; +use mlua::prelude::*; +use tokio::io::{AsyncRead, AsyncReadExt, AsyncWrite, AsyncWriteExt}; + +const CHUNK_SIZE: usize = 8; + +#[derive(Debug, Clone)] +pub struct ChildProcessReader(pub R); +#[derive(Debug, Clone)] +pub struct ChildProcessWriter(pub W); + +impl ChildProcessReader { + pub async fn read(&mut self) -> LuaResult> { + let mut buf = BytesMut::with_capacity(CHUNK_SIZE); + self.0.read_buf(&mut buf).await?; + + Ok(buf.to_vec()) + } +} + +impl LuaUserData for ChildProcessReader { + fn add_methods<'lua, M: LuaUserDataMethods<'lua, Self>>(methods: &mut M) { + methods.add_async_method_mut("read", |lua, this, ()| async { + Ok(lua.create_buffer(this.read().await?)) + }); + } +} + +impl ChildProcessWriter { + pub async fn write(&mut self, data: BString) -> LuaResult<()> { + self.0.write_all(data.as_ref()).await?; + Ok(()) + } +} + +impl LuaUserData for ChildProcessWriter { + fn add_methods<'lua, M: LuaUserDataMethods<'lua, Self>>(methods: &mut M) { + methods.add_async_method_mut("write", |_, this, data| async { this.write(data).await }); + } +} diff --git a/test.luau b/test.luau new file mode 100644 index 00000000..983c0a31 --- /dev/null +++ b/test.luau @@ -0,0 +1,7 @@ +local process = require("@lune/process") + +local a = process.spawn("yes", {}) + +print(a) +print(buffer.tostring(a.stdout:read())) +print(a.status()) \ No newline at end of file From 8ce47806eb6567e9d3d254b390543783a90ab68c Mon Sep 17 00:00:00 2001 From: Erica Marigold Date: Sun, 9 Jun 2024 13:05:14 +0530 Subject: [PATCH 02/35] feat: impl readToEnd convenience method for reader Also rename success key in table to be "ok", so that it is consistent with process.exec. --- crates/lune-std-process/src/lib.rs | 2 +- crates/lune-std-process/src/stream.rs | 10 ++++++++++ test.luau | 4 ++-- 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/crates/lune-std-process/src/lib.rs b/crates/lune-std-process/src/lib.rs index 04286ff9..3e5e163c 100644 --- a/crates/lune-std-process/src/lib.rs +++ b/crates/lune-std-process/src/lib.rs @@ -264,7 +264,7 @@ async fn process_spawn( TableBuilder::new(lua)? .with_value("code", code)? - .with_value("success", code == 0)? + .with_value("ok", code == 0)? .build_readonly() } })? diff --git a/crates/lune-std-process/src/stream.rs b/crates/lune-std-process/src/stream.rs index df46d6e1..9a67e0ed 100644 --- a/crates/lune-std-process/src/stream.rs +++ b/crates/lune-std-process/src/stream.rs @@ -17,6 +17,12 @@ impl ChildProcessReader { Ok(buf.to_vec()) } + + pub async fn read_to_end(&mut self) -> LuaResult> { + let mut buf = vec![]; + self.0.read_to_end(&mut buf).await?; + Ok(buf) + } } impl LuaUserData for ChildProcessReader { @@ -24,6 +30,10 @@ impl LuaUserData for ChildProcessReader { methods.add_async_method_mut("read", |lua, this, ()| async { Ok(lua.create_buffer(this.read().await?)) }); + + methods.add_async_method_mut("readToEnd", |lua, this, ()| async { + Ok(lua.create_buffer(this.read_to_end().await?)) + }); } } diff --git a/test.luau b/test.luau index 983c0a31..a7f5deca 100644 --- a/test.luau +++ b/test.luau @@ -1,7 +1,7 @@ local process = require("@lune/process") -local a = process.spawn("yes", {}) +local a = process.spawn("echo", {"hello"}) print(a) -print(buffer.tostring(a.stdout:read())) +print(buffer.tostring(a.stdout:readToEnd())) print(a.status()) \ No newline at end of file From c63caca39140a6c1c84b3b8763d003ca73d6437f Mon Sep 17 00:00:00 2001 From: Erica Marigold Date: Sun, 9 Jun 2024 13:06:11 +0530 Subject: [PATCH 03/35] chore: remove testing file --- test.luau | 7 ------- 1 file changed, 7 deletions(-) delete mode 100644 test.luau diff --git a/test.luau b/test.luau deleted file mode 100644 index a7f5deca..00000000 --- a/test.luau +++ /dev/null @@ -1,7 +0,0 @@ -local process = require("@lune/process") - -local a = process.spawn("echo", {"hello"}) - -print(a) -print(buffer.tostring(a.stdout:readToEnd())) -print(a.status()) \ No newline at end of file From 4b5b54eb0dcb8b54843dee13495ce5282a302cd0 Mon Sep 17 00:00:00 2001 From: Erica Marigold Date: Sun, 9 Jun 2024 13:10:06 +0530 Subject: [PATCH 04/35] fix: change code channel capacity should be i32 size --- crates/lune-std-process/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/lune-std-process/src/lib.rs b/crates/lune-std-process/src/lib.rs index 3e5e163c..e8b64e46 100644 --- a/crates/lune-std-process/src/lib.rs +++ b/crates/lune-std-process/src/lib.rs @@ -189,7 +189,7 @@ async fn process_spawn( let (stdin_tx, stdin_rx) = tokio::sync::oneshot::channel(); let (stdout_tx, stdout_rx) = tokio::sync::oneshot::channel(); let (stderr_tx, stderr_rx) = tokio::sync::oneshot::channel(); - let (code_tx, code_rx) = tokio::sync::broadcast::channel(100); + let (code_tx, code_rx) = tokio::sync::broadcast::channel(4); let code_rx_rc = Rc::new(RefCell::new(code_rx)); tokio::spawn(async move { From ce033bbdcb9b42a9eae78504a6ba305a2aff7257 Mon Sep 17 00:00:00 2001 From: Erica Marigold Date: Sun, 9 Jun 2024 13:15:52 +0530 Subject: [PATCH 05/35] fix: rename tests to for process.spawn->process.exec --- crates/lune/src/tests.rs | 16 ++++++++-------- tests/process/{spawn => exec}/async.luau | 2 +- tests/process/{spawn => exec}/basic.luau | 2 +- tests/process/{spawn => exec}/cwd.luau | 12 ++++++------ tests/process/{spawn => exec}/no_panic.luau | 2 +- tests/process/{spawn => exec}/shell.luau | 2 +- tests/process/{spawn => exec}/stdin.luau | 4 ++-- tests/process/{spawn => exec}/stdio.luau | 2 +- types/process.luau | 8 ++++---- 9 files changed, 25 insertions(+), 25 deletions(-) rename tests/process/{spawn => exec}/async.luau (93%) rename tests/process/{spawn => exec}/basic.luau (96%) rename tests/process/{spawn => exec}/cwd.luau (81%) rename tests/process/{spawn => exec}/no_panic.luau (73%) rename tests/process/{spawn => exec}/shell.luau (94%) rename tests/process/{spawn => exec}/stdin.luau (79%) rename tests/process/{spawn => exec}/stdio.luau (93%) diff --git a/crates/lune/src/tests.rs b/crates/lune/src/tests.rs index 2e866dc3..e1b22c25 100644 --- a/crates/lune/src/tests.rs +++ b/crates/lune/src/tests.rs @@ -138,13 +138,13 @@ create_tests! { process_cwd: "process/cwd", process_env: "process/env", process_exit: "process/exit", - process_spawn_async: "process/spawn/async", - process_spawn_basic: "process/spawn/basic", - process_spawn_cwd: "process/spawn/cwd", - process_spawn_no_panic: "process/spawn/no_panic", - process_spawn_shell: "process/spawn/shell", - process_spawn_stdin: "process/spawn/stdin", - process_spawn_stdio: "process/spawn/stdio", + process_exec_async: "process/exec/async", + process_exec_basic: "process/exec/basic", + process_exec_cwd: "process/exec/cwd", + process_exec_no_panic: "process/exec/no_panic", + process_exec_shell: "process/exec/shell", + process_exec_stdin: "process/exec/stdin", + process_exec_stdio: "process/exec/stdio", } #[cfg(feature = "std-regex")] @@ -249,6 +249,6 @@ create_tests! { task_cancel: "task/cancel", task_defer: "task/defer", task_delay: "task/delay", - task_spawn: "task/spawn", + task_exec: "task/exec", task_wait: "task/wait", } diff --git a/tests/process/spawn/async.luau b/tests/process/exec/async.luau similarity index 93% rename from tests/process/spawn/async.luau rename to tests/process/exec/async.luau index 2f60f3af..bab274b0 100644 --- a/tests/process/spawn/async.luau +++ b/tests/process/exec/async.luau @@ -31,7 +31,7 @@ for i = 1, SLEEP_SAMPLES, 1 do table.insert(args, 1, "-Milliseconds") end -- Windows does not have `sleep` as a process, so we use powershell instead. - process.spawn("sleep", args, if IS_WINDOWS then { shell = true } else nil) + process.exec("sleep", args, if IS_WINDOWS then { shell = true } else nil) sleepCounter += 1 end) end diff --git a/tests/process/spawn/basic.luau b/tests/process/exec/basic.luau similarity index 96% rename from tests/process/spawn/basic.luau rename to tests/process/exec/basic.luau index 012a8ee7..be782b5a 100644 --- a/tests/process/spawn/basic.luau +++ b/tests/process/exec/basic.luau @@ -12,7 +12,7 @@ end) local IS_WINDOWS = process.os == "windows" -local result = process.spawn( +local result = process.exec( if IS_WINDOWS then "cmd" else "ls", if IS_WINDOWS then { "/c", "dir" } else { "-a" } ) diff --git a/tests/process/spawn/cwd.luau b/tests/process/exec/cwd.luau similarity index 81% rename from tests/process/spawn/cwd.luau rename to tests/process/exec/cwd.luau index d9989dfc..96a7fe42 100644 --- a/tests/process/spawn/cwd.luau +++ b/tests/process/exec/cwd.luau @@ -6,7 +6,7 @@ local pwdCommand = if IS_WINDOWS then "cmd" else "pwd" local pwdArgs = if IS_WINDOWS then { "/c", "cd" } else {} -- Make sure the cwd option actually uses the directory we want -local rootPwd = process.spawn(pwdCommand, pwdArgs, { +local rootPwd = process.exec(pwdCommand, pwdArgs, { cwd = "/", }).stdout rootPwd = string.gsub(rootPwd, "^%s+", "") @@ -27,24 +27,24 @@ end -- Setting cwd should not change the cwd of this process -local pwdBefore = process.spawn(pwdCommand, pwdArgs).stdout -process.spawn("ls", {}, { +local pwdBefore = process.exec(pwdCommand, pwdArgs).stdout +process.exec("ls", {}, { cwd = "/", shell = true, }) -local pwdAfter = process.spawn(pwdCommand, pwdArgs).stdout +local pwdAfter = process.exec(pwdCommand, pwdArgs).stdout assert(pwdBefore == pwdAfter, "Current working directory changed after running child process") -- Setting the cwd on a child process should properly -- replace any leading ~ with the users real home dir -local homeDir1 = process.spawn("echo $HOME", nil, { +local homeDir1 = process.exec("echo $HOME", nil, { shell = true, }).stdout -- NOTE: Powershell for windows uses `$pwd.Path` instead of `pwd` as pwd would return -- a PathInfo object, using $pwd.Path gets the Path property of the PathInfo object -local homeDir2 = process.spawn(if IS_WINDOWS then "$pwd.Path" else "pwd", nil, { +local homeDir2 = process.exec(if IS_WINDOWS then "$pwd.Path" else "pwd", nil, { shell = true, cwd = "~", }).stdout diff --git a/tests/process/spawn/no_panic.luau b/tests/process/exec/no_panic.luau similarity index 73% rename from tests/process/spawn/no_panic.luau rename to tests/process/exec/no_panic.luau index 3a57a9bd..9c954213 100644 --- a/tests/process/spawn/no_panic.luau +++ b/tests/process/exec/no_panic.luau @@ -3,5 +3,5 @@ local process = require("@lune/process") -- Spawning a child process for a non-existent -- program should not panic, but should error -local success = pcall(process.spawn, "someProgramThatDoesNotExist") +local success = pcall(process.exec, "someProgramThatDoesNotExist") assert(not success, "Spawned a non-existent program") diff --git a/tests/process/spawn/shell.luau b/tests/process/exec/shell.luau similarity index 94% rename from tests/process/spawn/shell.luau rename to tests/process/exec/shell.luau index 6f64791d..729f15a5 100644 --- a/tests/process/spawn/shell.luau +++ b/tests/process/exec/shell.luau @@ -5,7 +5,7 @@ local IS_WINDOWS = process.os == "windows" -- Default shell should be /bin/sh on unix and powershell on Windows, -- note that powershell needs slightly different command flags for ls -local shellResult = process.spawn("ls", { +local shellResult = process.exec("ls", { if IS_WINDOWS then "-Force" else "-a", }, { shell = true, diff --git a/tests/process/spawn/stdin.luau b/tests/process/exec/stdin.luau similarity index 79% rename from tests/process/spawn/stdin.luau rename to tests/process/exec/stdin.luau index 56c77a50..f85cd0bf 100644 --- a/tests/process/spawn/stdin.luau +++ b/tests/process/exec/stdin.luau @@ -10,8 +10,8 @@ local echoMessage = "Hello from child process!" -- When passing stdin to powershell on windows we must "accept" using the double newline local result = if IS_WINDOWS - then process.spawn("powershell", { "echo" }, { stdin = echoMessage .. "\n\n" }) - else process.spawn("xargs", { "echo" }, { stdin = echoMessage }) + then process.exec("powershell", { "echo" }, { stdin = echoMessage .. "\n\n" }) + else process.exec("xargs", { "echo" }, { stdin = echoMessage }) local resultStdout = if IS_WINDOWS then string.sub(result.stdout, #result.stdout - #echoMessage - 1) diff --git a/tests/process/spawn/stdio.luau b/tests/process/exec/stdio.luau similarity index 93% rename from tests/process/spawn/stdio.luau rename to tests/process/exec/stdio.luau index 0ea5b1c3..d4ef3886 100644 --- a/tests/process/spawn/stdio.luau +++ b/tests/process/exec/stdio.luau @@ -5,7 +5,7 @@ local IS_WINDOWS = process.os == "windows" -- Inheriting stdio & environment variables should work local echoMessage = "Hello from child process!" -local echoResult = process.spawn("echo", { +local echoResult = process.exec("echo", { if IS_WINDOWS then '"$Env:TEST_VAR"' else '"$TEST_VAR"', }, { env = { TEST_VAR = echoMessage }, diff --git a/types/process.luau b/types/process.luau index 7b820527..97fe1ee6 100644 --- a/types/process.luau +++ b/types/process.luau @@ -12,7 +12,7 @@ export type SpawnOptionsStdio = { @interface SpawnOptions @within Process - A dictionary of options for `process.spawn`, with the following available values: + A dictionary of options for `process.exec`, with the following available values: * `cwd` - The current working directory for the process * `env` - Extra environment variables to give to the process @@ -32,7 +32,7 @@ export type SpawnOptions = { @interface SpawnResult @within Process - Result type for child processes in `process.spawn`. + Result type for child processes in `process.exec`. This is a dictionary containing the following values: @@ -74,7 +74,7 @@ export type SpawnResult = { print("Running " .. process.os .. " on " .. process.arch .. "!") -- Spawning a child process - local result = process.spawn("program", { + local result = process.exec("program", { "cli argument", "other cli argument" }) @@ -175,7 +175,7 @@ end @param options A dictionary of options for the child process @return A dictionary representing the result of the child process ]=] -function process.spawn(program: string, params: { string }?, options: SpawnOptions?): SpawnResult +function process.exec(program: string, params: { string }?, options: SpawnOptions?): SpawnResult return nil :: any end From 78a3d4db5f621a09a713048ac155daa8d40b3115 Mon Sep 17 00:00:00 2001 From: Erica Marigold Date: Sun, 9 Jun 2024 16:13:38 +0530 Subject: [PATCH 06/35] chore(types): include types for new process.spawn --- crates/lune-std-process/src/lib.rs | 1 + crates/lune-std-process/src/stream.rs | 3 + types/process.luau | 128 ++++++++++++++++++++++++-- 3 files changed, 124 insertions(+), 8 deletions(-) diff --git a/crates/lune-std-process/src/lib.rs b/crates/lune-std-process/src/lib.rs index e8b64e46..cc88cce2 100644 --- a/crates/lune-std-process/src/lib.rs +++ b/crates/lune-std-process/src/lib.rs @@ -221,6 +221,7 @@ async fn process_spawn( .expect("ExitCode receiver was unexpectedly dropped"); }); + // TODO: If not piped, don't return readers and writers instead of panicking TableBuilder::new(lua)? .with_value( "stdout", diff --git a/crates/lune-std-process/src/stream.rs b/crates/lune-std-process/src/stream.rs index 9a67e0ed..74b31062 100644 --- a/crates/lune-std-process/src/stream.rs +++ b/crates/lune-std-process/src/stream.rs @@ -19,6 +19,9 @@ impl ChildProcessReader { } pub async fn read_to_end(&mut self) -> LuaResult> { + // FIXME: This yields, but should rather only return the stdout + // till present moment instead, so we should have our own logic + // instead of using read_to_end let mut buf = vec![]; self.0.read_to_end(&mut buf).await?; Ok(buf) diff --git a/types/process.luau b/types/process.luau index 97fe1ee6..d6924cd1 100644 --- a/types/process.luau +++ b/types/process.luau @@ -5,6 +5,9 @@ export type SpawnOptionsStdioKind = "default" | "inherit" | "forward" | "none" export type SpawnOptionsStdio = { stdout: SpawnOptionsStdioKind?, stderr: SpawnOptionsStdioKind?, +} + +export type ExecuteOptionsStdio = SpawnOptionsStdio & { stdin: string?, } @@ -12,26 +15,110 @@ export type SpawnOptionsStdio = { @interface SpawnOptions @within Process - A dictionary of options for `process.exec`, with the following available values: + A dictionary of options for `process.spawn`, with the following available values: * `cwd` - The current working directory for the process * `env` - Extra environment variables to give to the process * `shell` - Whether to run in a shell or not - set to `true` to run using the default shell, or a string to run using a specific shell * `stdio` - How to treat output and error streams from the child process - see `SpawnOptionsStdioKind` and `SpawnOptionsStdio` for more info - * `stdin` - Optional standard input to pass to spawned child process ]=] export type SpawnOptions = { cwd: string?, env: { [string]: string }?, shell: (boolean | string)?, - stdio: (SpawnOptionsStdioKind | SpawnOptionsStdio)?, + stdio: (SpawnOptionsStdio | SpawnOptionsStdioKind)?, +} + +--[=[ + @interface ExecuteOptions + @within Process + + A dictionary of options for `process.exec`, with the following available values: + + * `cwd` - The current working directory for the process + * `env` - Extra environment variables to give to the process + * `shell` - Whether to run in a shell or not - set to `true` to run using the default shell, or a string to run using a specific shell + * `stdio` - How to treat output and error streams from the child process - see `SpawnOptionsStdioKind` and `ExecuteOptionsStdio` for more info + * `stdin` - Optional standard input to pass to executed child process +]=] +export type ExecuteOptions = SpawnOptions & { stdin: string?, -- TODO: Remove this since it is now available in stdio above, breaking change } +--[=[ + @class ChildProcessReader + @within Process + + A reader class to read data from a child process' streams in realtime. +]=] +local ChildProcessReader = {} + +--[=[ + @within ChildProcessReader + + Reads a chunk of data (8 bytes at a time) from the reader into a buffer. + Returns a buffer of size 0 if there is no more data to read. + + @return The buffer containing the data read from the reader +]=] +function ChildProcessReader:read(): buffer + return nil :: any +end + +--[=[ + @within ChildProcessReader + + Reads all the data currently present in the reader into a buffer. + + @return The buffer containing the data read from the reader +]=] +function ChildProcessReader:readToEnd(): buffer + return nil :: any +end + +--[=[ + @class ChildProcessWriter + @within Process + + A writer class to write data to a child process' streams in realtime. +]=] +local ChildProcessWriter = {} + +--[=[ + @within ChildProcessWriter + + Writes a buffer or string of data to the writer. + + @param data The data to write to the writer +]=] +function ChildProcessWriter:write(data: buffer | string): () + return nil :: any +end + --[=[ @interface SpawnResult @within Process + Result type for child processes in `process.spawn`. + + This is a dictionary containing the following values: + + * `stdin` - A writer to write to the child process' stdin - see `ChildProcessWriter` for more info + * `stdout` - A reader to read from the child process' stdout - see `ChildProcessReader` for more info + * `stderr` - A reader to read from the child process' stderr - see `ChildProcessReader` for more info + * `status` - A function that yields and returns the exit status of the child process +]=] +export type ChildProcess = { + stdin: typeof(ChildProcessWriter), + stdout: typeof(ChildProcessReader), + stderr: typeof(ChildProcessReader), + status: () -> { ok: boolean, code: number } +} + +--[=[ + @interface ExecuteResult + @within Process + Result type for child processes in `process.exec`. This is a dictionary containing the following values: @@ -41,7 +128,7 @@ export type SpawnOptions = { * `stdout` - The full contents written to stdout by the child process, or an empty string if nothing was written * `stderr` - The full contents written to stderr by the child process, or an empty string if nothing was written ]=] -export type SpawnResult = { +export type ExecuteResult = { ok: boolean, code: number, stdout: string, @@ -73,7 +160,7 @@ export type SpawnResult = { -- Getting the current os and processor architecture print("Running " .. process.os .. " on " .. process.arch .. "!") - -- Spawning a child process + -- Executeing a child process local result = process.exec("program", { "cli argument", "other cli argument" @@ -163,19 +250,44 @@ end --[=[ @within Process - Spawns a child process that will run the program `program`, and returns a dictionary that describes the final status and ouput of the child process. + Spawns a child process in the background that runs the program `program`, and immediately returns + readers and writers to communicate with it. + + In order to execute a command and wait for its output, see `process.exec`. The second argument, `params`, can be passed as a list of string parameters to give to the program. The third argument, `options`, can be passed as a dictionary of options to give to the child process. Refer to the documentation for `SpawnOptions` for specific option keys and their values. - @param program The program to spawn as a child process + @param program The program to Execute as a child process + @param params Additional parameters to pass to the program + @param options A dictionary of options for the child process + @return A dictionary with the readers and writers to communicate with the child process +]=] +function process.spawn(program: string, params: { string }?, options: SpawnOptions?): ChildProcess + return nil :: any +end + +--[=[ + @within Process + + Executes a child process that will execute the command `program`, waiting for it to exit. + Upon exit, it returns a dictionary that describes the final status and ouput of the child process. + + In order to spawn a child process in the background, see `process.spawn`. + + The second argument, `params`, can be passed as a list of string parameters to give to the program. + + The third argument, `options`, can be passed as a dictionary of options to give to the child process. + Refer to the documentation for `ExecuteOptions` for specific option keys and their values. + + @param program The program to Execute as a child process @param params Additional parameters to pass to the program @param options A dictionary of options for the child process @return A dictionary representing the result of the child process ]=] -function process.exec(program: string, params: { string }?, options: SpawnOptions?): SpawnResult +function process.exec(program: string, params: { string }?, options: ExecuteOptions?): ExecuteResult return nil :: any end From 50b1bcbd64f2794868e66652ea0803896c3f1941 Mon Sep 17 00:00:00 2001 From: Erica Marigold Date: Sun, 9 Jun 2024 16:22:47 +0530 Subject: [PATCH 07/35] fix: stop accepting stdio options for process.spawn --- crates/lune-std-process/src/lib.rs | 7 +++++-- test.lua | 16 ++++++++++++++++ types/process.luau | 2 +- 3 files changed, 22 insertions(+), 3 deletions(-) create mode 100644 test.lua diff --git a/crates/lune-std-process/src/lib.rs b/crates/lune-std-process/src/lib.rs index cc88cce2..f9cb7b56 100644 --- a/crates/lune-std-process/src/lib.rs +++ b/crates/lune-std-process/src/lib.rs @@ -15,6 +15,7 @@ use mlua::prelude::*; use lune_utils::TableBuilder; use mlua_luau_scheduler::{Functions, LuaSpawnExt}; +use options::ProcessSpawnOptionsStdio; use os_str_bytes::RawOsString; use stream::{ChildProcessReader, ChildProcessWriter}; use tokio::{io::AsyncWriteExt, process::Child}; @@ -183,8 +184,10 @@ async fn process_spawn( lua: &Lua, (program, args, options): (String, Option>, ProcessSpawnOptions), ) -> LuaResult { + // Spawn does not accept stdio options, so we remove them from the options + // and use the defaults instead let mut spawn_options = options.clone(); - spawn_options.stdio.stdin = None; + spawn_options.stdio = ProcessSpawnOptionsStdio::default(); let (stdin_tx, stdin_rx) = tokio::sync::oneshot::channel(); let (stdout_tx, stdout_rx) = tokio::sync::oneshot::channel(); @@ -221,7 +224,7 @@ async fn process_spawn( .expect("ExitCode receiver was unexpectedly dropped"); }); - // TODO: If not piped, don't return readers and writers instead of panicking + // TODO: Remove the lua errors since we no longer accept stdio options for spawn TableBuilder::new(lua)? .with_value( "stdout", diff --git a/test.lua b/test.lua new file mode 100644 index 00000000..fa5e8abe --- /dev/null +++ b/test.lua @@ -0,0 +1,16 @@ +local process = require("@lune/process") +local stdio = require("@lune/stdio") + +local child = process.spawn("luau-lsp", { "lsp" }) + +while true do + child.stdin:write("hello world") + local buf = child.stdout:read() + + if buffer.len(buf) == 0 then + break + end + + stdio.write(buffer.tostring(buf) .. "\n") + -- stdio.write(buffer.tostring(child.stderr:read() .. child.stderr:read() .. child.stderr:read() .. child.stderr:read())) +end \ No newline at end of file diff --git a/types/process.luau b/types/process.luau index d6924cd1..5fd2e8b5 100644 --- a/types/process.luau +++ b/types/process.luau @@ -26,7 +26,6 @@ export type SpawnOptions = { cwd: string?, env: { [string]: string }?, shell: (boolean | string)?, - stdio: (SpawnOptionsStdio | SpawnOptionsStdioKind)?, } --[=[ @@ -42,6 +41,7 @@ export type SpawnOptions = { * `stdin` - Optional standard input to pass to executed child process ]=] export type ExecuteOptions = SpawnOptions & { + stdio: (SpawnOptionsStdio | SpawnOptionsStdioKind)?, stdin: string?, -- TODO: Remove this since it is now available in stdio above, breaking change } From 6a2f5061d5951611b3bf17e62e81cda82c100dba Mon Sep 17 00:00:00 2001 From: Erica Marigold Date: Sun, 9 Jun 2024 21:54:56 +0530 Subject: [PATCH 08/35] chore(types + tests): update types and tests for exec --- .lune/hello_lune.luau | 2 +- crates/lune-std-process/src/stream.rs | 12 ++++----- scripts/generate_compression_test_files.luau | 4 +-- test.lua | 26 ++++++++++++-------- tests/datetime/formatLocalTime.luau | 2 +- tests/process/exec/stdio.luau | 2 +- types/process.luau | 10 +++++--- 7 files changed, 32 insertions(+), 26 deletions(-) diff --git a/.lune/hello_lune.luau b/.lune/hello_lune.luau index 197fe321..c50fd7b8 100644 --- a/.lune/hello_lune.luau +++ b/.lune/hello_lune.luau @@ -129,7 +129,7 @@ end ]] print("Sending 4 pings to google 🌏") -local result = process.spawn("ping", { +local result = process.exec("ping", { "google.com", "-c 4", }) diff --git a/crates/lune-std-process/src/stream.rs b/crates/lune-std-process/src/stream.rs index 74b31062..fe6bd8b0 100644 --- a/crates/lune-std-process/src/stream.rs +++ b/crates/lune-std-process/src/stream.rs @@ -11,18 +11,16 @@ pub struct ChildProcessReader(pub R); pub struct ChildProcessWriter(pub W); impl ChildProcessReader { - pub async fn read(&mut self) -> LuaResult> { - let mut buf = BytesMut::with_capacity(CHUNK_SIZE); + pub async fn read(&mut self, chunk_size: Option) -> LuaResult> { + let mut buf = BytesMut::with_capacity(chunk_size.unwrap_or(CHUNK_SIZE)); self.0.read_buf(&mut buf).await?; Ok(buf.to_vec()) } pub async fn read_to_end(&mut self) -> LuaResult> { - // FIXME: This yields, but should rather only return the stdout - // till present moment instead, so we should have our own logic - // instead of using read_to_end let mut buf = vec![]; + self.0.read_to_end(&mut buf).await?; Ok(buf) } @@ -30,8 +28,8 @@ impl ChildProcessReader { impl LuaUserData for ChildProcessReader { fn add_methods<'lua, M: LuaUserDataMethods<'lua, Self>>(methods: &mut M) { - methods.add_async_method_mut("read", |lua, this, ()| async { - Ok(lua.create_buffer(this.read().await?)) + methods.add_async_method_mut("read", |lua, this, chunk_size: Option| async move { + Ok(lua.create_buffer(this.read(chunk_size).await?)) }); methods.add_async_method_mut("readToEnd", |lua, this, ()| async { diff --git a/scripts/generate_compression_test_files.luau b/scripts/generate_compression_test_files.luau index 954929a1..ce7ac823 100644 --- a/scripts/generate_compression_test_files.luau +++ b/scripts/generate_compression_test_files.luau @@ -108,7 +108,7 @@ local BIN_ZLIB = if process.os == "macos" then "/opt/homebrew/bin/pigz" else "pi local function checkInstalled(program: string, args: { string }?) print("Checking if", program, "is installed") - local result = process.spawn(program, args) + local result = process.exec(program, args) if not result.ok then stdio.ewrite(string.format("Program '%s' is not installed\n", program)) process.exit(1) @@ -123,7 +123,7 @@ checkInstalled(BIN_ZLIB, { "--version" }) -- Run them to generate files local function run(program: string, args: { string }): string - local result = process.spawn(program, args) + local result = process.exec(program, args) if not result.ok then stdio.ewrite(string.format("Command '%s' failed\n", program)) if #result.stdout > 0 then diff --git a/test.lua b/test.lua index fa5e8abe..42a8621f 100644 --- a/test.lua +++ b/test.lua @@ -1,16 +1,22 @@ local process = require("@lune/process") local stdio = require("@lune/stdio") +local task = require("@lune/task") +local child = process.spawn("echo", { "lsp" }) +task.wait(1) -local child = process.spawn("luau-lsp", { "lsp" }) -while true do - child.stdin:write("hello world") - local buf = child.stdout:read() +stdio.write(buffer.tostring(child.stdout:readToEnd())) +stdio.write(buffer.tostring(child.stdout:readToEnd())) +stdio.write(buffer.tostring(child.stdout:readToEnd())) - if buffer.len(buf) == 0 then - break - end +-- while true do +-- child.stdin:write("hello world") +-- local buf = child.stdout:read() - stdio.write(buffer.tostring(buf) .. "\n") - -- stdio.write(buffer.tostring(child.stderr:read() .. child.stderr:read() .. child.stderr:read() .. child.stderr:read())) -end \ No newline at end of file +-- if buffer.len(buf) == 0 then +-- break +-- end + +-- stdio.write(buffer.tostring(buf) .. "\n") +-- -- stdio.write(buffer.tostring(child.stderr:read() .. child.stderr:read() .. child.stderr:read() .. child.stderr:read())) +-- end \ No newline at end of file diff --git a/tests/datetime/formatLocalTime.luau b/tests/datetime/formatLocalTime.luau index 4e2f6575..8bd000cc 100644 --- a/tests/datetime/formatLocalTime.luau +++ b/tests/datetime/formatLocalTime.luau @@ -31,7 +31,7 @@ if not runLocaleTests then return end -local dateCmd = process.spawn("bash", { "-c", "date +\"%A, %d %B %Y\" --date='@1693068988'" }, { +local dateCmd = process.exec("bash", { "-c", "date +\"%A, %d %B %Y\" --date='@1693068988'" }, { env = { LC_ALL = "fr_FR.UTF-8 ", }, diff --git a/tests/process/exec/stdio.luau b/tests/process/exec/stdio.luau index d4ef3886..524713b8 100644 --- a/tests/process/exec/stdio.luau +++ b/tests/process/exec/stdio.luau @@ -10,7 +10,7 @@ local echoResult = process.exec("echo", { }, { env = { TEST_VAR = echoMessage }, shell = if IS_WINDOWS then "powershell" else "bash", - stdio = "inherit", + stdio = "inherit" :: process.SpawnOptionsStdioKind, -- FIXME: This should just work without a cast? }) -- Windows uses \r\n (CRLF) and unix uses \n (LF) diff --git a/types/process.luau b/types/process.luau index 5fd2e8b5..a60f1d92 100644 --- a/types/process.luau +++ b/types/process.luau @@ -41,7 +41,7 @@ export type SpawnOptions = { * `stdin` - Optional standard input to pass to executed child process ]=] export type ExecuteOptions = SpawnOptions & { - stdio: (SpawnOptionsStdio | SpawnOptionsStdioKind)?, + stdio: (SpawnOptionsStdioKind | SpawnOptionsStdio)?, stdin: string?, -- TODO: Remove this since it is now available in stdio above, breaking change } @@ -56,12 +56,13 @@ local ChildProcessReader = {} --[=[ @within ChildProcessReader - Reads a chunk of data (8 bytes at a time) from the reader into a buffer. - Returns a buffer of size 0 if there is no more data to read. + Reads a chunk of data (specified length or a default of 8 bytes at a time) from + the reader into a buffer. Returns a buffer of size 0 if there is no more data to + read. @return The buffer containing the data read from the reader ]=] -function ChildProcessReader:read(): buffer +function ChildProcessReader:read(chunkSize: number?): buffer return nil :: any end @@ -69,6 +70,7 @@ end @within ChildProcessReader Reads all the data currently present in the reader into a buffer. + This function will yield until the process exits. @return The buffer containing the data read from the reader ]=] From d9cc71e5125fea4b8db9dbdd2325d4026368179d Mon Sep 17 00:00:00 2001 From: Erica Marigold Date: Sun, 9 Jun 2024 22:24:32 +0530 Subject: [PATCH 09/35] chore(tests): add new process.spawn tests --- tests/process/exec/async.luau | 2 +- tests/process/exec/basic.luau | 2 +- tests/process/exec/no_panic.luau | 4 ++-- tests/process/spawn/non_blocking.luau | 18 ++++++++++++++++++ tests/process/spawn/status.luau | 15 +++++++++++++++ tests/process/spawn/stream.luau | 18 ++++++++++++++++++ 6 files changed, 55 insertions(+), 4 deletions(-) create mode 100644 tests/process/spawn/non_blocking.luau create mode 100644 tests/process/spawn/status.luau create mode 100644 tests/process/spawn/stream.luau diff --git a/tests/process/exec/async.luau b/tests/process/exec/async.luau index bab274b0..205eccc6 100644 --- a/tests/process/exec/async.luau +++ b/tests/process/exec/async.luau @@ -4,7 +4,7 @@ local task = require("@lune/task") local IS_WINDOWS = process.os == "windows" --- Spawning a process should not block any lua thread(s) +-- Executing a command should not block any lua thread(s) local SLEEP_DURATION = 1 / 4 local SLEEP_SAMPLES = 2 diff --git a/tests/process/exec/basic.luau b/tests/process/exec/basic.luau index be782b5a..f0b35d14 100644 --- a/tests/process/exec/basic.luau +++ b/tests/process/exec/basic.luau @@ -2,7 +2,7 @@ local process = require("@lune/process") local stdio = require("@lune/stdio") local task = require("@lune/task") --- Spawning a child process should work, with options +-- Executing a comamnd should work, with options local thread = task.delay(1, function() stdio.ewrite("Spawning a process should take a reasonable amount of time\n") diff --git a/tests/process/exec/no_panic.luau b/tests/process/exec/no_panic.luau index 9c954213..a7d289f6 100644 --- a/tests/process/exec/no_panic.luau +++ b/tests/process/exec/no_panic.luau @@ -1,7 +1,7 @@ local process = require("@lune/process") --- Spawning a child process for a non-existent --- program should not panic, but should error +-- Executing a non existent command as a child process +-- should not panic, but should error local success = pcall(process.exec, "someProgramThatDoesNotExist") assert(not success, "Spawned a non-existent program") diff --git a/tests/process/spawn/non_blocking.luau b/tests/process/spawn/non_blocking.luau new file mode 100644 index 00000000..9b886c0f --- /dev/null +++ b/tests/process/spawn/non_blocking.luau @@ -0,0 +1,18 @@ +local process = require("@lune/process") + +-- Spawning a child process should not block the main thread + +local SAMPLES = 400 + +for _ = 1, SAMPLES do + local start = os.time() + local child = process.spawn("echo", { "hello, world" }) + + assert(child ~= nil, "Failed to spawn child process") + + local delta = os.time() - start + assert( + delta <= 1, + `Spawning a child process should not block the main thread, process.spawn took {delta}s to return it should return immediately` + ) +end diff --git a/tests/process/spawn/status.luau b/tests/process/spawn/status.luau new file mode 100644 index 00000000..3ee6f363 --- /dev/null +++ b/tests/process/spawn/status.luau @@ -0,0 +1,15 @@ +local process = require("@lune/process") + +-- The exit code of an child process should be correct + +local randomExitCode = math.random(1, 255) +local isOk = randomExitCode == 0 +local child = process.spawn("exit", { tostring(randomExitCode) }, { shell = true }) +local status = child.status() + +assert( + status.code == randomExitCode, + `Child process exited with wrong exit code, expected {randomExitCode}` +) + +assert(status.ok == isOk, `Child status should be {if status.ok then "ok" else "not ok"}`) diff --git a/tests/process/spawn/stream.luau b/tests/process/spawn/stream.luau new file mode 100644 index 00000000..7edd9e90 --- /dev/null +++ b/tests/process/spawn/stream.luau @@ -0,0 +1,18 @@ +local process = require("@lune/process") + +-- Should be able to write and read from child process streams + +local msg = "hello, world" + +local catChild = process.spawn("cat") +catChild.stdin:write(msg) +assert( + msg == buffer.tostring(catChild.stdout:read(#msg)), + "Failed to write to stdin or read from stdout of child process" +) + +local echoChild = process.spawn("echo", { msg, ">>/dev/stderr" }, { shell = true }) +assert( + msg == buffer.tostring(echoChild.stderr:read(#msg)), + "Failed to read from stderr of child process" +) From f0906c98a262d7c691772aad522ae0520504ab23 Mon Sep 17 00:00:00 2001 From: Erica Marigold Date: Sun, 9 Jun 2024 22:29:19 +0530 Subject: [PATCH 10/35] chore(tests): windows support for process.spawn stream test --- tests/process/spawn/stream.luau | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/process/spawn/stream.luau b/tests/process/spawn/stream.luau index 7edd9e90..bc5846b7 100644 --- a/tests/process/spawn/stream.luau +++ b/tests/process/spawn/stream.luau @@ -11,7 +11,10 @@ assert( "Failed to write to stdin or read from stdout of child process" ) -local echoChild = process.spawn("echo", { msg, ">>/dev/stderr" }, { shell = true }) +local echoChild = if process.os == "windows" + then process.spawn("Write-Error", { msg }, { shell = "powershell" }) + else process.spawn("echo", { msg, ">>/dev/stderr" }, { shell = true }) + assert( msg == buffer.tostring(echoChild.stderr:read(#msg)), "Failed to read from stderr of child process" From 48760b6a0f26d3c3ead528706497f5f2a1c0bfa6 Mon Sep 17 00:00:00 2001 From: Erica Marigold Date: Mon, 10 Jun 2024 09:07:00 +0530 Subject: [PATCH 11/35] chore(tests): status test should use 0 as an exit code too --- tests/process/spawn/status.luau | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/process/spawn/status.luau b/tests/process/spawn/status.luau index 3ee6f363..0da2d993 100644 --- a/tests/process/spawn/status.luau +++ b/tests/process/spawn/status.luau @@ -2,7 +2,7 @@ local process = require("@lune/process") -- The exit code of an child process should be correct -local randomExitCode = math.random(1, 255) +local randomExitCode = math.random(0, 255) local isOk = randomExitCode == 0 local child = process.spawn("exit", { tostring(randomExitCode) }, { shell = true }) local status = child.status() From d3cda4be0cb6e934b15e725ddbdba11f6d912e25 Mon Sep 17 00:00:00 2001 From: Erica Marigold Date: Mon, 10 Jun 2024 09:08:57 +0530 Subject: [PATCH 12/35] chore(types): add spawn example in process docs --- types/process.luau | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/types/process.luau b/types/process.luau index a60f1d92..4a6e9f12 100644 --- a/types/process.luau +++ b/types/process.luau @@ -162,7 +162,7 @@ export type ExecuteResult = { -- Getting the current os and processor architecture print("Running " .. process.os .. " on " .. process.arch .. "!") - -- Executeing a child process + -- Executing a command local result = process.exec("program", { "cli argument", "other cli argument" @@ -172,6 +172,19 @@ export type ExecuteResult = { else print(result.stderr) end + + -- Spawning a child process + local child = process.spawn("program", { + "cli argument", + "other cli argument" + }) + + -- Writing to the child process' stdin + child.stdin:write("Hello from Lune!") + + -- Reading from the child process' stdout + local data = child.stdout:read() + print(buffer.tostring(data)) ``` ]=] local process = {} From 821c6d9f8c6cf7e9b00fe9382d8729c452d5beaa Mon Sep 17 00:00:00 2001 From: Erica Marigold Date: Mon, 10 Jun 2024 09:12:03 +0530 Subject: [PATCH 13/35] chore(types): add yieldability docs for ChildProcessReader:read --- types/process.luau | 3 +++ 1 file changed, 3 insertions(+) diff --git a/types/process.luau b/types/process.luau index 4a6e9f12..9f5017f5 100644 --- a/types/process.luau +++ b/types/process.luau @@ -60,6 +60,9 @@ local ChildProcessReader = {} the reader into a buffer. Returns a buffer of size 0 if there is no more data to read. + This function may yield until there is new data to read from reader, if all data + has already been read. + @return The buffer containing the data read from the reader ]=] function ChildProcessReader:read(chunkSize: number?): buffer From fc26d132a0fa4377cda4648a7bf589fd4bc6f8a3 Mon Sep 17 00:00:00 2001 From: Erica Marigold Date: Mon, 10 Jun 2024 09:19:51 +0530 Subject: [PATCH 14/35] chore(types): clarify yieldability with exit for ChildProcessReader:read --- types/process.luau | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/types/process.luau b/types/process.luau index 9f5017f5..678c1e13 100644 --- a/types/process.luau +++ b/types/process.luau @@ -61,7 +61,7 @@ local ChildProcessReader = {} read. This function may yield until there is new data to read from reader, if all data - has already been read. + till present has already been read, and the process has not exited. @return The buffer containing the data read from the reader ]=] From 2a4c610b6f53926c2c52bb2500e4db3594756c64 Mon Sep 17 00:00:00 2001 From: Erica Marigold Date: Mon, 10 Jun 2024 09:25:49 +0530 Subject: [PATCH 15/35] feat(tests): update tests list --- crates/lune/src/tests.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/crates/lune/src/tests.rs b/crates/lune/src/tests.rs index e1b22c25..d4d30527 100644 --- a/crates/lune/src/tests.rs +++ b/crates/lune/src/tests.rs @@ -145,6 +145,9 @@ create_tests! { process_exec_shell: "process/exec/shell", process_exec_stdin: "process/exec/stdin", process_exec_stdio: "process/exec/stdio", + process_spawn_non_blocking: "process/spawn/non_blocking", + process_spawn_status: "process/spawn/status", + process_spawn_stream: "process/spawn/stream", } #[cfg(feature = "std-regex")] @@ -249,6 +252,6 @@ create_tests! { task_cancel: "task/cancel", task_defer: "task/defer", task_delay: "task/delay", - task_exec: "task/exec", + task_spawn: "task/spawn", task_wait: "task/wait", } From 968a9b4f803be893769277b102888ea5ab3e9b6c Mon Sep 17 00:00:00 2001 From: Erica Marigold Date: Mon, 10 Jun 2024 09:37:48 +0530 Subject: [PATCH 16/35] chore(tests): fix stream test for windows --- tests/process/spawn/stream.luau | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/process/spawn/stream.luau b/tests/process/spawn/stream.luau index bc5846b7..7616c837 100644 --- a/tests/process/spawn/stream.luau +++ b/tests/process/spawn/stream.luau @@ -12,7 +12,7 @@ assert( ) local echoChild = if process.os == "windows" - then process.spawn("Write-Error", { msg }, { shell = "powershell" }) + then process.spawn("echo", { msg, "2>&1" }, { shell = "cmd" }) else process.spawn("echo", { msg, ">>/dev/stderr" }, { shell = true }) assert( From 70583c6bf8e2e7042566e3831ae6487b871b8810 Mon Sep 17 00:00:00 2001 From: Erica Marigold Date: Mon, 10 Jun 2024 09:51:00 +0530 Subject: [PATCH 17/35] chore(tests): stream test on windows should redir stdout to stderr not other way around --- tests/process/spawn/stream.luau | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/process/spawn/stream.luau b/tests/process/spawn/stream.luau index 7616c837..511d1784 100644 --- a/tests/process/spawn/stream.luau +++ b/tests/process/spawn/stream.luau @@ -12,7 +12,7 @@ assert( ) local echoChild = if process.os == "windows" - then process.spawn("echo", { msg, "2>&1" }, { shell = "cmd" }) + then process.spawn("echo", { msg, "1>&2" }, { shell = "cmd" }) else process.spawn("echo", { msg, ">>/dev/stderr" }, { shell = true }) assert( From a30380cba8b14809297e6a3a407609e7bed4c289 Mon Sep 17 00:00:00 2001 From: Erica Marigold Date: Mon, 10 Jun 2024 14:56:01 +0530 Subject: [PATCH 18/35] fix: address todo comments --- crates/lune-std-process/src/lib.rs | 13 ++----------- test.lua | 22 ---------------------- tests/process/spawn/non_blocking.luau | 2 +- 3 files changed, 3 insertions(+), 34 deletions(-) delete mode 100644 test.lua diff --git a/crates/lune-std-process/src/lib.rs b/crates/lune-std-process/src/lib.rs index f9cb7b56..0a2d5b45 100644 --- a/crates/lune-std-process/src/lib.rs +++ b/crates/lune-std-process/src/lib.rs @@ -224,7 +224,6 @@ async fn process_spawn( .expect("ExitCode receiver was unexpectedly dropped"); }); - // TODO: Remove the lua errors since we no longer accept stdio options for spawn TableBuilder::new(lua)? .with_value( "stdout", @@ -232,9 +231,7 @@ async fn process_spawn( stdout_rx .await .expect("Stdout sender unexpectedly dropped") - .ok_or(LuaError::runtime( - "Cannot read from stdout when it is not piped", - ))?, + .unwrap(), ), )? .with_value( @@ -243,9 +240,7 @@ async fn process_spawn( stderr_rx .await .expect("Stderr sender unexpectedly dropped") - .ok_or(LuaError::runtime( - "Cannot read from stderr when it is not piped", - ))?, + .unwrap(), ), )? .with_value( @@ -284,10 +279,6 @@ async fn spawn_command( let stderr = options.stdio.stderr; let stdin = options.stdio.stdin.take(); - // TODO: Have an stdin_kind which the user can supply as piped or not - // TODO: Maybe even revamp the stdout/stderr kinds? User should only use - // piped when they are sure they want to read the stdout. Currently we default - // to piped let mut child = options .into_command(program, args) .stdin(Stdio::piped()) diff --git a/test.lua b/test.lua deleted file mode 100644 index 42a8621f..00000000 --- a/test.lua +++ /dev/null @@ -1,22 +0,0 @@ -local process = require("@lune/process") -local stdio = require("@lune/stdio") -local task = require("@lune/task") -local child = process.spawn("echo", { "lsp" }) -task.wait(1) - - -stdio.write(buffer.tostring(child.stdout:readToEnd())) -stdio.write(buffer.tostring(child.stdout:readToEnd())) -stdio.write(buffer.tostring(child.stdout:readToEnd())) - --- while true do --- child.stdin:write("hello world") --- local buf = child.stdout:read() - --- if buffer.len(buf) == 0 then --- break --- end - --- stdio.write(buffer.tostring(buf) .. "\n") --- -- stdio.write(buffer.tostring(child.stderr:read() .. child.stderr:read() .. child.stderr:read() .. child.stderr:read())) --- end \ No newline at end of file diff --git a/tests/process/spawn/non_blocking.luau b/tests/process/spawn/non_blocking.luau index 9b886c0f..39d87832 100644 --- a/tests/process/spawn/non_blocking.luau +++ b/tests/process/spawn/non_blocking.luau @@ -13,6 +13,6 @@ for _ = 1, SAMPLES do local delta = os.time() - start assert( delta <= 1, - `Spawning a child process should not block the main thread, process.spawn took {delta}s to return it should return immediately` + `Spawning a child process should not block the main thread, process.spawn took {delta}s to return when it should return immediately` ) end From daedbf9899772ad9bf4c4d4ae6267a5e315ac4a1 Mon Sep 17 00:00:00 2001 From: Erica Marigold Date: Mon, 10 Jun 2024 14:59:03 +0530 Subject: [PATCH 19/35] refactor: minor formatting change --- crates/lune-std-process/src/stream.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/lune-std-process/src/stream.rs b/crates/lune-std-process/src/stream.rs index fe6bd8b0..2babe8dd 100644 --- a/crates/lune-std-process/src/stream.rs +++ b/crates/lune-std-process/src/stream.rs @@ -20,8 +20,8 @@ impl ChildProcessReader { pub async fn read_to_end(&mut self) -> LuaResult> { let mut buf = vec![]; - self.0.read_to_end(&mut buf).await?; + Ok(buf) } } From 80ebab4d7097ac840dda09c996fefefebba6c7d2 Mon Sep 17 00:00:00 2001 From: Erica Marigold Date: Mon, 10 Jun 2024 20:10:04 +0530 Subject: [PATCH 20/35] chore(tests): attempt to fix windows cmd yielding --- tests/process/spawn/stream.luau | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/process/spawn/stream.luau b/tests/process/spawn/stream.luau index 511d1784..a6e137c4 100644 --- a/tests/process/spawn/stream.luau +++ b/tests/process/spawn/stream.luau @@ -12,7 +12,7 @@ assert( ) local echoChild = if process.os == "windows" - then process.spawn("echo", { msg, "1>&2" }, { shell = "cmd" }) + then process.spawn("cmd.exe", { "/c", "echo", msg, "1>&2" }) else process.spawn("echo", { msg, ">>/dev/stderr" }, { shell = true }) assert( From cf707a37ff5d8ed9b6fef66e4640d89be30a2217 Mon Sep 17 00:00:00 2001 From: Erica Marigold Date: Tue, 11 Jun 2024 09:09:47 +0530 Subject: [PATCH 21/35] chore(tests): fix stream test failing on windows cmd --- tests/process/spawn/stream.luau | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/process/spawn/stream.luau b/tests/process/spawn/stream.luau index a6e137c4..5b940e9e 100644 --- a/tests/process/spawn/stream.luau +++ b/tests/process/spawn/stream.luau @@ -12,7 +12,7 @@ assert( ) local echoChild = if process.os == "windows" - then process.spawn("cmd.exe", { "/c", "echo", msg, "1>&2" }) + then process.spawn("/c", { "echo", msg, "1>&2" }, { shell = "cmd" }) else process.spawn("echo", { msg, ">>/dev/stderr" }, { shell = true }) assert( From 7ed656cf3e59d867cabb84461f6726357ed4ae80 Mon Sep 17 00:00:00 2001 From: Erica Marigold Date: Sun, 23 Jun 2024 18:50:36 +0530 Subject: [PATCH 22/35] chore(tests): update comment specifying command for exec Co-authored-by: Filip Tibell --- tests/process/exec/basic.luau | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/process/exec/basic.luau b/tests/process/exec/basic.luau index f0b35d14..41b0847a 100644 --- a/tests/process/exec/basic.luau +++ b/tests/process/exec/basic.luau @@ -2,7 +2,7 @@ local process = require("@lune/process") local stdio = require("@lune/stdio") local task = require("@lune/task") --- Executing a comamnd should work, with options +-- Executing a command should work, with options local thread = task.delay(1, function() stdio.ewrite("Spawning a process should take a reasonable amount of time\n") From c9cbaf61830ca9e93dca7f63159c0764c42406f3 Mon Sep 17 00:00:00 2001 From: Erica Marigold Date: Sun, 23 Jun 2024 19:28:51 +0530 Subject: [PATCH 23/35] feat: return strings and null instead of buffers --- crates/lune-std-process/src/stream.rs | 10 ++++++++-- tests/process/spawn/stream.luau | 4 ++-- types/process.luau | 13 ++++++------- 3 files changed, 16 insertions(+), 11 deletions(-) diff --git a/crates/lune-std-process/src/stream.rs b/crates/lune-std-process/src/stream.rs index 2babe8dd..830e0556 100644 --- a/crates/lune-std-process/src/stream.rs +++ b/crates/lune-std-process/src/stream.rs @@ -29,11 +29,17 @@ impl ChildProcessReader { impl LuaUserData for ChildProcessReader { fn add_methods<'lua, M: LuaUserDataMethods<'lua, Self>>(methods: &mut M) { methods.add_async_method_mut("read", |lua, this, chunk_size: Option| async move { - Ok(lua.create_buffer(this.read(chunk_size).await?)) + let buf = this.read(chunk_size).await?; + + if buf.is_empty() { + return Ok(LuaValue::Nil); + } + + Ok(LuaValue::String(lua.create_string(buf)?)) }); methods.add_async_method_mut("readToEnd", |lua, this, ()| async { - Ok(lua.create_buffer(this.read_to_end().await?)) + Ok(lua.create_string(this.read_to_end().await?)) }); } } diff --git a/tests/process/spawn/stream.luau b/tests/process/spawn/stream.luau index 5b940e9e..ce5f7288 100644 --- a/tests/process/spawn/stream.luau +++ b/tests/process/spawn/stream.luau @@ -7,7 +7,7 @@ local msg = "hello, world" local catChild = process.spawn("cat") catChild.stdin:write(msg) assert( - msg == buffer.tostring(catChild.stdout:read(#msg)), + msg == catChild.stdout:read(#msg), "Failed to write to stdin or read from stdout of child process" ) @@ -16,6 +16,6 @@ local echoChild = if process.os == "windows" else process.spawn("echo", { msg, ">>/dev/stderr" }, { shell = true }) assert( - msg == buffer.tostring(echoChild.stderr:read(#msg)), + msg == echoChild.stderr:read(#msg), "Failed to read from stderr of child process" ) diff --git a/types/process.luau b/types/process.luau index 678c1e13..8161de1e 100644 --- a/types/process.luau +++ b/types/process.luau @@ -57,27 +57,26 @@ local ChildProcessReader = {} @within ChildProcessReader Reads a chunk of data (specified length or a default of 8 bytes at a time) from - the reader into a buffer. Returns a buffer of size 0 if there is no more data to - read. + the reader as a string. Returns nil if there is no more data to read. This function may yield until there is new data to read from reader, if all data till present has already been read, and the process has not exited. - @return The buffer containing the data read from the reader + @return The string containing the data read from the reader ]=] -function ChildProcessReader:read(chunkSize: number?): buffer +function ChildProcessReader:read(chunkSize: number?): string? return nil :: any end --[=[ @within ChildProcessReader - Reads all the data currently present in the reader into a buffer. + Reads all the data currently present in the reader as a string. This function will yield until the process exits. - @return The buffer containing the data read from the reader + @return The string containing the data read from the reader ]=] -function ChildProcessReader:readToEnd(): buffer +function ChildProcessReader:readToEnd(): string return nil :: any end From ad4b8a7c917f7c57c68e4b966eebab26f457bbf2 Mon Sep 17 00:00:00 2001 From: Erica Marigold Date: Sun, 23 Jun 2024 19:29:12 +0530 Subject: [PATCH 24/35] fix: lint in process lib.rs --- crates/lune-std-process/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/lune-std-process/src/lib.rs b/crates/lune-std-process/src/lib.rs index 0a2d5b45..9e0ee02d 100644 --- a/crates/lune-std-process/src/lib.rs +++ b/crates/lune-std-process/src/lib.rs @@ -47,8 +47,8 @@ pub fn module(lua: &Lua) -> LuaResult { cwd_str.push(MAIN_SEPARATOR); } // Create constants for OS & processor architecture - let os = lua.create_string(&OS.to_lowercase())?; - let arch = lua.create_string(&ARCH.to_lowercase())?; + let os = lua.create_string(OS.to_lowercase())?; + let arch = lua.create_string(ARCH.to_lowercase())?; // Create readonly args array let args_vec = lua .app_data_ref::>() From c08b7382fdfdd295105d2960d19839f9b7fb6f51 Mon Sep 17 00:00:00 2001 From: Erica Marigold Date: Sun, 23 Jun 2024 20:05:01 +0530 Subject: [PATCH 25/35] chore: remove bstr dep and rearrange bytes dep --- Cargo.lock | 1 - crates/lune-std-process/Cargo.toml | 3 +-- crates/lune-std-process/src/stream.rs | 1 - 3 files changed, 1 insertion(+), 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 434f2e60..7164f721 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1625,7 +1625,6 @@ dependencies = [ name = "lune-std-process" version = "0.1.1" dependencies = [ - "bstr", "bytes", "directories", "lune-utils", diff --git a/crates/lune-std-process/Cargo.toml b/crates/lune-std-process/Cargo.toml index 7af5ad26..abcbb271 100644 --- a/crates/lune-std-process/Cargo.toml +++ b/crates/lune-std-process/Cargo.toml @@ -20,7 +20,7 @@ directories = "5.0" pin-project = "1.0" os_str_bytes = { version = "7.0", features = ["conversions"] } -bstr = "1.9" +bytes = "1.6.0" tokio = { version = "1", default-features = false, features = [ "io-std", @@ -31,4 +31,3 @@ tokio = { version = "1", default-features = false, features = [ ] } lune-utils = { version = "0.1.0", path = "../lune-utils" } -bytes = "1.6.0" diff --git a/crates/lune-std-process/src/stream.rs b/crates/lune-std-process/src/stream.rs index 830e0556..12619806 100644 --- a/crates/lune-std-process/src/stream.rs +++ b/crates/lune-std-process/src/stream.rs @@ -1,4 +1,3 @@ -use bstr::BString; use bytes::BytesMut; use mlua::prelude::*; use tokio::io::{AsyncRead, AsyncReadExt, AsyncWrite, AsyncWriteExt}; From 70088c78e54a4f0acb6635a002857b9b998817c7 Mon Sep 17 00:00:00 2001 From: Erica Marigold Date: Sun, 23 Jun 2024 20:10:21 +0530 Subject: [PATCH 26/35] revert: remove bstr dep --- Cargo.lock | 1 + crates/lune-std-process/Cargo.toml | 1 + crates/lune-std-process/src/stream.rs | 1 + 3 files changed, 3 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index 7164f721..434f2e60 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1625,6 +1625,7 @@ dependencies = [ name = "lune-std-process" version = "0.1.1" dependencies = [ + "bstr", "bytes", "directories", "lune-utils", diff --git a/crates/lune-std-process/Cargo.toml b/crates/lune-std-process/Cargo.toml index abcbb271..5b19e034 100644 --- a/crates/lune-std-process/Cargo.toml +++ b/crates/lune-std-process/Cargo.toml @@ -20,6 +20,7 @@ directories = "5.0" pin-project = "1.0" os_str_bytes = { version = "7.0", features = ["conversions"] } +bstr = "1.9" bytes = "1.6.0" tokio = { version = "1", default-features = false, features = [ diff --git a/crates/lune-std-process/src/stream.rs b/crates/lune-std-process/src/stream.rs index 12619806..830e0556 100644 --- a/crates/lune-std-process/src/stream.rs +++ b/crates/lune-std-process/src/stream.rs @@ -1,3 +1,4 @@ +use bstr::BString; use bytes::BytesMut; use mlua::prelude::*; use tokio::io::{AsyncRead, AsyncReadExt, AsyncWrite, AsyncWriteExt}; From 0c346a59467c1b97190caad3ded8fc9c85673955 Mon Sep 17 00:00:00 2001 From: Erica Marigold Date: Mon, 24 Jun 2024 14:17:52 +0530 Subject: [PATCH 27/35] feat: rename process.spawn->process.create --- crates/lune-std-process/src/lib.rs | 2 +- tests/process/spawn/non_blocking.luau | 2 +- tests/process/spawn/status.luau | 2 +- tests/process/spawn/stream.luau | 6 +++--- types/process.luau | 10 +++++----- 5 files changed, 11 insertions(+), 11 deletions(-) diff --git a/crates/lune-std-process/src/lib.rs b/crates/lune-std-process/src/lib.rs index 9e0ee02d..4212a773 100644 --- a/crates/lune-std-process/src/lib.rs +++ b/crates/lune-std-process/src/lib.rs @@ -79,7 +79,7 @@ pub fn module(lua: &Lua) -> LuaResult { .with_value("env", env_tab)? .with_value("exit", process_exit)? .with_async_function("exec", process_exec)? - .with_async_function("spawn", process_spawn)? + .with_async_function("create", process_spawn)? .build_readonly() } diff --git a/tests/process/spawn/non_blocking.luau b/tests/process/spawn/non_blocking.luau index 39d87832..d44a9d2b 100644 --- a/tests/process/spawn/non_blocking.luau +++ b/tests/process/spawn/non_blocking.luau @@ -6,7 +6,7 @@ local SAMPLES = 400 for _ = 1, SAMPLES do local start = os.time() - local child = process.spawn("echo", { "hello, world" }) + local child = process.create("echo", { "hello, world" }) assert(child ~= nil, "Failed to spawn child process") diff --git a/tests/process/spawn/status.luau b/tests/process/spawn/status.luau index 0da2d993..418c132a 100644 --- a/tests/process/spawn/status.luau +++ b/tests/process/spawn/status.luau @@ -4,7 +4,7 @@ local process = require("@lune/process") local randomExitCode = math.random(0, 255) local isOk = randomExitCode == 0 -local child = process.spawn("exit", { tostring(randomExitCode) }, { shell = true }) +local child = process.create("exit", { tostring(randomExitCode) }, { shell = true }) local status = child.status() assert( diff --git a/tests/process/spawn/stream.luau b/tests/process/spawn/stream.luau index ce5f7288..965eab15 100644 --- a/tests/process/spawn/stream.luau +++ b/tests/process/spawn/stream.luau @@ -4,7 +4,7 @@ local process = require("@lune/process") local msg = "hello, world" -local catChild = process.spawn("cat") +local catChild = process.create("cat") catChild.stdin:write(msg) assert( msg == catChild.stdout:read(#msg), @@ -12,8 +12,8 @@ assert( ) local echoChild = if process.os == "windows" - then process.spawn("/c", { "echo", msg, "1>&2" }, { shell = "cmd" }) - else process.spawn("echo", { msg, ">>/dev/stderr" }, { shell = true }) + then process.create("/c", { "echo", msg, "1>&2" }, { shell = "cmd" }) + else process.create("echo", { msg, ">>/dev/stderr" }, { shell = true }) assert( msg == echoChild.stderr:read(#msg), diff --git a/types/process.luau b/types/process.luau index 8161de1e..7e9f50bf 100644 --- a/types/process.luau +++ b/types/process.luau @@ -15,7 +15,7 @@ export type ExecuteOptionsStdio = SpawnOptionsStdio & { @interface SpawnOptions @within Process - A dictionary of options for `process.spawn`, with the following available values: + A dictionary of options for `process.create`, with the following available values: * `cwd` - The current working directory for the process * `env` - Extra environment variables to give to the process @@ -103,7 +103,7 @@ end @interface SpawnResult @within Process - Result type for child processes in `process.spawn`. + Result type for child processes in `process.create`. This is a dictionary containing the following values: @@ -176,7 +176,7 @@ export type ExecuteResult = { end -- Spawning a child process - local child = process.spawn("program", { + local child = process.create("program", { "cli argument", "other cli argument" }) @@ -282,7 +282,7 @@ end @param options A dictionary of options for the child process @return A dictionary with the readers and writers to communicate with the child process ]=] -function process.spawn(program: string, params: { string }?, options: SpawnOptions?): ChildProcess +function process.create(program: string, params: { string }?, options: SpawnOptions?): ChildProcess return nil :: any end @@ -292,7 +292,7 @@ end Executes a child process that will execute the command `program`, waiting for it to exit. Upon exit, it returns a dictionary that describes the final status and ouput of the child process. - In order to spawn a child process in the background, see `process.spawn`. + In order to spawn a child process in the background, see `process.create`. The second argument, `params`, can be passed as a list of string parameters to give to the program. From aa10e88495961e03ebdbd51319a31c974068dcea Mon Sep 17 00:00:00 2001 From: Erica Marigold Date: Mon, 24 Jun 2024 15:14:36 +0530 Subject: [PATCH 28/35] chore(tests): replace old process.spawn usage with process.exec for stdio --- tests/stdio/format.luau | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/stdio/format.luau b/tests/stdio/format.luau index 7ade5f5b..c0cc7cfc 100644 --- a/tests/stdio/format.luau +++ b/tests/stdio/format.luau @@ -109,7 +109,7 @@ assertContains( local _, errorMessage = pcall(function() local function innerInnerFn() - process.spawn("PROGRAM_THAT_DOES_NOT_EXIST") + process.exec("PROGRAM_THAT_DOES_NOT_EXIST") end local function innerFn() innerInnerFn() From bd71b5e4b8ad4f8dca00ef84463e5e1b90627f4a Mon Sep 17 00:00:00 2001 From: Erica Marigold Date: Mon, 24 Jun 2024 18:25:09 +0530 Subject: [PATCH 29/35] feat: correct non blocking test for process.create --- crates/lune-std-process/src/lib.rs | 84 ++++++++++----------------- tests/process/spawn/non_blocking.luau | 21 +++---- tests/process/spawn/stream.luau | 5 +- 3 files changed, 39 insertions(+), 71 deletions(-) diff --git a/crates/lune-std-process/src/lib.rs b/crates/lune-std-process/src/lib.rs index 4212a773..a83f59db 100644 --- a/crates/lune-std-process/src/lib.rs +++ b/crates/lune-std-process/src/lib.rs @@ -79,7 +79,7 @@ pub fn module(lua: &Lua) -> LuaResult { .with_value("env", env_tab)? .with_value("exit", process_exit)? .with_async_function("exec", process_exec)? - .with_async_function("create", process_spawn)? + .with_function("create", process_spawn)? .build_readonly() } @@ -153,7 +153,7 @@ async fn process_exec( ) -> LuaResult { let res = lua .spawn(async move { - let cmd = spawn_command(program, args, options.clone()).await?; + let cmd = spawn_command_with_stdin(program, args, options.clone()).await?; wait_for_child(cmd, options.stdio.stdout, options.stdio.stderr).await }) .await?; @@ -180,7 +180,7 @@ async fn process_exec( } #[allow(clippy::await_holding_refcell_ref)] -async fn process_spawn( +fn process_spawn( lua: &Lua, (program, args, options): (String, Option>, ProcessSpawnOptions), ) -> LuaResult { @@ -189,26 +189,15 @@ async fn process_spawn( let mut spawn_options = options.clone(); spawn_options.stdio = ProcessSpawnOptionsStdio::default(); - let (stdin_tx, stdin_rx) = tokio::sync::oneshot::channel(); - let (stdout_tx, stdout_rx) = tokio::sync::oneshot::channel(); - let (stderr_tx, stderr_rx) = tokio::sync::oneshot::channel(); let (code_tx, code_rx) = tokio::sync::broadcast::channel(4); let code_rx_rc = Rc::new(RefCell::new(code_rx)); - tokio::spawn(async move { - let mut child = spawn_command(program, args, spawn_options) - .await - .expect("Could not spawn child process"); - stdin_tx - .send(child.stdin.take()) - .expect("Stdin receiver was unexpectedly dropped"); - stdout_tx - .send(child.stdout.take()) - .expect("Stdout receiver was unexpectedly dropped"); - stderr_tx - .send(child.stderr.take()) - .expect("Stderr receiver was unexpectedly dropped"); + let mut child = spawn_command(program, args, spawn_options)?; + let stdin = child.stdin.take().unwrap(); + let stdout = child.stdout.take().unwrap(); + let stderr = child.stderr.take().unwrap(); + tokio::spawn(async move { let res = child .wait_with_output() .await @@ -225,33 +214,9 @@ async fn process_spawn( }); TableBuilder::new(lua)? - .with_value( - "stdout", - ChildProcessReader( - stdout_rx - .await - .expect("Stdout sender unexpectedly dropped") - .unwrap(), - ), - )? - .with_value( - "stderr", - ChildProcessReader( - stderr_rx - .await - .expect("Stderr sender unexpectedly dropped") - .unwrap(), - ), - )? - .with_value( - "stdin", - ChildProcessWriter( - stdin_rx - .await - .expect("Stdin sender unexpectedly dropped") - .unwrap(), - ), - )? + .with_value("stdout", ChildProcessReader(stdout))? + .with_value("stderr", ChildProcessReader(stderr))? + .with_value("stdin", ChildProcessWriter(stdin))? .with_async_function("status", move |lua, ()| { let code_rx_rc_clone = Rc::clone(&code_rx_rc); async move { @@ -270,26 +235,37 @@ async fn process_spawn( .build_readonly() } -async fn spawn_command( +async fn spawn_command_with_stdin( program: String, args: Option>, mut options: ProcessSpawnOptions, +) -> LuaResult { + let stdin = options.stdio.stdin.take(); + + let mut child = spawn_command(program, args, options)?; + + if let Some(stdin) = stdin { + let mut child_stdin = child.stdin.take().unwrap(); + child_stdin.write_all(&stdin).await.into_lua_err()?; + } + + Ok(child) +} + +fn spawn_command( + program: String, + args: Option>, + options: ProcessSpawnOptions, ) -> LuaResult { let stdout = options.stdio.stdout; let stderr = options.stdio.stderr; - let stdin = options.stdio.stdin.take(); - let mut child = options + let child = options .into_command(program, args) .stdin(Stdio::piped()) .stdout(stdout.as_stdio()) .stderr(stderr.as_stdio()) .spawn()?; - if let Some(stdin) = stdin { - let mut child_stdin = child.stdin.take().unwrap(); - child_stdin.write_all(&stdin).await.into_lua_err()?; - } - Ok(child) } diff --git a/tests/process/spawn/non_blocking.luau b/tests/process/spawn/non_blocking.luau index d44a9d2b..82352a7a 100644 --- a/tests/process/spawn/non_blocking.luau +++ b/tests/process/spawn/non_blocking.luau @@ -1,18 +1,13 @@ local process = require("@lune/process") --- Spawning a child process should not block the main thread +-- Spawning a child process should not block the thread -local SAMPLES = 400 +local childThread = coroutine.create(process.create) -for _ = 1, SAMPLES do - local start = os.time() - local child = process.create("echo", { "hello, world" }) +local ok, err = coroutine.resume(childThread, "echo", { "hello, world" }) +assert(ok, err) - assert(child ~= nil, "Failed to spawn child process") - - local delta = os.time() - start - assert( - delta <= 1, - `Spawning a child process should not block the main thread, process.spawn took {delta}s to return when it should return immediately` - ) -end +assert( + coroutine.status(childThread) == "dead", + "Child process should not block the thread it is running on" +) diff --git a/tests/process/spawn/stream.luau b/tests/process/spawn/stream.luau index 965eab15..89bb61ac 100644 --- a/tests/process/spawn/stream.luau +++ b/tests/process/spawn/stream.luau @@ -15,7 +15,4 @@ local echoChild = if process.os == "windows" then process.create("/c", { "echo", msg, "1>&2" }, { shell = "cmd" }) else process.create("echo", { msg, ">>/dev/stderr" }, { shell = true }) -assert( - msg == echoChild.stderr:read(#msg), - "Failed to read from stderr of child process" -) +assert(msg == echoChild.stderr:read(#msg), "Failed to read from stderr of child process") From 53402d8522ad329fdfa44987d4b12b8e24584ab5 Mon Sep 17 00:00:00 2001 From: Erica Marigold Date: Tue, 25 Jun 2024 11:39:18 +0530 Subject: [PATCH 30/35] feat: kill() for process.create spawning --- crates/lune-std-process/src/lib.rs | 62 ++++++++++++++++++------------ 1 file changed, 38 insertions(+), 24 deletions(-) diff --git a/crates/lune-std-process/src/lib.rs b/crates/lune-std-process/src/lib.rs index a83f59db..f546d47d 100644 --- a/crates/lune-std-process/src/lib.rs +++ b/crates/lune-std-process/src/lib.rs @@ -9,6 +9,7 @@ use std::{ path::MAIN_SEPARATOR, process::Stdio, rc::Rc, + sync::Arc, }; use mlua::prelude::*; @@ -18,7 +19,7 @@ use mlua_luau_scheduler::{Functions, LuaSpawnExt}; use options::ProcessSpawnOptionsStdio; use os_str_bytes::RawOsString; use stream::{ChildProcessReader, ChildProcessWriter}; -use tokio::{io::AsyncWriteExt, process::Child}; +use tokio::{io::AsyncWriteExt, process::Child, sync::RwLock}; mod options; mod stream; @@ -79,7 +80,7 @@ pub fn module(lua: &Lua) -> LuaResult { .with_value("env", env_tab)? .with_value("exit", process_exit)? .with_async_function("exec", process_exec)? - .with_function("create", process_spawn)? + .with_function("create", process_create)? .build_readonly() } @@ -180,43 +181,56 @@ async fn process_exec( } #[allow(clippy::await_holding_refcell_ref)] -fn process_spawn( +fn process_create( lua: &Lua, (program, args, options): (String, Option>, ProcessSpawnOptions), ) -> LuaResult { - // Spawn does not accept stdio options, so we remove them from the options - // and use the defaults instead + // We do not want the user to provide stdio options for process.create, + // so we reset the options, regardless of what the user provides us let mut spawn_options = options.clone(); spawn_options.stdio = ProcessSpawnOptionsStdio::default(); let (code_tx, code_rx) = tokio::sync::broadcast::channel(4); let code_rx_rc = Rc::new(RefCell::new(code_rx)); - let mut child = spawn_command(program, args, spawn_options)?; - let stdin = child.stdin.take().unwrap(); - let stdout = child.stdout.take().unwrap(); - let stderr = child.stderr.take().unwrap(); - - tokio::spawn(async move { - let res = child - .wait_with_output() - .await - .expect("Failed to get status and output of spawned child process"); - - let code = res - .status - .code() - .unwrap_or(i32::from(!res.stderr.is_empty())); - - code_tx - .send(code) - .expect("ExitCode receiver was unexpectedly dropped"); + let child = spawn_command(program, args, spawn_options)?; + + let child_arc = Arc::new(RwLock::new(child)); + + let child_arc_clone = Arc::clone(&child_arc); + let mut child_lock = tokio::task::block_in_place(|| child_arc_clone.blocking_write()); + + let stdin = child_lock.stdin.take().unwrap(); + let stdout = child_lock.stdout.take().unwrap(); + let stderr = child_lock.stderr.take().unwrap(); + + let child_arc_inner = Arc::clone(&child_arc); + + // Spawn a background task to wait for the child to exit and send the exit code + let status_handle = tokio::spawn(async move { + let res = child_arc_inner.write().await.wait().await; + + if let Ok(output) = res { + let code = output.code().unwrap_or_default(); + + code_tx + .send(code) + .expect("ExitCode receiver was unexpectedly dropped"); + } }); TableBuilder::new(lua)? .with_value("stdout", ChildProcessReader(stdout))? .with_value("stderr", ChildProcessReader(stderr))? .with_value("stdin", ChildProcessWriter(stdin))? + .with_async_function("kill", move |_, ()| { + // First, stop the status task so the RwLock is dropped + status_handle.abort(); + let child_arc_clone = Arc::clone(&child_arc); + + // Then get another RwLock to write to the child process and kill it + async move { Ok(child_arc_clone.write().await.kill().await?) } + })? .with_async_function("status", move |lua, ()| { let code_rx_rc_clone = Rc::clone(&code_rx_rc); async move { From e143a508a10b0e1857beefa824d2beaceb47a6a8 Mon Sep 17 00:00:00 2001 From: Erica Marigold Date: Tue, 25 Jun 2024 11:39:54 +0530 Subject: [PATCH 31/35] chore(tests): rename tests to process.create --- tests/process/{spawn => create}/non_blocking.luau | 0 tests/process/{spawn => create}/status.luau | 0 tests/process/{spawn => create}/stream.luau | 0 3 files changed, 0 insertions(+), 0 deletions(-) rename tests/process/{spawn => create}/non_blocking.luau (100%) rename tests/process/{spawn => create}/status.luau (100%) rename tests/process/{spawn => create}/stream.luau (100%) diff --git a/tests/process/spawn/non_blocking.luau b/tests/process/create/non_blocking.luau similarity index 100% rename from tests/process/spawn/non_blocking.luau rename to tests/process/create/non_blocking.luau diff --git a/tests/process/spawn/status.luau b/tests/process/create/status.luau similarity index 100% rename from tests/process/spawn/status.luau rename to tests/process/create/status.luau diff --git a/tests/process/spawn/stream.luau b/tests/process/create/stream.luau similarity index 100% rename from tests/process/spawn/stream.luau rename to tests/process/create/stream.luau From e07d37eec6be57ef38b2ae9cc8e6624a8afcaaf8 Mon Sep 17 00:00:00 2001 From: Erica Marigold Date: Tue, 25 Jun 2024 11:41:10 +0530 Subject: [PATCH 32/35] chore(types): add kill function typedef for ChildProcess --- types/process.luau | 2 ++ 1 file changed, 2 insertions(+) diff --git a/types/process.luau b/types/process.luau index 7e9f50bf..beb5a8e5 100644 --- a/types/process.luau +++ b/types/process.luau @@ -110,12 +110,14 @@ end * `stdin` - A writer to write to the child process' stdin - see `ChildProcessWriter` for more info * `stdout` - A reader to read from the child process' stdout - see `ChildProcessReader` for more info * `stderr` - A reader to read from the child process' stderr - see `ChildProcessReader` for more info + * `kill` - A function that kills the child process * `status` - A function that yields and returns the exit status of the child process ]=] export type ChildProcess = { stdin: typeof(ChildProcessWriter), stdout: typeof(ChildProcessReader), stderr: typeof(ChildProcessReader), + kill: () -> (); status: () -> { ok: boolean, code: number } } From e134120afd6e1a9108a73e27ce6232d8919f99ab Mon Sep 17 00:00:00 2001 From: Erica Marigold Date: Tue, 25 Jun 2024 11:52:59 +0530 Subject: [PATCH 33/35] feat: add tests and default exit code for kill() on ChildProcess --- crates/lune-std-process/src/lib.rs | 8 +++----- tests/process/create/kill.luau | 21 +++++++++++++++++++++ 2 files changed, 24 insertions(+), 5 deletions(-) create mode 100644 tests/process/create/kill.luau diff --git a/crates/lune-std-process/src/lib.rs b/crates/lune-std-process/src/lib.rs index f546d47d..b66bc0df 100644 --- a/crates/lune-std-process/src/lib.rs +++ b/crates/lune-std-process/src/lib.rs @@ -234,11 +234,9 @@ fn process_create( .with_async_function("status", move |lua, ()| { let code_rx_rc_clone = Rc::clone(&code_rx_rc); async move { - let code = code_rx_rc_clone - .borrow_mut() - .recv() - .await - .expect("Code sender unexpectedly dropped"); + // Exit code of 9 corresponds to SIGKILL, which should be the only case where + // the receiver gets suddenly dropped + let code = code_rx_rc_clone.borrow_mut().recv().await.unwrap_or(9); TableBuilder::new(lua)? .with_value("code", code)? diff --git a/tests/process/create/kill.luau b/tests/process/create/kill.luau new file mode 100644 index 00000000..e0cbbc7b --- /dev/null +++ b/tests/process/create/kill.luau @@ -0,0 +1,21 @@ +local process = require("@lune/process") + +-- Killing a child process should work as expected + +local message = "Hello, world!" +local child = process.create("cat") + +child.stdin:write(message) +child.kill() + +assert(child.status().code == 9, "Child process should have an exit code of 9 (SIGKILL)") + +assert( + child.stdout:readToEnd() == message, + "Reading from stdout of child process should work even after kill" +) + +local stdinWriteOk = pcall(function() + child.stdin:write(message) +end) +assert(not stdinWriteOk, "Writing to stdin of child process should not work after kill") From 96ee1ba512de66840102a19b6eea7260352d6160 Mon Sep 17 00:00:00 2001 From: Erica Marigold Date: Tue, 25 Jun 2024 12:06:36 +0530 Subject: [PATCH 34/35] feat: update test paths for process.create --- crates/lune/src/tests.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/lune/src/tests.rs b/crates/lune/src/tests.rs index d4d30527..fd2c8488 100644 --- a/crates/lune/src/tests.rs +++ b/crates/lune/src/tests.rs @@ -145,9 +145,9 @@ create_tests! { process_exec_shell: "process/exec/shell", process_exec_stdin: "process/exec/stdin", process_exec_stdio: "process/exec/stdio", - process_spawn_non_blocking: "process/spawn/non_blocking", - process_spawn_status: "process/spawn/status", - process_spawn_stream: "process/spawn/stream", + process_spawn_non_blocking: "process/create/non_blocking", + process_spawn_status: "process/create/status", + process_spawn_stream: "process/create/stream", } #[cfg(feature = "std-regex")] From 6a713a881f49c136ecd8e30493dcc7526e5abc1d Mon Sep 17 00:00:00 2001 From: Erica Marigold Date: Sat, 29 Jun 2024 17:19:49 +0530 Subject: [PATCH 35/35] chore(types): use consistent class naming for `SpawnResult` --- types/process.luau | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/types/process.luau b/types/process.luau index beb5a8e5..6a4a12ec 100644 --- a/types/process.luau +++ b/types/process.luau @@ -100,7 +100,7 @@ function ChildProcessWriter:write(data: buffer | string): () end --[=[ - @interface SpawnResult + @interface ChildProcess @within Process Result type for child processes in `process.create`.