-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
chore: prefactor process manager (#7034)
### Description This PR lays some ground work for PTY support by hiding implementation details of building the command from users of the `ProcessManager`. The public interface for `ProcessManager` and `Child` after this PR no longer use any part of `tokio::process`. This PR adds our own `Command` struct that is intended to be used with `ProcessManager::spawn` instead of the raw command builder type which in the future might be `portable_pty::CommandBuilder`. This approach offers a few benefits: - It allows us to limit the API we expose for command building to only options supported by both regular child processes and child processes hooked up to a pseudo-terminal. - Commands can be decoupled from the spawning strategy - Allows us to avoid running into the [orphan rule](https://doc.rust-lang.org/book/ch19-03-advanced-traits.html#using-the-newtype-pattern-to-implement-external-traits-on-external-types)⚠️ Behavior change: we will now not have our child processes inherit our `stdin`. This was never intended behavior, but an oversight of us not explicitly calling `stdin(Stdio::piped())` or `stdin(Stdio::null())`. ### Testing Instructions Existing test suite and 👀 Each commit should be easily reviewable on its own. Closes TURBO-2070
- Loading branch information
1 parent
1c4dc1c
commit 92d8ceb
Showing
4 changed files
with
163 additions
and
85 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,131 @@ | ||
| use std::{ | ||
| collections::BTreeMap, | ||
| ffi::{OsStr, OsString}, | ||
| process::Stdio, | ||
| }; | ||
|
|
||
| use itertools::Itertools; | ||
| use turbopath::AbsoluteSystemPathBuf; | ||
|
|
||
| /// A command builder that can be used to build both regular | ||
| /// child processes and ones spawned hooked up to a PTY | ||
| pub struct Command { | ||
| program: OsString, | ||
| args: Vec<OsString>, | ||
| cwd: Option<AbsoluteSystemPathBuf>, | ||
| env: BTreeMap<OsString, OsString>, | ||
| open_stdin: bool, | ||
| env_clear: bool, | ||
| } | ||
|
|
||
| impl Command { | ||
| pub fn new(program: impl AsRef<OsStr>) -> Self { | ||
| let program = program.as_ref().to_os_string(); | ||
| Self { | ||
| program, | ||
| args: Vec::new(), | ||
| cwd: None, | ||
| env: BTreeMap::new(), | ||
| open_stdin: false, | ||
| env_clear: false, | ||
| } | ||
| } | ||
|
|
||
| pub fn args<I, S>(&mut self, args: I) -> &mut Self | ||
| where | ||
| I: IntoIterator<Item = S>, | ||
| S: AsRef<OsStr>, | ||
| { | ||
| self.args = args | ||
| .into_iter() | ||
| .map(|arg| arg.as_ref().to_os_string()) | ||
| .collect(); | ||
| self | ||
| } | ||
|
|
||
| pub fn current_dir(&mut self, dir: AbsoluteSystemPathBuf) -> &mut Self { | ||
| self.cwd = Some(dir); | ||
| self | ||
| } | ||
|
|
||
| pub fn envs<I, K, V>(&mut self, vars: I) -> &mut Self | ||
| where | ||
| I: IntoIterator<Item = (K, V)>, | ||
| K: AsRef<OsStr>, | ||
| V: AsRef<OsStr>, | ||
| { | ||
| for (ref key, ref val) in vars { | ||
| self.env | ||
| .insert(key.as_ref().to_os_string(), val.as_ref().to_os_string()); | ||
| } | ||
| self | ||
| } | ||
|
|
||
| pub fn env<K, V>(&mut self, key: K, val: V) -> &mut Self | ||
| where | ||
| K: AsRef<OsStr>, | ||
| V: AsRef<OsStr>, | ||
| { | ||
| self.env | ||
| .insert(key.as_ref().to_os_string(), val.as_ref().to_os_string()); | ||
| self | ||
| } | ||
|
|
||
| /// Configure the child process to spawn with a piped stdin | ||
| pub fn open_stdin(&mut self) -> &mut Self { | ||
| self.open_stdin = true; | ||
| self | ||
| } | ||
|
|
||
| /// Clears the environment variables for the child process | ||
| pub fn env_clear(&mut self) -> &mut Self { | ||
| self.env_clear = true; | ||
| self.env.clear(); | ||
| self | ||
| } | ||
|
|
||
| pub fn label(&self) -> String { | ||
| format!( | ||
| "({}) {} {}", | ||
| self.cwd | ||
| .as_deref() | ||
| .map(|dir| dir.as_str()) | ||
| .unwrap_or_default(), | ||
| self.program.to_string_lossy(), | ||
| self.args.iter().map(|s| s.to_string_lossy()).join(" ") | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| impl From<Command> for tokio::process::Command { | ||
| fn from(value: Command) -> Self { | ||
| let Command { | ||
| program, | ||
| args, | ||
| cwd, | ||
| env, | ||
| open_stdin, | ||
| env_clear, | ||
| } = value; | ||
|
|
||
| let mut cmd = tokio::process::Command::new(program); | ||
| if env_clear { | ||
| cmd.env_clear(); | ||
| } | ||
| cmd.args(args) | ||
| .envs(env) | ||
| // We always pipe stdout/stderr to allow us to capture task output | ||
| .stdout(Stdio::piped()) | ||
| .stderr(Stdio::piped()) | ||
| // Only open stdin if configured to do so | ||
| .stdin(if open_stdin { | ||
| Stdio::piped() | ||
| } else { | ||
| Stdio::null() | ||
| }); | ||
| if let Some(cwd) = cwd { | ||
| cmd.current_dir(cwd.as_std_path()); | ||
| } | ||
| cmd | ||
| } | ||
| } |
Oops, something went wrong.
92d8cebThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Successfully deployed to the following URLs:
turbo-site – ./docs
turbo.vercel.sh
turborepo.com
www.turbopack.org
turborepo.org
www.turbo.build
turbo.vercel.app
turbopack.org
www.turborepo.org
www.turborepo.com
turbo.build
turbo-site-git-main.vercel.sh
turbo-site.vercel.sh