feat(runtime): two-tier subprocess API

[crowlKats]

Fixes #11016

Left to do:

• documentation
• tests

Things ideally to have before landing:

• Support BYOB ReadableStream #11025
• Stabilize Deno.kill API #11757

[crowlKats]

My concerns with this API is that this has too much API surface and has multiple ways to do the same thing:
await new Command().status() and await new Command().spawn().wait()
await new Command().output() and await new Command().spawn().output()
making the Command methods superfluous except spawn(), which doesn't make much sense and could be "collapsed"/simplified to just Child, so don't see that much need for 2-tier subprocess

[crowlKats]

On top of previous concerns, do we really need both status() and output() if output() includes status()?

[bartlomieju]

My concerns with this API is that this has too much API surface and has multiple ways to do the same thing:
await new Command().status() and await new Command().spawn().wait()
await new Command().output() and await new Command().spawn().output()
making the Command methods superfluous except spawn(), which doesn't make much sense and could be "collapsed"/simplified to just Child, so don't see that much need for 2-tier subprocess

That's on purpose to mimic Rust's API. There's a difference between those APIs - eg. if you use await new Command().output() we should automatically make sure that stdout and stderr are set to piped.

On top of previous concerns, do we really need both status() and output() if output() includes status()?

Yes, there are situation where you might not want to collect output from the command - either discard it setting the pipes to null or use inherit.

Again - the idea was to mimic Rust's API which is very flexible yet it has a simple "happy path" that most of the users will use.

[crowlKats]

Yes, there are situation where you might not want to collect output from the command - either discard it setting the pipes to null or use inherit.

But output() takes care of that:

deno/runtime/ops/command.rs

Lines 171 to 183 in 046ae0c

	Ok(CommandOutput {
	status: output.status.into(),
	stdout: if &stdout == "piped" {
	Some(output.stdout.into())
	} else {
	None
	},
	stderr: if &stderr == "piped" {
	Some(output.stderr.into())
	} else {
	None
	},
	})

as else it just returns an empty uint8array

[crowlKats]

That's on purpose to mimic Rust's API. There's a difference between those APIs - eg. if you use await new Command().output() we should automatically make sure that stdout and stderr are set to piped.

Then it would rather make sense to move stdin, stderr, and stdout options from the main constructor to what currently is the spawn() method

[lucacasonato]

The ops are not marked as unstable.

[crowlKats]

@lucacasonato

deno/runtime/ops/command.rs

Line 133 in 9af9577

super::check_unstable(state, "Deno.spawn");

[lucacasonato]

CI is failing on linting, but other than that LGTM!

Thanks for seeing this through @crowlKats! Very exciting to have this finally done. 🎉

[bartlomieju]

LGTM. You are most likely the winner in "the longest time to merge the PR" contest in Deno @crowlKats 🤣

[crowlKats]

@bartlomieju and i think second place is webgpu, which took 7 months 🤣